Hi all
Whilst testing 2.0.55 we found that some of the changes to mod_proxy has
prevented one of our custom modules from working correctly. The problem
is not with the main purpose of the CL/TE security fixes, but a related
behaviour change that's happened along the way. Specifically, in
proxy_http.c:ap_proxy_http_request(), this:
if (r->main) {
[some lines snipped]
/* If you POST to a page that gets server-side parsed
* by mod_include, and the parsing results in a reverse
* proxy call, the proxied request will be a GET, but
* its request_rec will have inherited the Content-Length
* of the original request (the POST for the enclosing
* page). We can't send the original POST's request body
* as part of the proxied subrequest, so we need to avoid
* sending the corresponding content length. Otherwise,
* the server to which we're proxying will sit there
* forever, waiting for a request body that will never
* arrive.
*/
if ((r->method_number == M_GET) && headers_in[counter].key &&
!apr_strnatcasecmp(headers_in[counter].key,
"Content-Length")) {
continue;
}
has been replaced with:
/* sub-requests never use keepalives, and mustn't pass request bodies.
* Because the new logic looks at input_brigade, we will self-terminate
* input_brigade and jump past all of the request body logic...
* Reading anything with ap_get_brigade is likely to consume the
* main request's body or read beyond EOS - which would be unplesant.
*/
if (r->main) {
p_conn->close++;
if (old_cl_val) {
old_cl_val = NULL;
apr_table_unset(r->headers_in, "Content-Length");
}
if (old_te_val) {
old_te_val = NULL;
apr_table_unset(r->headers_in, "Transfer-Encoding");
}
rb_method = RB_STREAM_CL;
e = apr_bucket_eos_create(input_brigade->bucket_alloc);
APR_BRIGADE_INSERT_TAIL(input_brigade, e);
goto skip_body;
}
So in <=2.0.54 the CL header was not passed on for subrequests if it was
set and the request is a GET. Now both CL and TE are removed for *all*
subrequests, regardless of the HTTP method.
As you've probably guessed, in our module we turn [some] subrequests
into POSTs. (We then set an appropriate CL header and add an input
filter to the subrequest which generates the request body.) I've written
a simple patch which adds a check on the request method again (simply
skipping most of the above lines if it's a POST). However, the comment
"sub-requests ... mustn't pass request bodies" made me think perhaps
some discussion is required before I stick that in bugzilla. Is there a
reason behind the "musn't"? Ie: are POST subrequests a problem (assuming
there's a valid input_brigade to read and a corresponding CL)?
Some googling turns up a previous thread which touches on this issue:
http://marc.theaimsgroup.com/?l=apache-httpd-dev&m=111099194601082&w=2
As a side comment, the original check wouldn't be necessary if we knew
that the subrequest was well formed. The 2.0.54 code is there because
mod_include creates the subrequests as GETs, but leaves an invalid CL
header (in fact, it's using the main request's headers table - not a
copy). This probably applies to the other headers that mod_proxy ignores
as well.
TIA
--
Stuart Children
http://terminus.co.uk/
