Added support for "ProxyErrorOverride" directive in mod_proxy_http2. (#304)

Author: icing

ChangeLog

@@ -1,3 +1,4 @@
+  * Added support for "ProxyErrorOverride" directive in mod_proxy_http2.
   * Fix a bug in calculating the log2 value of integers, used in push
     diaries and proxy window size calculations. Apache PR69741.
     [Benjamin P. Kallus]

mod_http2/h2_proxy_session.c

@@ -49,6 +49,7 @@ typedef struct h2_proxy_stream {
     unsigned int waiting_on_ping : 1;
     unsigned int headers_ended : 1;
     uint32_t error_code;
+    int proxy_status;
 
     apr_bucket_brigade *input;
     apr_off_t data_sent;
@@ -310,6 +311,15 @@ static int on_frame_recv(nghttp2_session *ngh2, const nghttp2_frame *frame,
                     ap_send_interim_response(r, 1);
                 }
             }
+            else if (r->status >= 400) {
+                proxy_dir_conf *dconf;
+                dconf = ap_get_module_config(r->per_dir_config, &proxy_module);
+                if (ap_proxy_should_override(dconf, r->status)) {
+                    apr_table_setn(r->notes, "proxy-error-override", "1");
+                    nghttp2_submit_rst_stream(ngh2, NGHTTP2_FLAG_NONE,
+                          frame->hd.stream_id, NGHTTP2_STREAM_CLOSED);
+                }
+            }
             stream_resume(stream);
             break;
         case NGHTTP2_PING:
@@ -856,8 +866,8 @@ static apr_status_t open_stream(h2_proxy_session *session, const char *url,
              * Host: header */
             authority = r->server->server_hostname;
             ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(10511)
-                          "HTTP/0.9 request (with no host line) "
-                          "on incoming request and preserve host set "
+                          "incoming HTTP/0.9 request (with no Host header) "
+                          "and preserve host set, "
                           "forcing hostname to be %s for uri %s",
                           authority, r->uri);
             apr_table_setn(r->headers_in, "Host", authority);

mod_http2/mod_proxy_http2.c

@@ -239,9 +239,15 @@ static void request_done(h2_proxy_ctx *ctx, request_rec *r,
                       ctx->id, touched, error_code);
         ctx->r_done = 1;
         if (touched) ctx->r_may_retry = 0;
-        ctx->r_status = error_code? HTTP_BAD_GATEWAY :
-            ((status == APR_SUCCESS)? OK :
-             ap_map_http_request_error(status, HTTP_SERVICE_UNAVAILABLE));
+        if (apr_table_get(r->notes, "proxy-error-override")) {
+            ctx->r_status = r->status;
+            r->status = OK;
+        }
+        else {
+          ctx->r_status = error_code? HTTP_BAD_GATEWAY :
+              ((status == APR_SUCCESS)? OK :
+               ap_map_http_request_error(status, HTTP_SERVICE_UNAVAILABLE));
+        }
     }
 }    
 
@@ -428,7 +434,12 @@ static int proxy_http2_handler(request_rec *r,
     if (ctx->cfront->aborted) goto cleanup;
     status = ctx_run(ctx);
 
-    if (ctx->r_status != APR_SUCCESS && ctx->r_may_retry && !ctx->cfront->aborted) {
+    if (apr_table_get(r->notes, "proxy-error-override")) {
+        /* pass on out */
+        ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, ctx->cfront,
+                      "proxy-error-override status %d", ctx->r_status);
+    }
+    else if (ctx->r_status != APR_SUCCESS && ctx->r_may_retry && !ctx->cfront->aborted) {
         /* Not successfully processed, but may retry, tear down old conn and start over */
         if (ctx->p_conn) {
             ctx->p_conn->close = 1;
@@ -463,7 +474,7 @@ static int proxy_http2_handler(request_rec *r,
 
     ap_set_module_config(ctx->cfront->conn_config, &proxy_http2_module, NULL);
     ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, ctx->cfront,
-                  APLOGNO(03377) "leaving handler");
+                  APLOGNO(03377) "leaving handler -> %d", ctx->r_status);
     return ctx->r_status;
 }
 

test/modules/http2/env.py

@@ -116,6 +116,16 @@ def __init__(self, env: HttpdTestEnv, extras: Dict[str, Any] = None):
                 "<Location \"/h2test/tweak\">",
                 "    SetHandler h2test-tweak",
                 "</Location>",
+                'ErrorDocument 405 "*waggles finger*"',
+                'ErrorDocument 406 "*not acceptable*"',
+                '<Location "/proxy/">',
+                '    ErrorDocument 405 "*proxy waggles finger*"',
+                '    ProxyErrorOverride  On 405',
+                '</Location>',
+                '<Location "/h2proxy/">',
+                '    ErrorDocument 405 "*h2proxy waggles finger*"',
+                '    ProxyErrorOverride  On 405',
+                '</Location>',
             ]
         }))
 

test/modules/http2/mod_h2test/mod_h2test.c

@@ -517,6 +517,13 @@ static int h2test_error_handler(request_rec *r)
     if (error != APR_SUCCESS) {
         return ap_map_http_request_error(error, HTTP_BAD_REQUEST);
     }
+    if (r->status >= 400) {
+        b = ap_bucket_error_create(r->status, NULL, r->pool, c->bucket_alloc);
+        APR_BRIGADE_INSERT_TAIL(bb, b);
+        ap_pass_brigade(r->output_filters, bb);
+        return OK;
+    }
+
     /* flush response */
     b = apr_bucket_flush_create(c->bucket_alloc);
     APR_BRIGADE_INSERT_TAIL(bb, b);

test/modules/http2/test_500_proxy.py

@@ -167,3 +167,11 @@ def test_h2_500_32(self, env, repeat):
                 "AH01110"   # Network error reading response
             ]
         )
+
+    # produce a HTTP error on the proxied end, check that ProxyErrorOverride works
+    def test_h2_500_33(self, env, repeat):
+        url = env.mkurl("https", "cgi", "/proxy/h2test/error?status=405")
+        r = env.curl_get(url)
+        assert r.exit_code == 0
+        assert r.response['status'] == 405, f'{r}'
+        assert r.stdout == '*proxy waggles finger*', f'{r}'

test/modules/http2/test_600_h2proxy.py

@@ -198,3 +198,27 @@ def test_h2_600_32(self, env, repeat):
         # stream (exit_code != 0) or give a 503 response.
         if r.exit_code == 0:
             assert r.response['status'] in [502, 503]
+
+    # produce a HTTP error on the proxied end, check we see orig error doc
+    def test_h2_600_33(self, env, repeat):
+        conf = H2Conf(env)
+        conf.add_vhost_cgi(h2proxy_self=True)
+        conf.install()
+        assert env.apache_restart() == 0
+        url = env.mkurl("https", "cgi", "/h2proxy/h2test/error?status=406")
+        r = env.curl_get(url)
+        assert r.exit_code == 0
+        assert r.response['status'] == 406, f'{r}'
+        assert r.stdout == '*not acceptable*', f'{r}'
+
+    # produce a HTTP error on the proxied end, check that ProxyErrorOverride works
+    def test_h2_600_34(self, env, repeat):
+        conf = H2Conf(env)
+        conf.add_vhost_cgi(h2proxy_self=True)
+        conf.install()
+        assert env.apache_restart() == 0
+        url = env.mkurl("https", "cgi", "/h2proxy/h2test/error?status=405")
+        r = env.curl_get(url)
+        assert r.exit_code == 0
+        assert r.response['status'] == 405, f'{r}'
+        assert r.stdout == '*h2proxy waggles finger*', f'{r}'