Skip to content

Commit

Permalink
http2: set default enableConnectProtocol to 0
Browse files Browse the repository at this point in the history 
PR-URL: #31174
Refs: https://tools.ietf.org/html/rfc8441#section-9.1
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
@ZYSzys @BethGriggs
ZYSzys authored and BethGriggs committed Feb 6, 2020
1 parent 35b7ba6 commit 5dd7c92
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 21 deletions.
3 changes: 2 additions & 1 deletion doc/api/http2.md
View file
Expand Up @@ -2420,7 +2420,7 @@ properties.
header compression. The minimum allowed value is 0. The maximum allowed value
is 2<sup>32</sup>-1. **Default:** `4,096 octets`.
* `enablePush` {boolean} Specifies `true` if HTTP/2 Push Streams are to be
permitted on the `Http2Session` instances.
permitted on the `Http2Session` instances. **Default:** `true`.
* `initialWindowSize` {number} Specifies the *senders* initial window size
for stream-level flow control. The minimum allowed value is 0. The maximum
allowed value is 2<sup>32</sup>-1. **Default:** `65,535 bytes`.
Expand All @@ -2440,6 +2440,7 @@ properties.
Protocol" defined by [RFC 8441][] is to be enabled. This setting is only
meaningful if sent by the server. Once the `enableConnectProtocol` setting
has been enabled for a given `Http2Session`, it cannot be disabled.
**Default:** `false`.

All additional properties on the settings object are ignored.

Expand Down
9 changes: 9 additions & 0 deletions lib/internal/http2/core.js
View file
Expand Up @@ -261,6 +261,7 @@ const {
NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE,
NGHTTP2_SETTINGS_MAX_FRAME_SIZE,
NGHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE,
NGHTTP2_SETTINGS_ENABLE_CONNECT_PROTOCOL,

HTTP2_METHOD_GET,
HTTP2_METHOD_HEAD,
Expand Down Expand Up @@ -905,6 +906,7 @@ function pingCallback(cb) {
// 4. maxConcurrentStreams must be a number in the range 0 <= n <= kMaxStreams
// 5. maxHeaderListSize must be a number in the range 0 <= n <= kMaxInt
// 6. enablePush must be a boolean
// 7. enableConnectProtocol must be a boolean
// All settings are optional and may be left undefined
const validateSettings = hideStackFrames((settings) => {
if (settings === undefined) return;
Expand All @@ -928,6 +930,11 @@ const validateSettings = hideStackFrames((settings) => {
throw new ERR_HTTP2_INVALID_SETTING_VALUE('enablePush',
settings.enablePush);
}
if (settings.enableConnectProtocol !== undefined &&
typeof settings.enableConnectProtocol !== 'boolean') {
throw new ERR_HTTP2_INVALID_SETTING_VALUE('enableConnectProtocol',
settings.enableConnectProtocol);
}
});

// Creates the internal binding.Http2Session handle for an Http2Session
Expand Down Expand Up @@ -3094,6 +3101,8 @@ function getUnpackedSettings(buf, options = {}) {
case NGHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE:
settings.maxHeaderListSize = value;
break;
case NGHTTP2_SETTINGS_ENABLE_CONNECT_PROTOCOL:
settings.enableConnectProtocol = value !== 0;
}
offset += 4;
}
Expand Down
5 changes: 3 additions & 2 deletions lib/internal/http2/util.js
View file
Expand Up @@ -301,7 +301,7 @@ function getDefaultSettings() {
if ((flags & (1 << IDX_SETTINGS_ENABLE_CONNECT_PROTOCOL)) ===
(1 << IDX_SETTINGS_ENABLE_CONNECT_PROTOCOL)) {
holder.enableConnectProtocol =
settingsBuffer[IDX_SETTINGS_ENABLE_CONNECT_PROTOCOL];
settingsBuffer[IDX_SETTINGS_ENABLE_CONNECT_PROTOCOL] === 1;
}

return holder;
Expand All @@ -322,7 +322,8 @@ 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],
enableConnectProtocol: settingsBuffer[IDX_SETTINGS_ENABLE_CONNECT_PROTOCOL]
enableConnectProtocol:
!!settingsBuffer[IDX_SETTINGS_ENABLE_CONNECT_PROTOCOL]
};
}

Expand Down
7 changes: 6 additions & 1 deletion src/node_http2.cc
View file
Expand Up @@ -301,13 +301,16 @@ void Http2Session::Http2Settings::RefreshDefaults(Environment* env) {
DEFAULT_SETTINGS_MAX_FRAME_SIZE;
buffer[IDX_SETTINGS_MAX_HEADER_LIST_SIZE] =
DEFAULT_SETTINGS_MAX_HEADER_LIST_SIZE;
buffer[IDX_SETTINGS_ENABLE_CONNECT_PROTOCOL] =
DEFAULT_SETTINGS_ENABLE_CONNECT_PROTOCOL;
buffer[IDX_SETTINGS_COUNT] =
(1 << IDX_SETTINGS_HEADER_TABLE_SIZE) |
(1 << IDX_SETTINGS_ENABLE_PUSH) |
(1 << IDX_SETTINGS_MAX_CONCURRENT_STREAMS) |
(1 << IDX_SETTINGS_INITIAL_WINDOW_SIZE) |
(1 << IDX_SETTINGS_MAX_FRAME_SIZE) |
(1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE);
(1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE) |
(1 << IDX_SETTINGS_ENABLE_CONNECT_PROTOCOL);
}


Expand Down Expand Up @@ -3170,6 +3173,8 @@ void Initialize(Local<Object> target,
NODE_DEFINE_CONSTANT(constants, DEFAULT_SETTINGS_MAX_CONCURRENT_STREAMS);
NODE_DEFINE_CONSTANT(constants, DEFAULT_SETTINGS_INITIAL_WINDOW_SIZE);
NODE_DEFINE_CONSTANT(constants, DEFAULT_SETTINGS_MAX_FRAME_SIZE);
NODE_DEFINE_CONSTANT(constants, DEFAULT_SETTINGS_MAX_HEADER_LIST_SIZE);
NODE_DEFINE_CONSTANT(constants, DEFAULT_SETTINGS_ENABLE_CONNECT_PROTOCOL);
NODE_DEFINE_CONSTANT(constants, MAX_MAX_FRAME_SIZE);
NODE_DEFINE_CONSTANT(constants, MIN_MAX_FRAME_SIZE);
NODE_DEFINE_CONSTANT(constants, MAX_INITIAL_WINDOW_SIZE);
Expand Down
1 change: 1 addition & 0 deletions src/node_http2.h
View file
Expand Up @@ -45,6 +45,7 @@ using performance::PerformanceEntry;
#define DEFAULT_SETTINGS_INITIAL_WINDOW_SIZE 65535
#define DEFAULT_SETTINGS_MAX_FRAME_SIZE 16384
#define DEFAULT_SETTINGS_MAX_HEADER_LIST_SIZE 65535
#define DEFAULT_SETTINGS_ENABLE_CONNECT_PROTOCOL 0
#define MAX_MAX_FRAME_SIZE 16777215
#define MIN_MAX_FRAME_SIZE DEFAULT_SETTINGS_MAX_FRAME_SIZE
#define MAX_INITIAL_WINDOW_SIZE 2147483647
Expand Down
4 changes: 3 additions & 1 deletion test/parallel/test-http2-binding.js
View file
Expand Up @@ -230,7 +230,9 @@ const defaultSettings = {
DEFAULT_SETTINGS_ENABLE_PUSH: 1,
DEFAULT_SETTINGS_MAX_CONCURRENT_STREAMS: 4294967295,
DEFAULT_SETTINGS_INITIAL_WINDOW_SIZE: 65535,
DEFAULT_SETTINGS_MAX_FRAME_SIZE: 16384
DEFAULT_SETTINGS_MAX_FRAME_SIZE: 16384,
DEFAULT_SETTINGS_MAX_HEADER_LIST_SIZE: 65535,
DEFAULT_SETTINGS_ENABLE_CONNECT_PROTOCOL: 0
};

for (const name of Object.keys(constants)) {
Expand Down
60 changes: 44 additions & 16 deletions test/parallel/test-http2-getpackedsettings.js
View file
Expand Up @@ -11,7 +11,8 @@ const check = Buffer.from([0x00, 0x01, 0x00, 0x00, 0x10, 0x00,
0x00, 0x05, 0x00, 0x00, 0x40, 0x00,
0x00, 0x04, 0x00, 0x00, 0xff, 0xff,
0x00, 0x06, 0x00, 0x00, 0xff, 0xff,
0x00, 0x02, 0x00, 0x00, 0x00, 0x01]);
0x00, 0x02, 0x00, 0x00, 0x00, 0x01,
0x00, 0x08, 0x00, 0x00, 0x00, 0x00]);
const val = http2.getPackedSettings(http2.getDefaultSettings());
assert.deepStrictEqual(val, check);

Expand Down Expand Up @@ -67,12 +68,27 @@ http2.getPackedSettings({ enablePush: false });
});
});

[
1, null, '', Infinity, new Date(), {}, NaN, [false]
].forEach((i) => {
assert.throws(() => {
http2.getPackedSettings({ enableConnectProtocol: i });
}, {
code: 'ERR_HTTP2_INVALID_SETTING_VALUE',
name: 'TypeError',
message: `Invalid value for setting "enableConnectProtocol": ${i}`
});
});

{
const check = Buffer.from([
0x00, 0x01, 0x00, 0x00, 0x00, 0x64, 0x00, 0x03, 0x00, 0x00,
0x00, 0xc8, 0x00, 0x05, 0x00, 0x00, 0x4e, 0x20, 0x00, 0x04,
0x00, 0x00, 0x00, 0x64, 0x00, 0x06, 0x00, 0x00, 0x00, 0x64,
0x00, 0x02, 0x00, 0x00, 0x00, 0x01]);
0x00, 0x01, 0x00, 0x00, 0x00, 0x64,
0x00, 0x03, 0x00, 0x00, 0x00, 0xc8,
0x00, 0x05, 0x00, 0x00, 0x4e, 0x20,
0x00, 0x04, 0x00, 0x00, 0x00, 0x64,
0x00, 0x06, 0x00, 0x00, 0x00, 0x64,
0x00, 0x02, 0x00, 0x00, 0x00, 0x01,
0x00, 0x08, 0x00, 0x00, 0x00, 0x00]);

const packed = http2.getPackedSettings({
headerTableSize: 100,
Expand All @@ -81,9 +97,10 @@ http2.getPackedSettings({ enablePush: false });
maxConcurrentStreams: 200,
maxHeaderListSize: 100,
enablePush: true,
enableConnectProtocol: false,
foo: 'ignored'
});
assert.strictEqual(packed.length, 36);
assert.strictEqual(packed.length, 42);
assert.deepStrictEqual(packed, check);
}

Expand All @@ -95,10 +112,13 @@ http2.getPackedSettings({ enablePush: false });

{
const packed = Buffer.from([
0x00, 0x01, 0x00, 0x00, 0x00, 0x64, 0x00, 0x03, 0x00, 0x00,
0x00, 0xc8, 0x00, 0x05, 0x00, 0x00, 0x4e, 0x20, 0x00, 0x04,
0x00, 0x00, 0x00, 0x64, 0x00, 0x06, 0x00, 0x00, 0x00, 0x64,
0x00, 0x02, 0x00, 0x00, 0x00, 0x01]);
0x00, 0x01, 0x00, 0x00, 0x00, 0x64,
0x00, 0x03, 0x00, 0x00, 0x00, 0xc8,
0x00, 0x05, 0x00, 0x00, 0x4e, 0x20,
0x00, 0x04, 0x00, 0x00, 0x00, 0x64,
0x00, 0x06, 0x00, 0x00, 0x00, 0x64,
0x00, 0x02, 0x00, 0x00, 0x00, 0x01,
0x00, 0x08, 0x00, 0x00, 0x00, 0x00]);

[1, true, '', [], {}, NaN].forEach((input) => {
common.expectsError(() => {
Expand Down Expand Up @@ -129,30 +149,38 @@ http2.getPackedSettings({ enablePush: false });
assert.strictEqual(settings.maxConcurrentStreams, 200);
assert.strictEqual(settings.maxHeaderListSize, 100);
assert.strictEqual(settings.enablePush, true);
assert.strictEqual(settings.enableConnectProtocol, false);
}

{
const packed = Buffer.from([
0x00, 0x02, 0x00, 0x00, 0x00, 0x00]);
0x00, 0x02, 0x00, 0x00, 0x00, 0x00,
0x00, 0x08, 0x00, 0x00, 0x00, 0x00]);

const settings = http2.getUnpackedSettings(packed, { validate: true });
assert.strictEqual(settings.enablePush, false);
assert.strictEqual(settings.enableConnectProtocol, false);
}
{
const packed = Buffer.from([
0x00, 0x02, 0x00, 0x00, 0x00, 0x64]);
0x00, 0x02, 0x00, 0x00, 0x00, 0x64,
0x00, 0x08, 0x00, 0x00, 0x00, 0x64]);

const settings = http2.getUnpackedSettings(packed, { validate: true });
assert.strictEqual(settings.enablePush, true);
assert.strictEqual(settings.enableConnectProtocol, true);
}

// Verify that passing {validate: true} does not throw.
{
const packed = Buffer.from([
0x00, 0x01, 0x00, 0x00, 0x00, 0x64, 0x00, 0x03, 0x00, 0x00,
0x00, 0xc8, 0x00, 0x05, 0x00, 0x00, 0x4e, 0x20, 0x00, 0x04,
0x00, 0x00, 0x00, 0x64, 0x00, 0x06, 0x00, 0x00, 0x00, 0x64,
0x00, 0x02, 0x00, 0x00, 0x00, 0x01]);
0x00, 0x01, 0x00, 0x00, 0x00, 0x64,
0x00, 0x03, 0x00, 0x00, 0x00, 0xc8,
0x00, 0x05, 0x00, 0x00, 0x4e, 0x20,
0x00, 0x04, 0x00, 0x00, 0x00, 0x64,
0x00, 0x06, 0x00, 0x00, 0x00, 0x64,
0x00, 0x02, 0x00, 0x00, 0x00, 0x01,
0x00, 0x08, 0x00, 0x00, 0x00, 0x00]);

http2.getUnpackedSettings(packed, { validate: true });
}
Expand Down

0 comments on commit 5dd7c92

Please sign in to comment.