From ce265908eb84aa0518dcadc57d59e7fbb353a256 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Wed, 20 Mar 2019 12:18:16 +0100 Subject: [PATCH] http2: remove side effects from validateSettings 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: https://github.com/nodejs/node/pull/26809 Reviewed-By: Matteo Collina Reviewed-By: James M Snell --- lib/internal/http2/core.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 824d7d604c9a09..69cc72c7ae5905 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -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); @@ -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 @@ -1144,7 +1143,7 @@ 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(); @@ -1152,7 +1151,7 @@ class Http2Session extends EventEmitter { 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; @@ -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(); }