You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by rp...@apache.org on 2010/08/24 08:33:01 UTC
svn commit: r988400 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS
modules/http/http_request.c modules/mappers/mod_dir.c
modules/mappers/mod_negotiation.c
Author: rpluem
Date: Tue Aug 24 06:33:01 2010
New Revision: 988400
URL: http://svn.apache.org/viewvc?rev=988400&view=rev
Log:
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
Modified:
httpd/httpd/branches/2.2.x/CHANGES
httpd/httpd/branches/2.2.x/STATUS
httpd/httpd/branches/2.2.x/modules/http/http_request.c
httpd/httpd/branches/2.2.x/modules/mappers/mod_dir.c
httpd/httpd/branches/2.2.x/modules/mappers/mod_negotiation.c
Modified: httpd/httpd/branches/2.2.x/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/CHANGES?rev=988400&r1=988399&r2=988400&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/CHANGES [utf-8] (original)
+++ httpd/httpd/branches/2.2.x/CHANGES [utf-8] Tue Aug 24 06:33:01 2010
@@ -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]
Modified: httpd/httpd/branches/2.2.x/STATUS
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/STATUS?rev=988400&r1=988399&r2=988400&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/STATUS (original)
+++ httpd/httpd/branches/2.2.x/STATUS Tue Aug 24 06:33:01 2010
@@ -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 ]
Modified: httpd/httpd/branches/2.2.x/modules/http/http_request.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/http/http_request.c?rev=988400&r1=988399&r2=988400&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/modules/http/http_request.c (original)
+++ httpd/httpd/branches/2.2.x/modules/http/http_request.c Tue Aug 24 06:33:01 2010
@@ -399,16 +399,46 @@ static request_rec *internal_internal_re
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_redire
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_redire
*/
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)
Modified: httpd/httpd/branches/2.2.x/modules/mappers/mod_dir.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/mappers/mod_dir.c?rev=988400&r1=988399&r2=988400&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/modules/mappers/mod_dir.c (original)
+++ httpd/httpd/branches/2.2.x/modules/mappers/mod_dir.c Tue Aug 24 06:33:01 2010
@@ -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
Modified: httpd/httpd/branches/2.2.x/modules/mappers/mod_negotiation.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/mappers/mod_negotiation.c?rev=988400&r1=988399&r2=988400&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/modules/mappers/mod_negotiation.c (original)
+++ httpd/httpd/branches/2.2.x/modules/mappers/mod_negotiation.c Tue Aug 24 06:33:01 2010
@@ -1165,8 +1165,10 @@ static int read_types_multi(negotiation_
/* 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
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);