From 2b53b16b3f9f9a78b2236d7146fc75db92c07df2 Mon Sep 17 00:00:00 2001 From: Marten Richter Date: Sat, 5 Aug 2023 11:32:52 +0000 Subject: [PATCH 1/3] http2: addtl http/2 settings Currently, node.js http/2 is limited in sending SETTINGs, that are currently implemented by nghttp2. However, nghttp2 has the ability to send arbitary SETTINGs, that are not known beforehand. This patch adds this feature including a fall back mechanism, if a SETTING is implemented in a later nghttp2 or node version. Fixes: https://github.com/nodejs/node/issues/1337 --- lib/internal/errors.js | 2 + lib/internal/http2/core.js | 6 +++ lib/internal/http2/util.js | 79 ++++++++++++++++++++++++++++++++++++++ src/node_http2.cc | 15 +++++++- src/node_http2.h | 2 +- src/node_http2_state.h | 19 ++++++--- 6 files changed, 116 insertions(+), 7 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 39e512762a470c..27f63469154270 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1279,6 +1279,8 @@ E('ERR_HTTP2_STREAM_CANCEL', function(error) { E('ERR_HTTP2_STREAM_ERROR', 'Stream closed with error code %s', Error); E('ERR_HTTP2_STREAM_SELF_DEPENDENCY', 'A stream cannot depend on itself', Error); +E('ERR_HTTP2_TOO_MANY_CUSTOM_SETTINGS', + 'Number of custom settings exceeds MAX_ADDITIONAL_SETTINGS', Error); E('ERR_HTTP2_TOO_MANY_INVALID_FRAMES', 'Too many invalid HTTP/2 frames', Error); E('ERR_HTTP2_TRAILERS_ALREADY_SENT', 'Trailing headers have already been sent', Error); diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 58d0783701286d..e6056c395cda68 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -946,6 +946,8 @@ function pingCallback(cb) { // All settings are optional and may be left undefined const validateSettings = hideStackFrames((settings) => { if (settings === undefined) return; + assertIsObject.withoutStackTrace(settings.customSettings, 'customSettings', 'Number'); + assertWithinRange.withoutStackTrace('headerTableSize', settings.headerTableSize, 0, kMaxInt); @@ -3387,6 +3389,10 @@ function getUnpackedSettings(buf, options = kEmptyObject) { break; case NGHTTP2_SETTINGS_ENABLE_CONNECT_PROTOCOL: settings.enableConnectProtocol = value !== 0; + break; + default: + if (!settings.customSettings) settings.customSettings = {}; + settings.customSettings[id] = value; } offset += 4; } diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js index a9d636c2f20581..8578cc9cc8e5fb 100644 --- a/lib/internal/http2/util.js +++ b/lib/internal/http2/util.js @@ -8,6 +8,7 @@ const { Error, MathMax, Number, + NumberIsNaN, ObjectKeys, SafeSet, String, @@ -24,6 +25,7 @@ const { ERR_HTTP2_INVALID_CONNECTION_HEADERS, ERR_HTTP2_INVALID_PSEUDOHEADER: { HideStackFramesError: ERR_HTTP2_INVALID_PSEUDOHEADER }, ERR_HTTP2_INVALID_SETTING_VALUE, + ERR_HTTP2_TOO_MANY_CUSTOM_SETTINGS, ERR_INVALID_ARG_TYPE, ERR_INVALID_HTTP_TOKEN, }, @@ -190,6 +192,9 @@ const IDX_SETTINGS_MAX_HEADER_LIST_SIZE = 5; const IDX_SETTINGS_ENABLE_CONNECT_PROTOCOL = 6; const IDX_SETTINGS_FLAGS = 7; +// Maximum number of allowed additional settings +const MAX_ADDITIONAL_SETTINGS = 10; + const IDX_SESSION_STATE_EFFECTIVE_LOCAL_WINDOW_SIZE = 0; const IDX_SESSION_STATE_EFFECTIVE_RECV_DATA_LENGTH = 1; const IDX_SESSION_STATE_NEXT_STREAM_ID = 2; @@ -348,6 +353,80 @@ function getSettings(session, remote) { function updateSettingsBuffer(settings) { let flags = 0; + let numCustomSettings = 0; + + if (typeof settings.customSettings === 'object') { + const customSettings = settings.customSettings; + for (const setting in customSettings) { + const val = customSettings[setting]; + if (typeof val === 'number') { + let set = false; + const nsetting = Number(setting); + if (NumberIsNaN(nsetting) || + typeof nsetting !== 'number' || + 0 >= nsetting || + nsetting > 0xffff) + throw new ERR_HTTP2_INVALID_SETTING_VALUE.RangeError( + 'Range Error', nsetting, 0, 0xffff); + if (NumberIsNaN(val) || + typeof val !== 'number' || + 0 >= val || + val > 0xffffffff) + throw new ERR_HTTP2_INVALID_SETTING_VALUE.RangeError( + 'Range Error', val, 0, 0xffffffff); + if (nsetting < IDX_SETTINGS_FLAGS) { + set = true; + switch (nsetting) { + case IDX_SETTINGS_HEADER_TABLE_SIZE: + flags |= (1 << IDX_SETTINGS_HEADER_TABLE_SIZE); + settingsBuffer[IDX_SETTINGS_HEADER_TABLE_SIZE] = + val; + break; + case IDX_SETTINGS_ENABLE_PUSH: + flags |= (1 << IDX_SETTINGS_ENABLE_PUSH); + settingsBuffer[IDX_SETTINGS_ENABLE_PUSH] = val; + break; + case IDX_SETTINGS_INITIAL_WINDOW_SIZE: + flags |= (1 << IDX_SETTINGS_INITIAL_WINDOW_SIZE); + settingsBuffer[IDX_SETTINGS_INITIAL_WINDOW_SIZE] = + val; + break; + case IDX_SETTINGS_MAX_FRAME_SIZE: + flags |= (1 << IDX_SETTINGS_MAX_FRAME_SIZE); + settingsBuffer[IDX_SETTINGS_MAX_FRAME_SIZE] = + val; + break; + case IDX_SETTINGS_MAX_CONCURRENT_STREAMS: + flags |= (1 << IDX_SETTINGS_MAX_CONCURRENT_STREAMS); + settingsBuffer[IDX_SETTINGS_MAX_CONCURRENT_STREAMS] = val; + break; + case IDX_SETTINGS_MAX_HEADER_LIST_SIZE: + flags |= (1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE); + settingsBuffer[IDX_SETTINGS_MAX_HEADER_LIST_SIZE] = + val; + break; + case IDX_SETTINGS_ENABLE_CONNECT_PROTOCOL: + flags |= (1 << IDX_SETTINGS_ENABLE_CONNECT_PROTOCOL); + settingsBuffer[IDX_SETTINGS_ENABLE_CONNECT_PROTOCOL] = val; + break; + default: + set = false; + break; + } + } + if (!set) { // not supported + if (numCustomSettings === MAX_ADDITIONAL_SETTINGS) + throw new ERR_HTTP2_TOO_MANY_CUSTOM_SETTINGS(); + + settingsBuffer[IDX_SETTINGS_FLAGS + 1 + 2 * numCustomSettings + 1] = nsetting; + settingsBuffer[IDX_SETTINGS_FLAGS + 1 + 2 * numCustomSettings + 2] = val; + numCustomSettings++; + } + } + } + } + settingsBuffer[IDX_SETTINGS_FLAGS + 1] = numCustomSettings; + if (typeof settings.headerTableSize === 'number') { flags |= (1 << IDX_SETTINGS_HEADER_TABLE_SIZE); settingsBuffer[IDX_SETTINGS_HEADER_TABLE_SIZE] = diff --git a/src/node_http2.cc b/src/node_http2.cc index 070b40ae0a6ad6..ebb1ab63c3ff80 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -228,6 +228,16 @@ size_t Http2Settings::Init( HTTP2_SETTINGS(V) #undef V + uint32_t numAddSettings = buffer[IDX_SETTINGS_COUNT + 1]; + if (numAddSettings > 0) { + uint32_t offset = IDX_SETTINGS_COUNT + 1 + 1; + for (uint32_t i = 0; i < numAddSettings; i++) { + uint32_t key = buffer[offset + i * 2 + 0]; + uint32_t val = buffer[offset + i * 2 + 1]; + entries[count++] = nghttp2_settings_entry{(int32_t)key, val}; + } + } + return count; } #undef GRABSETTING @@ -262,7 +272,7 @@ Local Http2Settings::Pack() { } Local Http2Settings::Pack(Http2State* state) { - nghttp2_settings_entry entries[IDX_SETTINGS_COUNT]; + nghttp2_settings_entry entries[IDX_SETTINGS_COUNT + MAX_ADDITIONAL_SETTINGS]; size_t count = Init(state, entries); return Pack(state->env(), count, entries); } @@ -298,6 +308,8 @@ void Http2Settings::Update(Http2Session* session, get_setting fn) { fn(session->session(), NGHTTP2_SETTINGS_ ## name); HTTP2_SETTINGS(V) #undef V + buffer[IDX_SETTINGS_COUNT + 1] = + 0; // no additional settings are coming, clear them } // Initializes the shared TypedArray with the default settings values. @@ -314,6 +326,7 @@ void Http2Settings::RefreshDefaults(Http2State* http2_state) { #undef V buffer[IDX_SETTINGS_COUNT] = flags; + buffer[IDX_SETTINGS_COUNT + 1] = 0; // no additional settings } diff --git a/src/node_http2.h b/src/node_http2.h index 87f6ab8305a7d8..6b7fd746021507 100644 --- a/src/node_http2.h +++ b/src/node_http2.h @@ -1035,7 +1035,7 @@ class Http2Settings : public AsyncWrap { v8::Global callback_; uint64_t startTime_; size_t count_ = 0; - nghttp2_settings_entry entries_[IDX_SETTINGS_COUNT]; + nghttp2_settings_entry entries_[IDX_SETTINGS_COUNT + MAX_ADDITIONAL_SETTINGS]; }; class Origins { diff --git a/src/node_http2_state.h b/src/node_http2_state.h index f9ac6b40c3410a..487ddad51d8c22 100644 --- a/src/node_http2_state.h +++ b/src/node_http2_state.h @@ -21,6 +21,9 @@ namespace http2 { IDX_SETTINGS_COUNT }; + // number of max additional settings, thus settings not implemented by nghttp2 + const size_t MAX_ADDITIONAL_SETTINGS = 10; + enum Http2SessionStateIndex { IDX_SESSION_STATE_EFFECTIVE_LOCAL_WINDOW_SIZE, IDX_SESSION_STATE_EFFECTIVE_RECV_DATA_LENGTH, @@ -108,10 +111,11 @@ class Http2State : public BaseObject { offsetof(http2_state_internal, options_buffer), IDX_OPTIONS_FLAGS + 1, root_buffer), - settings_buffer(realm->isolate(), - offsetof(http2_state_internal, settings_buffer), - IDX_SETTINGS_COUNT + 1, - root_buffer) {} + settings_buffer( + realm->isolate(), + offsetof(http2_state_internal, settings_buffer), + IDX_SETTINGS_COUNT + 1 + 1 + 2 * MAX_ADDITIONAL_SETTINGS, + root_buffer) {} AliasedUint8Array root_buffer; AliasedFloat64Array session_state_buffer; @@ -135,7 +139,12 @@ class Http2State : public BaseObject { double stream_stats_buffer[IDX_STREAM_STATS_COUNT]; double session_stats_buffer[IDX_SESSION_STATS_COUNT]; uint32_t options_buffer[IDX_OPTIONS_FLAGS + 1]; - uint32_t settings_buffer[IDX_SETTINGS_COUNT + 1]; + // first + 1: number of actual nghttp2 supported settings + // second + 1: number of additional settings not suppoted by nghttp2 + // 2 * MAX_ADDITIONAL_SETTINGS: settings id and value for each + // additional setting + uint32_t settings_buffer[IDX_SETTINGS_COUNT + 1 + 1 + + 2 * MAX_ADDITIONAL_SETTINGS]; }; }; From 6bedd4cea0969bf7d33325f93afa8cae78de8231 Mon Sep 17 00:00:00 2001 From: Marten Richter Date: Sun, 13 Aug 2023 23:38:57 +0200 Subject: [PATCH 2/3] http2: add tests for customSettings Test for the http2 setting's custom settings were added. --- test/parallel/test-http2-getpackedsettings.js | 88 ++++++++++++++++++- test/parallel/test-http2-update-settings.js | 3 +- 2 files changed, 87 insertions(+), 4 deletions(-) diff --git a/test/parallel/test-http2-getpackedsettings.js b/test/parallel/test-http2-getpackedsettings.js index 4f5970522f13b3..05bf8eb6c1245d 100644 --- a/test/parallel/test-http2-getpackedsettings.js +++ b/test/parallel/test-http2-getpackedsettings.js @@ -29,6 +29,7 @@ assert.deepStrictEqual(val, check); ['maxHeaderListSize', 2 ** 32 - 1], ['maxHeaderSize', 0], ['maxHeaderSize', 2 ** 32 - 1], + ['customSettings', { '9999': 301 }], ].forEach((i) => { // Valid options should not throw. http2.getPackedSettings({ [i[0]]: i[1] }); @@ -93,6 +94,7 @@ http2.getPackedSettings({ enablePush: false }); 0x00, 0x05, 0x00, 0x00, 0x4e, 0x20, 0x00, 0x06, 0x00, 0x00, 0x00, 0x64, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00, + 0x27, 0x0F, 0x00, 0x00, 0x01, 0x2d, ]); const packed = http2.getPackedSettings({ @@ -104,12 +106,90 @@ http2.getPackedSettings({ enablePush: false }); maxHeaderSize: 100, enablePush: true, enableConnectProtocol: false, - foo: 'ignored' + foo: 'ignored', + customSettings: { '9999': 301 } }); - assert.strictEqual(packed.length, 42); + assert.strictEqual(packed.length, 48); assert.deepStrictEqual(packed, check); } +// Check if multiple custom settings can be set +{ + const check = Buffer.from([ + 0x00, 0x01, 0x00, 0x00, 0x00, 0x64, + 0x00, 0x02, 0x00, 0x00, 0x00, 0x01, + 0x00, 0x03, 0x00, 0x00, 0x00, 0xc8, + 0x00, 0x04, 0x00, 0x00, 0x00, 0x64, + 0x00, 0x05, 0x00, 0x00, 0x4e, 0x20, + 0x00, 0x06, 0x00, 0x00, 0x00, 0x64, + 0x00, 0x08, 0x00, 0x00, 0x00, 0x00, + 0x03, 0xf3, 0x00, 0x00, 0x07, 0x9F, + 0x0a, 0x2e, 0x00, 0x00, 0x00, 0x58, + ]); + + const packed = http2.getPackedSettings({ + headerTableSize: 100, + initialWindowSize: 100, + maxFrameSize: 20000, + maxConcurrentStreams: 200, + maxHeaderListSize: 100, + maxHeaderSize: 100, + enablePush: true, + enableConnectProtocol: false, + customSettings: { '2606': 88, '1011': 1951 } + }); + assert.strictEqual(packed.length, 54); + assert.deepStrictEqual(packed, check); +} + +{ + // Check if wrong custom settings cause an error + + assert.throws(() => { + http2.getPackedSettings({ + customSettings: { '-1': 659685 } + }); + }, { + code: 'ERR_HTTP2_INVALID_SETTING_VALUE', + name: 'RangeError' + }); + + assert.throws(() => { + http2.getPackedSettings({ + customSettings: { '10': 34577577777 } + }); + }, { + code: 'ERR_HTTP2_INVALID_SETTING_VALUE', + name: 'RangeError' + }); + + assert.throws(() => { + http2.getPackedSettings({ + customSettings: { 'notvalid': -777 } + }); + }, { + code: 'ERR_HTTP2_INVALID_SETTING_VALUE', + name: 'RangeError' + }); + + assert.throws(() => { + http2.getPackedSettings({ + customSettings: { '11': 11, '12': 12, '13': 13, '14': 14, '15': 15, '16': 16, + '17': 17, '18': 18, '19': 19, '20': 20, '21': 21 } + }); + }, { + code: 'ERR_HTTP2_TOO_MANY_CUSTOM_SETTINGS' + }); + assert.throws(() => { + http2.getPackedSettings({ + customSettings: { '11': 11, '12': 12, '13': 13, '14': 14, '15': 15, '16': 16, + '17': 17, '18': 18, '19': 19, '20': 20, '21': 21, '22': 22 } + }); + }, { + code: 'ERR_HTTP2_TOO_MANY_CUSTOM_SETTINGS' + }); +} + // Check for not passing settings. { const packed = http2.getPackedSettings(); @@ -124,7 +204,8 @@ http2.getPackedSettings({ enablePush: false }); 0x00, 0x04, 0x00, 0x00, 0x00, 0x64, 0x00, 0x06, 0x00, 0x00, 0x00, 0x64, 0x00, 0x02, 0x00, 0x00, 0x00, 0x01, - 0x00, 0x08, 0x00, 0x00, 0x00, 0x00]); + 0x00, 0x08, 0x00, 0x00, 0x00, 0x00, + 0x27, 0x0F, 0x00, 0x00, 0x01, 0x2d]); [1, true, '', [], {}, NaN].forEach((input) => { assert.throws(() => { @@ -157,6 +238,7 @@ http2.getPackedSettings({ enablePush: false }); assert.strictEqual(settings.maxHeaderSize, 100); assert.strictEqual(settings.enablePush, true); assert.strictEqual(settings.enableConnectProtocol, false); + assert.deepStrictEqual(settings.customSettings, { '9999': 301 }); } { diff --git a/test/parallel/test-http2-update-settings.js b/test/parallel/test-http2-update-settings.js index e99ee2bcf150d8..98b1a3790c3781 100644 --- a/test/parallel/test-http2-update-settings.js +++ b/test/parallel/test-http2-update-settings.js @@ -19,7 +19,8 @@ testUpdateSettingsWith({ 'maxHeaderListSize': 1, 'maxFrameSize': 16385, 'enablePush': false, - 'enableConnectProtocol': true + 'enableConnectProtocol': true, + 'customSettings': { '9999': 301 } } }); testUpdateSettingsWith({ From 2860c6502abb07e8ff8c1ae330fa2184aab20cd4 Mon Sep 17 00:00:00 2001 From: Marten Richter Date: Mon, 21 Aug 2023 09:12:00 +0200 Subject: [PATCH 3/3] http2: customSettings add documentation Add an explanation to the documentation for Http2Settings to explain the usage of customSettings and its limitations. Co-authored-by: James M Snell --- doc/api/errors.md | 6 ++++++ doc/api/http2.md | 13 +++++++++++++ 2 files changed, 19 insertions(+) diff --git a/doc/api/errors.md b/doc/api/errors.md index 38a29a1aae5000..38de652257c0e1 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1710,6 +1710,12 @@ When setting the priority for an HTTP/2 stream, the stream may be marked as a dependency for a parent stream. This error code is used when an attempt is made to mark a stream and dependent of itself. + + +### `ERR_HTTP2_TOO_MANY_CUSTOM_SETTINGS` + +The number of supported custom settings (10) has been exceeded. + ### `ERR_HTTP2_TOO_MANY_INVALID_FRAMES` diff --git a/doc/api/http2.md b/doc/api/http2.md index c45e5142ca753c..fea6c9a39acb9f 100644 --- a/doc/api/http2.md +++ b/doc/api/http2.md @@ -3010,6 +3010,19 @@ properties. meaningful if sent by the server. Once the `enableConnectProtocol` setting has been enabled for a given `Http2Session`, it cannot be disabled. **Default:** `false`. +* `customSettings` {Object} Specifies additional settings, yet not implemented + in node and the underlying libraries. The key of the object defines the + numeric value of the settings type (as defined in the "HTTP/2 SETTINGS" + registry established by \[RFC 7540]) and the values the actual numeric value + of the settings. + The settings type has to be an integer in the range from 1 to 2^16-1. + It should not be a settings type already handled by node, i.e. currently + it should be greater than 6, although it is not an error. + The values need to be unsigned integers in the range from 0 to 2^32-1. + Currently, a maximum of up 10 custom settings is supported. + It is only supported for sending SETTINGS. + Custom settings are not supported for the functions retrieving remote and + local settings as nghttp2 does not pass unknown HTTP/2 settings to Node.js. All additional properties on the settings object are ignored.