http2: explicitly disallow nested push streams

jasnell · rvagg · commit 7223a91a5080 · 2018-08-15T20:23:17.000+10:00
Fixes: #19095 PR-URL: #22245 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
diff --git a/doc/api/errors.md b/doc/api/errors.md
@@ -956,6 +956,12 @@ required to send an acknowledgment that it has received and applied the new
 be sent at any given time. This error code is used when that limit has been
 reached.
 
+<a id="ERR_HTTP2_NESTED_PUSH"></a>
+### ERR_HTTP2_NESTED_PUSH
+
+An attempt was made to initiate a new push stream from within a push stream.
+Nested push streams are not permitted.
+
 <a id="ERR_HTTP2_NO_SOCKET_MANIPULATION"></a>
 ### ERR_HTTP2_NO_SOCKET_MANIPULATION
 
diff --git a/doc/api/http2.md b/doc/api/http2.md
@@ -1256,6 +1256,9 @@ Setting the weight of a push stream is not allowed in the `HEADERS` frame. Pass
 a `weight` value to `http2stream.priority` with the `silent` option set to
 `true` to enable server-side bandwidth balancing between concurrent streams.
 
+Calling `http2stream.pushStream()` from within a pushed stream is not permitted
+and will throw an error.
+
 #### http2stream.respond([headers[, options]])
 <!-- YAML
 added: v8.4.0
diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -568,6 +568,8 @@ E('ERR_HTTP2_INVALID_SETTING_VALUE',
 E('ERR_HTTP2_INVALID_STREAM', 'The stream has been destroyed', Error);
 E('ERR_HTTP2_MAX_PENDING_SETTINGS_ACK',
   'Maximum number of pending settings acknowledgements', Error);
+E('ERR_HTTP2_NESTED_PUSH',
+  'A push stream cannot initiate another push stream.', Error);
 E('ERR_HTTP2_NO_SOCKET_MANIPULATION',
   'HTTP/2 sockets should not be directly manipulated (e.g. read and written)',
   Error);
diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
@@ -47,6 +47,7 @@ const {
     ERR_HTTP2_INVALID_SETTING_VALUE,
     ERR_HTTP2_INVALID_STREAM,
     ERR_HTTP2_MAX_PENDING_SETTINGS_ACK,
+    ERR_HTTP2_NESTED_PUSH,
     ERR_HTTP2_NO_SOCKET_MANIPULATION,
     ERR_HTTP2_OUT_OF_STREAMS,
     ERR_HTTP2_PAYLOAD_FORBIDDEN,
@@ -2158,6 +2159,8 @@ class ServerHttp2Stream extends Http2Stream {
   pushStream(headers, options, callback) {
     if (!this.pushAllowed)
       throw new ERR_HTTP2_PUSH_DISABLED();
+    if (this[kID] % 2 === 0)
+      throw new ERR_HTTP2_NESTED_PUSH();
 
     const session = this[kSession];
 
diff --git a/test/parallel/test-http2-server-push-stream.js b/test/parallel/test-http2-server-push-stream.js
@@ -22,6 +22,14 @@ server.on('stream', common.mustCall((stream, headers) => {
         'x-push-data': 'pushed by server',
       });
       push.end('pushed by server data');
+
+      common.expectsError(() => {
+        push.pushStream({}, common.mustNotCall());
+      }, {
+        code: 'ERR_HTTP2_NESTED_PUSH',
+        type: Error
+      });
+
       stream.end('test');
     }));
   }
