http2: refactor multiple internals

* eliminate pooling of Nghttp2Stream instances. After testing,
  the pooling is not having any tangible benefit
  and makes things more complicated. Simplify. Simplify.

* refactor inbound headers

* Enforce MAX_HEADERS_LIST setting and limit the number of header
  pairs accepted from the peer. Use the ENHANCE_YOUR_CALM error
  code when receiving either too many headers or too many octets.
  Use a vector to store the headers instead of a queue

PR-URL: #16676
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

doc/api/http2.md

@@ -1470,11 +1470,18 @@ not be emitted.
 ### http2.createServer(options[, onRequestHandler])
 <!-- YAML
 added: v8.4.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/16676
+    description: Added the `maxHeaderListPairs` option with a default limit of
+                 128 header pairs.
 -->
 
 * `options` {Object}
   * `maxDeflateDynamicTableSize` {number} Sets the maximum dynamic table size
     for deflating header fields. **Default:** `4Kib`
+  * `maxHeaderListPairs` {number} Sets the maximum number of header entries.
+    **Default:** `128`. The minimum value is `4`.
   * `maxSendHeaderBlockLength` {number} Sets the maximum allowed size for a
     serialized, compressed block of headers. Attempts to send headers that
     exceed this limit will result in a `'frameError'` event being emitted
@@ -1525,6 +1532,11 @@ server.listen(80);
 ### http2.createSecureServer(options[, onRequestHandler])
 <!-- YAML
 added: v8.4.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/16676
+    description: Added the `maxHeaderListPairs` option with a default limit of
+                 128 header pairs.
 -->
 
 * `options` {Object}
@@ -1533,6 +1545,8 @@ added: v8.4.0
     `false`. See the [`'unknownProtocol'`][] event. See [ALPN negotiation][].
   * `maxDeflateDynamicTableSize` {number} Sets the maximum dynamic table size
     for deflating header fields. **Default:** `4Kib`
+  * `maxHeaderListPairs` {number} Sets the maximum number of header entries.
+    **Default:** `128`. The minimum value is `4`.
   * `maxSendHeaderBlockLength` {number} Sets the maximum allowed size for a
     serialized, compressed block of headers. Attempts to send headers that
     exceed this limit will result in a `'frameError'` event being emitted
@@ -1590,12 +1604,19 @@ server.listen(80);
 ### http2.connect(authority[, options][, listener])
 <!-- YAML
 added: v8.4.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/16676
+    description: Added the `maxHeaderListPairs` option with a default limit of
+                 128 header pairs.
 -->
 
 * `authority` {string|URL}
 * `options` {Object}
   * `maxDeflateDynamicTableSize` {number} Sets the maximum dynamic table size
     for deflating header fields. **Default:** `4Kib`
+  * `maxHeaderListPairs` {number} Sets the maximum number of header entries.
+    **Default:** `128`. The minimum value is `1`.
   * `maxReservedRemoteStreams` {number} Sets the maximum number of reserved push
     streams the client will accept at any given time. Once the current number of
     currently reserved push streams exceeds reaches this limit, new push streams
@@ -1747,7 +1768,13 @@ server.on('stream', (stream, headers) => {
 ```
 
 ### Settings Object
-
+<!-- YAML
+added: v8.4.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/16676
+    description: The `maxHeaderListSize` setting is now strictly enforced.
+-->
 The `http2.getDefaultSettings()`, `http2.getPackedSettings()`,
 `http2.createServer()`, `http2.createSecureServer()`,
 `http2session.settings()`, `http2session.localSettings`, and
@@ -1773,8 +1800,8 @@ properties.
   concurrently at any given time in an `Http2Session`. The minimum value is
   0. The maximum allowed value is 2<sup>31</sup>-1.
 * `maxHeaderListSize` {number} Specifies the maximum size (uncompressed octets)
-  of header list that will be accepted. There is no default value. The minimum
-  allowed value is 0. The maximum allowed value is 2<sup>32</sup>-1.
+  of header list that will be accepted. The minimum allowed value is 0. The
+  maximum allowed value is 2<sup>32</sup>-1. **Default:** 65535.
 
 All additional properties on the settings object are ignored.
 

lib/internal/http2/util.js

@@ -172,7 +172,8 @@ const IDX_OPTIONS_MAX_RESERVED_REMOTE_STREAMS = 1;
 const IDX_OPTIONS_MAX_SEND_HEADER_BLOCK_LENGTH = 2;
 const IDX_OPTIONS_PEER_MAX_CONCURRENT_STREAMS = 3;
 const IDX_OPTIONS_PADDING_STRATEGY = 4;
-const IDX_OPTIONS_FLAGS = 5;
+const IDX_OPTIONS_MAX_HEADER_LIST_PAIRS = 5;
+const IDX_OPTIONS_FLAGS = 6;
 
 function updateOptionsBuffer(options) {
   var flags = 0;
@@ -201,6 +202,11 @@ function updateOptionsBuffer(options) {
     optionsBuffer[IDX_OPTIONS_PADDING_STRATEGY] =
       options.paddingStrategy;
   }
+  if (typeof options.maxHeaderListPairs === 'number') {
+    flags |= (1 << IDX_OPTIONS_MAX_HEADER_LIST_PAIRS);
+    optionsBuffer[IDX_OPTIONS_MAX_HEADER_LIST_PAIRS] =
+      options.maxHeaderListPairs;
+  }
   optionsBuffer[IDX_OPTIONS_FLAGS] = flags;
 }
 

src/node_http2.cc

@@ -5,6 +5,7 @@
 #include "node_http2_state.h"
 
 #include <queue>
+#include <algorithm>
 
 namespace node {
 
@@ -20,8 +21,6 @@ using v8::Undefined;
 
 namespace http2 {
 
-Freelist<Nghttp2Stream, FREELIST_MAX> stream_free_list;
-
 Nghttp2Session::Callbacks Nghttp2Session::callback_struct_saved[2] = {
     Callbacks(false),
     Callbacks(true)};
@@ -67,6 +66,10 @@ Http2Options::Http2Options(Environment* env) {
             buffer.GetValue(IDX_OPTIONS_PADDING_STRATEGY));
     SetPaddingStrategy(strategy);
   }
+
+  if (flags & (1 << IDX_OPTIONS_MAX_HEADER_LIST_PAIRS)) {
+    SetMaxHeaderPairs(buffer[IDX_OPTIONS_MAX_HEADER_LIST_PAIRS]);
+  }
 }
 
 Http2Settings::Http2Settings(Environment* env) : env_(env) {
@@ -173,11 +176,14 @@ inline void Http2Settings::RefreshDefaults(Environment* env) {
       DEFAULT_SETTINGS_INITIAL_WINDOW_SIZE;
   buffer[IDX_SETTINGS_MAX_FRAME_SIZE] =
       DEFAULT_SETTINGS_MAX_FRAME_SIZE;
+  buffer[IDX_SETTINGS_MAX_HEADER_LIST_SIZE] =
+      DEFAULT_SETTINGS_MAX_HEADER_LIST_SIZE;
   buffer[IDX_SETTINGS_COUNT] =
     (1 << IDX_SETTINGS_HEADER_TABLE_SIZE) |
     (1 << IDX_SETTINGS_ENABLE_PUSH) |
     (1 << IDX_SETTINGS_INITIAL_WINDOW_SIZE) |
-    (1 << IDX_SETTINGS_MAX_FRAME_SIZE);
+    (1 << IDX_SETTINGS_MAX_FRAME_SIZE) |
+    (1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE);
 }
 
 Http2Priority::Http2Priority(Environment* env,
@@ -204,7 +210,10 @@ Http2Session::Http2Session(Environment* env,
 
   padding_strategy_ = opts.GetPaddingStrategy();
 
-  Init(type, *opts);
+  int32_t maxHeaderPairs = opts.GetMaxHeaderPairs();
+  maxHeaderPairs = type == NGHTTP2_SESSION_SERVER ?
+      std::max(maxHeaderPairs, 4) : std::max(maxHeaderPairs, 1);
+  Init(type, *opts, nullptr, maxHeaderPairs);
 
   // For every node::Http2Session instance, there is a uv_prepare_t handle
   // whose callback is triggered on every tick of the event loop. When
@@ -901,7 +910,8 @@ void Http2Session::OnTrailers(Nghttp2Stream* stream,
 
 void Http2Session::OnHeaders(
     Nghttp2Stream* stream,
-    std::queue<nghttp2_header>* headers,
+    nghttp2_header* headers,
+    size_t count,
     nghttp2_headers_category cat,
     uint8_t flags) {
   Local<Context> context = env()->context();
@@ -926,18 +936,19 @@ void Http2Session::OnHeaders(
   // like {name1: value1, name2: value2, name3: [value3, value4]}. We do it
   // this way for performance reasons (it's faster to generate and pass an
   // array than it is to generate and pass the object).
-  do {
+  size_t n = 0;
+  while (count > 0) {
     size_t j = 0;
-    while (!headers->empty() && j < arraysize(argv) / 2) {
-      nghttp2_header item = headers->front();
+    while (count > 0 && j < arraysize(argv) / 2) {
+      nghttp2_header item = headers[n++];
       // The header name and value are passed as external one-byte strings
       name_str =
           ExternalHeader::New<true>(env(), item.name).ToLocalChecked();
       value_str =
           ExternalHeader::New<false>(env(), item.value).ToLocalChecked();
       argv[j * 2] = name_str;
       argv[j * 2 + 1] = value_str;
-      headers->pop();
+      count--;
       j++;
     }
     // For performance, we pass name and value pairs to array.protototype.push
@@ -946,7 +957,7 @@ void Http2Session::OnHeaders(
     if (j > 0) {
       fn->Call(env()->context(), holder, j * 2, argv).ToLocalChecked();
     }
-  } while (!headers->empty());
+  }
 
   Local<Value> args[4] = {
     Integer::New(isolate, stream->id()),

src/node_http2.h

@@ -291,14 +291,6 @@ const char* nghttp2_errname(int rv) {
   }
 }
 
-#define DEFAULT_SETTINGS_HEADER_TABLE_SIZE 4096
-#define DEFAULT_SETTINGS_ENABLE_PUSH 1
-#define DEFAULT_SETTINGS_INITIAL_WINDOW_SIZE 65535
-#define DEFAULT_SETTINGS_MAX_FRAME_SIZE 16384
-#define MAX_MAX_FRAME_SIZE 16777215
-#define MIN_MAX_FRAME_SIZE DEFAULT_SETTINGS_MAX_FRAME_SIZE
-#define MAX_INITIAL_WINDOW_SIZE 2147483647
-
 // This allows for 4 default-sized frames with their frame headers
 static const size_t kAllocBufferSize = 4 * (16384 + 9);
 
@@ -323,19 +315,25 @@ class Http2Options {
     return options_;
   }
 
+  void SetMaxHeaderPairs(uint32_t max) {
+    max_header_pairs_ = max;
+  }
+
+  uint32_t GetMaxHeaderPairs() const {
+    return max_header_pairs_;
+  }
+
   void SetPaddingStrategy(padding_strategy_type val) {
-#if DEBUG
-    CHECK_LE(val, PADDING_STRATEGY_CALLBACK);
-#endif
     padding_strategy_ = static_cast<padding_strategy_type>(val);
   }
 
-  padding_strategy_type GetPaddingStrategy() {
+  padding_strategy_type GetPaddingStrategy() const {
     return padding_strategy_;
   }
 
  private:
   nghttp2_option* options_;
+  uint32_t max_header_pairs_ = DEFAULT_MAX_HEADER_LIST_PAIRS;
   padding_strategy_type padding_strategy_ = PADDING_STRATEGY_NONE;
 };
 
@@ -426,7 +424,8 @@ class Http2Session : public AsyncWrap,
 
   void OnHeaders(
       Nghttp2Stream* stream,
-      std::queue<nghttp2_header>* headers,
+      nghttp2_header* headers,
+      size_t count,
       nghttp2_headers_category cat,
       uint8_t flags) override;
   void OnStreamClose(int32_t id, uint32_t code) override;