From be12754eed565c3d57bbce5cf6e146fb941d58b1 Mon Sep 17 00:00:00 2001 From: Yann Ylavic Date: Fri, 6 Feb 2015 16:16:52 +0000 Subject: [PATCH] http: Make ap_die() robust against any HTTP error code and not modify response status (finally logged) when nothing is to be done. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1657881 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES | 3 ++ modules/http/http_request.c | 68 ++++++++++++++----------------------- 2 files changed, 29 insertions(+), 42 deletions(-) diff --git a/CHANGES b/CHANGES index 7349ad6093..a9b3723bb9 100644 --- a/CHANGES +++ b/CHANGES @@ -6,6 +6,9 @@ Changes with Apache 2.5.0 calls r:wsupgrade() can cause a child process crash. [Edward Lu ] + *) 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/modules/http/http_request.c b/modules/http/http_request.c index cdfec8b568..e0f7fc1e15 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. */ @@ -100,7 +103,7 @@ AP_DECLARE(void) ap_die(int type, request_rec *r) */ if (next) { ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01579) - "Custom error page caused AP_FILTER_ERROR"); + "Invalid response status %i", type); type = HTTP_INTERNAL_SERVER_ERROR; } else { @@ -108,20 +111,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 +137,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 +217,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) { if (c->keepalive != AP_CONN_CLOSE) { @@ -337,18 +343,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); } @@ -631,8 +626,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); @@ -648,12 +643,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 @@ -674,15 +664,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, ...) -- 2.50.1