Skip to content

Commit

Permalink
http2: add maxHeaderSize option to http2
Browse files Browse the repository at this point in the history
add maxHeaderSize to http2 as an alias for maxHeaderListSize.

Fixes: #33517
PR-URL: #33636
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Pranshu Srivastava <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
  • Loading branch information
preyunk authored and MylesBorins committed Aug 17, 2020
1 parent 6832568 commit ef356f6
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 26 deletions.
1 change: 1 addition & 0 deletions doc/api/http2.md
Original file line number Diff line number Diff line change
Expand Up @@ -2570,6 +2570,7 @@ properties.
* `maxHeaderListSize` {number} Specifies the maximum size (uncompressed octets)
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`.
* `maxHeaderSize` {number} Alias for `maxHeaderListSize`.
* `enableConnectProtocol`{boolean} Specifies `true` if the "Extended Connect
Protocol" defined by [RFC 8441][] is to be enabled. This setting is only
meaningful if sent by the server. Once the `enableConnectProtocol` setting
Expand Down
5 changes: 4 additions & 1 deletion lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -908,6 +908,9 @@ const validateSettings = hideStackFrames((settings) => {
assertWithinRange('maxHeaderListSize',
settings.maxHeaderListSize,
0, kMaxInt);
assertWithinRange('maxHeaderSize',
settings.maxHeaderSize,
0, kMaxInt);
if (settings.enablePush !== undefined &&
typeof settings.enablePush !== 'boolean') {
throw new ERR_HTTP2_INVALID_SETTING_VALUE('enablePush',
Expand Down Expand Up @@ -3102,7 +3105,7 @@ function getUnpackedSettings(buf, options = {}) {
settings.maxFrameSize = value;
break;
case NGHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE:
settings.maxHeaderListSize = value;
settings.maxHeaderListSize = settings.maxHeaderSize = value;
break;
case NGHTTP2_SETTINGS_ENABLE_CONNECT_PROTOCOL:
settings.enableConnectProtocol = value !== 0;
Expand Down
19 changes: 15 additions & 4 deletions lib/internal/http2/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ function getDefaultSettings() {

if ((flags & (1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE)) ===
(1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE)) {
holder.maxHeaderListSize =
holder.maxHeaderListSize = holder.maxHeaderSize =
settingsBuffer[IDX_SETTINGS_MAX_HEADER_LIST_SIZE];
}

Expand Down Expand Up @@ -328,6 +328,7 @@ function getSettings(session, remote) {
maxFrameSize: settingsBuffer[IDX_SETTINGS_MAX_FRAME_SIZE],
maxConcurrentStreams: settingsBuffer[IDX_SETTINGS_MAX_CONCURRENT_STREAMS],
maxHeaderListSize: settingsBuffer[IDX_SETTINGS_MAX_HEADER_LIST_SIZE],
maxHeaderSize: settingsBuffer[IDX_SETTINGS_MAX_HEADER_LIST_SIZE],
enableConnectProtocol:
!!settingsBuffer[IDX_SETTINGS_ENABLE_CONNECT_PROTOCOL]
};
Expand Down Expand Up @@ -355,10 +356,20 @@ function updateSettingsBuffer(settings) {
settingsBuffer[IDX_SETTINGS_MAX_FRAME_SIZE] =
settings.maxFrameSize;
}
if (typeof settings.maxHeaderListSize === 'number') {
if (typeof settings.maxHeaderListSize === 'number' ||
typeof settings.maxHeaderSize === 'number') {
flags |= (1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE);
settingsBuffer[IDX_SETTINGS_MAX_HEADER_LIST_SIZE] =
settings.maxHeaderListSize;
if (settings.maxHeaderSize !== undefined &&
(settings.maxHeaderSize !== settings.maxHeaderListSize)) {
process.emitWarning(
'settings.maxHeaderSize overwrite settings.maxHeaderListSize'
);
settingsBuffer[IDX_SETTINGS_MAX_HEADER_LIST_SIZE] =
settings.maxHeaderSize;
} else {
settingsBuffer[IDX_SETTINGS_MAX_HEADER_LIST_SIZE] =
settings.maxHeaderListSize;
}
}
if (typeof settings.enablePush === 'boolean') {
flags |= (1 << IDX_SETTINGS_ENABLE_PUSH);
Expand Down
2 changes: 2 additions & 0 deletions test/parallel/test-http2-client-settings-before-connect.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ server.listen(0, common.mustCall(() => {
['maxConcurrentStreams', 2 ** 32, RangeError],
['maxHeaderListSize', -1, RangeError],
['maxHeaderListSize', 2 ** 32, RangeError],
['maxHeaderSize', -1, RangeError],
['maxHeaderSize', 2 ** 32, RangeError],
['enablePush', 'a', TypeError],
['enablePush', 1, TypeError],
['enablePush', 0, TypeError],
Expand Down
10 changes: 8 additions & 2 deletions test/parallel/test-http2-getpackedsettings.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ assert.deepStrictEqual(val, check);
['maxConcurrentStreams', 0],
['maxConcurrentStreams', 2 ** 31 - 1],
['maxHeaderListSize', 0],
['maxHeaderListSize', 2 ** 32 - 1]
['maxHeaderListSize', 2 ** 32 - 1],
['maxHeaderSize', 0],
['maxHeaderSize', 2 ** 32 - 1]
].forEach((i) => {
// Valid options should not throw.
http2.getPackedSettings({ [i[0]]: i[1] });
Expand All @@ -45,7 +47,9 @@ http2.getPackedSettings({ enablePush: false });
['maxConcurrentStreams', -1],
['maxConcurrentStreams', 2 ** 32],
['maxHeaderListSize', -1],
['maxHeaderListSize', 2 ** 32]
['maxHeaderListSize', 2 ** 32],
['maxHeaderSize', -1],
['maxHeaderSize', 2 ** 32]
].forEach((i) => {
assert.throws(() => {
http2.getPackedSettings({ [i[0]]: i[1] });
Expand Down Expand Up @@ -97,6 +101,7 @@ http2.getPackedSettings({ enablePush: false });
maxFrameSize: 20000,
maxConcurrentStreams: 200,
maxHeaderListSize: 100,
maxHeaderSize: 100,
enablePush: true,
enableConnectProtocol: false,
foo: 'ignored'
Expand Down Expand Up @@ -149,6 +154,7 @@ http2.getPackedSettings({ enablePush: false });
assert.strictEqual(settings.maxFrameSize, 20000);
assert.strictEqual(settings.maxConcurrentStreams, 200);
assert.strictEqual(settings.maxHeaderListSize, 100);
assert.strictEqual(settings.maxHeaderSize, 100);
assert.strictEqual(settings.enablePush, true);
assert.strictEqual(settings.enableConnectProtocol, false);
}
Expand Down
5 changes: 4 additions & 1 deletion test/parallel/test-http2-session-settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ server.on(
assert.strictEqual(typeof settings.maxFrameSize, 'number');
assert.strictEqual(typeof settings.maxConcurrentStreams, 'number');
assert.strictEqual(typeof settings.maxHeaderListSize, 'number');
assert.strictEqual(typeof settings.maxHeaderSize, 'number');
};

const localSettings = stream.session.localSettings;
Expand Down Expand Up @@ -100,7 +101,9 @@ server.listen(
['maxFrameSize', 16383],
['maxFrameSize', 2 ** 24],
['maxHeaderListSize', -1],
['maxHeaderListSize', 2 ** 32]
['maxHeaderListSize', 2 ** 32],
['maxHeaderSize', -1],
['maxHeaderSize', 2 ** 32]
].forEach((i) => {
const settings = {};
settings[i[0]] = i[1];
Expand Down
38 changes: 20 additions & 18 deletions test/parallel/test-http2-too-large-headers.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,24 +9,26 @@ const {
NGHTTP2_ENHANCE_YOUR_CALM
} = http2.constants;

const server = http2.createServer({ settings: { maxHeaderListSize: 100 } });
server.on('stream', common.mustNotCall());
for (const prototype of ['maxHeaderListSize', 'maxHeaderSize']) {
const server = http2.createServer({ settings: { [prototype]: 100 } });
server.on('stream', common.mustNotCall());

server.listen(0, common.mustCall(() => {
const client = http2.connect(`http://localhost:${server.address().port}`);
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',
name: 'Error',
message: 'Stream closed with error code NGHTTP2_ENHANCE_YOUR_CALM'
}));
req.on('close', common.mustCall(() => {
assert.strictEqual(req.rstCode, NGHTTP2_ENHANCE_YOUR_CALM);
server.close();
client.close();
}));
});
client.on('remoteSettings', () => {
const req = client.request({ 'foo': 'a'.repeat(1000) });
req.on('error', common.expectsError({
code: 'ERR_HTTP2_STREAM_ERROR',
name: 'Error',
message: 'Stream closed with error code NGHTTP2_ENHANCE_YOUR_CALM'
}));
req.on('close', common.mustCall(() => {
assert.strictEqual(req.rstCode, NGHTTP2_ENHANCE_YOUR_CALM);
server.close();
client.close();
}));
});

}));
}));
}

0 comments on commit ef356f6

Please sign in to comment.