From: Justin Erenkrantz <jerenkrantz@apache.org>
Date: Tue, 28 May 2002 23:38:31 +0000 (+0000)
Subject: Correctly return 413 when an invalid chunk size is given on input.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=9bd73bf6409e7b0cf5bd15150cd89d442ca3a35e;p=apache

Correctly return 413 when an invalid chunk size is given on input.

- If get_chunk_size() returns a negative number, that probably implies
  an overflow.  So, create a 413 error and pass it to the output filters.
- Modify ap_discard_request_body() to return OK quickly if we're a subreq
  or our status code implies that we will be dropping the connection.
- Modify ap_die() so that if the new status implies that we will drop
  the connection, that we correctly indicate that we can not keepalive
  this connection.  (Without this, the error is returned, but the connection
  is not closed.)


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@95331 13f79535-47bb-0310-9956-ffa450edef68
---

diff --git a/CHANGES b/CHANGES
index 5541b06676..86efd0c86a 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,5 +1,10 @@
 Changes with Apache 2.0.37
 
+  *) Correctly return 413 when an invalid chunk size is given on
+     input.  Also modify ap_discard_request_body to not do anything
+     on sub-requests or when the connection will be dropped.
+     [Justin Erenkrantz]
+
   *) Fix the TIME_* SSL var lookups to be threadsafe.  PR 9469.
      [Cliff Woolley]
 
diff --git a/modules/http/http_protocol.c b/modules/http/http_protocol.c
index a41dca5c76..c5301f0841 100644
--- a/modules/http/http_protocol.c
+++ b/modules/http/http_protocol.c
@@ -852,6 +852,17 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b,
             apr_brigade_flatten(bb, line, &len);
 
             ctx->remaining = get_chunk_size(line);
+            /* Detect invalid chunk sizes. */
+            if (ctx->remaining < 0) {
+                apr_brigade_cleanup(bb);
+                e = ap_bucket_error_create(HTTP_REQUEST_ENTITY_TOO_LARGE, NULL,
+                                           f->r->connection->pool,
+                                           f->r->connection->bucket_alloc);
+                APR_BRIGADE_INSERT_TAIL(bb, e);
+                e = apr_bucket_eos_create(f->r->connection->bucket_alloc);
+                APR_BRIGADE_INSERT_TAIL(bb, e);
+                return ap_pass_brigade(f->r->output_filters, bb);
+            }
         } 
     }
 
@@ -890,6 +901,17 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b,
                 apr_brigade_flatten(bb, line, &len);
                 ctx->remaining = get_chunk_size(line);
 
+                /* Detect invalid chunk sizes. */
+                if (ctx->remaining < 0) {
+                    apr_brigade_cleanup(bb);
+                    e = ap_bucket_error_create(HTTP_REQUEST_ENTITY_TOO_LARGE,
+                                               NULL, c->pool, c->bucket_alloc);
+                    APR_BRIGADE_INSERT_TAIL(bb, e);
+                    e = apr_bucket_eos_create(c->bucket_alloc);
+                    APR_BRIGADE_INSERT_TAIL(bb, e);
+                    return ap_pass_brigade(f->r->output_filters, bb);
+                }
+
                 if (!ctx->remaining) {
                     /* Handle trailers by calling ap_get_mime_headers again! */
                     ctx->state = BODY_NONE;
@@ -1777,6 +1799,17 @@ AP_DECLARE(int) ap_discard_request_body(request_rec *r)
 {
     int rv;
 
+    /* Sometimes we'll get in a state where the input handling has
+     * detected an error where we want to drop the connection, so if
+     * that's the case, don't read the data as that is what we're trying
+     * to avoid.
+     *
+     * This function is also a no-op on a subrequest.
+     */
+    if (r->main || ap_status_drops_connection(r->status)) {
+        return OK;
+    }
+
     if (r->read_length == 0) {  /* if not read already */
         if ((rv = ap_setup_client_block(r, REQUEST_CHUNKED_DECHUNK))) {
             return rv;
diff --git a/modules/http/http_request.c b/modules/http/http_request.c
index d848916f00..22c7d7861a 100644
--- a/modules/http/http_request.c
+++ b/modules/http/http_request.c
@@ -139,13 +139,16 @@ AP_DECLARE(void) ap_die(int type, request_rec *r)
         r->status = HTTP_PROXY_AUTHENTICATION_REQUIRED;
     }
 
-    /*
-     * If we want to keep the connection, be sure that the request body
-     * (if any) has been read.
+    /* If we don't want to keep the connection, make sure we mark that the
+     * connection is not eligible for keepalive.  If we want to keep the
+     * connection, be sure that the request body (if any) has been read.
      */
-    if ((r->status != HTTP_NOT_MODIFIED) && (r->status != HTTP_NO_CONTENT)
-        && !ap_status_drops_connection(r->status)
-        && r->connection && (r->connection->keepalive != -1)) {
+    if (ap_status_drops_connection(r->status)) {
+        r->connection->keepalive = 0;
+    }
+    else if ((r->status != HTTP_NOT_MODIFIED) &&
+             (r->status != HTTP_NO_CONTENT) &&
+             r->connection && (r->connection->keepalive != -1)) {
 
         (void) ap_discard_request_body(r);
     }