diff --git a/src/node_http2.cc b/src/node_http2.cc index b439ae588a7756..deb61107f5d2ff 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -418,6 +418,16 @@ inline Http2Stream* Http2Session::FindStream(int32_t id) { return s != streams_.end() ? s->second : nullptr; } +inline bool Http2Session::CanAddStream() { + uint32_t maxConcurrentStreams = + nghttp2_session_get_local_settings( + session_, NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS); + size_t maxSize = + std::min(streams_.max_size(), static_cast(maxConcurrentStreams)); + // We can add a new stream so long as we are less than the current + // maximum on concurrent streams + return streams_.size() < maxSize; +} inline void Http2Session::AddStream(Http2Stream* stream) { streams_[stream->id()] = stream; @@ -528,7 +538,14 @@ inline int Http2Session::OnBeginHeadersCallback(nghttp2_session* handle, Http2Stream* stream = session->FindStream(id); if (stream == nullptr) { - new Http2Stream(session, id, frame->headers.cat); + if (session->CanAddStream()) { + new Http2Stream(session, id, frame->headers.cat); + } else { + // Too many concurrent streams being opened + nghttp2_submit_rst_stream(**session, NGHTTP2_FLAG_NONE, id, + NGHTTP2_ENHANCE_YOUR_CALM); + return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; + } } else { stream->StartHeaders(frame->headers.cat); } diff --git a/src/node_http2.h b/src/node_http2.h index e4b6226e82f659..7032313f49a275 100644 --- a/src/node_http2.h +++ b/src/node_http2.h @@ -819,6 +819,8 @@ class Http2Session : public AsyncWrap { // Returns pointer to the stream, or nullptr if stream does not exist inline Http2Stream* FindStream(int32_t id); + inline bool CanAddStream(); + // Adds a stream instance to this session inline void AddStream(Http2Stream* stream); diff --git a/test/parallel/test-http2-too-many-streams.js b/test/parallel/test-http2-too-many-streams.js new file mode 100644 index 00000000000000..ff66f80ce71de8 --- /dev/null +++ b/test/parallel/test-http2-too-many-streams.js @@ -0,0 +1,75 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const Countdown = require('../common/countdown'); +const http2 = require('http2'); +const assert = require('assert'); + +const server = http2.createServer({ settings: { maxConcurrentStreams: 1 } }); + +let c = 0; + +server.on('stream', (stream) => { + assert.strictEqual(++c, 1); + stream.respond(); + setImmediate(() => { + stream.end('ok'); + assert.strictEqual(--c, 0); + }); +}); + +server.listen(0, common.mustCall(() => { + const client = http2.connect(`http://localhost:${server.address().port}`); + + const countdown = new Countdown(3, common.mustCall(() => { + server.close(); + client.destroy(); + })); + + // Test that the maxConcurrentStreams setting is strictly enforced + + client.on('remoteSettings', common.mustCall(() => { + assert.strictEqual(client.remoteSettings.maxConcurrentStreams, 1); + + // This one should be ok + { + const req = client.request(); + req.resume(); + req.on('end', () => { + countdown.dec(); + + setImmediate(() => { + const req = client.request(); + req.resume(); + req.on('end', () => countdown.dec()); + }); + }); + } + + // This one should fail + { + const req = client.request(); + req.resume(); + // TODO(jasnell): Still investigating precisely why, but on Windows, + // the error is not emitted because the underlying + // mechanism ensures that only one request is sent + // at a time per the maxConcurrentStreams setting. + // This likely has to do with the way the response + // is being handled on the server side. This is safe + // to ignore on Windows because of the assert.strictEqual + // check in the on('stream') handler which ensures that + // only one request is being handled at any given time. + if (!common.isWindows) { + req.on('error', common.expectsError({ + code: 'ERR_HTTP2_STREAM_ERROR', + type: Error, + message: 'Stream closed with error code 11' + })); + } + req.on('end', () => countdown.dec()); + } + })); +}));