From c20901a7f574d7ecea70dfc852a3307d7178ddba Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Thu, 7 Sep 2017 07:25:43 -0400 Subject: [PATCH] http2: correct behaviour for enablePush unpack The only valid values for enablePush are 0 and 1. If validation is requested, we should verify that it wasn't set to another value rather than casting to Boolean regardless of value. PR-URL: https://github.com/nodejs/node/pull/15167 Reviewed-By: Anna Henningsen Reviewed-By: Benjamin Gruenbaum Reviewed-By: Luigi Pinca Reviewed-By: James M Snell Reviewed-By: Ruben Bridgewater --- lib/internal/http2/core.js | 16 +++++++------- test/parallel/test-http2-getpackedsettings.js | 21 +++++++++++++++++++ 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 8646f843a5bc88..15b6f7bdd4f4db 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -2547,7 +2547,7 @@ function getUnpackedSettings(buf, options = {}) { settings.headerTableSize = value; break; case NGHTTP2_SETTINGS_ENABLE_PUSH: - settings.enablePush = Boolean(value); + settings.enablePush = value; break; case NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS: settings.maxConcurrentStreams = value; @@ -2569,6 +2569,9 @@ function getUnpackedSettings(buf, options = {}) { assertWithinRange('headerTableSize', settings.headerTableSize, 0, 2 ** 32 - 1); + assertWithinRange('enablePush', + settings.enablePush, + 0, 1); assertWithinRange('initialWindowSize', settings.initialWindowSize, 0, 2 ** 32 - 1); @@ -2581,13 +2584,10 @@ function getUnpackedSettings(buf, options = {}) { assertWithinRange('maxHeaderListSize', settings.maxHeaderListSize, 0, 2 ** 32 - 1); - 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; - } + } + + if (settings.enablePush !== undefined) { + settings.enablePush = !!settings.enablePush; } return settings; diff --git a/test/parallel/test-http2-getpackedsettings.js b/test/parallel/test-http2-getpackedsettings.js index e5e49571800797..8c46afcb57fd82 100644 --- a/test/parallel/test-http2-getpackedsettings.js +++ b/test/parallel/test-http2-getpackedsettings.js @@ -126,6 +126,27 @@ 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]); + + const settings = http2.getUnpackedSettings(packed, { validate: true }); + assert.strictEqual(settings.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' + })); +} + //check for what happens if passing {validate: true} and no errors happen { const packed = Buffer.from([