Fix LimitRequestBody directive by moving the relevant code from

ap_*_client_block to ap_http_filter (aka HTTP_IN).  This is the
only appropriate place for limit checking to occur (otherwise,
chunked input is not correctly limited).

Also changed the type of limit_req_body to apr_off_t to match the
other types inside of HTTP_IN.  Also made the strtol call for
limit_req_body a bit more robust.

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

diff --git a/CHANGES b/CHANGES
index 8ac6dc3fcce17dd03b4ca770e4c1866d55a92db5..8a62d84e3510d4780984f03327cc75682850b976 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,4 +1,8 @@
 Changes with Apache 2.0.30-dev
+
+  *) Fix LimitRequestBody directive by placing it in the HTTP
+     filter.  [Justin Erenkrantz]
+
   *) Fix mod_proxy seg fault when the proxied server returns 
      an HTTP/0.9 response or a bogus status line.
      [Adam Sussman]

diff --git a/include/http_core.h b/include/http_core.h
index 7220f2d7cf219afb55fd35f9e79e001ae2b37c09..402d51152cb0b5c8d9aeaa0d109cb4d8877721c6 100644 (file)
--- a/include/http_core.h
+++ b/include/http_core.h
@@ -234,9 +234,9 @@ AP_DECLARE(apr_port_t) ap_get_server_port(const request_rec *r);
  * Return the limit on bytes in request msg body 
  * @param r The current request
  * @return the maximum number of bytes in the request msg body
- * @deffunc unsigned long ap_get_limit_req_body(const request_rec *r)
+ * @deffunc apr_off_t ap_get_limit_req_body(const request_rec *r)
  */
-AP_DECLARE(unsigned long) ap_get_limit_req_body(const request_rec *r);
+AP_DECLARE(apr_off_t) ap_get_limit_req_body(const request_rec *r);
 
 /**
  * Return the limit on bytes in XML request msg body
@@ -471,7 +471,7 @@ typedef struct {
 #ifdef RLIMIT_NPROC
     struct rlimit *limit_nproc;
 #endif
-    unsigned long limit_req_body;  /* limit on bytes in request msg body */
+    apr_off_t limit_req_body;      /* limit on bytes in request msg body */
     long limit_xml_body;           /* limit on bytes in XML request msg body */
 
     /* logging options */

diff --git a/modules/http/http_protocol.c b/modules/http/http_protocol.c
index a5cf8f37b856463fa906cd1048dfa225fb8eb1b0..b8db820e04979da500e46cf87cf998ff41df77b0 100644 (file)
--- a/modules/http/http_protocol.c
+++ b/modules/http/http_protocol.c
@@ -510,6 +510,8 @@ static long get_chunk_size(char *);
 
 typedef struct http_filter_ctx {
     apr_off_t remaining;
+    apr_off_t limit;
+    apr_off_t limit_used;
     enum {
         BODY_NONE,
         BODY_LENGTH,
@@ -536,6 +538,9 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b,
         const char *tenc, *lenp;
         f->ctx = ctx = apr_palloc(f->r->pool, sizeof(*ctx));
         ctx->state = BODY_NONE;
+        ctx->remaining = 0;
+        ctx->limit_used = 0;
+        ctx->limit = ap_get_limit_req_body(f->r);
 
         tenc = apr_table_get(f->r->headers_in, "Transfer-Encoding");
         lenp = apr_table_get(f->r->headers_in, "Content-Length");
@@ -562,6 +567,18 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b,
                 ctx->state = BODY_LENGTH;
                 ctx->remaining = atol(lenp);
             }
+            
+            /* If we have a limit in effect and we know the C-L ahead of
+             * time, stop it here if it is invalid. 
+             */ 
+            if (ctx->limit && ctx->limit < ctx->remaining) {
+                ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, 0, f->r,
+                          "Requested content-length of %" APR_OFF_T_FMT 
+                          " is larger than the configured limit"
+                          " of %" APR_OFF_T_FMT, ctx->remaining, ctx->limit);
+                ap_die(HTTP_REQUEST_ENTITY_TOO_LARGE, f->r);
+                return APR_EGENERAL;
+            }
         }
     }
 
@@ -620,6 +637,22 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b,
         ctx->remaining -= *readbytes;
     }
 
+    /* We have a limit in effect. */
+    if (ctx->limit) {
+        /* FIXME: Note that we might get slightly confused on chunked inputs
+         * as we'd need to compensate for the chunk lengths which may not
+         * really count.  This seems to be up for interpretation.  */
+        ctx->limit_used += *readbytes;
+        if (ctx->limit < ctx->limit_used) {
+            ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, 0, f->r,
+                          "Read content-length of %" APR_OFF_T_FMT 
+                          " is larger than the configured limit"
+                          " of %" APR_OFF_T_FMT, ctx->limit_used, ctx->limit);
+            ap_die(HTTP_REQUEST_ENTITY_TOO_LARGE, f->r);
+            return APR_EGENERAL;
+        }
+    }
+
     return APR_SUCCESS;
 }
 
@@ -1283,7 +1316,6 @@ AP_DECLARE(int) ap_setup_client_block(request_rec *r, int read_policy)
 {
     const char *tenc = apr_table_get(r->headers_in, "Transfer-Encoding");
     const char *lenp = apr_table_get(r->headers_in, "Content-Length");
-    apr_off_t max_body;
 
     r->read_body = read_policy;
     r->read_chunked = 0;
@@ -1325,16 +1357,6 @@ AP_DECLARE(int) ap_setup_client_block(request_rec *r, int read_policy)
         return HTTP_REQUEST_ENTITY_TOO_LARGE;
     }
 
-    max_body = ap_get_limit_req_body(r);
-    if (max_body && (r->remaining > max_body)) {
-        /* XXX shouldn't we enforce this for chunked encoding too? */
-        ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, 0, r,
-                      "Request content-length of %s is larger than "
-                      "the configured limit of %" APR_OFF_T_FMT, lenp,
-                      max_body);
-        return HTTP_REQUEST_ENTITY_TOO_LARGE;
-    }
-
 #ifdef AP_DEBUG
     {
         /* Make sure ap_getline() didn't leave any droppings. */

diff --git a/server/core.c b/server/core.c
index 7c21d65c66143b63dc7187bf5ac184079ae97f09..3128e2f6df43277fbaa1cd5a78e703fb4d1bd439 100644 (file)
--- a/server/core.c
+++ b/server/core.c
@@ -778,7 +778,7 @@ AP_DECLARE(char *) ap_construct_url(apr_pool_t *p, const char *uri,
     return apr_psprintf(p, "%s://%s:%u%s", ap_http_method(r), host, port, uri);
 }
 
-AP_DECLARE(unsigned long) ap_get_limit_req_body(const request_rec *r)
+AP_DECLARE(apr_off_t) ap_get_limit_req_body(const request_rec *r)
 {
     core_dir_config *d =
       (core_dir_config *)ap_get_module_config(r->per_dir_config, &core_module);
@@ -2093,6 +2093,7 @@ static const char *set_limit_req_body(cmd_parms *cmd, void *conf_,
 {
     core_dir_config *conf=conf_;
     const char *err = ap_check_cmd_context(cmd, NOT_IN_LIMIT);
+    char *errp;
     if (err != NULL) {
         return err;
     }
@@ -2101,7 +2102,10 @@ static const char *set_limit_req_body(cmd_parms *cmd, void *conf_,
      *      Instead we have an idiotic define in httpd.h that prevents
      *      it from being used even when it is available. Sheesh.
      */
-    conf->limit_req_body = (unsigned long)strtol(arg, (char **)NULL, 10);
+    conf->limit_req_body = (apr_off_t)strtol(arg, &errp, 10);
+    if (*errp != '\0') {
+        return "LimitRequestBody requires a non-negative integer.";
+    }
     return NULL;
 }