Skip to content

Commit 72d0e7e

Browse files
jasnellcjihrig
jasnell
authored and
cjihrig
committed
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: nodejs#16676 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
1 parent 9c39d79 commit 72d0e7e

11 files changed

Lines changed: 275 additions & 146 deletions

‎doc/api/http2.md‎

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1470,11 +1470,18 @@ not be emitted.
14701470
### http2.createServer(options[, onRequestHandler])
14711471
<!-- YAML
14721472
added: v8.4.0
1473+
changes:
1474+
- version: REPLACEME
1475+
pr-url: https://github.com/nodejs/node/pull/16676
1476+
description: Added the `maxHeaderListPairs` option with a default limit of
1477+
128 header pairs.
14731478
-->
14741479

14751480
* `options` {Object}
14761481
* `maxDeflateDynamicTableSize` {number} Sets the maximum dynamic table size
14771482
for deflating header fields. **Default:** `4Kib`
1483+
* `maxHeaderListPairs` {number} Sets the maximum number of header entries.
1484+
**Default:** `128`. The minimum value is `4`.
14781485
* `maxSendHeaderBlockLength` {number} Sets the maximum allowed size for a
14791486
serialized, compressed block of headers. Attempts to send headers that
14801487
exceed this limit will result in a `'frameError'` event being emitted
@@ -1525,6 +1532,11 @@ server.listen(80);
15251532
### http2.createSecureServer(options[, onRequestHandler])
15261533
<!-- YAML
15271534
added: v8.4.0
1535+
changes:
1536+
- version: REPLACEME
1537+
pr-url: https://github.com/nodejs/node/pull/16676
1538+
description: Added the `maxHeaderListPairs` option with a default limit of
1539+
128 header pairs.
15281540
-->
15291541

15301542
* `options` {Object}
@@ -1533,6 +1545,8 @@ added: v8.4.0
15331545
`false`. See the [`'unknownProtocol'`][] event. See [ALPN negotiation][].
15341546
* `maxDeflateDynamicTableSize` {number} Sets the maximum dynamic table size
15351547
for deflating header fields. **Default:** `4Kib`
1548+
* `maxHeaderListPairs` {number} Sets the maximum number of header entries.
1549+
**Default:** `128`. The minimum value is `4`.
15361550
* `maxSendHeaderBlockLength` {number} Sets the maximum allowed size for a
15371551
serialized, compressed block of headers. Attempts to send headers that
15381552
exceed this limit will result in a `'frameError'` event being emitted
@@ -1590,12 +1604,19 @@ server.listen(80);
15901604
### http2.connect(authority[, options][, listener])
15911605
<!-- YAML
15921606
added: v8.4.0
1607+
changes:
1608+
- version: REPLACEME
1609+
pr-url: https://github.com/nodejs/node/pull/16676
1610+
description: Added the `maxHeaderListPairs` option with a default limit of
1611+
128 header pairs.
15931612
-->
15941613

15951614
* `authority` {string|URL}
15961615
* `options` {Object}
15971616
* `maxDeflateDynamicTableSize` {number} Sets the maximum dynamic table size
15981617
for deflating header fields. **Default:** `4Kib`
1618+
* `maxHeaderListPairs` {number} Sets the maximum number of header entries.
1619+
**Default:** `128`. The minimum value is `1`.
15991620
* `maxReservedRemoteStreams` {number} Sets the maximum number of reserved push
16001621
streams the client will accept at any given time. Once the current number of
16011622
currently reserved push streams exceeds reaches this limit, new push streams
@@ -1747,7 +1768,13 @@ server.on('stream', (stream, headers) => {
17471768
```
17481769

17491770
### Settings Object
1750-
1771+
<!-- YAML
1772+
added: v8.4.0
1773+
changes:
1774+
- version: REPLACEME
1775+
pr-url: https://github.com/nodejs/node/pull/16676
1776+
description: The `maxHeaderListSize` setting is now strictly enforced.
1777+
-->
17511778
The `http2.getDefaultSettings()`, `http2.getPackedSettings()`,
17521779
`http2.createServer()`, `http2.createSecureServer()`,
17531780
`http2session.settings()`, `http2session.localSettings`, and
@@ -1773,8 +1800,8 @@ properties.
17731800
concurrently at any given time in an `Http2Session`. The minimum value is
17741801
0. The maximum allowed value is 2<sup>31</sup>-1.
17751802
* `maxHeaderListSize` {number} Specifies the maximum size (uncompressed octets)
1776-
of header list that will be accepted. There is no default value. The minimum
1777-
allowed value is 0. The maximum allowed value is 2<sup>32</sup>-1.
1803+
of header list that will be accepted. The minimum allowed value is 0. The
1804+
maximum allowed value is 2<sup>32</sup>-1. **Default:** 65535.
17781805

17791806
All additional properties on the settings object are ignored.
17801807

‎lib/internal/http2/util.js‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,8 @@ const IDX_OPTIONS_MAX_RESERVED_REMOTE_STREAMS = 1;
172172
const IDX_OPTIONS_MAX_SEND_HEADER_BLOCK_LENGTH = 2;
173173
const IDX_OPTIONS_PEER_MAX_CONCURRENT_STREAMS = 3;
174174
const IDX_OPTIONS_PADDING_STRATEGY = 4;
175-
const IDX_OPTIONS_FLAGS = 5;
175+
const IDX_OPTIONS_MAX_HEADER_LIST_PAIRS = 5;
176+
const IDX_OPTIONS_FLAGS = 6;
176177

177178
function updateOptionsBuffer(options) {
178179
var flags = 0;
@@ -201,6 +202,11 @@ function updateOptionsBuffer(options) {
201202
optionsBuffer[IDX_OPTIONS_PADDING_STRATEGY] =
202203
options.paddingStrategy;
203204
}
205+
if (typeof options.maxHeaderListPairs === 'number') {
206+
flags |= (1 << IDX_OPTIONS_MAX_HEADER_LIST_PAIRS);
207+
optionsBuffer[IDX_OPTIONS_MAX_HEADER_LIST_PAIRS] =
208+
options.maxHeaderListPairs;
209+
}
204210
optionsBuffer[IDX_OPTIONS_FLAGS] = flags;
205211
}
206212

‎src/node_http2.cc‎

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include "node_http2_state.h"
66

77
#include <queue>
8+
#include <algorithm>
89

910
namespace node {
1011

@@ -20,8 +21,6 @@ using v8::Undefined;
2021

2122
namespace http2 {
2223

23-
Freelist<Nghttp2Stream, FREELIST_MAX> stream_free_list;
24-
2524
Nghttp2Session::Callbacks Nghttp2Session::callback_struct_saved[2] = {
2625
Callbacks(false),
2726
Callbacks(true)};
@@ -67,6 +66,10 @@ Http2Options::Http2Options(Environment* env) {
6766
buffer.GetValue(IDX_OPTIONS_PADDING_STRATEGY));
6867
SetPaddingStrategy(strategy);
6968
}
69+
70+
if (flags & (1 << IDX_OPTIONS_MAX_HEADER_LIST_PAIRS)) {
71+
SetMaxHeaderPairs(buffer[IDX_OPTIONS_MAX_HEADER_LIST_PAIRS]);
72+
}
7073
}
7174

7275
Http2Settings::Http2Settings(Environment* env) : env_(env) {
@@ -173,11 +176,14 @@ inline void Http2Settings::RefreshDefaults(Environment* env) {
173176
DEFAULT_SETTINGS_INITIAL_WINDOW_SIZE;
174177
buffer[IDX_SETTINGS_MAX_FRAME_SIZE] =
175178
DEFAULT_SETTINGS_MAX_FRAME_SIZE;
179+
buffer[IDX_SETTINGS_MAX_HEADER_LIST_SIZE] =
180+
DEFAULT_SETTINGS_MAX_HEADER_LIST_SIZE;
176181
buffer[IDX_SETTINGS_COUNT] =
177182
(1 << IDX_SETTINGS_HEADER_TABLE_SIZE) |
178183
(1 << IDX_SETTINGS_ENABLE_PUSH) |
179184
(1 << IDX_SETTINGS_INITIAL_WINDOW_SIZE) |
180-
(1 << IDX_SETTINGS_MAX_FRAME_SIZE);
185+
(1 << IDX_SETTINGS_MAX_FRAME_SIZE) |
186+
(1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE);
181187
}
182188

183189

@@ -192,7 +198,10 @@ Http2Session::Http2Session(Environment* env,
192198

193199
padding_strategy_ = opts.GetPaddingStrategy();
194200

195-
Init(type, *opts);
201+
int32_t maxHeaderPairs = opts.GetMaxHeaderPairs();
202+
maxHeaderPairs = type == NGHTTP2_SESSION_SERVER ?
203+
std::max(maxHeaderPairs, 4) : std::max(maxHeaderPairs, 1);
204+
Init(type, *opts, nullptr, maxHeaderPairs);
196205

197206
// For every node::Http2Session instance, there is a uv_prepare_t handle
198207
// whose callback is triggered on every tick of the event loop. When
@@ -911,7 +920,8 @@ void Http2Session::OnTrailers(Nghttp2Stream* stream,
911920

912921
void Http2Session::OnHeaders(
913922
Nghttp2Stream* stream,
914-
std::queue<nghttp2_header>* headers,
923+
nghttp2_header* headers,
924+
size_t count,
915925
nghttp2_headers_category cat,
916926
uint8_t flags) {
917927
Local<Context> context = env()->context();
@@ -936,18 +946,19 @@ void Http2Session::OnHeaders(
936946
// like {name1: value1, name2: value2, name3: [value3, value4]}. We do it
937947
// this way for performance reasons (it's faster to generate and pass an
938948
// array than it is to generate and pass the object).
939-
do {
949+
size_t n = 0;
950+
while (count > 0) {
940951
size_t j = 0;
941-
while (!headers->empty() && j < arraysize(argv) / 2) {
942-
nghttp2_header item = headers->front();
952+
while (count > 0 && j < arraysize(argv) / 2) {
953+
nghttp2_header item = headers[n++];
943954
// The header name and value are passed as external one-byte strings
944955
name_str =
945956
ExternalHeader::New<true>(env(), item.name).ToLocalChecked();
946957
value_str =
947958
ExternalHeader::New<false>(env(), item.value).ToLocalChecked();
948959
argv[j * 2] = name_str;
949960
argv[j * 2 + 1] = value_str;
950-
headers->pop();
961+
count--;
951962
j++;
952963
}
953964
// For performance, we pass name and value pairs to array.protototype.push
@@ -956,7 +967,7 @@ void Http2Session::OnHeaders(
956967
if (j > 0) {
957968
fn->Call(env()->context(), holder, j * 2, argv).ToLocalChecked();
958969
}
959-
} while (!headers->empty());
970+
}
960971

961972
Local<Value> args[4] = {
962973
Integer::New(isolate, stream->id()),

‎src/node_http2.h‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -292,14 +292,6 @@ const char* nghttp2_errname(int rv) {
292292
}
293293
}
294294

295-
#define DEFAULT_SETTINGS_HEADER_TABLE_SIZE 4096
296-
#define DEFAULT_SETTINGS_ENABLE_PUSH 1
297-
#define DEFAULT_SETTINGS_INITIAL_WINDOW_SIZE 65535
298-
#define DEFAULT_SETTINGS_MAX_FRAME_SIZE 16384
299-
#define MAX_MAX_FRAME_SIZE 16777215
300-
#define MIN_MAX_FRAME_SIZE DEFAULT_SETTINGS_MAX_FRAME_SIZE
301-
#define MAX_INITIAL_WINDOW_SIZE 2147483647
302-
303295
// This allows for 4 default-sized frames with their frame headers
304296
static const size_t kAllocBufferSize = 4 * (16384 + 9);
305297

@@ -324,19 +316,25 @@ class Http2Options {
324316
return options_;
325317
}
326318

319+
void SetMaxHeaderPairs(uint32_t max) {
320+
max_header_pairs_ = max;
321+
}
322+
323+
uint32_t GetMaxHeaderPairs() const {
324+
return max_header_pairs_;
325+
}
326+
327327
void SetPaddingStrategy(padding_strategy_type val) {
328-
#if DEBUG
329-
CHECK_LE(val, PADDING_STRATEGY_CALLBACK);
330-
#endif
331328
padding_strategy_ = static_cast<padding_strategy_type>(val);
332329
}
333330

334-
padding_strategy_type GetPaddingStrategy() {
331+
padding_strategy_type GetPaddingStrategy() const {
335332
return padding_strategy_;
336333
}
337334

338335
private:
339336
nghttp2_option* options_;
337+
uint32_t max_header_pairs_ = DEFAULT_MAX_HEADER_LIST_PAIRS;
340338
padding_strategy_type padding_strategy_ = PADDING_STRATEGY_NONE;
341339
};
342340

@@ -413,7 +411,8 @@ class Http2Session : public AsyncWrap,
413411

414412
void OnHeaders(
415413
Nghttp2Stream* stream,
416-
std::queue<nghttp2_header>* headers,
414+
nghttp2_header* headers,
415+
size_t count,
417416
nghttp2_headers_category cat,
418417
uint8_t flags) override;
419418
void OnStreamClose(int32_t id, uint32_t code) override;

0 commit comments

Comments
 (0)