Skip to content

Commit

Permalink
http2: remove side effects from validateSettings
Browse files Browse the repository at this point in the history
The function did not only validate the input so far but it also made
a copy of the input object and returned that copy to the callee
function. That copy was not necessary for all call sites and it was
not obvious that the function did not only validate the input but
that it also returned a copy of it. This makes sure the function does
nothing more than validation and copying is happening in the callee
function when required.

PR-URL: #26809
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
BridgeAR committed Mar 27, 2019
1 parent 28e2c37 commit ce26590
Showing 1 changed file with 5 additions and 5 deletions.
10 changes: 5 additions & 5 deletions lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -784,7 +784,7 @@ function pingCallback(cb) {
// 6. enablePush must be a boolean
// All settings are optional and may be left undefined
const validateSettings = hideStackFrames((settings) => {
settings = { ...settings };
if (settings === undefined) return;
assertWithinRange('headerTableSize',
settings.headerTableSize,
0, kMaxInt);
Expand All @@ -805,7 +805,6 @@ const validateSettings = hideStackFrames((settings) => {
throw new ERR_HTTP2_INVALID_SETTING_VALUE('enablePush',
settings.enablePush);
}
return settings;
});

// Creates the internal binding.Http2Session handle for an Http2Session
Expand Down Expand Up @@ -1144,15 +1143,15 @@ class Http2Session extends EventEmitter {
if (this.destroyed)
throw new ERR_HTTP2_INVALID_SESSION();
assertIsObject(settings, 'settings');
settings = validateSettings(settings);
validateSettings(settings);

if (callback && typeof callback !== 'function')
throw new ERR_INVALID_CALLBACK();
debug(`Http2Session ${sessionName(this[kType])}: sending settings`);

this[kState].pendingAck++;

const settingsFn = submitSettings.bind(this, settings, callback);
const settingsFn = submitSettings.bind(this, { ...settings }, callback);
if (this.connecting) {
this.once('connect', settingsFn);
return;
Expand Down Expand Up @@ -2817,7 +2816,8 @@ function createServer(options, handler) {
// HTTP2-Settings header frame.
function getPackedSettings(settings) {
assertIsObject(settings, 'settings');
updateSettingsBuffer(validateSettings(settings));
validateSettings(settings);
updateSettingsBuffer({ ...settings });
return binding.packSettings();
}

Expand Down

0 comments on commit ce26590

Please sign in to comment.