core: follow up to r1876664: allow ErrorDocument to read body when applicable

Unless ap_read_request() failed to read the request line or header, or
Transfer-Encoding is invalid, we can still provide the request body to custom
error handlers (ErrorDocument) that ask it (e.g. internal redirects to CGI).

So this commit splits early failure path (previously die_early label) in two,
die_unusable_input and die_before_hooks, where the latter preserves input
filters (including HTTP_IN).

Also, the code to apply the connection timeout and r->per_dir_config from the
server is now in a new apply_server_config() helper since it's used multiple
times. Note that apr_socket_timeout_set() is a noop if the new timeout is the
same as the one already in place, so there is no need to cache the old timeout
nor use apr_socket_timeout_get(). Likewise, r->server is initially set to
c->base_server so apply_server_config() is overall a noop when no change is
needed.

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

diff --git a/modules/http2/h2_request.c b/modules/http2/h2_request.c
index a3d3696704cf4340537ab0c16fe92698a55f1db0..9d96c300f331c0dd0a52588692c61820e2eb4b96 100644 (file)
--- a/modules/http2/h2_request.c
+++ b/modules/http2/h2_request.c
@@ -289,6 +289,8 @@ request_rec *h2_request_create_rec(const h2_request *req, conn_rec *c)
 
     /* Validate HTTP/1 request and select vhost. */
     if (!ap_parse_request_line(r) || !ap_check_request_header(r)) {
+        /* we may have switched to another server still */
+        r->per_dir_config = r->server->lookup_defaults;
         access_status = r->status;
         r->status = HTTP_OK;
         goto die;
@@ -328,12 +330,12 @@ die:
      * end the request with an EOR bucket for stream/pipeline accounting.
      */
     {
-        apr_bucket_brigade *tmp_bb;
-        tmp_bb = ap_acquire_brigade(c);
-        APR_BRIGADE_INSERT_TAIL(tmp_bb,
+        apr_bucket_brigade *eor_bb;
+        eor_bb = ap_acquire_brigade(c);
+        APR_BRIGADE_INSERT_TAIL(eor_bb,
                                 ap_bucket_eor_create(c->bucket_alloc, r));
-        ap_pass_brigade(c->output_filters, tmp_bb);
-        ap_release_brigade(c, tmp_bb);
+        ap_pass_brigade(c->output_filters, eor_bb);
+        ap_release_brigade(c, eor_bb);
     }
 
     r = NULL;

diff --git a/server/protocol.c b/server/protocol.c
index 57c696aba4a837bbe12ef9bac5e98a428318919d..aeff2d20020cf9c5250df4c85d59815a15ed8728 100644 (file)
--- a/server/protocol.c
+++ b/server/protocol.c
@@ -1430,12 +1430,21 @@ AP_DECLARE(request_rec *) ap_create_request(conn_rec *conn)
     return r;
 }
 
+/* Apply the server's timeout/config to the connection/request. */
+static void apply_server_config(request_rec *r)
+{
+    apr_socket_t *csd;
+
+    csd = ap_get_conn_socket(r->connection);
+    apr_socket_timeout_set(csd, r->server->timeout);
+
+    r->per_dir_config = r->server->lookup_defaults;
+}
+
 request_rec *ap_read_request(conn_rec *conn)
 {
     int access_status;
     apr_bucket_brigade *tmp_bb;
-    apr_socket_t *csd;
-    apr_interval_time_t cur_timeout;
 
     request_rec *r = ap_create_request(conn);
 
@@ -1461,7 +1470,7 @@ request_rec *ap_read_request(conn_rec *conn)
                               "request failed: malformed request line");
             }
             access_status = r->status;
-            goto die_early;
+            goto die_unusable_input;
 
         case HTTP_REQUEST_TIME_OUT:
             /* Just log, no further action on this connection. */
@@ -1481,12 +1490,7 @@ request_rec *ap_read_request(conn_rec *conn)
      * to the normal timeout mode as we fetch the header lines,
      * as necessary.
      */
-    csd = ap_get_conn_socket(conn);
-    apr_socket_timeout_get(csd, &cur_timeout);
-    if (cur_timeout != conn->base_server->timeout) {
-        apr_socket_timeout_set(csd, conn->base_server->timeout);
-        cur_timeout = conn->base_server->timeout;
-    }
+    apply_server_config(r);
 
     if (!r->assbackwards) {
         const char *tenc;
@@ -1497,7 +1501,7 @@ request_rec *ap_read_request(conn_rec *conn)
             ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00567)
                           "request failed: error reading the headers");
             access_status = r->status;
-            goto die_early;
+            goto die_unusable_input;
         }
 
         tenc = apr_table_get(r->headers_in, "Transfer-Encoding");
@@ -1513,7 +1517,7 @@ request_rec *ap_read_request(conn_rec *conn)
                               "client sent unknown Transfer-Encoding "
                               "(%s): %s", tenc, r->uri);
                 access_status = HTTP_BAD_REQUEST;
-                goto die_early;
+                goto die_unusable_input;
             }
 
             /* http://tools.ietf.org/html/draft-ietf-httpbis-p1-messaging-23
@@ -1526,22 +1530,6 @@ request_rec *ap_read_request(conn_rec *conn)
         }
     }
 
-    if (!ap_check_request_header(r)) {
-        access_status = r->status;
-        goto die_early;
-    }
-
-    /* we may have switched to another server */
-    r->per_dir_config = r->server->lookup_defaults;
-
-    /* Toggle to the Host:-based vhost's timeout mode to fetch the
-     * request body and send the response body, if needed.
-     */
-    if (cur_timeout != r->server->timeout) {
-        apr_socket_timeout_set(csd, r->server->timeout);
-        cur_timeout = r->server->timeout;
-    }
-
     /*
      * Add the HTTP_IN filter here to ensure that ap_discard_request_body
      * called by ap_die and by ap_send_error_response works correctly on
@@ -1551,6 +1539,17 @@ request_rec *ap_read_request(conn_rec *conn)
     ap_add_input_filter_handle(ap_http_input_filter_handle,
                                NULL, r, r->connection);
 
+    /* Validate Host/Expect headers and select vhost. */
+    if (!ap_check_request_header(r)) {
+        /* we may have switched to another server still */
+        apply_server_config(r);
+        access_status = r->status;
+        goto die_before_hooks;
+    }
+
+    /* we may have switched to another server */
+    apply_server_config(r);
+
     if ((access_status = ap_run_post_read_request(r))) {
         goto die;
     }
@@ -1560,7 +1559,9 @@ request_rec *ap_read_request(conn_rec *conn)
                             r->status);
     return r;
 
-die_early:
+    /* Everything falls through on failure */
+
+die_unusable_input:
     /* Input filters are in an undeterminate state, cleanup (including
      * CORE_IN's socket) such that any further attempt to read is EOF.
      */
@@ -1579,21 +1580,25 @@ die_early:
         conn->keepalive = AP_CONN_CLOSE;
     }
 
-    /* fallthru ap_die() (non recursive) */
+die_before_hooks:
+    /* First call to ap_die() (non recursive) */
     r->status = HTTP_OK;
+
 die:
     ap_die(access_status, r);
 
     /* ap_die() sent the response through the output filters, we must now
      * end the request with an EOR bucket for stream/pipeline accounting.
      */
-    tmp_bb = ap_acquire_brigade(conn);
-    APR_BRIGADE_INSERT_TAIL(tmp_bb,
-                            ap_bucket_eor_create(conn->bucket_alloc, r));
-    ap_pass_brigade(conn->output_filters, tmp_bb);
-    ap_release_brigade(conn, tmp_bb);
+    {
+        apr_bucket_brigade *eor_bb;
+        eor_bb = ap_acquire_brigade(conn);
+        APR_BRIGADE_INSERT_TAIL(eor_bb,
+                                ap_bucket_eor_create(conn->bucket_alloc, r));
+        ap_pass_brigade(conn->output_filters, eor_bb);
+        ap_release_brigade(conn, eor_bb);
+    }
 
-    /* fallthru */
 ignore:
     r = NULL;
     AP_READ_REQUEST_FAILURE((uintptr_t)r);