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

diff --git a/CHANGES b/CHANGES
index 7349ad6093e69f7f86766e4ff7c250dc20badfe4..a9b3723bb9fdcec2388238bd8f35b02c9f1c8cd2 100644 (file)
--- 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 <Chaosed0 gmail.com>]
 
+  *) 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 cdfec8b5689a8b5b536c7de7d36a4e1f8549624c..e0f7fc1e15b66d95412a257710e9fd086a4278f4 100644 (file)
--- 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, ...)