httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Justin Erenkrantz <jerenkra...@apache.org>
Subject [PATCH] ap_discard_request_body() can't be called more than once
Date Sun, 02 Jun 2002 23:40:41 GMT
This patch combined with the last few patches I've posted today allow
chunked trailer support again and now passes all httpd-test cases.
What we try to do is to ensure that ap_discard_request_body() is not
called before the handler "accepts" the request and begins generating
the output.  It is still possible for a bad module to call discard
more than once or improperly.

This effectively reverts Ryan's patch to http_protocol.c, so
I'd appreciate it gets some review (preferably from Ryan
himself!).  -- justin

Index: modules/dav/main/mod_dav.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/dav/main/mod_dav.c,v
retrieving revision 1.79
diff -u -r1.79 mod_dav.c
--- modules/dav/main/mod_dav.c	17 May 2002 11:33:08 -0000	1.79
+++ modules/dav/main/mod_dav.c	2 Jun 2002 22:50:08 -0000
@@ -1084,11 +1097,6 @@
     int result;
     int depth;
 
-    /* We don't use the request body right now, so torch it. */
-    if ((result = ap_discard_request_body(r)) != OK) {
-        return result;
-    }
-
     /* Ask repository module to resolve the resource */
     err = dav_get_resource(r, 0 /* label_allowed */, 0 /* use_checked_in */,
                            &resource);
@@ -2680,11 +2646,6 @@
                                   "and Overwrite has been specified.");
     }
 
-    /* ### for now, we don't need anything in the body */
-    if ((result = ap_discard_request_body(r)) != OK) {
-        return result;
-    }
-
     if ((err = dav_open_lockdb(r, 0, &lockdb)) != NULL) {
         /* ### add a higher-level description? */
         return dav_handle_err(r, err, NULL);
@@ -3461,10 +3422,6 @@
     if (vsn_hooks == NULL)
         return DECLINED;
 
-    if ((result = ap_discard_request_body(r)) != OK) {
-        return result;
-    }
-
     /* Ask repository module to resolve the resource */
     err = dav_get_resource(r, 0 /* label_allowed */, 0 /* use_checked_in */,
                            &resource);
@@ -3506,6 +3463,11 @@
         return dav_handle_err(r, err, NULL);
     }
 
+    /* Discard the body now. */
+    if ((result = ap_discard_request_body(r)) != OK) {
+        return result;
+    }
+
     /* no body */
     ap_set_content_length(r, 0);
 
@@ -4056,11 +4018,6 @@
                            &resource);
     if (err != NULL)
         return dav_handle_err(r, err, NULL);
-
-    /* MKACTIVITY does not have a defined request body. */
-    if ((result = ap_discard_request_body(r)) != OK) {
-        return result;
-    }
 
     /* Check request preconditions */
 
Index: modules/http/http_protocol.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/http/http_protocol.c,v
retrieving revision 1.432
diff -u -r1.432 http_protocol.c
--- modules/http/http_protocol.c	31 May 2002 20:41:06 -0000	1.432
+++ modules/http/http_protocol.c	2 Jun 2002 22:50:14 -0000
@@ -903,11 +903,6 @@
     if (!ctx->remaining) {
         switch (ctx->state) {
         case BODY_NONE:
-            if (f->r->proxyreq != PROXYREQ_RESPONSE) {
-                e = apr_bucket_eos_create(f->c->bucket_alloc);
-                APR_BRIGADE_INSERT_TAIL(b, e);
-                return APR_SUCCESS;
-            }
             break;
         case BODY_LENGTH:
             e = apr_bucket_eos_create(f->c->bucket_alloc);
@@ -2285,14 +2280,8 @@
         const char *title = status_lines[idx];
         const char *h1;
 
-        /* XXX This is a major hack that should be fixed cleanly.  The
-         * problem is that we have the information we need in a previous
-         * request, but the text of the page must be sent down the last
-         * request_rec's filter stack.  rbb
-         */
-        request_rec *rlast = r;
-        while (rlast->next) {
-            rlast = rlast->next;
+        if (ap_discard_request_body(r) == AP_FILTER_ERROR) {
+            return;
         }
 
         /* Accept a status_line set by a module, but only if it begins
@@ -2315,24 +2304,24 @@
          * so do ebcdic->ascii translation explicitly (if needed)
          */
 
-        ap_rvputs_proto_in_ascii(rlast,
+        ap_rvputs_proto_in_ascii(r,
                   DOCTYPE_HTML_2_0
                   "<html><head>\n<title>", title,
                   "</title>\n</head><body>\n<h1>", h1, "</h1>\n",
                   NULL);
 
-        ap_rvputs_proto_in_ascii(rlast,
+        ap_rvputs_proto_in_ascii(r,
                                  get_canned_error_string(status, r, location),
                                  NULL);
 
         if (recursive_error) {
-            ap_rvputs_proto_in_ascii(rlast, "<p>Additionally, a ",
+            ap_rvputs_proto_in_ascii(r, "<p>Additionally, a ",
                       status_lines[ap_index_of_response(recursive_error)],
                       "\nerror was encountered while trying to use an "
                       "ErrorDocument to handle the request.</p>\n", NULL);
         }
-        ap_rvputs_proto_in_ascii(rlast, ap_psignature("<hr />\n", r), NULL);
-        ap_rvputs_proto_in_ascii(rlast, "</body></html>\n", NULL);
+        ap_rvputs_proto_in_ascii(r, ap_psignature("<hr />\n", r), NULL);
+        ap_rvputs_proto_in_ascii(r, "</body></html>\n", NULL);
     }
     ap_finalize_request_protocol(r);
 }
Index: modules/http/http_request.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/http/http_request.c,v
retrieving revision 1.145
diff -u -r1.145 http_request.c
--- modules/http/http_request.c	31 May 2002 07:37:19 -0000	1.145
+++ modules/http/http_request.c	2 Jun 2002 22:50:15 -0000
@@ -145,17 +145,6 @@
     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)) {
-        /* If the discard returns AP_FILTER_ERROR, it means that we went
-         * recursive on ourselves and we should abort.
-         */
-        int errstatus = ap_discard_request_body(r);
-        if (errstatus == AP_FILTER_ERROR) {
-            return;
-        }
-    }
 
     /*
      * Two types of custom redirects --- plain text, and URLs. Plain text has
Index: modules/mappers/mod_negotiation.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/mappers/mod_negotiation.c,v
retrieving revision 1.102
diff -u -r1.102 mod_negotiation.c
--- modules/mappers/mod_negotiation.c	17 May 2002 11:24:16 -0000	1.102
+++ modules/mappers/mod_negotiation.c	2 Jun 2002 22:50:18 -0000
@@ -2818,9 +2818,6 @@
         apr_bucket *e;
 
         ap_allow_standard_methods(r, REPLACE_ALLOW, M_GET, M_OPTIONS, M_POST, -1);
-        if ((res = ap_discard_request_body(r)) != OK) {
-            return res;
-        }
         /*if (r->method_number == M_OPTIONS) {
          *    return ap_send_http_options(r);
          *}
@@ -2841,6 +2838,9 @@
             return res;
         }
 
+        if ((res = ap_discard_request_body(r)) != OK) {
+            return res;
+        }
         bb = apr_brigade_create(r->pool, c->bucket_alloc);
         e = apr_bucket_file_create(map, best->body, 
                                    (apr_size_t)best->bytes, r->pool,
Index: server/core.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/core.c,v
retrieving revision 1.182
diff -u -r1.182 core.c
--- server/core.c	31 May 2002 20:52:28 -0000	1.182
+++ server/core.c	2 Jun 2002 22:50:23 -0000
@@ -3196,15 +3196,6 @@
 
     ap_allow_standard_methods(r, MERGE_ALLOW, M_GET, M_OPTIONS, M_POST, -1);
 
-    /* If filters intend to consume the request body, they must
-     * register an InputFilter to slurp the contents of the POST
-     * data from the POST input stream.  It no longer exists when
-     * the output filters are invoked by the default handler.
-     */
-    if ((errstatus = ap_discard_request_body(r)) != OK) {
-        return errstatus;
-    }
-
     if (r->method_number == M_GET || r->method_number == M_POST) {
         if (r->finfo.filetype == 0) {
             ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
@@ -3242,6 +3233,11 @@
         if (bld_content_md5) {
             apr_table_setn(r->headers_out, "Content-MD5",
                            ap_md5digest(r->pool, fd));
+        }
+
+        /* We know we are okay to respond to this, so discard the body. */
+        if ((errstatus = ap_discard_request_body(r)) != OK) {
+            return errstatus;
         }
 
         bb = apr_brigade_create(r->pool, c->bucket_alloc);
Index: server/protocol.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/protocol.c,v
retrieving revision 1.104
diff -u -r1.104 protocol.c
--- server/protocol.c	17 May 2002 11:11:37 -0000	1.104
+++ server/protocol.c	2 Jun 2002 22:50:25 -0000
@@ -968,8 +968,9 @@
             ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r,
                           "client sent an unrecognized expectation value of "
                           "Expect: %s", expect);
-            ap_send_error_response(r, 0);
-            (void) ap_discard_request_body(r);
+            if (ap_discard_request_body(r) != AP_FILTER_ERROR) {
+                ap_send_error_response(r, 0);
+            }
             ap_run_log_transaction(r);
             return r;
         }

Mime
View raw message