Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial proposal for additional http2settings #49025

Merged
merged 3 commits into from
Dec 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

<a id="ERR_HTTP2_TOO_MANY_CUSTOM_SETTINGS"></a>

### `ERR_HTTP2_TOO_MANY_CUSTOM_SETTINGS`

The number of supported custom settings (10) has been exceeded.

<a id="ERR_HTTP2_TOO_MANY_INVALID_FRAMES"></a>

### `ERR_HTTP2_TOO_MANY_INVALID_FRAMES`
Expand Down
13 changes: 13 additions & 0 deletions doc/api/http2.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
jasnell marked this conversation as resolved.
Show resolved Hide resolved

All additional properties on the settings object are ignored.

Expand Down
2 changes: 2 additions & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 6 additions & 0 deletions lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
Expand Down
79 changes: 79 additions & 0 deletions lib/internal/http2/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const {
Error,
MathMax,
Number,
NumberIsNaN,
ObjectKeys,
SafeSet,
String,
Expand All @@ -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,
},
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
jasnell marked this conversation as resolved.
Show resolved Hide resolved
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) {
jasnell marked this conversation as resolved.
Show resolved Hide resolved
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;
jasnell marked this conversation as resolved.
Show resolved Hide resolved
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] =
Expand Down
15 changes: 14 additions & 1 deletion src/node_http2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -262,7 +272,7 @@ Local<Value> Http2Settings::Pack() {
}

Local<Value> 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);
}
Expand Down Expand Up @@ -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.
Expand All @@ -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
}


Expand Down
2 changes: 1 addition & 1 deletion src/node_http2.h
Original file line number Diff line number Diff line change
Expand Up @@ -1035,7 +1035,7 @@ class Http2Settings : public AsyncWrap {
v8::Global<v8::Function> 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 {
Expand Down
19 changes: 14 additions & 5 deletions src/node_http2_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand All @@ -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];
jasnell marked this conversation as resolved.
Show resolved Hide resolved
};
};

Expand Down
88 changes: 85 additions & 3 deletions test/parallel/test-http2-getpackedsettings.js
Original file line number Diff line number Diff line change
Expand Up @@ -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] });
Expand Down Expand Up @@ -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({
Expand All @@ -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();
Expand All @@ -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(() => {
Expand Down Expand Up @@ -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 });
jasnell marked this conversation as resolved.
Show resolved Hide resolved
}

{
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-http2-update-settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ testUpdateSettingsWith({
'maxHeaderListSize': 1,
'maxFrameSize': 16385,
'enablePush': false,
'enableConnectProtocol': true
'enableConnectProtocol': true,
'customSettings': { '9999': 301 }
}
});
testUpdateSettingsWith({
Expand Down