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: https://github.com/nodejs/node/pull/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
+  of header list that will be accepted. The minimum allowed value is 0. The
-  allowed value is 0. The maximum allowed value is 2<sup>32</sup>-1.
+  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);
 }
 
 
@@ -192,7 +198,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
@@ -911,7 +920,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();
@@ -936,10 +946,11 @@ 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) {
+    while (count > 0 && j < arraysize(argv) / 2) {
-      nghttp2_header item = headers->front();
+      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();
@@ -947,7 +958,7 @@ void Http2Session::OnHeaders(
           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
@@ -956,7 +967,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

@@ -292,14 +292,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);
 
@@ -324,19 +316,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;
 };
 
@@ -413,7 +411,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;

src/node_http2_core-inl.h

@@ -5,17 +5,11 @@
 
 #include "node_http2_core.h"
 #include "node_internals.h"  // arraysize
-#include "freelist.h"
+#include <algorithm>
 
 namespace node {
 namespace http2 {
 
-#define FREELIST_MAX 10240
-
-// Instances of Nghttp2Stream are created and pooled in order to speed
-// allocation under load.
-extern Freelist<Nghttp2Stream, FREELIST_MAX> stream_free_list;
-
 #ifdef NODE_DEBUG_HTTP2
 inline int Nghttp2Session::OnNghttpError(nghttp2_session* session,
                                          const char* message,
@@ -45,13 +39,36 @@ inline int Nghttp2Session::OnBeginHeadersCallback(nghttp2_session* session,
 
   Nghttp2Stream* stream = handle->FindStream(id);
   if (stream == nullptr) {
-    Nghttp2Stream::Init(id, handle, frame->headers.cat);
+    new Nghttp2Stream(id, handle, frame->headers.cat);
   } else {
     stream->StartHeaders(frame->headers.cat);
   }
   return 0;
 }
 
+inline size_t GetBufferLength(nghttp2_rcbuf* buf) {
+  return nghttp2_rcbuf_get_buf(buf).len;
+}
+
+inline bool Nghttp2Stream::AddHeader(nghttp2_rcbuf* name,
+                                     nghttp2_rcbuf* value,
+                                     uint8_t flags) {
+  size_t length = GetBufferLength(name) + GetBufferLength(value) + 32;
+  if (current_headers_.size() == max_header_pairs_ ||
+      current_headers_length_ + length > max_header_length_) {
+    return false;
+  }
+  nghttp2_header header;
+  header.name = name;
+  header.value = value;
+  header.flags = flags;
+  current_headers_.push_back(header);
+  nghttp2_rcbuf_incref(name);
+  nghttp2_rcbuf_incref(value);
+  current_headers_length_ += length;
+  return true;
+}
+
 // nghttp2 calls this once for every header name-value pair in a HEADERS
 // or PUSH_PROMISE block. CONTINUATION frames are handled automatically
 // and transparently so we do not need to worry about those at all.
@@ -68,15 +85,12 @@ inline int Nghttp2Session::OnHeaderCallback(nghttp2_session* session,
       frame->push_promise.promised_stream_id :
       frame->hd.stream_id;
   Nghttp2Stream* stream = handle->FindStream(id);
-  // The header name and value are stored in a reference counted buffer
+  if (!stream->AddHeader(name, value, flags)) {
-  // provided to us by nghttp2. We need to increment the reference counter
+    // This will only happen if the connected peer sends us more
-  // here, then decrement it when we're done using it later.
+    // than the allowed number of header items at any given time
-  nghttp2_rcbuf_incref(name);
+    stream->SubmitRstStream(NGHTTP2_ENHANCE_YOUR_CALM);
-  nghttp2_rcbuf_incref(value);
+    return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
-  nghttp2_header header;
+  }
-  header.name = name;
-  header.value = value;
-  stream->headers()->emplace(header);
   return 0;
 }
 
@@ -447,6 +461,7 @@ inline void Nghttp2Session::HandleHeadersFrame(const nghttp2_frame* frame) {
 #endif
   OnHeaders(stream,
             stream->headers(),
+            stream->headers_count(),
             stream->headers_category(),
             frame->hd.flags);
 }
@@ -551,12 +566,15 @@ inline void Nghttp2Session::SendPendingData() {
 // uv_loop_t.
 inline int Nghttp2Session::Init(const nghttp2_session_type type,
                                 nghttp2_option* options,
-                                nghttp2_mem* mem) {
+                                nghttp2_mem* mem,
+                                uint32_t maxHeaderPairs) {
   session_type_ = type;
   DEBUG_HTTP2("Nghttp2Session %s: initializing session\n", TypeName());
   destroying_ = false;
   int ret = 0;
 
+  max_header_pairs_ = maxHeaderPairs;
+
   nghttp2_session_callbacks* callbacks
       = callback_struct_saved[HasGetPaddingCallback() ? 1 : 0].callbacks;
 
@@ -637,44 +655,31 @@ inline void Nghttp2Session::RemoveStream(int32_t id) {
 
 // Implementation for Nghttp2Stream functions
 
-inline Nghttp2Stream* Nghttp2Stream::Init(
+Nghttp2Stream::Nghttp2Stream(
     int32_t id,
     Nghttp2Session* session,
     nghttp2_headers_category category,
-    int options) {
+    int options) : id_(id),
-  DEBUG_HTTP2("Nghttp2Stream %d: initializing stream\n", id);
+                   session_(session),
-  Nghttp2Stream* stream = stream_free_list.pop();
+                   current_headers_category_(category) {
-  stream->ResetState(id, session, category, options);
+  // Limit the number of header pairs
-  session->AddStream(stream);
+  max_header_pairs_ = session->GetMaxHeaderPairs();
-  return stream;
+  if (max_header_pairs_ == 0)
-}
+  max_header_pairs_ = DEFAULT_MAX_HEADER_LIST_PAIRS;
+  current_headers_.reserve(max_header_pairs_);
 
+  // Limit the number of header octets
+  max_header_length_ =
+      std::min(
+        nghttp2_session_get_local_settings(
+          session->session(),
+          NGHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE),
+      MAX_MAX_HEADER_LIST_SIZE);
 
-// Resets the state of the stream instance to defaults
-inline void Nghttp2Stream::ResetState(
-    int32_t id,
-    Nghttp2Session* session,
-    nghttp2_headers_category category,
-    int options) {
-  DEBUG_HTTP2("Nghttp2Stream %d: resetting stream state\n", id);
-  session_ = session;
-  while (!queue_.empty()) {
-    nghttp2_stream_write* head = queue_.front();
-    delete head;
-    queue_.pop();
-  }
-  while (!data_chunks_.empty())
-    data_chunks_.pop();
-  while (!current_headers_.empty())
-    current_headers_.pop();
-  current_headers_category_ = category;
-  flags_ = NGHTTP2_STREAM_FLAG_NONE;
-  id_ = id;
-  code_ = NGHTTP2_NO_ERROR;
-  prev_local_window_size_ = 65535;
-  queue_index_ = 0;
-  queue_offset_ = 0;
   getTrailers_ = options & STREAM_OPTION_GET_TRAILERS;
+  if (options & STREAM_OPTION_EMPTY_PAYLOAD)
+    Shutdown();
+  session->AddStream(this);
 }
 
 
@@ -687,14 +692,16 @@ inline void Nghttp2Stream::Destroy() {
   Nghttp2Session* session = this->session_;
 
   if (session != nullptr) {
-    // Remove this stream from the associated session
     session_->RemoveStream(this->id());
     session_ = nullptr;
   }
 
   // Free any remaining incoming data chunks.
-  while (!data_chunks_.empty())
+  while (!data_chunks_.empty()) {
+    uv_buf_t buf = data_chunks_.front();
+    free(buf.base);
     data_chunks_.pop();
+  }
 
   // Free any remaining outgoing data chunks.
   while (!queue_.empty()) {
@@ -704,12 +711,7 @@ inline void Nghttp2Stream::Destroy() {
     queue_.pop();
   }
 
-  // Free any remaining headers
+  delete this;
-  while (!current_headers_.empty())
-    current_headers_.pop();
-
-  // Return this stream instance to the freelist
-  stream_free_list.push(this);
 }
 
 // Submit informational headers for a stream.
@@ -759,9 +761,9 @@ inline int32_t Nghttp2Stream::SubmitPushPromise(
                                             id_, nva, len,
                                             nullptr);
   if (ret > 0) {
-    auto stream = Nghttp2Stream::Init(ret, session_);
+    auto stream = new Nghttp2Stream(ret, session_,
-    if (options & STREAM_OPTION_EMPTY_PAYLOAD)
+                                    NGHTTP2_HCAT_HEADERS,
-      stream->Shutdown();
+                                    options);
     if (assigned != nullptr) *assigned = stream;
   }
   return ret;
@@ -837,11 +839,7 @@ inline int32_t Nghttp2Session::SubmitRequest(
                                        provider, nullptr);
   // Assign the Nghttp2Stream handle
   if (ret > 0) {
-    Nghttp2Stream* stream = Nghttp2Stream::Init(ret, this,
+    auto stream = new Nghttp2Stream(ret, this, NGHTTP2_HCAT_HEADERS, options);
-                                                NGHTTP2_HCAT_HEADERS,
-                                                options);
-    if (options & STREAM_OPTION_EMPTY_PAYLOAD)
-      stream->Shutdown();
     if (assigned != nullptr) *assigned = stream;
   }
   return ret;

src/node_http2_core.h

@@ -9,6 +9,7 @@
 #include "nghttp2/nghttp2.h"
 
 #include <queue>
+#include <vector>
 #include <stdio.h>
 #include <unordered_map>
 
@@ -36,6 +37,19 @@ void inline debug_vfprintf(const char* format, ...) {
   } while (0)
 #endif
 
+#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 DEFAULT_SETTINGS_MAX_HEADER_LIST_SIZE 65535
+#define MAX_MAX_FRAME_SIZE 16777215
+#define MIN_MAX_FRAME_SIZE DEFAULT_SETTINGS_MAX_FRAME_SIZE
+#define MAX_INITIAL_WINDOW_SIZE 2147483647
+
+#define MAX_MAX_HEADER_LIST_SIZE 16777215u
+#define DEFAULT_MAX_HEADER_LIST_PAIRS 128u
+
+
 class Nghttp2Session;
 class Nghttp2Stream;
 
@@ -86,6 +100,7 @@ struct nghttp2_stream_write {
 struct nghttp2_header {
   nghttp2_rcbuf* name = nullptr;
   nghttp2_rcbuf* value = nullptr;
+  uint8_t flags = 0;
 };
 
 // Handle Types
@@ -95,7 +110,8 @@ class Nghttp2Session {
   inline int Init(
       const nghttp2_session_type type = NGHTTP2_SESSION_SERVER,
       nghttp2_option* options = nullptr,
-      nghttp2_mem* mem = nullptr);
+      nghttp2_mem* mem = nullptr,
+      uint32_t maxHeaderPairs = DEFAULT_MAX_HEADER_LIST_PAIRS);
 
   // Frees this session instance
   inline ~Nghttp2Session();
@@ -104,6 +120,10 @@ class Nghttp2Session {
     return destroying_;
   }
 
+  uint32_t GetMaxHeaderPairs() const {
+    return max_header_pairs_;
+  }
+
   inline const char* TypeName() {
     switch (session_type_) {
       case NGHTTP2_SESSION_SERVER: return "server";
@@ -156,7 +176,8 @@ class Nghttp2Session {
 
   virtual void OnHeaders(
       Nghttp2Stream* stream,
-      std::queue<nghttp2_header>* headers,
+      nghttp2_header* headers,
+      size_t count,
       nghttp2_headers_category cat,
       uint8_t flags) {}
   virtual void OnStreamClose(int32_t id, uint32_t code) {}
@@ -288,6 +309,7 @@ class Nghttp2Session {
 
   nghttp2_session* session_;
   nghttp2_session_type session_type_;
+  uint32_t max_header_pairs_ = DEFAULT_MAX_HEADER_LIST_PAIRS;
   std::unordered_map<int32_t, Nghttp2Stream*> streams_;
   bool destroying_ = false;
 
@@ -298,37 +320,21 @@ class Nghttp2Session {
 
 class Nghttp2Stream {
  public:
-  static inline Nghttp2Stream* Init(
+  // Resets the state of the stream instance to defaults
+  Nghttp2Stream(
     int32_t id,
     Nghttp2Session* session,
     nghttp2_headers_category category = NGHTTP2_HCAT_HEADERS,
     int options = 0);
 
-  inline ~Nghttp2Stream() {
+  inline ~Nghttp2Stream() {}
-#if defined(DEBUG) && DEBUG
-    CHECK_EQ(session_, nullptr);
-#endif
-    DEBUG_HTTP2("Nghttp2Stream %d: freed\n", id_);
-  }
 
   inline void FlushDataChunks();
 
-  // Resets the state of the stream instance to defaults
-  inline void ResetState(
-      int32_t id,
-      Nghttp2Session* session,
-      nghttp2_headers_category category = NGHTTP2_HCAT_HEADERS,
-      int options = 0);
-
   // Destroy this stream instance and free all held memory.
   // Note that this will free queued outbound and inbound
   // data chunks and inbound headers, so it's important not
   // to call this until those are fully consumed.
-  //
-  // Also note: this does not actually destroy the instance.
-  // instead, it frees the held memory, removes the stream
-  // from the parent session, and returns the instance to
-  // the FreeList so that it can be reused.
   inline void Destroy();
 
   // Returns true if this stream has been destroyed
@@ -434,34 +440,44 @@ class Nghttp2Stream {
     return id_;
   }
 
-  inline std::queue<nghttp2_header>* headers() {
+  inline bool AddHeader(nghttp2_rcbuf* name,
-    return &current_headers_;
+                        nghttp2_rcbuf* value,
+                        uint8_t flags);
+
+  inline nghttp2_header* headers() {
+    return current_headers_.data();
   }
 
   inline nghttp2_headers_category headers_category() const {
     return current_headers_category_;
   }
 
+  inline size_t headers_count() const {
+    return current_headers_.size();
+  }
+
   void StartHeaders(nghttp2_headers_category category) {
     DEBUG_HTTP2("Nghttp2Stream %d: starting headers, category: %d\n",
                 id_, category);
-    // We shouldn't be in the middle of a headers block already.
+    current_headers_length_ = 0;
-    // Something bad happened if this fails
+    current_headers_.clear();
-#if defined(DEBUG) && DEBUG
-    CHECK(current_headers_.empty());
-#endif
     current_headers_category_ = category;
   }
 
  private:
-  // The Parent HTTP/2 Session
-  Nghttp2Session* session_ = nullptr;
-
   // The Stream Identifier
-  int32_t id_ = 0;
+  int32_t id_;
+
+  // The Parent HTTP/2 Session
+  Nghttp2Session* session_;
 
   // Internal state flags
-  int flags_ = 0;
+  int flags_ = NGHTTP2_STREAM_FLAG_NONE;
+  uint32_t max_header_pairs_ = DEFAULT_MAX_HEADER_LIST_PAIRS;
+  uint32_t max_header_length_ = DEFAULT_SETTINGS_MAX_HEADER_LIST_SIZE;
+
+  // The RST_STREAM code used to close this stream
+  int32_t code_ = NGHTTP2_NO_ERROR;
 
   // Outbound Data... This is the data written by the JS layer that is
   // waiting to be written out to the socket.
@@ -471,23 +487,19 @@ class Nghttp2Stream {
   int64_t fd_offset_ = 0;
   int64_t fd_length_ = -1;
 
+  // True if this stream will have outbound trailers
+  bool getTrailers_ = false;
+
   // The Current Headers block... As headers are received for this stream,
   // they are temporarily stored here until the OnFrameReceived is called
   // signalling the end of the HEADERS frame
   nghttp2_headers_category current_headers_category_ = NGHTTP2_HCAT_HEADERS;
-  std::queue<nghttp2_header> current_headers_;
+  uint32_t current_headers_length_ = 0;  // total number of octets
+  std::vector<nghttp2_header> current_headers_;
 
   // Inbound Data... This is the data received via DATA frames for this stream.
   std::queue<uv_buf_t> data_chunks_;
 
-  // The RST_STREAM code used to close this stream
-  int32_t code_ = NGHTTP2_NO_ERROR;
-
-  int32_t prev_local_window_size_ = 65535;
-
-  // True if this stream will have outbound trailers
-  bool getTrailers_ = false;
-
   friend class Nghttp2Session;
 };
 

src/node_http2_state.h

@@ -47,6 +47,7 @@ namespace http2 {
     IDX_OPTIONS_MAX_SEND_HEADER_BLOCK_LENGTH,
     IDX_OPTIONS_PEER_MAX_CONCURRENT_STREAMS,
     IDX_OPTIONS_PADDING_STRATEGY,
+    IDX_OPTIONS_MAX_HEADER_LIST_PAIRS,
     IDX_OPTIONS_FLAGS
   };
 

test/parallel/test-http2-getpackedsettings.js

@@ -6,9 +6,11 @@ if (!common.hasCrypto)
 const assert = require('assert');
 const http2 = require('http2');
 
-const check = Buffer.from([0x00, 0x01, 0x00, 0x00, 0x10, 0x00, 0x00, 0x05,
+const check = Buffer.from([0x00, 0x01, 0x00, 0x00, 0x10, 0x00,
-                           0x00, 0x00, 0x40, 0x00, 0x00, 0x04, 0x00, 0x00,
+                           0x00, 0x05, 0x00, 0x00, 0x40, 0x00,
-                           0xff, 0xff, 0x00, 0x02, 0x00, 0x00, 0x00, 0x01]);
+                           0x00, 0x04, 0x00, 0x00, 0xff, 0xff,
+                           0x00, 0x06, 0x00, 0x00, 0xff, 0xff,
+                           0x00, 0x02, 0x00, 0x00, 0x00, 0x01]);
 const val = http2.getPackedSettings(http2.getDefaultSettings());
 assert.deepStrictEqual(val, check);
 

test/parallel/test-http2-too-large-headers.js

@@ -0,0 +1,32 @@
+'use strict';
+
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+const http2 = require('http2');
+const assert = require('assert');
+const {
+  NGHTTP2_ENHANCE_YOUR_CALM
+} = http2.constants;
+
+const server = http2.createServer({ settings: { maxHeaderListSize: 100 } });
+server.on('stream', common.mustNotCall());
+
+server.listen(0, common.mustCall(() => {
+  const client = http2.connect(`http://localhost:${server.address().port}`);
+
+  client.on('remoteSettings', () => {
+    const req = client.request({ 'foo': 'a'.repeat(1000) });
+    req.on('error', common.expectsError({
+      code: 'ERR_HTTP2_STREAM_ERROR',
+      type: Error,
+      message: 'Stream closed with error code 11'
+    }));
+    req.on('streamClosed', common.mustCall((code) => {
+      assert.strictEqual(code, NGHTTP2_ENHANCE_YOUR_CALM);
+      server.close();
+      client.destroy();
+    }));
+  });
+
+}));

test/parallel/test-http2-too-many-headers.js

@@ -0,0 +1,34 @@
+'use strict';
+
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+const http2 = require('http2');
+const assert = require('assert');
+const {
+  NGHTTP2_ENHANCE_YOUR_CALM
+} = http2.constants;
+
+// By default, the maximum number of header fields allowed per
+// block is 128, including the HTTP pseudo-header fields. The
+// minimum value for servers is 4, setting this to any value
+// less than 4 will still leave the minimum to 4.
+const server = http2.createServer({ maxHeaderListPairs: 0 });
+server.on('stream', common.mustNotCall());
+
+server.listen(0, common.mustCall(() => {
+  const client = http2.connect(`http://localhost:${server.address().port}`);
+
+  const req = client.request({ foo: 'bar' });
+  req.on('error', common.expectsError({
+    code: 'ERR_HTTP2_STREAM_ERROR',
+    type: Error,
+    message: 'Stream closed with error code 11'
+  }));
+  req.on('streamClosed', common.mustCall((code) => {
+    assert.strictEqual(code, NGHTTP2_ENHANCE_YOUR_CALM);
+    server.close();
+    client.destroy();
+  }));
+
+}));

test/parallel/test-http2-util-update-options-buffer.js

@@ -15,7 +15,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;
 
 {
   updateOptionsBuffer({
@@ -23,7 +24,8 @@ const IDX_OPTIONS_FLAGS = 5;
     maxReservedRemoteStreams: 2,
     maxSendHeaderBlockLength: 3,
     peerMaxConcurrentStreams: 4,
-    paddingStrategy: 5
+    paddingStrategy: 5,
+    maxHeaderListPairs: 6
   });
 
   strictEqual(optionsBuffer[IDX_OPTIONS_MAX_DEFLATE_DYNAMIC_TABLE_SIZE], 1);
@@ -31,6 +33,7 @@ const IDX_OPTIONS_FLAGS = 5;
   strictEqual(optionsBuffer[IDX_OPTIONS_MAX_SEND_HEADER_BLOCK_LENGTH], 3);
   strictEqual(optionsBuffer[IDX_OPTIONS_PEER_MAX_CONCURRENT_STREAMS], 4);
   strictEqual(optionsBuffer[IDX_OPTIONS_PADDING_STRATEGY], 5);
+  strictEqual(optionsBuffer[IDX_OPTIONS_MAX_HEADER_LIST_PAIRS], 6);
 
   const flags = optionsBuffer[IDX_OPTIONS_FLAGS];
 
@@ -39,6 +42,7 @@ const IDX_OPTIONS_FLAGS = 5;
   ok(flags & (1 << IDX_OPTIONS_MAX_SEND_HEADER_BLOCK_LENGTH));
   ok(flags & (1 << IDX_OPTIONS_PEER_MAX_CONCURRENT_STREAMS));
   ok(flags & (1 << IDX_OPTIONS_PADDING_STRATEGY));
+  ok(flags & (1 << IDX_OPTIONS_MAX_HEADER_LIST_PAIRS));
 }
 
 {
@@ -48,7 +52,8 @@ const IDX_OPTIONS_FLAGS = 5;
     maxDeflateDynamicTableSize: 1,
     maxReservedRemoteStreams: 2,
     peerMaxConcurrentStreams: 4,
-    paddingStrategy: 5
+    paddingStrategy: 5,
+    maxHeaderListPairs: 6
   });
 
   strictEqual(optionsBuffer[IDX_OPTIONS_MAX_DEFLATE_DYNAMIC_TABLE_SIZE], 1);
@@ -56,6 +61,7 @@ const IDX_OPTIONS_FLAGS = 5;
   strictEqual(optionsBuffer[IDX_OPTIONS_MAX_SEND_HEADER_BLOCK_LENGTH], 0);
   strictEqual(optionsBuffer[IDX_OPTIONS_PEER_MAX_CONCURRENT_STREAMS], 4);
   strictEqual(optionsBuffer[IDX_OPTIONS_PADDING_STRATEGY], 5);
+  strictEqual(optionsBuffer[IDX_OPTIONS_MAX_HEADER_LIST_PAIRS], 6);
 
   const flags = optionsBuffer[IDX_OPTIONS_FLAGS];
 
@@ -64,4 +70,5 @@ const IDX_OPTIONS_FLAGS = 5;
   ok(!(flags & (1 << IDX_OPTIONS_MAX_SEND_HEADER_BLOCK_LENGTH)));
   ok(flags & (1 << IDX_OPTIONS_PEER_MAX_CONCURRENT_STREAMS));
   ok(flags & (1 << IDX_OPTIONS_PADDING_STRATEGY));
+  ok(flags & (1 << IDX_OPTIONS_MAX_HEADER_LIST_PAIRS));
 }