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: #15167 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
nodejs · Sep 20, 2017 · 2e421ff · 2e421ff
1 parent 4d68064
commit 2e421ff
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 8 deletions.
diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
@@ -2535,7 +2535,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;
@@ -2557,6 +2557,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);
@@ -2569,13 +2572,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
@@ -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([