From b5a71a439dce1d38ba96d9a11f6de2ef18b5073d Mon Sep 17 00:00:00 2001 From: ZYSzys Date: Fri, 3 Jan 2020 17:50:04 +0800 Subject: [PATCH] http2: set default enableConnectProtocol to 0 PR-URL: https://github.com/nodejs/node/pull/31174 Refs: https://tools.ietf.org/html/rfc8441#section-9.1 Reviewed-By: James M Snell Reviewed-By: Colin Ihrig Reviewed-By: Rich Trott --- doc/api/http2.md | 3 +- lib/internal/http2/core.js | 9 +++ lib/internal/http2/util.js | 5 +- src/node_http2.cc | 7 ++- src/node_http2.h | 1 + test/parallel/test-http2-binding.js | 4 +- test/parallel/test-http2-getpackedsettings.js | 60 ++++++++++++++----- 7 files changed, 68 insertions(+), 21 deletions(-) diff --git a/doc/api/http2.md b/doc/api/http2.md index c81f94a278100e..42b0693bda2b25 100644 --- a/doc/api/http2.md +++ b/doc/api/http2.md @@ -2413,7 +2413,7 @@ properties. header compression. The minimum allowed value is 0. The maximum allowed value is 232-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 232-1. **Default:** `65,535 bytes`. @@ -2433,6 +2433,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. diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index f912a65dd07932..3e1bbe3e97638a 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -253,6 +253,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, @@ -886,6 +887,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; @@ -909,6 +911,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 @@ -3105,6 +3112,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; } diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js index ba2526177a133d..3c9b35bb774960 100644 --- a/lib/internal/http2/util.js +++ b/lib/internal/http2/util.js @@ -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; @@ -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] }; } diff --git a/src/node_http2.cc b/src/node_http2.cc index 52cc41594af386..94e1c88506ff54 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -297,13 +297,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); } @@ -3137,6 +3140,8 @@ void Initialize(Local 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); diff --git a/src/node_http2.h b/src/node_http2.h index 07febd7da40a9b..1cbdadc4b0639d 100644 --- a/src/node_http2.h +++ b/src/node_http2.h @@ -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 diff --git a/test/parallel/test-http2-binding.js b/test/parallel/test-http2-binding.js index 0de00fcc75159c..81f49691e3ecff 100644 --- a/test/parallel/test-http2-binding.js +++ b/test/parallel/test-http2-binding.js @@ -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)) { diff --git a/test/parallel/test-http2-getpackedsettings.js b/test/parallel/test-http2-getpackedsettings.js index f9ca7da1e865b6..4aa5747a053bd1 100644 --- a/test/parallel/test-http2-getpackedsettings.js +++ b/test/parallel/test-http2-getpackedsettings.js @@ -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); @@ -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, @@ -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); } @@ -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) => { assert.throws(() => { @@ -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 }); }