Merge r1881620, r1881635 from trunk:

Process early errors via a dummy HTTP/1.1 request as well

Process early errors via a dummy HTTP/1.1 request as well to ensure
that the request gets logged correctly and possible custom error
pages are considered. The previous way of directly sending a HTTP/2
answer with the HTTP status code appropriate for the error is more
efficient, but does not log the request nor sents a possible custom
error page.

* modules/http2/h2.h: Add http_status to h2_request struct and define
  H2_HTTP_STATUS_UNSET.

* modules/http2/h2_request.c(h2_request_create_rec): Check if
  http_status is set for the request and die with the
  status code it contains if set.

* modules/http2/h2_session.c(on_header_cb): Adjust the error condition
  now that we mark early errors via http_status: Only return an error
  if the status is not success and http_status is not H2_HTTP_STATUS_UNSET.

* modules/http2/h2_stream.c(set_error_response): Set http_status
  on the request instead of creating headers for a response and a
  respective brigade.

Github: closes #137

* Changelog for r1881620

Submitted by: rpluem
Reviewed by: rpluem, giovanni, ylavic

Github: closes #142

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1883475 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index 2603312f31ca6add39ee8727086dc39da74c5eb3..92c4d1d85ebba71aa8d1fde5f1a9b93e15af0392 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,10 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.4.47
 
+  *) mod_http2: Log requests and sent the configured error response in case of
+     early detected errors like too many or too long headers.
+     [Ruediger Pluem, Stefan Eissing]
+
   *) mod_md: lowered the required minimal libcurl version from 7.50 to 7.29
      as proposed by <alexander.gerasimov@codeit.pro>. [Stefan Eissing]
 

diff --git a/STATUS b/STATUS
index 3591db74edc4fc2eaeecf0e75ae58b3ab47abdda..e5416fc7bca0f025c62a1c287579af1bb8345ee8 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -154,16 +154,6 @@ PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
      2.4.x patch: http://people.apache.org/~covener/patches/wstunnel-decline.diff
      +1 covener, jim, rpluem, jfclere
 
-  *) mod_http2: Log requests and sent the configured error response in case of
-     early detected errors like too many or too long headers.
-     Trunk version of patch:
-        https://svn.apache.org/r1881620
-        https://svn.apache.org/r1881635
-     Backport version for 2.4.x of patch:
-        https://github.com/apache/httpd/pull/142
-        https://github.com/apache/httpd/pull/142.diff
-     +1: rpluem, giovanni, ylavic
-
   *) core: Avoid a core dump if 'AllowOverride nonfatal' is used in a configuration
      file.
      Trunk version of patch:

diff --git a/modules/http2/h2.h b/modules/http2/h2.h
index e074204c7390cc486a3578295b9d9d2db62dde40..08f59c44f903d07b17288ea535f5190ba7ee5cbc 100644 (file)
--- a/modules/http2/h2.h
+++ b/modules/http2/h2.h
@@ -141,8 +141,19 @@ struct h2_request {
     unsigned int chunked : 1;   /* iff request body needs to be forwarded as chunked */
     unsigned int serialize : 1; /* iff this request is written in HTTP/1.1 serialization */
     apr_off_t raw_bytes;        /* RAW network bytes that generated this request - if known. */
+    int http_status;            /* Store a possible HTTP status code that gets
+                                 * defined before creating the dummy HTTP/1.1
+                                 * request e.g. due to an error already
+                                 * detected.
+                                 */
 };
 
+/*
+ * A possible HTTP status code is not defined yet. See the http_status field
+ * in struct h2_request above for further explanation.
+ */
+#define H2_HTTP_STATUS_UNSET (0)
+
 typedef struct h2_headers h2_headers;
 
 struct h2_headers {

diff --git a/modules/http2/h2_request.c b/modules/http2/h2_request.c
index 149acad8182fa1f1a8c1be7170712d913455964c..51f61843630f78ea35ec6fbb3c30778c1e509d77 100644 (file)
--- a/modules/http2/h2_request.c
+++ b/modules/http2/h2_request.c
@@ -79,11 +79,12 @@ apr_status_t h2_request_rcreate(h2_request **preq, apr_pool_t *pool,
     }
     
     req = apr_pcalloc(pool, sizeof(*req));
-    req->method    = apr_pstrdup(pool, r->method);
-    req->scheme    = scheme;
-    req->authority = authority;
-    req->path      = path;
-    req->headers   = apr_table_make(pool, 10);
+    req->method      = apr_pstrdup(pool, r->method);
+    req->scheme      = scheme;
+    req->authority   = authority;
+    req->path        = path;
+    req->headers     = apr_table_make(pool, 10);
+    req->http_status = H2_HTTP_STATUS_UNSET;
     if (r->server) {
         req->serialize = h2_config_rgeti(r, H2_CONF_SER_HEADERS);
     }
@@ -327,6 +328,13 @@ request_rec *h2_request_create_rec(const h2_request *req, conn_rec *c)
         }
     }
 
+    if (req->http_status != H2_HTTP_STATUS_UNSET) {
+        access_status = req->http_status;
+        r->status = HTTP_OK;
+        /* Be safe and close the connection */
+        c->keepalive = AP_CONN_CLOSE;
+    }
+
     /*
      * 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

diff --git a/modules/http2/h2_session.c b/modules/http2/h2_session.c
index 33c9f7f28d1acaa045096f77f32032b075af9750..56254f2a8989cb8191d4b5ed532e1b7ddfb1c8b9 100644 (file)
--- a/modules/http2/h2_session.c
+++ b/modules/http2/h2_session.c
@@ -311,7 +311,9 @@ static int on_header_cb(nghttp2_session *ngh2, const nghttp2_frame *frame,
     
     status = h2_stream_add_header(stream, (const char *)name, namelen,
                                   (const char *)value, valuelen);
-    if (status != APR_SUCCESS && !h2_stream_is_ready(stream)) {
+    if (status != APR_SUCCESS
+        && (!stream->rtmp
+            || stream->rtmp->http_status == H2_HTTP_STATUS_UNSET)) {
         return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
     }
     return 0;

diff --git a/modules/http2/h2_stream.c b/modules/http2/h2_stream.c
index 77b764e1ded1f12dffb955af8d88d9bc1bd46601..dce1fb45b4be0643cb38bc6405bafa8e0bbdc286 100644 (file)
--- a/modules/http2/h2_stream.c
+++ b/modules/http2/h2_stream.c
@@ -639,16 +639,7 @@ void h2_stream_set_request(h2_stream *stream, const h2_request *r)
 static void set_error_response(h2_stream *stream, int http_status)
 {
     if (!h2_stream_is_ready(stream)) {
-        conn_rec *c = stream->session->c;
-        apr_bucket *b;
-        h2_headers *response;
-        
-        response = h2_headers_die(http_status, stream->request, stream->pool);
-        prep_output(stream);
-        b = apr_bucket_eos_create(c->bucket_alloc);
-        APR_BRIGADE_INSERT_HEAD(stream->out_buffer, b);
-        b = h2_bucket_headers_create(c->bucket_alloc, response);
-        APR_BRIGADE_INSERT_HEAD(stream->out_buffer, b);
+        stream->rtmp->http_status = http_status;
     }
 }