POST subrequests

[Stuart Children <st...@terminus.co.uk>]

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/

Re: POST subrequests

[Joe Schaefer <jo...@sunstarsys.com>]

Stuart Children <st...@terminus.co.uk> writes:

> 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.

Good question.  In apreq I focused on the case where the main request
was issued a POST, and the subrequest was a GET.  In that case the 
subrequest doesn't get access to the parsed body via apreq.  OTOH,
I do think it should be possible to do a POST subrequest in httpd,
especially when you're being very careful about the state of the input 
filter chain.

> 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)?

I'm not sure what the rationale is behind forbidding it within httpd,
other than it being a complex situation that wasn't really possible 
in 1.3.

-- 
Joe Schaefer