mod_proxy_http: Make sure that when an ErrorDocument is served

from a reverse proxied URL, that the subrequest respects the status
of the original request. This brings the behaviour of proxy_handler
in line with default_handler. PR 47106.


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@909899 13f79535-47bb-0310-9956-ffa450edef68

2 files changed, 43 insertions, 16 deletions

diff --git a/CHANGES b/CHANGES
index ffe4e476ec..a8c87f589c 100644
--- a/CHANGES
+++ b/CHANGES
@@ -2,6 +2,11 @@
 
 Changes with Apache 2.3.7
 
+  *) mod_proxy_http: Make sure that when an ErrorDocument is served
+     from a reverse proxied URL, that the subrequest respects the status
+     of the original request. This brings the behaviour of proxy_handler
+     in line with default_handler. PR 47106. [Graham Leggett]
+
   *) Support wildcards in both the directory and file components of
      the path specified by the Include directive. [Graham Leggett]
 
diff --git a/modules/proxy/mod_proxy_http.c b/modules/proxy/mod_proxy_http.c
index 93a5533832..0bedea01d3 100644
--- a/modules/proxy/mod_proxy_http.c
+++ b/modules/proxy/mod_proxy_http.c
@@ -1368,6 +1368,10 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
         {"Keep-Alive", "Proxy-Authenticate", "TE", "Trailer", "Upgrade", NULL};
     int i;
     const char *te = NULL;
+    int original_status = r->status;
+    int proxy_status = OK;
+    const char *original_status_line = r->status_line;
+    const char *proxy_status_line = NULL;
 
     bb = apr_brigade_create(p, c->bucket_alloc);
     pass_bb = apr_brigade_create(p, c->bucket_alloc);
@@ -1481,7 +1485,7 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
 
             keepchar = buffer[12];
             buffer[12] = '\0';
-            r->status = atoi(&buffer[9]);
+            proxy_status = atoi(&buffer[9]);
 
             if (keepchar != '\0') {
                 buffer[12] = keepchar;
@@ -1492,8 +1496,13 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
                 buffer[12] = ' ';
                 buffer[13] = '\0';
             }
-            r->status_line = apr_pstrdup(p, &buffer[9]);
+            proxy_status_line = apr_pstrdup(p, &buffer[9]);
 
+            /* The status out of the front is the same as the status coming in
+             * from the back, until further notice.
+             */
+            r->status = proxy_status;
+            r->status_line = proxy_status_line;
 
             /* read the headers. */
             /* N.B. for HTTP/1.0 clients, we have to fold line-wrapped headers*/
@@ -1569,7 +1578,7 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
             if ((buf = apr_table_get(r->headers_out, "Content-Type"))) {
                 ap_set_content_type(r, apr_pstrdup(p, buf));
             }
-            if (!ap_is_HTTP_INFO(r->status)) {
+            if (!ap_is_HTTP_INFO(proxy_status)) {
                 ap_proxy_pre_http_request(origin, rp);
             }
 
@@ -1620,7 +1629,7 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
             backend->close += 1;
         }
 
-        if (ap_is_HTTP_INFO(r->status)) {
+        if (ap_is_HTTP_INFO(proxy_status)) {
             interim_response++;
         }
         else {
@@ -1659,7 +1668,7 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
          * ProxyPassReverse/etc from here to ap_proxy_read_headers
          */
 
-        if ((r->status == 401) && (conf->error_override)) {
+        if ((proxy_status == 401) && (conf->error_override)) {
             const char *buf;
             const char *wa = "WWW-Authenticate";
             if ((buf = apr_table_get(r->headers_out, wa))) {
@@ -1699,8 +1708,8 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
         /* send body - but only if a body is expected */
         if ((!r->header_only) &&                   /* not HEAD request */
             !interim_response &&                   /* not any 1xx response */
-            (r->status != HTTP_NO_CONTENT) &&      /* not 204 */
-            (r->status != HTTP_NOT_MODIFIED)) {    /* not 304 */
+            (proxy_status != HTTP_NO_CONTENT) &&      /* not 204 */
+            (proxy_status != HTTP_NOT_MODIFIED)) {    /* not 304 */
 
             /* We need to copy the output headers and treat them as input
              * headers as well.  BUT, we need to do this before we remove
@@ -1726,11 +1735,22 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
              * if we are overriding the errors, we can't put the content
              * of the page into the brigade
              */
-            if (!conf->error_override || !ap_is_HTTP_ERROR(r->status)) {
+            if (!conf->error_override || !ap_is_HTTP_ERROR(proxy_status)) {
                 /* read the body, pass it to the output filters */
                 apr_read_type_e mode = APR_NONBLOCK_READ;
                 int finish = FALSE;
 
+                /* Handle the case where the error document is itself reverse
+                 * proxied and was successful. We must maintain any previous
+                 * error status so that an underlying error (eg HTTP_NOT_FOUND)
+                 * doesn't become an HTTP_OK.
+                 */
+                if (conf->error_override && !ap_is_HTTP_ERROR(proxy_status)
+                        && ap_is_HTTP_ERROR(original_status)) {
+                    r->status = original_status;
+                    r->status_line = original_status_line;
+                }
+
                 do {
                     apr_off_t readbytes;
                     apr_status_t rv;
@@ -1847,25 +1867,27 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
 
     if (conf->error_override) {
         /* the code above this checks for 'OK' which is what the hook expects */
-        if (!ap_is_HTTP_ERROR(r->status))
+        if (!ap_is_HTTP_ERROR(proxy_status)) {
             return OK;
+        }
         else {
             /* clear r->status for override error, otherwise ErrorDocument
              * thinks that this is a recursive error, and doesn't find the
              * custom error page
              */
-            int status = r->status;
             r->status = HTTP_OK;
             /* Discard body, if one is expected */
             if (!r->header_only && /* not HEAD request */
-                (status != HTTP_NO_CONTENT) && /* not 204 */
-                (status != HTTP_NOT_MODIFIED)) { /* not 304 */
-               ap_discard_request_body(rp);
-           }
-            return status;
+                (proxy_status != HTTP_NO_CONTENT) && /* not 204 */
+                (proxy_status != HTTP_NOT_MODIFIED)) { /* not 304 */
+                ap_discard_request_body(rp);
+            }
+            return proxy_status;
         }
-    } else
+    }
+    else {
         return OK;
+    }
 }
 
 static