Merge of PR 297

    Fixing limits on response header size

      * Fixed bug in handling over long response headers. When the 64 KB limit
        of nghttp2 was exceeded, the request was not reset and the client was
        left hanging, waiting for it. Now the stream is reset.
      * Added new directive `H2MaxHeaderBlockLen` to set the limit on response
        header sizes.

Author: icing

ChangeLog

@@ -1,3 +1,8 @@
+  * Fixed bug in handling over long response headers. When the 64 KB limit
+    of nghttp2 was exceeded, the request was not reset and the client was
+    left hanging, waiting for it. Now the stream is reset.
+  * Added new directive `H2MaxHeaderBlockLen` to set the limit on response
+    header sizes.
   * Fixed handling of Timeout vs. KeepAliveTimeout when first request on a
     connection was reset.
 

README.md

@@ -135,6 +135,7 @@ The following configuration directives are available:
   * [H2EarlyHint](#h2earlyhint)
   * [H2EarlyHints](#h2earlyhints)
   * [H2MaxDataFrameLen](#h2maxdataframelen)
+  * [H2MaxHeaderBlockLen](#h2maxheaderblocklen)
   * [H2MaxSessionStreams](#h2maxsessionstreams)
   * [H2MaxWorkerIdleSeconds](#h2maxworkeridleseconds)
   * [H2MaxWorkers](#h2maxworkers)
@@ -225,6 +226,14 @@ H2MaxDataFrameLen limits the maximum amount of response body bytes placed into a
 
 The module, by default, tries to use the maximum size possible, which is somewhat around 16KB. This sets the maximum. When less response data is available, smaller frames will be sent.
 
+### H2MaxHeaderBlockLen
+```
+Syntax:  H2MaxHeaderBlockLen n
+Default: H2MaxHeaderBlockLen 0
+Context: server config, virtual host
+```
+H2MaxHeaderBlockLen set the limit on the maximum size of the uncompressed headers of a response. This is the accumulated size of all headers in a response. Setting this to 0 means the nghttp2 default of 64 KB applies.
+
 ### H2MaxSessionStreams
 
 ```

mod_http2/h2_config.c

@@ -77,6 +77,7 @@ typedef struct h2_config {
     int output_buffered;
     apr_interval_time_t stream_timeout;/* beam timeout */
     int max_data_frame_len;          /* max # bytes in a single h2 DATA frame */
+    int max_hd_block_len;            /* max # bytes in a response header block */
     int proxy_requests;              /* act as forward proxy */
     int h2_websockets;               /* if mod_h2 negotiating WebSockets */
 } h2_config;
@@ -117,6 +118,7 @@ static h2_config defconf = {
     1,                      /* stream output buffered */
     -1,                     /* beam timeout */
     0,                      /* max DATA frame len, 0 == no extra limit */
+    0,                      /* max header block len, 0 == no extra limit */
     0,                      /* forward proxy */
     0,                      /* WebSockets negotiation, enabled */
 };
@@ -165,6 +167,7 @@ void *h2_config_create_svr(apr_pool_t *pool, server_rec *s)
     conf->output_buffered      = DEF_VAL;
     conf->stream_timeout       = DEF_VAL;
     conf->max_data_frame_len   = DEF_VAL;
+    conf->max_hd_block_len     = DEF_VAL;
     conf->proxy_requests       = DEF_VAL;
     conf->h2_websockets        = DEF_VAL;
     return conf;
@@ -216,6 +219,7 @@ static void *h2_config_merge(apr_pool_t *pool, void *basev, void *addv)
     n->padding_always       = H2_CONFIG_GET(add, base, padding_always);
     n->stream_timeout       = H2_CONFIG_GET(add, base, stream_timeout);
     n->max_data_frame_len   = H2_CONFIG_GET(add, base, max_data_frame_len);
+    n->max_hd_block_len     = H2_CONFIG_GET(add, base, max_hd_block_len);
     n->proxy_requests       = H2_CONFIG_GET(add, base, proxy_requests);
     n->h2_websockets        = H2_CONFIG_GET(add, base, h2_websockets);
     return n;
@@ -313,6 +317,8 @@ static apr_int64_t h2_srv_config_geti64(const h2_config *conf, h2_config_var_t v
             return H2_CONFIG_GET(conf, &defconf, proxy_requests);
         case H2_CONF_WEBSOCKETS:
             return H2_CONFIG_GET(conf, &defconf, h2_websockets);
+        case H2_CONF_MAX_HEADER_BLOCK_LEN:
+            return H2_CONFIG_GET(conf, &defconf, max_hd_block_len);
         default:
             return DEF_VAL;
     }
@@ -381,6 +387,8 @@ static void h2_srv_config_seti(h2_config *conf, h2_config_var_t var, int val)
         case H2_CONF_WEBSOCKETS:
             H2_CONFIG_SET(conf, h2_websockets, val);
             break;
+        case H2_CONF_MAX_HEADER_BLOCK_LEN:
+            H2_CONFIG_SET(conf, max_hd_block_len, val);
         default:
             break;
     }
@@ -650,6 +658,17 @@ static const char *h2_conf_set_max_data_frame_len(cmd_parms *cmd,
     return NULL;
 }
 
+static const char *h2_conf_set_max_hd_block_len(cmd_parms *cmd,
+                                                void *dirconf, const char *value)
+{
+    int val = (int)apr_atoi64(value);
+    if (val < 0) {
+        return "value must be 0 or larger";
+    }
+    CONFIG_CMD_SET(cmd, dirconf, H2_CONF_MAX_HEADER_BLOCK_LEN, val);
+    return NULL;
+}
+
 static const char *h2_conf_set_session_extra_files(cmd_parms *cmd,
                                                    void *dirconf, const char *value)
 {
@@ -1071,6 +1090,8 @@ const command_rec h2_cmds[] = {
                   RSRC_CONF, "set stream timeout"),
     AP_INIT_TAKE1("H2MaxDataFrameLen", h2_conf_set_max_data_frame_len, NULL,
                   RSRC_CONF, "maximum number of bytes in a single HTTP/2 DATA frame"),
+    AP_INIT_TAKE1("H2MaxHeaderBlockLen", h2_conf_set_max_hd_block_len, NULL,
+                  RSRC_CONF, "maximum number of bytes in a response header block"),
     AP_INIT_TAKE2("H2EarlyHint", h2_conf_add_early_hint, NULL,
                    OR_FILEINFO|OR_AUTHCFG, "add a a 'Link:' header for a 103 Early Hints response."),
     AP_INIT_TAKE1("H2ProxyRequests", h2_conf_set_proxy_requests, NULL,

mod_http2/h2_config.h

@@ -46,6 +46,7 @@ typedef enum {
     H2_CONF_MAX_DATA_FRAME_LEN,
     H2_CONF_PROXY_REQUESTS,
     H2_CONF_WEBSOCKETS,
+    H2_CONF_MAX_HEADER_BLOCK_LEN,
 } h2_config_var_t;
 
 struct apr_hash_t;

mod_http2/h2_session.c

@@ -624,6 +624,29 @@ static int on_frame_send_cb(nghttp2_session *ngh2,
     return 0;
 }
 
+static int on_frame_not_send_cb(nghttp2_session *ngh2,
+                            const nghttp2_frame *frame,
+                            int ngh2_err,
+                            void *user_data)
+{
+    h2_session *session = user_data;
+    int stream_id = frame->hd.stream_id;
+    h2_stream *stream;
+    char buffer[256];
+
+    stream = get_stream(session, stream_id);
+    h2_util_frame_print(frame, buffer, sizeof(buffer)/sizeof(buffer[0]));
+    ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, session->c1,
+                  H2_SSSN_LOG(APLOGNO(), session,
+                  "not sent FRAME[%s], error %d: %s"),
+                  buffer, ngh2_err, nghttp2_strerror(ngh2_err));
+    if(stream) {
+        h2_stream_rst(stream, NGHTTP2_PROTOCOL_ERROR);
+        return 0;
+    }
+    return NGHTTP2_ERR_CALLBACK_FAILURE;
+}
+
 #ifdef H2_NG2_INVALID_HEADER_CB
 static int on_invalid_header_cb(nghttp2_session *ngh2,
                                 const nghttp2_frame *frame, 
@@ -699,6 +722,7 @@ static apr_status_t init_callbacks(conn_rec *c, nghttp2_session_callbacks **pcb)
     NGH2_SET_CALLBACK(*pcb, on_header, on_header_cb);
     NGH2_SET_CALLBACK(*pcb, send_data, on_send_data_cb);
     NGH2_SET_CALLBACK(*pcb, on_frame_send, on_frame_send_cb);
+    NGH2_SET_CALLBACK(*pcb, on_frame_not_send, on_frame_not_send_cb);
 #ifdef H2_NG2_INVALID_HEADER_CB
     NGH2_SET_CALLBACK(*pcb, on_invalid_header, on_invalid_header_cb);
 #endif
@@ -988,6 +1012,11 @@ apr_status_t h2_session_create(h2_session **psession, conn_rec *c, request_rec *
      * handle them, just like the HTTP/1.1 parser does. */
     nghttp2_option_set_no_rfc9113_leading_and_trailing_ws_validation(options, 1);
 #endif
+
+    if(h2_config_sgeti(s, H2_CONF_MAX_HEADER_BLOCK_LEN) > 0)
+        nghttp2_option_set_max_send_header_block_length(options,
+            h2_config_sgeti(s, H2_CONF_MAX_HEADER_BLOCK_LEN));
+
     rv = nghttp2_session_server_new2(&session->ngh2, callbacks,
                                      session, options);
     nghttp2_session_callbacks_del(callbacks);

mod_http2/h2_stream.c

@@ -1721,10 +1721,10 @@ static apr_status_t stream_do_response(h2_stream *stream)
     if (nghttp2_is_fatal(ngrv)) {
         rv = APR_EGENERAL;
         h2_session_dispatch_event(stream->session,
-                                 H2_SESSION_EV_PROTO_ERROR, ngrv, nghttp2_strerror(rv));
+                                 H2_SESSION_EV_PROTO_ERROR, ngrv, nghttp2_strerror(ngrv));
         ap_log_cerror(APLOG_MARK, APLOG_ERR, rv, c1,
                       APLOGNO(10402) "submit_response: %s",
-                      nghttp2_strerror(rv));
+                      nghttp2_strerror(ngrv));
         goto cleanup;
     }
 

test/modules/http2/env.py

@@ -113,6 +113,9 @@ def __init__(self, env: HttpdTestEnv, extras: Dict[str, Any] = None):
                 "<Location \"/h2test/error\">",
                 "    SetHandler h2test-error",
                 "</Location>",
+                "<Location \"/h2test/tweak\">",
+                "    SetHandler h2test-tweak",
+                "</Location>",
             ]
         }))