http2: set default maxConcurrentStreams

ZYSzys · BridgeAR · commit 3f153789b559 · 2019-10-09T21:44:52.000+02:00
Set the default maxConcurrentStreams to NGHTTP2_DEFAULT_MAX_CONCURRENT_STREAMS. PR-URL: #29833 Fixes: #29763 Refs: nghttp2/nghttp2@16c4611 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
diff --git a/doc/api/http2.md b/doc/api/http2.md
@@ -2357,6 +2357,9 @@ server.on('stream', (stream, headers) => {
 <!-- YAML
 added: v8.4.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/29833
+    description: The `maxConcurrentStreams` setting is stricter.
   - version: v8.9.3
     pr-url: https://github.com/nodejs/node/pull/16676
     description: The `maxHeaderListSize` setting is now strictly enforced.
@@ -2382,9 +2385,10 @@ properties.
   is 2<sup>24</sup>-1. **Default:** `16,384 bytes`.
 * `maxConcurrentStreams` {number} Specifies the maximum number of concurrent
   streams permitted on an `Http2Session`. There is no default value which
-  implies, at least theoretically, 2<sup>31</sup>-1 streams may be open
+  implies, at least theoretically, 2<sup>32</sup>-1 streams may be open
   concurrently at any given time in an `Http2Session`. The minimum value
-  is 0. The maximum allowed value is 2<sup>31</sup>-1.
+  is 0. The maximum allowed value is 2<sup>32</sup>-1. **Default:**
+  `4294967295`.
 * `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`.
diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
@@ -161,7 +161,7 @@ function debugSessionObj(session, message, ...args) {
 
 const kMaxFrameSize = (2 ** 24) - 1;
 const kMaxInt = (2 ** 32) - 1;
-const kMaxStreams = (2 ** 31) - 1;
+const kMaxStreams = (2 ** 32) - 1;
 const kMaxALTSVC = (2 ** 14) - 2;
 
 // eslint-disable-next-line no-control-regex
diff --git a/src/node_http2.cc b/src/node_http2.cc
@@ -292,6 +292,8 @@ void Http2Session::Http2Settings::RefreshDefaults(Environment* env) {
       DEFAULT_SETTINGS_HEADER_TABLE_SIZE;
   buffer[IDX_SETTINGS_ENABLE_PUSH] =
       DEFAULT_SETTINGS_ENABLE_PUSH;
+  buffer[IDX_SETTINGS_MAX_CONCURRENT_STREAMS] =
+      DEFAULT_SETTINGS_MAX_CONCURRENT_STREAMS;
   buffer[IDX_SETTINGS_INITIAL_WINDOW_SIZE] =
       DEFAULT_SETTINGS_INITIAL_WINDOW_SIZE;
   buffer[IDX_SETTINGS_MAX_FRAME_SIZE] =
@@ -301,6 +303,7 @@ void Http2Session::Http2Settings::RefreshDefaults(Environment* env) {
   buffer[IDX_SETTINGS_COUNT] =
     (1 << IDX_SETTINGS_HEADER_TABLE_SIZE) |
     (1 << IDX_SETTINGS_ENABLE_PUSH) |
+    (1 << IDX_SETTINGS_MAX_CONCURRENT_STREAMS) |
     (1 << IDX_SETTINGS_INITIAL_WINDOW_SIZE) |
     (1 << IDX_SETTINGS_MAX_FRAME_SIZE) |
     (1 << IDX_SETTINGS_MAX_HEADER_LIST_SIZE);
@@ -3233,6 +3236,7 @@ void Initialize(Local<Object> target,
 
   NODE_DEFINE_CONSTANT(constants, DEFAULT_SETTINGS_HEADER_TABLE_SIZE);
   NODE_DEFINE_CONSTANT(constants, DEFAULT_SETTINGS_ENABLE_PUSH);
+  NODE_DEFINE_CONSTANT(constants, DEFAULT_SETTINGS_MAX_CONCURRENT_STREAMS);
   NODE_DEFINE_CONSTANT(constants, DEFAULT_SETTINGS_INITIAL_WINDOW_SIZE);
   NODE_DEFINE_CONSTANT(constants, DEFAULT_SETTINGS_MAX_FRAME_SIZE);
   NODE_DEFINE_CONSTANT(constants, MAX_MAX_FRAME_SIZE);
diff --git a/src/node_http2.h b/src/node_http2.h
@@ -40,6 +40,7 @@ using performance::PerformanceEntry;
 // These are the standard HTTP/2 defaults as specified by the RFC
 #define DEFAULT_SETTINGS_HEADER_TABLE_SIZE 4096
 #define DEFAULT_SETTINGS_ENABLE_PUSH 1
+#define DEFAULT_SETTINGS_MAX_CONCURRENT_STREAMS 0xffffffffu
 #define DEFAULT_SETTINGS_INITIAL_WINDOW_SIZE 65535
 #define DEFAULT_SETTINGS_MAX_FRAME_SIZE 16384
 #define DEFAULT_SETTINGS_MAX_HEADER_LIST_SIZE 65535
diff --git a/test/parallel/test-http2-binding.js b/test/parallel/test-http2-binding.js
@@ -16,6 +16,7 @@ assert.strictEqual(typeof binding.Http2Session, 'function');
 const settings = http2.getDefaultSettings();
 assert.strictEqual(settings.headerTableSize, 4096);
 assert.strictEqual(settings.enablePush, true);
+assert.strictEqual(settings.maxConcurrentStreams, 4294967295);
 assert.strictEqual(settings.initialWindowSize, 65535);
 assert.strictEqual(settings.maxFrameSize, 16384);
 
@@ -227,6 +228,7 @@ const expectedNGConstants = {
 const defaultSettings = {
   DEFAULT_SETTINGS_HEADER_TABLE_SIZE: 4096,
   DEFAULT_SETTINGS_ENABLE_PUSH: 1,
+  DEFAULT_SETTINGS_MAX_CONCURRENT_STREAMS: 4294967295,
   DEFAULT_SETTINGS_INITIAL_WINDOW_SIZE: 65535,
   DEFAULT_SETTINGS_MAX_FRAME_SIZE: 16384
 };
diff --git a/test/parallel/test-http2-client-setNextStreamID-errors.js b/test/parallel/test-http2-client-setNextStreamID-errors.js
@@ -26,14 +26,14 @@ server.listen(0, common.mustCall(() => {
   const client = http2.connect(`http://localhost:${server.address().port}`);
 
   client.on('connect', () => {
-    const outOfRangeNum = 2 ** 31;
+    const outOfRangeNum = 2 ** 32;
     common.expectsError(
       () => client.setNextStreamID(outOfRangeNum),
       {
         type: RangeError,
         code: 'ERR_OUT_OF_RANGE',
         message: 'The value of "id" is out of range.' +
-           ' It must be > 0 and <= 2147483647. Received ' + outOfRangeNum
+           ' It must be > 0 and <= 4294967295. Received ' + outOfRangeNum
       }
     );
 
diff --git a/test/parallel/test-http2-client-settings-before-connect.js b/test/parallel/test-http2-client-settings-before-connect.js
@@ -28,7 +28,7 @@ server.listen(0, common.mustCall(() => {
     ['maxFrameSize', 1, RangeError],
     ['maxFrameSize', 2 ** 24, RangeError],
     ['maxConcurrentStreams', -1, RangeError],
-    ['maxConcurrentStreams', 2 ** 31, RangeError],
+    ['maxConcurrentStreams', 2 ** 32, RangeError],
     ['maxHeaderListSize', -1, RangeError],
     ['maxHeaderListSize', 2 ** 32, RangeError],
     ['enablePush', 'a', TypeError],
diff --git a/test/parallel/test-http2-getpackedsettings.js b/test/parallel/test-http2-getpackedsettings.js
@@ -7,6 +7,7 @@ const assert = require('assert');
 const http2 = require('http2');
 
 const check = Buffer.from([0x00, 0x01, 0x00, 0x00, 0x10, 0x00,
+                           0x00, 0x03, 0xff, 0xff, 0xff, 0xff,
                            0x00, 0x05, 0x00, 0x00, 0x40, 0x00,
                            0x00, 0x04, 0x00, 0x00, 0xff, 0xff,
                            0x00, 0x06, 0x00, 0x00, 0xff, 0xff,
@@ -41,7 +42,7 @@ http2.getPackedSettings({ enablePush: false });
   ['maxFrameSize', 16383],
   ['maxFrameSize', 2 ** 24],
   ['maxConcurrentStreams', -1],
-  ['maxConcurrentStreams', 2 ** 31],
+  ['maxConcurrentStreams', 2 ** 32],
   ['maxHeaderListSize', -1],
   ['maxHeaderListSize', 2 ** 32]
 ].forEach((i) => {
@@ -168,16 +169,3 @@ http2.getPackedSettings({ enablePush: false });
     message: 'Invalid value for setting "maxFrameSize": 16777216'
   });
 }
-
-// Check for maxConcurrentStreams failing the max number.
-{
-  const packed = Buffer.from([0x00, 0x03, 0xFF, 0xFF, 0xFF, 0xFF]);
-
-  common.expectsError(() => {
-    http2.getUnpackedSettings(packed, { validate: true });
-  }, {
-    code: 'ERR_HTTP2_INVALID_SETTING_VALUE',
-    type: RangeError,
-    message: 'Invalid value for setting "maxConcurrentStreams": 4294967295'
-  });
-}
