Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 67abc1e

Browse filesBrowse files
jasnellMylesBorins
jasnell
authored and
MylesBorins
committed
http2: reduce code duplication in settings
PR-URL: #17328 Fixes: #15303 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Sebastiaan Deckers <sebdeckers83@gmail.com>
1 parent e5f92cd commit 67abc1e
Copy full SHA for 67abc1e

File tree

Expand file treeCollapse file tree

2 files changed

+35
-81
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+35
-81
lines changed
Open diff view settings
Collapse file

‎lib/internal/http2/core.js‎

Copy file name to clipboardExpand all lines: lib/internal/http2/core.js
+33-73Lines changed: 33 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -659,6 +659,33 @@ function pingCallback(cb) {
659659
};
660660
}
661661

662+
function validateSettings(settings) {
663+
settings = Object.assign({}, settings);
664+
assertWithinRange('headerTableSize',
665+
settings.headerTableSize,
666+
0, kMaxInt);
667+
assertWithinRange('initialWindowSize',
668+
settings.initialWindowSize,
669+
0, kMaxInt);
670+
assertWithinRange('maxFrameSize',
671+
settings.maxFrameSize,
672+
16384, kMaxFrameSize);
673+
assertWithinRange('maxConcurrentStreams',
674+
settings.maxConcurrentStreams,
675+
0, kMaxStreams);
676+
assertWithinRange('maxHeaderListSize',
677+
settings.maxHeaderListSize,
678+
0, kMaxInt);
679+
if (settings.enablePush !== undefined &&
680+
typeof settings.enablePush !== 'boolean') {
681+
const err = new errors.TypeError('ERR_HTTP2_INVALID_SETTING_VALUE',
682+
'enablePush', settings.enablePush);
683+
err.actual = settings.enablePush;
684+
throw err;
685+
}
686+
return settings;
687+
}
688+
662689
// Upon creation, the Http2Session takes ownership of the socket. The session
663690
// may not be ready to use immediately if the socket is not yet fully connected.
664691
class Http2Session extends EventEmitter {
@@ -842,29 +869,7 @@ class Http2Session extends EventEmitter {
842869

843870
// Validate the input first
844871
assertIsObject(settings, 'settings');
845-
settings = Object.assign(Object.create(null), settings);
846-
assertWithinRange('headerTableSize',
847-
settings.headerTableSize,
848-
0, kMaxInt);
849-
assertWithinRange('initialWindowSize',
850-
settings.initialWindowSize,
851-
0, kMaxInt);
852-
assertWithinRange('maxFrameSize',
853-
settings.maxFrameSize,
854-
16384, kMaxFrameSize);
855-
assertWithinRange('maxConcurrentStreams',
856-
settings.maxConcurrentStreams,
857-
0, kMaxStreams);
858-
assertWithinRange('maxHeaderListSize',
859-
settings.maxHeaderListSize,
860-
0, kMaxInt);
861-
if (settings.enablePush !== undefined &&
862-
typeof settings.enablePush !== 'boolean') {
863-
const err = new errors.TypeError('ERR_HTTP2_INVALID_SETTING_VALUE',
864-
'enablePush', settings.enablePush);
865-
err.actual = settings.enablePush;
866-
throw err;
867-
}
872+
settings = validateSettings(settings);
868873
if (state.pendingAck === state.maxPendingAck) {
869874
throw new errors.Error('ERR_HTTP2_MAX_PENDING_SETTINGS_ACK',
870875
this[kState].pendingAck);
@@ -2362,30 +2367,7 @@ function createServer(options, handler) {
23622367
// HTTP2-Settings header frame.
23632368
function getPackedSettings(settings) {
23642369
assertIsObject(settings, 'settings');
2365-
settings = settings || Object.create(null);
2366-
assertWithinRange('headerTableSize',
2367-
settings.headerTableSize,
2368-
0, kMaxInt);
2369-
assertWithinRange('initialWindowSize',
2370-
settings.initialWindowSize,
2371-
0, kMaxInt);
2372-
assertWithinRange('maxFrameSize',
2373-
settings.maxFrameSize,
2374-
16384, kMaxFrameSize);
2375-
assertWithinRange('maxConcurrentStreams',
2376-
settings.maxConcurrentStreams,
2377-
0, kMaxStreams);
2378-
assertWithinRange('maxHeaderListSize',
2379-
settings.maxHeaderListSize,
2380-
0, kMaxInt);
2381-
if (settings.enablePush !== undefined &&
2382-
typeof settings.enablePush !== 'boolean') {
2383-
const err = new errors.TypeError('ERR_HTTP2_INVALID_SETTING_VALUE',
2384-
'enablePush', settings.enablePush);
2385-
err.actual = settings.enablePush;
2386-
throw err;
2387-
}
2388-
updateSettingsBuffer(settings);
2370+
updateSettingsBuffer(validateSettings(settings));
23892371
return binding.packSettings();
23902372
}
23912373

@@ -2396,7 +2378,7 @@ function getUnpackedSettings(buf, options = {}) {
23962378
}
23972379
if (buf.length % 6 !== 0)
23982380
throw new errors.RangeError('ERR_HTTP2_INVALID_PACKED_SETTINGS_LENGTH');
2399-
const settings = Object.create(null);
2381+
const settings = {};
24002382
let offset = 0;
24012383
while (offset < buf.length) {
24022384
const id = buf.readUInt16BE(offset);
@@ -2407,7 +2389,7 @@ function getUnpackedSettings(buf, options = {}) {
24072389
settings.headerTableSize = value;
24082390
break;
24092391
case NGHTTP2_SETTINGS_ENABLE_PUSH:
2410-
settings.enablePush = value;
2392+
settings.enablePush = value !== 0;
24112393
break;
24122394
case NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS:
24132395
settings.maxConcurrentStreams = value;
@@ -2425,30 +2407,8 @@ function getUnpackedSettings(buf, options = {}) {
24252407
offset += 4;
24262408
}
24272409

2428-
if (options != null && options.validate) {
2429-
assertWithinRange('headerTableSize',
2430-
settings.headerTableSize,
2431-
0, kMaxInt);
2432-
assertWithinRange('enablePush',
2433-
settings.enablePush,
2434-
0, 1);
2435-
assertWithinRange('initialWindowSize',
2436-
settings.initialWindowSize,
2437-
0, kMaxInt);
2438-
assertWithinRange('maxFrameSize',
2439-
settings.maxFrameSize,
2440-
16384, kMaxFrameSize);
2441-
assertWithinRange('maxConcurrentStreams',
2442-
settings.maxConcurrentStreams,
2443-
0, kMaxStreams);
2444-
assertWithinRange('maxHeaderListSize',
2445-
settings.maxHeaderListSize,
2446-
0, kMaxInt);
2447-
}
2448-
2449-
if (settings.enablePush !== undefined) {
2450-
settings.enablePush = !!settings.enablePush;
2451-
}
2410+
if (options != null && options.validate)
2411+
validateSettings(settings);
24522412

24532413
return settings;
24542414
}
Collapse file

‎test/parallel/test-http2-getpackedsettings.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-http2-getpackedsettings.js
+2-8Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,6 @@ assert.doesNotThrow(() => http2.getPackedSettings({ enablePush: false }));
128128
assert.strictEqual(settings.enablePush, true);
129129
}
130130

131-
//should throw if enablePush is not 0 or 1
132131
{
133132
const packed = Buffer.from([
134133
0x00, 0x02, 0x00, 0x00, 0x00, 0x00]);
@@ -140,13 +139,8 @@ assert.doesNotThrow(() => http2.getPackedSettings({ enablePush: false }));
140139
const packed = Buffer.from([
141140
0x00, 0x02, 0x00, 0x00, 0x00, 0x64]);
142141

143-
assert.throws(() => {
144-
http2.getUnpackedSettings(packed, { validate: true });
145-
}, common.expectsError({
146-
code: 'ERR_HTTP2_INVALID_SETTING_VALUE',
147-
type: RangeError,
148-
message: 'Invalid value for setting "enablePush": 100'
149-
}));
142+
const settings = http2.getUnpackedSettings(packed, { validate: true });
143+
assert.strictEqual(settings.enablePush, true);
150144
}
151145

152146
//check for what happens if passing {validate: true} and no errors happen

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.