Skip to content

Commit

Permalink
http2: reduce code duplication in settings
Browse files Browse the repository at this point in the history 
PR-URL: #17328
Fixes: #15303
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Sebastiaan Deckers <sebdeckers83@gmail.com>
  • Loading branch information
@jasnell @MylesBorins
jasnell authored and MylesBorins committed Dec 12, 2017
1 parent e5f92cd commit 67abc1e
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 81 deletions.
106 changes: 33 additions & 73 deletions lib/internal/http2/core.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,33 @@ function pingCallback(cb) {
};
}

function validateSettings(settings) {
settings = Object.assign({}, settings);
assertWithinRange('headerTableSize',
settings.headerTableSize,
0, kMaxInt);
assertWithinRange('initialWindowSize',
settings.initialWindowSize,
0, kMaxInt);
assertWithinRange('maxFrameSize',
settings.maxFrameSize,
16384, kMaxFrameSize);
assertWithinRange('maxConcurrentStreams',
settings.maxConcurrentStreams,
0, kMaxStreams);
assertWithinRange('maxHeaderListSize',
settings.maxHeaderListSize,
0, kMaxInt);
if (settings.enablePush !== undefined &&
typeof settings.enablePush !== 'boolean') {
const err = new errors.TypeError('ERR_HTTP2_INVALID_SETTING_VALUE',
'enablePush', settings.enablePush);
err.actual = settings.enablePush;
throw err;
}
return settings;
}

// Upon creation, the Http2Session takes ownership of the socket. The session
// may not be ready to use immediately if the socket is not yet fully connected.
class Http2Session extends EventEmitter {
Expand Down Expand Up @@ -842,29 +869,7 @@ class Http2Session extends EventEmitter {

// Validate the input first
assertIsObject(settings, 'settings');
settings = Object.assign(Object.create(null), settings);
assertWithinRange('headerTableSize',
settings.headerTableSize,
0, kMaxInt);
assertWithinRange('initialWindowSize',
settings.initialWindowSize,
0, kMaxInt);
assertWithinRange('maxFrameSize',
settings.maxFrameSize,
16384, kMaxFrameSize);
assertWithinRange('maxConcurrentStreams',
settings.maxConcurrentStreams,
0, kMaxStreams);
assertWithinRange('maxHeaderListSize',
settings.maxHeaderListSize,
0, kMaxInt);
if (settings.enablePush !== undefined &&
typeof settings.enablePush !== 'boolean') {
const err = new errors.TypeError('ERR_HTTP2_INVALID_SETTING_VALUE',
'enablePush', settings.enablePush);
err.actual = settings.enablePush;
throw err;
}
settings = validateSettings(settings);
if (state.pendingAck === state.maxPendingAck) {
throw new errors.Error('ERR_HTTP2_MAX_PENDING_SETTINGS_ACK',
this[kState].pendingAck);
Expand Down Expand Up @@ -2362,30 +2367,7 @@ function createServer(options, handler) {
// HTTP2-Settings header frame.
function getPackedSettings(settings) {
assertIsObject(settings, 'settings');
settings = settings || Object.create(null);
assertWithinRange('headerTableSize',
settings.headerTableSize,
0, kMaxInt);
assertWithinRange('initialWindowSize',
settings.initialWindowSize,
0, kMaxInt);
assertWithinRange('maxFrameSize',
settings.maxFrameSize,
16384, kMaxFrameSize);
assertWithinRange('maxConcurrentStreams',
settings.maxConcurrentStreams,
0, kMaxStreams);
assertWithinRange('maxHeaderListSize',
settings.maxHeaderListSize,
0, kMaxInt);
if (settings.enablePush !== undefined &&
typeof settings.enablePush !== 'boolean') {
const err = new errors.TypeError('ERR_HTTP2_INVALID_SETTING_VALUE',
'enablePush', settings.enablePush);
err.actual = settings.enablePush;
throw err;
}
updateSettingsBuffer(settings);
updateSettingsBuffer(validateSettings(settings));
return binding.packSettings();
}

Expand All @@ -2396,7 +2378,7 @@ function getUnpackedSettings(buf, options = {}) {
}
if (buf.length % 6 !== 0)
throw new errors.RangeError('ERR_HTTP2_INVALID_PACKED_SETTINGS_LENGTH');
const settings = Object.create(null);
const settings = {};
let offset = 0;
while (offset < buf.length) {
const id = buf.readUInt16BE(offset);
Expand All @@ -2407,7 +2389,7 @@ function getUnpackedSettings(buf, options = {}) {
settings.headerTableSize = value;
break;
case NGHTTP2_SETTINGS_ENABLE_PUSH:
settings.enablePush = value;
settings.enablePush = value !== 0;
break;
case NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS:
settings.maxConcurrentStreams = value;
Expand All @@ -2425,30 +2407,8 @@ function getUnpackedSettings(buf, options = {}) {
offset += 4;
}

if (options != null && options.validate) {
assertWithinRange('headerTableSize',
settings.headerTableSize,
0, kMaxInt);
assertWithinRange('enablePush',
settings.enablePush,
0, 1);
assertWithinRange('initialWindowSize',
settings.initialWindowSize,
0, kMaxInt);
assertWithinRange('maxFrameSize',
settings.maxFrameSize,
16384, kMaxFrameSize);
assertWithinRange('maxConcurrentStreams',
settings.maxConcurrentStreams,
0, kMaxStreams);
assertWithinRange('maxHeaderListSize',
settings.maxHeaderListSize,
0, kMaxInt);
}

if (settings.enablePush !== undefined) {
settings.enablePush = !!settings.enablePush;
}
if (options != null && options.validate)
validateSettings(settings);

return settings;
}
Expand Down
10 changes: 2 additions & 8 deletions test/parallel/test-http2-getpackedsettings.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,6 @@ assert.doesNotThrow(() => http2.getPackedSettings({ enablePush: false }));
assert.strictEqual(settings.enablePush, true);
}

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

assert.throws(() => {
http2.getUnpackedSettings(packed, { validate: true });
}, common.expectsError({
code: 'ERR_HTTP2_INVALID_SETTING_VALUE',
type: RangeError,
message: 'Invalid value for setting "enablePush": 100'
}));
const settings = http2.getUnpackedSettings(packed, { validate: true });
assert.strictEqual(settings.enablePush, true);
}

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

0 comments on commit 67abc1e

Please sign in to comment.