*) mod_http2: Fixes LimitRequestFields configuration handling to compensate for

    browsers that send request headers as multiple ones to make best use of HTTP/2
    compression.
    Trunk version of patch:
      http://svn.apache.org/r1879832
    2.4.x:
      svn merge -c 1879832 ^/httpd/httpd/trunk .
    +1: icing, rpluem, minfrin

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

diff --git a/CHANGES b/CHANGES
index d69cfa81ceab6506f3a0b86df2935e1a3242d5f9..57800b3161f1dcc7a588b8630bb9b63da880a87f 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,14 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.4.44
 
+  *) mod_http2: 
+     Fixes <https://github.com/icing/mod_h2/issues/200>: 
+     "LimitRequestFields 0" now disables the limit, as documented.
+     Fixes <https://github.com/icing/mod_h2/issues/201>: 
+     Do not count repeated headers with same name against the field
+     count limit. The are merged internally, as if sent in a single HTTP/1 line.
+     [Stefan Eissing]
+
   *) mod_http2: Avoid segfaults in case of handling certain responses for
      already aborted connections.  [Stefan Eissing, Ruediger Pluem]
 

diff --git a/modules/http2/h2_request.c b/modules/http2/h2_request.c
index 202d56066a0612149723376ce266a79199afb8e9..149acad8182fa1f1a8c1be7170712d913455964c 100644 (file)
--- a/modules/http2/h2_request.c
+++ b/modules/http2/h2_request.c
@@ -47,9 +47,9 @@ typedef struct {
 static int set_h1_header(void *ctx, const char *key, const char *value)
 {
     h1_ctx *x = ctx;
-    x->status = h2_req_add_header(x->headers, x->pool, key, strlen(key), 
-                                  value, strlen(value));
-    return (x->status == APR_SUCCESS)? 1 : 0;
+    int was_added;
+    h2_req_add_header(x->headers, x->pool, key, strlen(key), value, strlen(value), 0, &was_added);
+    return 1;
 }
 
 apr_status_t h2_request_rcreate(h2_request **preq, apr_pool_t *pool, 
@@ -99,10 +99,12 @@ apr_status_t h2_request_rcreate(h2_request **preq, apr_pool_t *pool,
 
 apr_status_t h2_request_add_header(h2_request *req, apr_pool_t *pool, 
                                    const char *name, size_t nlen,
-                                   const char *value, size_t vlen)
+                                   const char *value, size_t vlen,
+                                   size_t max_field_len, int *pwas_added)
 {
     apr_status_t status = APR_SUCCESS;
     
+    *pwas_added = 0;
     if (nlen <= 0) {
         return status;
     }
@@ -143,8 +145,9 @@ apr_status_t h2_request_add_header(h2_request *req, apr_pool_t *pool,
         }
     }
     else {
-        /* non-pseudo header, append to work bucket of stream */
-        status = h2_req_add_header(req->headers, pool, name, nlen, value, vlen);
+        /* non-pseudo header, add to table */
+        status = h2_req_add_header(req->headers, pool, name, nlen, value, vlen, 
+                                   max_field_len, pwas_added);
     }
     
     return status;

diff --git a/modules/http2/h2_request.h b/modules/http2/h2_request.h
index 48aee09d956d8ce084e61bda8c3c7c139d9fac58..b4a1a05a081a9c229f6649aa7db2490982bbdacf 100644 (file)
--- a/modules/http2/h2_request.h
+++ b/modules/http2/h2_request.h
@@ -24,7 +24,8 @@ apr_status_t h2_request_rcreate(h2_request **preq, apr_pool_t *pool,
 
 apr_status_t h2_request_add_header(h2_request *req, apr_pool_t *pool,
                                    const char *name, size_t nlen,
-                                   const char *value, size_t vlen);
+                                   const char *value, size_t vlen,
+                                   size_t max_field_len, int *pwas_added);
 
 apr_status_t h2_request_add_trailer(h2_request *req, apr_pool_t *pool,
                                     const char *name, size_t nlen,

diff --git a/modules/http2/h2_stream.c b/modules/http2/h2_stream.c
index eb61add2a50116dbe6045f0c6cb9dead29a6e664..77b764e1ded1f12dffb955af8d88d9bc1bd46601 100644 (file)
--- a/modules/http2/h2_stream.c
+++ b/modules/http2/h2_stream.c
@@ -654,11 +654,14 @@ static void set_error_response(h2_stream *stream, int http_status)
 
 static apr_status_t add_trailer(h2_stream *stream,
                                 const char *name, size_t nlen,
-                                const char *value, size_t vlen)
+                                const char *value, size_t vlen,
+                                size_t max_field_len, int *pwas_added)
 {
     conn_rec *c = stream->session->c;
     char *hname, *hvalue;
+    const char *existing;
 
+    *pwas_added = 0;
     if (nlen == 0 || name[0] == ':') {
         ap_log_cerror(APLOG_MARK, APLOG_DEBUG, APR_EINVAL, c, 
                       H2_STRM_LOG(APLOGNO(03060), stream, 
@@ -672,8 +675,15 @@ static apr_status_t add_trailer(h2_stream *stream,
         stream->trailers = apr_table_make(stream->pool, 5);
     }
     hname = apr_pstrndup(stream->pool, name, nlen);
-    hvalue = apr_pstrndup(stream->pool, value, vlen);
     h2_util_camel_case_header(hname, nlen);
+    existing = apr_table_get(stream->trailers, hname);
+    if (max_field_len 
+        && ((existing? strlen(existing)+2 : 0) + vlen + nlen + 2 > max_field_len)) {
+        /* "key: (oldval, )?nval" is too long */
+        return APR_EINVAL;
+    }
+    if (!existing) *pwas_added = 1;
+    hvalue = apr_pstrndup(stream->pool, value, vlen);
     apr_table_mergen(stream->trailers, hname, hvalue);
     ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c, 
                   H2_STRM_MSG(stream, "added trailer '%s: %s'"), hname, hvalue);
@@ -686,13 +696,13 @@ apr_status_t h2_stream_add_header(h2_stream *stream,
                                   const char *value, size_t vlen)
 {
     h2_session *session = stream->session;
-    int error = 0;
-    apr_status_t status;
+    int error = 0, was_added = 0;
+    apr_status_t status = APR_SUCCESS;
     
     if (stream->has_response) {
         return APR_EINVAL;    
     }
-    ++stream->request_headers_added;
+
     if (name[0] == ':') {
         if ((vlen) > session->s->limit_req_line) {
             /* pseudo header: approximation of request line size check */
@@ -703,9 +713,36 @@ apr_status_t h2_stream_add_header(h2_stream *stream,
                                           "LimitRequestFieldSize: %s"), name);
             }
             error = HTTP_REQUEST_URI_TOO_LARGE;
+            goto cleanup;
+        }
+    }
+    
+    if (session->s->limit_req_fields > 0 
+        && stream->request_headers_added > session->s->limit_req_fields) {
+        /* already over limit, count this attempt, but do not take it in */
+        ++stream->request_headers_added;
+    }
+    else if (H2_SS_IDLE == stream->state) {
+        if (!stream->rtmp) {
+            stream->rtmp = h2_req_create(stream->id, stream->pool, 
+                                         NULL, NULL, NULL, NULL, NULL, 0);
         }
+        status = h2_request_add_header(stream->rtmp, stream->pool,
+                                       name, nlen, value, vlen,
+                                       session->s->limit_req_fieldsize, &was_added);
+        if (was_added) ++stream->request_headers_added;
+    }
+    else if (H2_SS_OPEN == stream->state) {
+        status = add_trailer(stream, name, nlen, value, vlen,
+                             session->s->limit_req_fieldsize, &was_added);
+        if (was_added) ++stream->request_headers_added;
     }
-    else if ((nlen + 2 + vlen) > session->s->limit_req_fieldsize) {
+    else {
+        status = APR_EINVAL;
+        goto cleanup;
+    }
+    
+    if (APR_EINVAL == status) {
         /* header too long */
         if (!h2_stream_is_ready(stream)) {
             ap_log_cerror(APLOG_MARK, APLOG_INFO, 0, session->c,
@@ -714,13 +751,14 @@ apr_status_t h2_stream_add_header(h2_stream *stream,
                           (int)H2MIN(nlen, 80), name);
         }
         error = HTTP_REQUEST_HEADER_FIELDS_TOO_LARGE;
+        goto cleanup;
     }
     
-    if (stream->request_headers_added > session->s->limit_req_fields + 4) {
-        /* too many header lines, include 4 pseudo headers */
-        if (stream->request_headers_added 
-            > session->s->limit_req_fields + 4 + 100) {
-            /* yeah, right */
+    if (session->s->limit_req_fields > 0 
+        && stream->request_headers_added > session->s->limit_req_fields) {
+        /* too many header lines */
+        if (stream->request_headers_added > session->s->limit_req_fields + 100) {
+            /* yeah, right, this request is way over the limit, say goodbye */
             h2_stream_rst(stream, H2_ERR_ENHANCE_YOUR_CALM);
             return APR_ECONNRESET;
         }
@@ -730,28 +768,15 @@ apr_status_t h2_stream_add_header(h2_stream *stream,
                                       "exceeds LimitRequestFields"));
         }
         error = HTTP_REQUEST_HEADER_FIELDS_TOO_LARGE;
+        goto cleanup;
     }
     
+cleanup:
     if (error) {
         set_error_response(stream, error);
         return APR_EINVAL; 
     }
-    else if (H2_SS_IDLE == stream->state) {
-        if (!stream->rtmp) {
-            stream->rtmp = h2_req_create(stream->id, stream->pool, 
-                                         NULL, NULL, NULL, NULL, NULL, 0);
-        }
-        status = h2_request_add_header(stream->rtmp, stream->pool,
-                                       name, nlen, value, vlen);
-    }
-    else if (H2_SS_OPEN == stream->state) {
-        status = add_trailer(stream, name, nlen, value, vlen);
-    }
-    else {
-        status = APR_EINVAL;
-    }
-    
-    if (status != APR_SUCCESS) {
+    else if (status != APR_SUCCESS) {
         ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, session->c,
                       H2_STRM_MSG(stream, "header %s not accepted"), name);
         h2_stream_dispatch(stream, H2_SEV_CANCELLED);

diff --git a/modules/http2/h2_util.c b/modules/http2/h2_util.c
index dd758f3d01b2fbeb7eab00c667044e02327f1bc3..a00dba4fa736561f5626e2418dfb8cd456d923de 100644 (file)
--- a/modules/http2/h2_util.c
+++ b/modules/http2/h2_util.c
@@ -1803,22 +1803,29 @@ int h2_res_ignore_trailer(const char *name, size_t len)
 }
 
 apr_status_t h2_req_add_header(apr_table_t *headers, apr_pool_t *pool, 
-                               const char *name, size_t nlen,
-                               const char *value, size_t vlen)
+                              const char *name, size_t nlen,
+                              const char *value, size_t vlen,
+                              size_t max_field_len, int *pwas_added)
 {
     char *hname, *hvalue;
+    const char *existing;
     
+    *pwas_added = 0;
     if (h2_req_ignore_header(name, nlen)) {
         return APR_SUCCESS;
     }
     else if (H2_HD_MATCH_LIT("cookie", name, nlen)) {
-        const char *existing = apr_table_get(headers, "cookie");
+        existing = apr_table_get(headers, "cookie");
         if (existing) {
             char *nval;
             
             /* Cookie header come separately in HTTP/2, but need
              * to be merged by "; " (instead of default ", ")
              */
+            if (max_field_len && strlen(existing) + vlen + nlen + 4 > max_field_len) {
+                /* "key: oldval, nval" is too long */
+                return APR_EINVAL;
+            }
             hvalue = apr_pstrndup(pool, value, vlen);
             nval = apr_psprintf(pool, "%s; %s", existing, hvalue);
             apr_table_setn(headers, "Cookie", nval);
@@ -1832,8 +1839,16 @@ apr_status_t h2_req_add_header(apr_table_t *headers, apr_pool_t *pool,
     }
     
     hname = apr_pstrndup(pool, name, nlen);
-    hvalue = apr_pstrndup(pool, value, vlen);
     h2_util_camel_case_header(hname, nlen);
+    existing = apr_table_get(headers, hname);
+    if (max_field_len) {
+        if ((existing? strlen(existing)+2 : 0) + vlen + nlen + 2 > max_field_len) {
+            /* "key: (oldval, )?nval" is too long */
+            return APR_EINVAL;
+        }
+    }
+    if (!existing) *pwas_added = 1;
+    hvalue = apr_pstrndup(pool, value, vlen);
     apr_table_mergen(headers, hname, hvalue);
     
     return APR_SUCCESS;

diff --git a/modules/http2/h2_util.h b/modules/http2/h2_util.h
index 470eeb050f6fdc493dafa961ac03f1c6688273cb..d182caf03354ee992e50c1a279bae8c0e9c696c0 100644 (file)
--- a/modules/http2/h2_util.h
+++ b/modules/http2/h2_util.h
@@ -410,9 +410,14 @@ apr_status_t h2_res_create_ngheader(h2_ngheader **ph, apr_pool_t *p,
 apr_status_t h2_req_create_ngheader(h2_ngheader **ph, apr_pool_t *p, 
                                     const struct h2_request *req);
 
+/**
+ * Add a HTTP/2 header and return the table key if it really was added
+ * and not ignored.
+ */
 apr_status_t h2_req_add_header(apr_table_t *headers, apr_pool_t *pool, 
                                const char *name, size_t nlen,
-                               const char *value, size_t vlen);
+                               const char *value, size_t vlen,
+                               size_t max_field_len, int *pwas_added);
 
 /*******************************************************************************
  * h2_request helpers

diff --git a/modules/http2/h2_version.h b/modules/http2/h2_version.h
index b12c40c4fc01be595ff5421ec1dc4c58f4d3655a..7083f097cba7cd610a093eddefcd1c398525c725 100644 (file)
--- a/modules/http2/h2_version.h
+++ b/modules/http2/h2_version.h
@@ -35,6 +35,6 @@
  * release. This is a 24 bit number with 8 bits for major number, 8 bits
  * for minor and 8 bits for patch. Version 1.2.3 becomes 0x010203.
  */
-#define MOD_HTTP2_VERSION_NUM 0x010f0b
+#define MOD_HTTP2_VERSION_NUM 0x010f0c
 
 #endif /* mod_h2_h2_version_h */