From: Jim Jagielski Date: Fri, 22 May 2015 13:30:43 +0000 (+0000) Subject: Merge r1657881, r1665643 from trunk: X-Git-Tag: 2.4.13~62 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=8e2ecf60172d09b10bcdacc4debddd8c714bafc8;p=apache Merge r1657881, r1665643 from trunk: http: Make ap_die() robust against any HTTP error code and not modify response status (finally logged) when nothing is to be done. ap_die(): follow up to r1657881. Use log level DEBUG for AP_FILTER_ERROR => HTTP_INTERNAL_SERVER_ERROR. Submitted by: ylavic Reviewed/backported by: jim git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1681114 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index dea2424d3a..79c085824e 100644 --- a/CHANGES +++ b/CHANGES @@ -134,6 +134,9 @@ Changes with Apache 2.4.13 a ProxyRemote forward-proxy. PR 55892. [Hendrik Harms , William Rowe, Yann Ylavic] + *) http: Make ap_die() robust against any HTTP error code and not modify + response status (finally logged) when nothing is to be done. [Yann Ylavic] + *) mod_proxy_connect/wstunnel: If both client and backend sides get readable at the same time, don't lose errors occuring while forwarding on the first side when none occurs next on the other side, and abort. [Yann Ylavic] diff --git a/STATUS b/STATUS index a0c97a775d..07a5c94d6f 100644 --- a/STATUS +++ b/STATUS @@ -105,12 +105,6 @@ RELEASE SHOWSTOPPERS: PATCHES ACCEPTED TO BACKPORT FROM TRUNK: [ start all new proposals below, under PATCHES PROPOSED. ] - *) http: Make ap_die() robust against any HTTP error code and not modify - response status (finally logged) when nothing is to be done. PR 56035. - trunk patch: http://svn.apache.org/r1657881 - http://svn.apache.org/r1665643 - 2.4.x patch: trunk works (modulo CHANGES) - +1: ylavic, minfrin, jim PATCHES PROPOSED TO BACKPORT FROM TRUNK: diff --git a/modules/http/http_request.c b/modules/http/http_request.c index 9f23a9ddec..7b06def940 100644 --- a/modules/http/http_request.c +++ b/modules/http/http_request.c @@ -73,19 +73,22 @@ static void update_r_in_filters(ap_filter_t *f, } } -AP_DECLARE(void) ap_die(int type, request_rec *r) +static void ap_die_r(int type, request_rec *r, int recursive_error) { - int error_index = ap_index_of_response(type); - char *custom_response = ap_response_code_string(r, error_index); - int recursive_error = 0; + char *custom_response; request_rec *r_1st_err = r; - if (type == AP_FILTER_ERROR) { + if (type == OK || type == DONE) { + ap_finalize_request_protocol(r); + return; + } + + if (!ap_is_HTTP_VALID_RESPONSE(type)) { 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 + * this is the case we should not ignore the 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. */ @@ -99,8 +102,14 @@ AP_DECLARE(void) ap_die(int type, request_rec *r) * next->frec == ap_http_header_filter */ if (next) { - ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01579) - "Custom error page caused AP_FILTER_ERROR"); + if (type != AP_FILTER_ERROR) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01579) + "Invalid response status %i", type); + } + else { + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02831) + "Response from AP_FILTER_ERROR"); + } type = HTTP_INTERNAL_SERVER_ERROR; } else { @@ -108,20 +117,13 @@ AP_DECLARE(void) ap_die(int type, request_rec *r) } } - if (type == DONE) { - ap_finalize_request_protocol(r); - return; - } - /* * The following takes care of Apache redirects to custom response URLs * Note that if we are already dealing with the response to some other * error condition, we just report on the original error, and give up on * any attempt to handle the other thing "intelligently"... */ - if (r->status != HTTP_OK) { - recursive_error = type; - + if (recursive_error != HTTP_OK) { while (r_1st_err->prev && (r_1st_err->prev->status != HTTP_OK)) r_1st_err = r_1st_err->prev; /* Get back to original error */ @@ -141,6 +143,11 @@ AP_DECLARE(void) ap_die(int type, request_rec *r) custom_response = NULL; /* Do NOT retry the custom thing! */ } + else { + int error_index = ap_index_of_response(type); + custom_response = ap_response_code_string(r, error_index); + recursive_error = 0; + } r->status = type; @@ -216,6 +223,11 @@ AP_DECLARE(void) ap_die(int type, request_rec *r) ap_send_error_response(r_1st_err, recursive_error); } +AP_DECLARE(void) ap_die(int type, request_rec *r) +{ + ap_die_r(type, r, r->status); +} + static void check_pipeline(conn_rec *c, apr_bucket_brigade *bb) { if (c->keepalive != AP_CONN_CLOSE && !c->aborted) { @@ -346,18 +358,7 @@ void ap_process_async_request(request_rec *r) apr_thread_mutex_unlock(r->invoke_mtx); #endif - if (access_status == DONE) { - /* e.g., something not in storage like TRACE */ - access_status = OK; - } - - if (access_status == OK) { - ap_finalize_request_protocol(r); - } - else { - r->status = HTTP_OK; - ap_die(access_status, r); - } + ap_die_r(access_status, r, HTTP_OK); ap_process_request_after_handler(r); } @@ -640,8 +641,8 @@ AP_DECLARE(void) ap_internal_fast_redirect(request_rec *rr, request_rec *r) AP_DECLARE(void) ap_internal_redirect(const char *new_uri, request_rec *r) { - request_rec *new = internal_internal_redirect(new_uri, r); int access_status; + request_rec *new = internal_internal_redirect(new_uri, r); AP_INTERNAL_REDIRECT(r->uri, new_uri); @@ -657,12 +658,7 @@ AP_DECLARE(void) ap_internal_redirect(const char *new_uri, request_rec *r) access_status = ap_invoke_handler(new); } } - if (access_status == OK) { - ap_finalize_request_protocol(new); - } - else { - ap_die(access_status, new); - } + ap_die(access_status, new); } /* This function is designed for things like actions or CGI scripts, when @@ -683,15 +679,9 @@ AP_DECLARE(void) ap_internal_redirect_handler(const char *new_uri, request_rec * ap_set_content_type(new, r->content_type); access_status = ap_process_request_internal(new); if (access_status == OK) { - if ((access_status = ap_invoke_handler(new)) != 0) { - ap_die(access_status, new); - return; - } - ap_finalize_request_protocol(new); - } - else { - ap_die(access_status, new); + access_status = ap_invoke_handler(new); } + ap_die(access_status, new); } AP_DECLARE(void) ap_allow_methods(request_rec *r, int reset, ...)