From: Ruediger Pluem Date: Tue, 24 Aug 2010 06:33:01 +0000 (+0000) Subject: Merge r620133, r724515, r724805, r725077, r952828 from trunk: X-Git-Tag: 2.2.17~73 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8ef0ba69d5d93cee33b6583ccb19ee418d08573a;p=thirdparty%2Fapache%2Fhttpd.git Merge r620133, r724515, r724805, r725077, r952828 from trunk: Sub-requests are created and used with two purposes; sometimes simply to 'see' what a request would do; as to fill out an SSI, validate access or similar - and is then discarded. And sometimes as the precursor to becoming the actual request; e.g. when mod_dir checks if an /index.html can be served for a '/'. In the latter case it is important to preserve the output filters 'for real'; whereas in the first case they have to be reset to purely the minimal proto filters (if at all). This patch instates the output filters in 3 cases where sub-requests are/may in fact be used as the real request later on. This is a relatively risky change (which should not be back-ported without further discussion) and may break caches in combination with internal redirects/vary/negotiation in subtle ways. See the thread starting at [1] and in particular the general concerns of rpluem at [2] with respect to sub requests and (fast_)internal redirects possibly needing a more thorough overhaul. 1: http://mail-archives.apache.org/mod_mbox/httpd-dev/200802.mbox/ajax/%3c335D1A4B-25E2-4FF1-8CDF-5010A7FBD293@webweaving.org%3e 2: http://mail-archives.apache.org/mod_mbox/httpd-dev/200802.mbox/%3c47ACE1D4.4060702@apache.org%3e * Correctly remove the SUBREQ_CORE filter from the filter chain if we do an internal fast redirect and if the new redirected request is NO subrequest. This fixes at least one of the possible subtle issues mentioned in the comment to r620133. reset chain if we need to... Hopefully the final fix for the subreq/filter issue. The prob was that we at this point could still have some stale and incorrect refs when we adjusted the f-stack. So move the update earlier so when we adjust, we're affecting r. Ruediger and Jim pretty much simultaneously :) * modules/http/http_request.c (internal_internal_redirect): For a subrequest, preserve any filters in the output filter chain which were not specific to the subrequest across the redirect (where f->r does not point to the subreq's request_rec). PR: 17629, 43939 Reviewed by: rpluem, jim, niq git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.x@988400 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index fc7c31181a8..74e4fd6aa55 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,13 @@ -*- coding: utf-8 -*- Changes with Apache 2.2.17 + *) mod_dir, mod_negotiation: Pass the output filter information + to newly created sub requests; as these are later on used + as true requests with an internal redirect. This allows for + mod_cache et.al. to trap the results of the redirect. + PR 17629, 43939 + [Dirk-Willem van Gulik, Jim Jagielski, Joe Orton, Ruediger Pluem] + *) rotatelogs: Fix possible buffer overflow if admin configures a mongo log file path. [Jeff Trawick] diff --git a/STATUS b/STATUS index 56c3f5ae8a5..f38c46bb618 100644 --- a/STATUS +++ b/STATUS @@ -97,27 +97,6 @@ PATCHES ACCEPTED TO BACKPORT FROM TRUNK: 2.2.x patch: http://people.apache.org/~sf/log_cookie_28037.diff +1: sf, jim, rpluem - * core: Pass the output filter information to newly created sub requests; - as these are later on used as true requests with an internal redirect. - This allows for mod_cache et.al. to trap the results of the redirect. - Furthermore adjust the output filter chain correctly in an internal - redirect from a subrequest, preserving filters from the main request as - necessary. - PR 17629, PR 43939 - Hint: Both PR need all revisions applied neither can be fixed by a subset - of the revisions below. - Trunk version of patch: - http://svn.apache.org/viewcvs.cgi?rev=620133&view=rev - http://svn.apache.org/viewcvs.cgi?rev=724515&view=rev - http://svn.apache.org/viewcvs.cgi?rev=724805&view=rev - http://svn.apache.org/viewcvs.cgi?rev=725077&view=rev - http://svn.apache.org/viewcvs.cgi?rev=952828&view=rev - Backport version for 2.2.x of patch: - Trunk version of patch works - For convenience (CHANGES might need some addition): - http://people.apache.org/~rjung/patches/pr17629-2_2_x.patch - +1: rpluem, jim, niq - PATCHES PROPOSED TO BACKPORT FROM TRUNK: [ New proposals should be added at the end of the list ] diff --git a/modules/http/http_request.c b/modules/http/http_request.c index 3a7e5ef9858..251aa33b417 100644 --- a/modules/http/http_request.c +++ b/modules/http/http_request.c @@ -399,16 +399,46 @@ static request_rec *internal_internal_redirect(const char *new_uri, new->proto_output_filters = r->proto_output_filters; new->proto_input_filters = r->proto_input_filters; - new->output_filters = new->proto_output_filters; new->input_filters = new->proto_input_filters; if (new->main) { - /* Add back the subrequest filter, which we lost when - * we set output_filters to include only the protocol - * output filters from the original request. - */ - ap_add_output_filter_handle(ap_subreq_core_filter_handle, - NULL, new, new->connection); + ap_filter_t *f, *nextf; + + /* If this is a subrequest, the filter chain may contain a + * mixture of filters specific to the old request (r), and + * some inherited from r->main. Here, inherit that filter + * chain, and remove all those which are specific to the old + * request; ensuring the subreq filter is left in place. */ + new->output_filters = r->output_filters; + + f = new->output_filters; + do { + nextf = f->next; + + if (f->r == r && f->frec != ap_subreq_core_filter_handle) { + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, + "dropping filter '%s' in internal redirect from %s to %s", + f->frec->name, r->unparsed_uri, new_uri); + + /* To remove the filter, first set f->r to the *new* + * request_rec, so that ->output_filters on 'new' is + * changed (if necessary) when removing the filter. */ + f->r = new; + ap_remove_output_filter(f); + } + + f = nextf; + + /* Stop at the protocol filters. If a protocol filter has + * been newly installed for this resource, better leave it + * in place, though it's probably a misconfiguration or + * filter bug to get into this state. */ + } while (f && f != new->proto_output_filters); + } + else { + /* If this is not a subrequest, clear out all + * resource-specific filters. */ + new->output_filters = new->proto_output_filters; } update_r_in_filters(new->input_filters, r, new); @@ -466,15 +496,6 @@ AP_DECLARE(void) ap_internal_fast_redirect(request_rec *rr, request_rec *r) r->output_filters = rr->output_filters; r->input_filters = rr->input_filters; - if (r->main) { - ap_add_output_filter_handle(ap_subreq_core_filter_handle, - NULL, r, r->connection); - } - else if (r->output_filters->frec == ap_subreq_core_filter_handle) { - ap_remove_output_filter(r->output_filters); - r->output_filters = r->output_filters->next; - } - /* If any filters pointed at the now-defunct rr, we must point them * at our "new" instance of r. In particular, some of rr's structures * will now be bogus (say rr->headers_out). If a filter tried to modify @@ -483,6 +504,32 @@ AP_DECLARE(void) ap_internal_fast_redirect(request_rec *rr, request_rec *r) */ update_r_in_filters(r->input_filters, rr, r); update_r_in_filters(r->output_filters, rr, r); + + if (r->main) { + ap_add_output_filter_handle(ap_subreq_core_filter_handle, + NULL, r, r->connection); + } + else { + /* + * We need to check if we now have the SUBREQ_CORE filter in our filter + * chain. If this is the case we need to remove it since we are NO + * subrequest. But we need to keep in mind that the SUBREQ_CORE filter + * does not necessarily need to be the first filter in our chain. So we + * need to go through the chain. But we only need to walk up the chain + * until the proto_output_filters as the SUBREQ_CORE filter is below the + * protocol filters. + */ + ap_filter_t *next; + + next = r->output_filters; + while (next && (next->frec != ap_subreq_core_filter_handle) + && (next != r->proto_output_filters)) { + next = next->next; + } + if (next && (next->frec == ap_subreq_core_filter_handle)) { + ap_remove_output_filter(next); + } + } } AP_DECLARE(void) ap_internal_redirect(const char *new_uri, request_rec *r) diff --git a/modules/mappers/mod_dir.c b/modules/mappers/mod_dir.c index ab215ecf537..860b9dcec85 100644 --- a/modules/mappers/mod_dir.c +++ b/modules/mappers/mod_dir.c @@ -232,7 +232,7 @@ static int fixup_dir(request_rec *r) name_ptr = apr_pstrcat(r->pool, name_ptr, "?", r->args, NULL); } - rr = ap_sub_req_lookup_uri(name_ptr, r, NULL); + rr = ap_sub_req_lookup_uri(name_ptr, r, r->output_filters); /* The sub request lookup is very liberal, and the core map_to_storage * handler will almost always result in HTTP_OK as /foo/index.html diff --git a/modules/mappers/mod_negotiation.c b/modules/mappers/mod_negotiation.c index d3ae87c6231..6feb58c0a56 100644 --- a/modules/mappers/mod_negotiation.c +++ b/modules/mappers/mod_negotiation.c @@ -1165,8 +1165,10 @@ static int read_types_multi(negotiation_state *neg) /* Double check, we still don't multi-resolve non-ordinary files */ - if (sub_req->finfo.filetype != APR_REG) + if (sub_req->finfo.filetype != APR_REG) { + /* XXX sub req not destroyed -- may be a bug/unintentional ? */ continue; + } /* If it has a handler, we'll pretend it's a CGI script, * since that's a good indication of the sort of thing it @@ -2712,7 +2714,7 @@ static int setup_choice_response(request_rec *r, negotiation_state *neg, if (!variant->sub_req) { int status; - sub_req = ap_sub_req_lookup_file(variant->file_name, r, NULL); + sub_req = ap_sub_req_lookup_file(variant->file_name, r, r->output_filters); status = sub_req->status; if (status != HTTP_OK && @@ -3123,7 +3125,7 @@ static int handle_multi(request_rec *r) * a sub_req structure yet. Get one now. */ - sub_req = ap_sub_req_lookup_file(best->file_name, r, NULL); + sub_req = ap_sub_req_lookup_file(best->file_name, r, r->output_filters); if (sub_req->status != HTTP_OK) { res = sub_req->status; ap_destroy_sub_req(sub_req);