Skip to content

Commit

Permalink
http2: explicitly disallow nested push streams
Browse files Browse the repository at this point in the history
Fixes: #19095

PR-URL: #22245
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
  • Loading branch information
jasnell authored and rvagg committed Aug 15, 2018
1 parent f570c19 commit 7223a91
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 0 deletions.
6 changes: 6 additions & 0 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 3 additions & 0 deletions doc/api/http2.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 3 additions & 0 deletions lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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];

Expand Down
8 changes: 8 additions & 0 deletions test/parallel/test-http2-server-push-stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}));
}
Expand Down

0 comments on commit 7223a91

Please sign in to comment.