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;
}
