http2: add maxHeaderSize option to http2

add maxHeaderSize to http2 as an alias for maxHeaderListSize. Fixes: #33517 PR-URL: #33636 Reviewed-By: Zeyu Yang <himself65@outlook.com> Reviewed-By: Pranshu Srivastava <rexagod@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
nodejs · Aug 20, 2020 · 6ee2578 · 6ee2578
1 parent b933eef
commit 6ee2578
Show file tree

Hide file tree

Showing 7 changed files with 54 additions and 26 deletions.
diff --git a/doc/api/http2.md b/doc/api/http2.md
@@ -2570,6 +2570,7 @@ properties.
 * `maxHeaderListSize` {number} Specifies the maximum size (uncompressed octets)
   of header list that will be accepted. The minimum allowed value is 0. The
   maximum allowed value is 2<sup>32</sup>-1. **Default:** `65535`.
+* `maxHeaderSize` {number} Alias for `maxHeaderListSize`.
 * `enableConnectProtocol`{boolean} Specifies `true` if the "Extended Connect
   Protocol" defined by [RFC 8441][] is to be enabled. This setting is only
   meaningful if sent by the server. Once the `enableConnectProtocol` setting

diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
@@ -908,6 +908,9 @@ const validateSettings = hideStackFrames((settings) => {
   assertWithinRange('maxHeaderListSize',
                     settings.maxHeaderListSize,
                     0, kMaxInt);
+  assertWithinRange('maxHeaderSize',
+                    settings.maxHeaderSize,
+                    0, kMaxInt);
   if (settings.enablePush !== undefined &&
       typeof settings.enablePush !== 'boolean') {
     throw new ERR_HTTP2_INVALID_SETTING_VALUE('enablePush',
@@ -3102,7 +3105,7 @@ function getUnpackedSettings(buf, options = {}) {
         settings.maxFrameSize = value;
         break;
       case NGHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE:
-        settings.maxHeaderListSize = value;
+        settings.maxHeaderListSize = settings.maxHeaderSize = value;
         break;
       case NGHTTP2_SETTINGS_ENABLE_CONNECT_PROTOCOL:
         settings.enableConnectProtocol = value !== 0;

diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js
@@ -300,7 +300,7 @@ function getDefaultSettings() {
 
   if ((flags & (1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE)) ===
       (1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE)) {
-    holder.maxHeaderListSize =
+    holder.maxHeaderListSize = holder.maxHeaderSize =
       settingsBuffer[IDX_SETTINGS_MAX_HEADER_LIST_SIZE];
   }
 
@@ -328,6 +328,7 @@ function getSettings(session, remote) {
     maxFrameSize: settingsBuffer[IDX_SETTINGS_MAX_FRAME_SIZE],
     maxConcurrentStreams: settingsBuffer[IDX_SETTINGS_MAX_CONCURRENT_STREAMS],
     maxHeaderListSize: settingsBuffer[IDX_SETTINGS_MAX_HEADER_LIST_SIZE],
+    maxHeaderSize: settingsBuffer[IDX_SETTINGS_MAX_HEADER_LIST_SIZE],
     enableConnectProtocol:
       !!settingsBuffer[IDX_SETTINGS_ENABLE_CONNECT_PROTOCOL]
   };
@@ -355,10 +356,20 @@ function updateSettingsBuffer(settings) {
     settingsBuffer[IDX_SETTINGS_MAX_FRAME_SIZE] =
       settings.maxFrameSize;
   }
-  if (typeof settings.maxHeaderListSize === 'number') {
+  if (typeof settings.maxHeaderListSize === 'number' ||
+      typeof settings.maxHeaderSize === 'number') {
     flags |= (1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE);
-    settingsBuffer[IDX_SETTINGS_MAX_HEADER_LIST_SIZE] =
-      settings.maxHeaderListSize;
+    if (settings.maxHeaderSize !== undefined &&
+      (settings.maxHeaderSize !== settings.maxHeaderListSize)) {
+      process.emitWarning(
+        'settings.maxHeaderSize overwrite settings.maxHeaderListSize'
+      );
+      settingsBuffer[IDX_SETTINGS_MAX_HEADER_LIST_SIZE] =
+        settings.maxHeaderSize;
+    } else {
+      settingsBuffer[IDX_SETTINGS_MAX_HEADER_LIST_SIZE] =
+        settings.maxHeaderListSize;
+    }
   }
   if (typeof settings.enablePush === 'boolean') {
     flags |= (1 << IDX_SETTINGS_ENABLE_PUSH);

diff --git a/test/parallel/test-http2-client-settings-before-connect.js b/test/parallel/test-http2-client-settings-before-connect.js
@@ -32,6 +32,8 @@ server.listen(0, common.mustCall(() => {
     ['maxConcurrentStreams', 2 ** 32, RangeError],
     ['maxHeaderListSize', -1, RangeError],
     ['maxHeaderListSize', 2 ** 32, RangeError],
+    ['maxHeaderSize', -1, RangeError],
+    ['maxHeaderSize', 2 ** 32, RangeError],
     ['enablePush', 'a', TypeError],
     ['enablePush', 1, TypeError],
     ['enablePush', 0, TypeError],

diff --git a/test/parallel/test-http2-getpackedsettings.js b/test/parallel/test-http2-getpackedsettings.js
@@ -26,7 +26,9 @@ assert.deepStrictEqual(val, check);
   ['maxConcurrentStreams', 0],
   ['maxConcurrentStreams', 2 ** 31 - 1],
   ['maxHeaderListSize', 0],
-  ['maxHeaderListSize', 2 ** 32 - 1]
+  ['maxHeaderListSize', 2 ** 32 - 1],
+  ['maxHeaderSize', 0],
+  ['maxHeaderSize', 2 ** 32 - 1]
 ].forEach((i) => {
   // Valid options should not throw.
   http2.getPackedSettings({ [i[0]]: i[1] });
@@ -45,7 +47,9 @@ http2.getPackedSettings({ enablePush: false });
   ['maxConcurrentStreams', -1],
   ['maxConcurrentStreams', 2 ** 32],
   ['maxHeaderListSize', -1],
-  ['maxHeaderListSize', 2 ** 32]
+  ['maxHeaderListSize', 2 ** 32],
+  ['maxHeaderSize', -1],
+  ['maxHeaderSize', 2 ** 32]
 ].forEach((i) => {
   assert.throws(() => {
     http2.getPackedSettings({ [i[0]]: i[1] });
@@ -97,6 +101,7 @@ http2.getPackedSettings({ enablePush: false });
     maxFrameSize: 20000,
     maxConcurrentStreams: 200,
     maxHeaderListSize: 100,
+    maxHeaderSize: 100,
     enablePush: true,
     enableConnectProtocol: false,
     foo: 'ignored'
@@ -149,6 +154,7 @@ http2.getPackedSettings({ enablePush: false });
   assert.strictEqual(settings.maxFrameSize, 20000);
   assert.strictEqual(settings.maxConcurrentStreams, 200);
   assert.strictEqual(settings.maxHeaderListSize, 100);
+  assert.strictEqual(settings.maxHeaderSize, 100);
   assert.strictEqual(settings.enablePush, true);
   assert.strictEqual(settings.enableConnectProtocol, false);
 }

diff --git a/test/parallel/test-http2-session-settings.js b/test/parallel/test-http2-session-settings.js
@@ -19,6 +19,7 @@ server.on(
       assert.strictEqual(typeof settings.maxFrameSize, 'number');
       assert.strictEqual(typeof settings.maxConcurrentStreams, 'number');
       assert.strictEqual(typeof settings.maxHeaderListSize, 'number');
+      assert.strictEqual(typeof settings.maxHeaderSize, 'number');
     };
 
     const localSettings = stream.session.localSettings;
@@ -100,7 +101,9 @@ server.listen(
         ['maxFrameSize', 16383],
         ['maxFrameSize', 2 ** 24],
         ['maxHeaderListSize', -1],
-        ['maxHeaderListSize', 2 ** 32]
+        ['maxHeaderListSize', 2 ** 32],
+        ['maxHeaderSize', -1],
+        ['maxHeaderSize', 2 ** 32]
       ].forEach((i) => {
         const settings = {};
         settings[i[0]] = i[1];

diff --git a/test/parallel/test-http2-too-large-headers.js b/test/parallel/test-http2-too-large-headers.js
@@ -9,24 +9,26 @@ const {
   NGHTTP2_ENHANCE_YOUR_CALM
 } = http2.constants;
 
-const server = http2.createServer({ settings: { maxHeaderListSize: 100 } });
-server.on('stream', common.mustNotCall());
+for (const prototype of ['maxHeaderListSize', 'maxHeaderSize']) {
+  const server = http2.createServer({ settings: { [prototype]: 100 } });
+  server.on('stream', common.mustNotCall());
 
-server.listen(0, common.mustCall(() => {
-  const client = http2.connect(`http://localhost:${server.address().port}`);
+  server.listen(0, common.mustCall(() => {
+    const client = http2.connect(`http://localhost:${server.address().port}`);
 
-  client.on('remoteSettings', () => {
-    const req = client.request({ 'foo': 'a'.repeat(1000) });
-    req.on('error', common.expectsError({
-      code: 'ERR_HTTP2_STREAM_ERROR',
-      name: 'Error',
-      message: 'Stream closed with error code NGHTTP2_ENHANCE_YOUR_CALM'
-    }));
-    req.on('close', common.mustCall(() => {
-      assert.strictEqual(req.rstCode, NGHTTP2_ENHANCE_YOUR_CALM);
-      server.close();
-      client.close();
-    }));
-  });
+    client.on('remoteSettings', () => {
+      const req = client.request({ 'foo': 'a'.repeat(1000) });
+      req.on('error', common.expectsError({
+        code: 'ERR_HTTP2_STREAM_ERROR',
+        name: 'Error',
+        message: 'Stream closed with error code NGHTTP2_ENHANCE_YOUR_CALM'
+      }));
+      req.on('close', common.mustCall(() => {
+        assert.strictEqual(req.rstCode, NGHTTP2_ENHANCE_YOUR_CALM);
+        server.close();
+        client.close();
+      }));
+    });
 
-}));
+  }));
+}