httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Aaron Bannert <aa...@clove.org>
Subject [PATCH] fix LimitRequestBody for all input handlers
Date Thu, 11 Apr 2002 20:02:37 GMT
Currently, if you set the LimitRequestBody and then do a POST request
against a CGI resource with a body larger than the LimitRequestBody,
one of two broken things happen:

1) You get a closed socket and no returned data.
2) you get the "normal" data from the cgi script, but no 413 error.

It appears to be random which of these broken behaviors is returned.

It seems to me that the proper way to handle this is not in the
http input filter, but instead in the ap_get_client_block function.
The following patch fixes it for me, but since there are so many hidden
cases I want to make sure that I got everything. Could a couple of
you filter gurus please review this?

-aaron


Index: modules/http/http_protocol.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/http/http_protocol.c,v
retrieving revision 1.407
diff -u -u -r1.407 http_protocol.c
--- modules/http/http_protocol.c	1 Apr 2002 22:26:09 -0000	1.407
+++ modules/http/http_protocol.c	11 Apr 2002 19:49:50 -0000
@@ -651,8 +651,6 @@
 
 typedef struct http_filter_ctx {
     apr_off_t remaining;
-    apr_off_t limit;
-    apr_off_t limit_used;
     enum {
         BODY_NONE,
         BODY_LENGTH,
@@ -684,19 +682,6 @@
         f->ctx = ctx = apr_palloc(f->r->pool, sizeof(*ctx));
         ctx->state = BODY_NONE;
         ctx->remaining = 0;
-        ctx->limit_used = 0;
-
-        /* LimitRequestBody does not apply to proxied responses.
-         * Consider implementing this check in its own filter. 
-         * Would adding a directive to limit the size of proxied 
-         * responses be useful?
-         */
-        if (!f->r->proxyreq) {
-            ctx->limit = ap_get_limit_req_body(f->r);
-        }
-        else {
-            ctx->limit = 0;
-        }
 
         tenc = apr_table_get(f->r->headers_in, "Transfer-Encoding");
         lenp = apr_table_get(f->r->headers_in, "Content-Length");
@@ -723,18 +708,6 @@
                 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;
-            }
         }
     }
 
@@ -801,22 +774,6 @@
         ctx->remaining -= totalread;
     }
 
-    /* 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 += totalread;
-        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;
 }
 
@@ -1641,6 +1598,7 @@
 {
     apr_size_t total;
     apr_status_t rv;
+    apr_off_t limit;
     apr_bucket *b;
     const char *tempbuf;
     core_request_config *req_cfg =
@@ -1648,6 +1606,18 @@
                                                     &core_module);
     apr_bucket_brigade *bb = req_cfg->bb;
 
+    /* LimitRequestBody does not apply to proxied responses.
+     * Consider implementing this check in its own filter. 
+     * Would adding a directive to limit the size of proxied 
+     * responses be useful?
+     */
+    if (!r->proxyreq) {
+        limit = ap_get_limit_req_body(r);
+    }
+    else {
+        limit = 0;
+    }
+
     /* read until we get a non-empty brigade */
     while (APR_BRIGADE_EMPTY(bb)) {
         if (ap_get_brigade(r->input_filters, bb, AP_MODE_READBYTES,
@@ -1694,11 +1664,18 @@
         memcpy(buffer, tempbuf, len_read);
         buffer += len_read;
         total += len_read;
-        /* XXX the next field shouldn't be mucked with here,
-         * as it is in terms of bytes in the unfiltered body;
-         * gotta see if anybody else actually uses it
-         */
-        r->read_length += len_read; /* XXX yank me? */
+
+        r->read_length += len_read;
+
+        /* Check that we haven't exceeded our LimitRequestBody */
+        if (limit && r->read_length > limit) {
+            ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, 0, r,
+                          "Length of request body is larger than the "
+                          "configured limit of %" APR_OFF_T_FMT, limit);
+            ap_die(HTTP_REQUEST_ENTITY_TOO_LARGE, r);
+            return -1;
+        }
+
         old = b;
         b = APR_BUCKET_NEXT(b);
         apr_bucket_delete(old);

Mime
View raw message