Skip to content

Commit e7a727e

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 Backport-PR-URL: #18050 PR-URL: #16766 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Sebastiaan Deckers <[email protected]>
1 parent 06aaaa8 commit e7a727e

File tree

3 files changed

+80
-1
lines changed

3 files changed

+80
-1
lines changed

src/node_http2.cc

+18-1
Original file line numberDiff line numberDiff line change
@@ -655,6 +655,16 @@ inline Http2Stream* Http2Session::FindStream(int32_t id) {
655655
return s != streams_.end() ? s->second : nullptr;
656656
}
657657

658+
inline bool Http2Session::CanAddStream() {
659+
uint32_t maxConcurrentStreams =
660+
nghttp2_session_get_local_settings(
661+
session_, NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS);
662+
size_t maxSize =
663+
std::min(streams_.max_size(), static_cast<size_t>(maxConcurrentStreams));
664+
// We can add a new stream so long as we are less than the current
665+
// maximum on concurrent streams
666+
return streams_.size() < maxSize;
667+
}
658668

659669
inline void Http2Session::AddStream(Http2Stream* stream) {
660670
CHECK_GE(++statistics_.stream_count, 0);
@@ -765,7 +775,14 @@ inline int Http2Session::OnBeginHeadersCallback(nghttp2_session* handle,
765775

766776
Http2Stream* stream = session->FindStream(id);
767777
if (stream == nullptr) {
768-
new Http2Stream(session, id, frame->headers.cat);
778+
if (session->CanAddStream()) {
779+
new Http2Stream(session, id, frame->headers.cat);
780+
} else {
781+
// Too many concurrent streams being opened
782+
nghttp2_submit_rst_stream(**session, NGHTTP2_FLAG_NONE, id,
783+
NGHTTP2_ENHANCE_YOUR_CALM);
784+
return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
785+
}
769786
} else {
770787
// If the stream has already been destroyed, ignore.
771788
if (stream->IsDestroyed())

src/node_http2.h

+2
Original file line numberDiff line numberDiff line change
@@ -835,6 +835,8 @@ class Http2Session : public AsyncWrap {
835835
// Returns pointer to the stream, or nullptr if stream does not exist
836836
inline Http2Stream* FindStream(int32_t id);
837837

838+
inline bool CanAddStream();
839+
838840
// Adds a stream instance to this session
839841
inline void AddStream(Http2Stream* stream);
840842

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)