From: Eric Covener Date: Tue, 2 Dec 2008 22:28:21 +0000 (+0000) Subject: Merge r722213,r722081, and r721679 from trunk: X-Git-Tag: 2.2.11~8 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=47168163bddae558c166769b483e2df30f6a55c5;p=thirdparty%2Fapache%2Fhttpd.git Merge r722213,r722081, and r721679 from trunk: * Avoid sending no answer at all if a custom error page causes an AP_FILTER_ERROR. * allow ap_invoke_handler() to pass-through AP_FILTER_ERROR as if it were a reserved status code (OK/DECLINED/SUSPENDED). Prevents ap_die() from seeing a 500 error when the http header filter has already taken care of the proper error response * To be safe, consume the entire brigade after processing an error bucket in the HTTP output filter. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.x@722642 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index 8c3af59b385..052e3769230 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,16 @@ -*- coding: utf-8 -*- Changes with Apache 2.2.11 + *) core: When the ap_http_header_filter processes an error bucket, cleanup + the passed brigade before returning AP_FILTER_ERROR down the filter + chain. This unambiguously ensures the same error bucket isn't revisited + [Ruediger Pluem] + + *) core: Error responses set by filters were being coerced into 500 errors, + sometimes appended to the original error response. Log entry of: + 'Handler for (null) returned invalid result code -3' + [Eric Covener] + *) configure: Don't reject libtool 2.x PR 44817 [Arfrever Frehtes Taifersar Arahesis ] diff --git a/STATUS b/STATUS index db3dd4c75f3..73f98716379 100644 --- a/STATUS +++ b/STATUS @@ -86,27 +86,6 @@ RELEASE SHOWSTOPPERS: PATCHES ACCEPTED TO BACKPORT FROM TRUNK: [ start all new proposals below, under PATCHES PROPOSED. ] - * core: Allow ap_invoke_handler to pass AP_FILTER_ERROR through as if it - were a reserved return code (OK/DECLINED) instead of converting it to - internal server error. - Additionally: - * Clear the brigade in ap_http_header_filter when handling an error bucket. - * Avoid sending no answer at all if a custom error page causes - an AP_FILTER_ERROR. - Trunk version of patch: - http://svn.apache.org/viewvc?rev=721679&view=rev - http://svn.apache.org/viewvc?rev=722081&view=rev - http://svn.apache.org/viewvc?rev=722213&view=rev - Backport version for 2.2.x of updated patch: - http://people.apache.org/~covener/2.2.x-ap_filter_error+722081+722213.diff - +1: covener, rpluem, niq - rpluem says: 1. There is a bogus entry in your patch to CHANGES - 2. I guess we should also consider r722213, because - with the current state of the patch we IMHO create - a regression in the case that a custom error page - results in AP_FILTER_ERROR before sending the page. - covener says: r722213 added and extra CHANGES entry removed (reset niq's +1) - PATCHES PROPOSED TO BACKPORT FROM TRUNK: [ New proposals should be added at the end of the list ] diff --git a/modules/http/http_filters.c b/modules/http/http_filters.c index 9620fc9dfb0..89263d4dcb6 100644 --- a/modules/http/http_filters.c +++ b/modules/http/http_filters.c @@ -1135,7 +1135,11 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_http_header_filter(ap_filter_t *f, } } if (eb) { - ap_die(eb->status, r); + int status; + + status = eb->status; + apr_brigade_cleanup(b); + ap_die(status, r); return AP_FILTER_ERROR; } diff --git a/modules/http/http_request.c b/modules/http/http_request.c index 8287c938365..fe10a196220 100644 --- a/modules/http/http_request.c +++ b/modules/http/http_request.c @@ -80,7 +80,31 @@ AP_DECLARE(void) ap_die(int type, request_rec *r) request_rec *r_1st_err = r; if (type == AP_FILTER_ERROR) { - return; + ap_filter_t *next; + + /* + * Check if we still have the ap_http_header_filter in place. If + * this is the case we should not ignore AP_FILTER_ERROR here because + * it means that we have not sent any response at all and never + * will. This is bad. Sent an internal server error instead. + */ + next = r->output_filters; + while (next && (next->frec != ap_http_header_filter_handle)) { + next = next->next; + } + + /* + * If next != NULL then we left the while above because of + * next->frec == ap_http_header_filter + */ + if (next) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, + "Custom error page caused AP_FILTER_ERROR"); + type = HTTP_INTERNAL_SERVER_ERROR; + } + else { + return; + } } if (type == DONE) { diff --git a/server/config.c b/server/config.c index d10f4afe264..9747c284a63 100644 --- a/server/config.c +++ b/server/config.c @@ -378,6 +378,7 @@ AP_CORE_DECLARE(int) ap_invoke_handler(request_rec *r) "handler \"%s\" not found for: %s", r->handler, r->filename); } if ((result != OK) && (result != DONE) && (result != DECLINED) + && (result != AP_FILTER_ERROR) && !ap_is_HTTP_VALID_RESPONSE(result)) { /* If a module is deliberately returning something else * (request_rec in non-HTTP or proprietary extension?)