Skip to content

Commit 67abc1e

Browse files
jasnellMylesBorins
authored andcommitted
http2: reduce code duplication in settings
PR-URL: #17328 Fixes: #15303 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Sebastiaan Deckers <[email protected]>
1 parent e5f92cd commit 67abc1e

File tree

2 files changed

+35
-81
lines changed

2 files changed

+35
-81
lines changed

lib/internal/http2/core.js

+33-73
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
}

test/parallel/test-http2-getpackedsettings.js

+2-8
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)