Skip to content

Commit 59944fe

Browse files
jasnellMylesBorins
authored andcommitted
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 PR-URL: #16766 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Sebastiaan Deckers <[email protected]>
1 parent d89cce5 commit 59944fe

File tree

3 files changed

+80
-1
lines changed

3 files changed

+80
-1
lines changed

src/node_http2.cc

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,16 @@ inline Http2Stream* Http2Session::FindStream(int32_t id) {
418418
return s != streams_.end() ? s->second : nullptr;
419419
}
420420

421+
inline bool Http2Session::CanAddStream() {
422+
uint32_t maxConcurrentStreams =
423+
nghttp2_session_get_local_settings(
424+
session_, NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS);
425+
size_t maxSize =
426+
std::min(streams_.max_size(), static_cast<size_t>(maxConcurrentStreams));
427+
// We can add a new stream so long as we are less than the current
428+
// maximum on concurrent streams
429+
return streams_.size() < maxSize;
430+
}
421431

422432
inline void Http2Session::AddStream(Http2Stream* stream) {
423433
streams_[stream->id()] = stream;
@@ -528,7 +538,14 @@ inline int Http2Session::OnBeginHeadersCallback(nghttp2_session* handle,
528538

529539
Http2Stream* stream = session->FindStream(id);
530540
if (stream == nullptr) {
531-
new Http2Stream(session, id, frame->headers.cat);
541+
if (session->CanAddStream()) {
542+
new Http2Stream(session, id, frame->headers.cat);
543+
} else {
544+
// Too many concurrent streams being opened
545+
nghttp2_submit_rst_stream(**session, NGHTTP2_FLAG_NONE, id,
546+
NGHTTP2_ENHANCE_YOUR_CALM);
547+
return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
548+
}
532549
} else {
533550
stream->StartHeaders(frame->headers.cat);
534551
}

src/node_http2.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -818,6 +818,8 @@ class Http2Session : public AsyncWrap {
818818
// Returns pointer to the stream, or nullptr if stream does not exist
819819
inline Http2Stream* FindStream(int32_t id);
820820

821+
inline bool CanAddStream();
822+
821823
// Adds a stream instance to this session
822824
inline void AddStream(Http2Stream* stream);
823825

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
if (!common.hasCrypto)
5+
common.skip('missing crypto');
6+
7+
const Countdown = require('../common/countdown');
8+
const http2 = require('http2');
9+
const assert = require('assert');
10+
11+
// Test that the maxConcurrentStreams setting is strictly enforced
12+
13+
const server = http2.createServer({ settings: { maxConcurrentStreams: 1 } });
14+
15+
let c = 0;
16+
17+
server.on('stream', common.mustCall((stream) => {
18+
// Because we only allow one open stream at a time,
19+
// c should never be greater than 1.
20+
assert.strictEqual(++c, 1);
21+
stream.respond();
22+
// Force some asynchronos stuff.
23+
setImmediate(() => {
24+
stream.end('ok');
25+
assert.strictEqual(--c, 0);
26+
});
27+
}, 3));
28+
29+
server.listen(0, common.mustCall(() => {
30+
const client = http2.connect(`http://localhost:${server.address().port}`);
31+
32+
const countdown = new Countdown(3, common.mustCall(() => {
33+
server.close();
34+
client.destroy();
35+
}));
36+
37+
client.on('remoteSettings', common.mustCall(() => {
38+
assert.strictEqual(client.remoteSettings.maxConcurrentStreams, 1);
39+
40+
{
41+
const req = client.request();
42+
req.resume();
43+
req.on('close', () => {
44+
countdown.dec();
45+
46+
setImmediate(() => {
47+
const req = client.request();
48+
req.resume();
49+
req.on('close', () => countdown.dec());
50+
});
51+
});
52+
}
53+
54+
{
55+
const req = client.request();
56+
req.resume();
57+
req.on('close', () => countdown.dec());
58+
}
59+
}));
60+
}));

0 commit comments

Comments
 (0)