http2: reduce code duplication in settings

PR-URL: https://github.com/nodejs/node/pull/17328
Fixes: https://github.com/nodejs/node/issues/15303
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Sebastiaan Deckers <sebdeckers83@gmail.com>
This commit is contained in:
James M Snell 2017-11-25 12:44:07 -08:00
parent d4111d0286
commit df33d8d0b3
2 changed files with 35 additions and 81 deletions

View File

@ -661,6 +661,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 {
@ -844,29 +871,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);
@ -2364,30 +2369,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();
}
@ -2398,7 +2380,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);
@ -2409,7 +2391,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;
@ -2427,30 +2409,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;
}

View File

@ -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]);
@ -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