You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by wr...@apache.org on 2010/01/21 08:20:07 UTC
svn commit: r901578 - in /httpd/httpd/trunk: CHANGES
modules/http/http_request.c modules/metadata/mod_headers.c server/protocol.c
Author: wrowe
Date: Thu Jan 21 07:19:41 2010
New Revision: 901578
URL: http://svn.apache.org/viewvc?rev=901578&view=rev
Log:
Correctly align the behavior of headers_in to be consistent with the
treatment of headers_out, resolving PR 48359 by keeping subrequest
scope changes out of the main request headers. This ensures that all
requests-without-bodies behave as the requests-with-bodies code has.
Modified:
httpd/httpd/trunk/CHANGES
httpd/httpd/trunk/modules/http/http_request.c
httpd/httpd/trunk/modules/metadata/mod_headers.c
httpd/httpd/trunk/server/protocol.c
Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=901578&r1=901577&r2=901578&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Thu Jan 21 07:19:41 2010
@@ -2,6 +2,11 @@
Changes with Apache 2.3.5
+ *) Ensure each subrequest has a shallow copy of headers_in so that the
+ parent request headers are not corrupted. Elimiates a problematic
+ optimization in the case of no request body. PR 48359
+ [Jake Scott, William Rowe]
+
*) Turn static function get_server_name_for_url() into public
ap_get_server_name_for_url() and use it where appropriate. This
fixes mod_rewrite generating invalid URLs for redirects to IPv6
@@ -23,9 +28,6 @@
*) mod_proxy_balancer: Fix crash in balancer-manager. [Rainer Jung]
- *) mod_headers: Ensure that changes to the main request remain valid when
- the subrequest is destroyed. PR 48359 [Nick Kew, Ruediger Pluem]
-
*) Core HTTP: disable keepalive when the Client has sent
Expect: 100-continue
but we respond directly with a non-100 response.
Modified: httpd/httpd/trunk/modules/http/http_request.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_request.c?rev=901578&r1=901577&r2=901578&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http/http_request.c (original)
+++ httpd/httpd/trunk/modules/http/http_request.c Thu Jan 21 07:19:41 2010
@@ -442,7 +442,7 @@
new->request_time = r->request_time;
new->main = r->main;
- new->headers_in = r->headers_in;
+ new->headers_in = apr_table_copy(r->pool, r->headers_in);
new->headers_out = apr_table_make(r->pool, 12);
new->err_headers_out = r->err_headers_out;
new->subprocess_env = rename_original_env(r->pool, r->subprocess_env);
@@ -515,6 +515,8 @@
r->per_dir_config = rr->per_dir_config;
/* copy output headers from subrequest, but leave negotiation headers */
r->notes = apr_table_overlay(r->pool, rr->notes, r->notes);
+ r->headers_in = apr_table_overlay(r->pool, rr->headers_in,
+ r->headers_in);
r->headers_out = apr_table_overlay(r->pool, rr->headers_out,
r->headers_out);
r->err_headers_out = apr_table_overlay(r->pool, rr->err_headers_out,
Modified: httpd/httpd/trunk/modules/metadata/mod_headers.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/metadata/mod_headers.c?rev=901578&r1=901577&r2=901578&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/metadata/mod_headers.c (original)
+++ httpd/httpd/trunk/modules/metadata/mod_headers.c Thu Jan 21 07:19:41 2010
@@ -547,7 +547,7 @@
* Concatenate the return from each handler into one string that is
* returned from this call.
*/
-static char* process_tags(header_entry *hdr, request_rec *r, request_rec *rr)
+static char* process_tags(header_entry *hdr, request_rec *r)
{
int i;
const char *s;
@@ -558,9 +558,9 @@
for (i = 0; i < hdr->ta->nelts; i++) {
s = tag[i].func(r, tag[i].arg);
if (str == NULL)
- str = apr_pstrdup(rr->pool, s);
+ str = apr_pstrdup(r->pool, s);
else
- str = apr_pstrcat(rr->pool, str, s, NULL);
+ str = apr_pstrcat(r->pool, str, s, NULL);
}
return str ? str : "";
}
@@ -627,12 +627,6 @@
echo_do v;
int i;
const char *val;
- request_rec *rr;
-
- rr = r;
- while (rr->main != NULL) {
- rr = rr->main;
- }
for (i = 0; i < fixup->nelts; ++i) {
header_entry *hdr = &((header_entry *) (fixup->elts))[i];
@@ -673,17 +667,17 @@
switch (hdr->action) {
case hdr_add:
- apr_table_addn(headers, hdr->header, process_tags(hdr, r, rr));
+ apr_table_addn(headers, hdr->header, process_tags(hdr, r));
break;
case hdr_append:
- apr_table_mergen(headers, hdr->header, process_tags(hdr, r, rr));
+ apr_table_mergen(headers, hdr->header, process_tags(hdr, r));
break;
case hdr_merge:
val = apr_table_get(headers, hdr->header);
if (val == NULL) {
- apr_table_addn(headers, hdr->header, process_tags(hdr, r, rr));
+ apr_table_addn(headers, hdr->header, process_tags(hdr, r));
} else {
- char *new_val = process_tags(hdr, r, rr);
+ char *new_val = process_tags(hdr, r);
apr_size_t new_val_len = strlen(new_val);
int tok_found = 0;
@@ -720,9 +714,9 @@
break;
case hdr_set:
if (!strcasecmp(hdr->header, "Content-Type")) {
- ap_set_content_type(r, process_tags(hdr, r, rr));
+ ap_set_content_type(r, process_tags(hdr, r));
}
- apr_table_setn(headers, hdr->header, process_tags(hdr, r, rr));
+ apr_table_setn(headers, hdr->header, process_tags(hdr, r));
break;
case hdr_unset:
apr_table_unset(headers, hdr->header);
@@ -742,7 +736,7 @@
if (apr_table_get(headers, hdr->header)) {
edit_do ed;
- ed.p = rr->pool;
+ ed.p = r->pool;
ed.hdr = hdr;
ed.t = apr_table_make(r->pool, 5);
apr_table_do(edit_header, (void *) &ed, headers, hdr->header,
Modified: httpd/httpd/trunk/server/protocol.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/protocol.c?rev=901578&r1=901577&r2=901578&view=diff
==============================================================================
--- httpd/httpd/trunk/server/protocol.c (original)
+++ httpd/httpd/trunk/server/protocol.c Thu Jan 21 07:19:41 2010
@@ -1074,15 +1074,13 @@
return r;
}
-/* if a request with a body creates a subrequest, clone the original request's
- * input headers minus any headers pertaining to the body which has already
- * been read. out-of-line helper function for ap_set_sub_req_protocol.
+/* if a request with a body creates a subrequest, remove original request's
+ * input headers which pertain to the body which has already been read.
+ * out-of-line helper function for ap_set_sub_req_protocol.
*/
-static void clone_headers_no_body(request_rec *rnew,
- const request_rec *r)
+static void strip_headers_request_body(request_rec *rnew)
{
- rnew->headers_in = apr_table_copy(rnew->pool, r->headers_in);
apr_table_unset(rnew->headers_in, "Content-Encoding");
apr_table_unset(rnew->headers_in, "Content-Language");
apr_table_unset(rnew->headers_in, "Content-Length");
@@ -1116,15 +1114,14 @@
rnew->status = HTTP_OK;
+ rnew->headers_in = apr_table_copy(rnew->pool, r->headers_in);
+
/* did the original request have a body? (e.g. POST w/SSI tags)
* if so, make sure the subrequest doesn't inherit body headers
*/
if (!r->kept_body && (apr_table_get(r->headers_in, "Content-Length")
|| apr_table_get(r->headers_in, "Transfer-Encoding"))) {
- clone_headers_no_body(rnew, r);
- } else {
- /* no body (common case). clone headers the cheap way */
- rnew->headers_in = r->headers_in;
+ strip_headers_request_body(rnew);
}
rnew->subprocess_env = apr_table_copy(rnew->pool, r->subprocess_env);
rnew->headers_out = apr_table_make(rnew->pool, 5);
Re: svn commit: r901578 - in /httpd/httpd/trunk: CHANGES modules/http/http_request.c
modules/metadata/mod_headers.c server/protocol.c
Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 1/21/2010 1:52 AM, Ruediger Pluem wrote:
> On 21.01.2010 08:20, wrowe@apache.org wrote:
>> Author: wrowe
>> Date: Thu Jan 21 07:19:41 2010
>> New Revision: 901578
>>
>> URL: http://svn.apache.org/viewvc?rev=901578&view=rev
>> Log:
>> Correctly align the behavior of headers_in to be consistent with the
>> treatment of headers_out, resolving PR 48359 by keeping subrequest
>> scope changes out of the main request headers. This ensures that all
>> requests-without-bodies behave as the requests-with-bodies code has.
>
> I think this is wrong. Think of the case where the subrequest does not change
> anything on headers_in. In this case the contents of headers_in of the request
> and the subrequest are the same (the subrequest has a copy of the requests
> table). Doing an apr_table_overlay would result in a table that has all key
> value pairs of the original headers_in doubled.
Makes sense; reverted that bit.
Re: svn commit: r901578 - in /httpd/httpd/trunk: CHANGES modules/http/http_request.c
modules/metadata/mod_headers.c server/protocol.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 21.01.2010 08:20, wrowe@apache.org wrote:
> Author: wrowe
> Date: Thu Jan 21 07:19:41 2010
> New Revision: 901578
>
> URL: http://svn.apache.org/viewvc?rev=901578&view=rev
> Log:
> Correctly align the behavior of headers_in to be consistent with the
> treatment of headers_out, resolving PR 48359 by keeping subrequest
> scope changes out of the main request headers. This ensures that all
> requests-without-bodies behave as the requests-with-bodies code has.
>
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/modules/http/http_request.c
> httpd/httpd/trunk/modules/metadata/mod_headers.c
> httpd/httpd/trunk/server/protocol.c
>
> Modified: httpd/httpd/trunk/modules/http/http_request.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_request.c?rev=901578&r1=901577&r2=901578&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/http/http_request.c (original)
> +++ httpd/httpd/trunk/modules/http/http_request.c Thu Jan 21 07:19:41 2010
> @@ -442,7 +442,7 @@
> new->request_time = r->request_time;
> new->main = r->main;
>
> - new->headers_in = r->headers_in;
> + new->headers_in = apr_table_copy(r->pool, r->headers_in);
> new->headers_out = apr_table_make(r->pool, 12);
> new->err_headers_out = r->err_headers_out;
> new->subprocess_env = rename_original_env(r->pool, r->subprocess_env);
> @@ -515,6 +515,8 @@
> r->per_dir_config = rr->per_dir_config;
> /* copy output headers from subrequest, but leave negotiation headers */
> r->notes = apr_table_overlay(r->pool, rr->notes, r->notes);
> + r->headers_in = apr_table_overlay(r->pool, rr->headers_in,
> + r->headers_in);
I think this is wrong. Think of the case where the subrequest does not change
anything on headers_in. In this case the contents of headers_in of the request
and the subrequest are the same (the subrequest has a copy of the requests
table). Doing an apr_table_overlay would result in a table that has all key
value pairs of the original headers_in doubled.
Regards
Rüdiger