From c915bc54d4279971c34b1c2a5c662176ca6608ab Mon Sep 17 00:00:00 2001 From: James M Snell Date: Tue, 21 Nov 2017 10:52:33 -0800 Subject: [PATCH] http2: strictly limit number on concurrent streams Strictly limit the number of concurrent streams based on the current setting of the MAX_CONCURRENT_STREAMS setting Backport-PR-URL: https://github.com/nodejs/node/pull/18050 Backport-PR-URL: https://github.com/nodejs/node/pull/20456 PR-URL: https://github.com/nodejs/node/pull/16766 Reviewed-By: Matteo Collina Reviewed-By: Anatoli Papirovski Reviewed-By: Sebastiaan Deckers --- src/node_http2.cc | 19 ++++++- src/node_http2.h | 2 + test/parallel/test-http2-too-many-streams.js | 60 ++++++++++++++++++++ 3 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-http2-too-many-streams.js diff --git a/src/node_http2.cc b/src/node_http2.cc index 21265bb15b91bd..8b9c35778ec496 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -656,6 +656,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) { CHECK_GE(++statistics_.stream_count, 0); @@ -766,7 +776,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 { // If the stream has already been destroyed, ignore. if (stream->IsDestroyed()) diff --git a/src/node_http2.h b/src/node_http2.h index cdad6e16690c5d..afba35959a192b 100644 --- a/src/node_http2.h +++ b/src/node_http2.h @@ -835,6 +835,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..a4a67befa0f50a --- /dev/null +++ b/test/parallel/test-http2-too-many-streams.js @@ -0,0 +1,60 @@ +'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'); + +// Test that the maxConcurrentStreams setting is strictly enforced + +const server = http2.createServer({ settings: { maxConcurrentStreams: 1 } }); + +let c = 0; + +server.on('stream', common.mustCall((stream) => { + // Because we only allow one open stream at a time, + // c should never be greater than 1. + assert.strictEqual(++c, 1); + stream.respond(); + // Force some asynchronos stuff. + setImmediate(() => { + stream.end('ok'); + assert.strictEqual(--c, 0); + }); +}, 3)); + +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(); + })); + + client.on('remoteSettings', common.mustCall(() => { + assert.strictEqual(client.remoteSettings.maxConcurrentStreams, 1); + + { + const req = client.request(); + req.resume(); + req.on('close', () => { + countdown.dec(); + + setImmediate(() => { + const req = client.request(); + req.resume(); + req.on('close', () => countdown.dec()); + }); + }); + } + + { + const req = client.request(); + req.resume(); + req.on('close', () => countdown.dec()); + } + })); +}));