FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

http2: retain header memory in session accounting · nodejs/node@34ed88a · GitHub

/ node Public

Commit 34ed88a

Browse files
mcollina
authored and
RafaelGSS
committed
http2: retain header memory in session accounting
Signed-off-by: Matteo Collina <hello@matteocollina.com> PR-URL: #63752 Reviewed-By: Tim Perry <pimterry@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day> CVE-ID: CVE-2026-56846
1 parent 9de263a commit 34ed88a

4 files changed

Lines changed: 96 additions & 11 deletions

File tree

‎doc/api/http2.md‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2860,9 +2860,10 @@ changes:
28602860
This is a credit based limit, existing `Http2Stream`s may cause this
28612861
limit to be exceeded, but new `Http2Stream` instances will be rejected
28622862
while this limit is exceeded. The current number of `Http2Stream` sessions,
2863-
the current memory use of the header compression tables, current data
2864-
queued to be sent, and unacknowledged `PING` and `SETTINGS` frames are all
2865-
counted towards the current limit. **Default:** `10`.
2863+
the current memory use of the header compression tables, header blocks
2864+
retained by open streams, current data queued to be sent, and
2865+
unacknowledged `PING` and `SETTINGS` frames are all counted towards the
2866+
current limit. **Default:** `10`.
28662867
* `maxHeaderListPairs` {number} Sets the maximum number of header entries.
28672868
This is similar to [`server.maxHeadersCount`][] or
28682869
[`request.maxHeadersCount`][] in the `node:http` module. The minimum value
@@ -3077,9 +3078,10 @@ changes:
30773078
credit based limit, existing `Http2Stream`s may cause this
30783079
limit to be exceeded, but new `Http2Stream` instances will be rejected
30793080
while this limit is exceeded. The current number of `Http2Stream` sessions,
3080-
the current memory use of the header compression tables, current data
3081-
queued to be sent, and unacknowledged `PING` and `SETTINGS` frames are all
3082-
counted towards the current limit. **Default:** `10`.
3081+
the current memory use of the header compression tables, header blocks
3082+
retained by open streams, current data queued to be sent, and
3083+
unacknowledged `PING` and `SETTINGS` frames are all counted towards the
3084+
current limit. **Default:** `10`.
30833085
* `maxHeaderListPairs` {number} Sets the maximum number of header entries.
30843086
This is similar to [`server.maxHeadersCount`][] or
30853087
[`request.maxHeadersCount`][] in the `node:http` module. The minimum value
@@ -3257,9 +3259,10 @@ changes:
32573259
This is a credit based limit, existing `Http2Stream`s may cause this
32583260
limit to be exceeded, but new `Http2Stream` instances will be rejected
32593261
while this limit is exceeded. The current number of `Http2Stream` sessions,
3260-
the current memory use of the header compression tables, current data
3261-
queued to be sent, and unacknowledged `PING` and `SETTINGS` frames are all
3262-
counted towards the current limit. **Default:** `10`.
3262+
the current memory use of the header compression tables, header blocks
3263+
retained by open streams, current data queued to be sent, and
3264+
unacknowledged `PING` and `SETTINGS` frames are all counted towards the
3265+
current limit. **Default:** `10`.
32633266
* `maxHeaderListPairs` {number} Sets the maximum number of header entries.
32643267
This is similar to [`server.maxHeadersCount`][] or
32653268
[`request.maxHeadersCount`][] in the `node:http` module. The minimum value

‎src/node_http2.cc‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -902,6 +902,14 @@ BaseObjectPtr<Http2Stream> Http2Session::RemoveStream(int32_t id) {
902902
stream = FindStream(id);
903903
if (stream) {
904904
streams_.erase(id);
905+
if (stream->current_headers_length_ > 0) {
906+
DecrementCurrentSessionMemory(stream->current_headers_length_);
907+
stream->current_headers_length_ = 0;
908+
}
909+
if (stream->retained_headers_length_ > 0) {
910+
DecrementCurrentSessionMemory(stream->retained_headers_length_);
911+
stream->retained_headers_length_ = 0;
912+
}
905913
DecrementCurrentSessionMemory(sizeof(*stream));
906914
}
907915
return stream;
@@ -1548,7 +1556,10 @@ void Http2Session::HandleHeadersFrame(const nghttp2_frame* frame) {
15481556
});
15491557
CHECK_EQ(stream->headers_count(), 0);
15501558

1551-
DecrementCurrentSessionMemory(stream->current_headers_length_);
1559+
// Keep the header block charged against maxSessionMemory while the
1560+
// corresponding JS objects can still keep it alive for the lifetime of
1561+
// the stream.
1562+
stream->retained_headers_length_ += stream->current_headers_length_;
15521563
stream->current_headers_length_ = 0;
15531564

15541565
Local<Value> args[] = {

‎src/node_http2.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -490,9 +490,11 @@ class Http2Stream : public AsyncWrap,
490490

491491
// The Current Headers block... As headers are received for this stream,
492492
// they are temporarily stored here until the OnFrameReceived is called
493-
// signalling the end of the HEADERS frame
493+
// signalling the end of the HEADERS frame.
494494
nghttp2_headers_category current_headers_category_ = NGHTTP2_HCAT_HEADERS;
495495
uint32_t current_headers_length_ = 0; // total number of octets
496+
// Charged against maxSessionMemory while headers stay alive in JS.
497+
uint64_t retained_headers_length_ = 0;
496498
std::vector<Http2Header> current_headers_;
497499

498500
// This keeps track of the amount of data read from the socket while the

‎test/parallel/test-http2-max-session-memory-stalled-headers.js‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
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 assert = require('assert');
9+
const http2 = require('http2');
10+
11+
const {
12+
NGHTTP2_ENHANCE_YOUR_CALM,
13+
} = http2.constants;
14+
15+
// Regression test: header blocks retained by stalled streams should continue
16+
// to count against maxSessionMemory after they have been handed to JS.
17+
const maxSessionMemory = 1;
18+
const totalRequests = 400;
19+
const cookieCrumbs = 120;
20+
21+
let accepted = 0;
22+
let rejected = 0;
23+
24+
const server = http2.createServer({ maxSessionMemory });
25+
server.on('stream', (stream) => {
26+
accepted++;
27+
stream.on('error', () => {});
28+
stream.respond();
29+
stream.write('x');
30+
});
31+
32+
server.listen(0, common.mustCall(() => {
33+
const client = http2.connect(`http://localhost:${server.address().port}`, {
34+
settings: {
35+
initialWindowSize: 0,
36+
},
37+
});
38+
client.on('error', () => {});
39+
40+
client.on('remoteSettings', common.mustCall(() => {
41+
let destroyed = false;
42+
const countdown = new Countdown(totalRequests, common.mustCall(() => {
43+
assert(rejected > 0);
44+
assert(accepted < totalRequests);
45+
server.close();
46+
}));
47+
48+
for (let i = 0; i < totalRequests; i++) {
49+
const headers = [':path', '/'];
50+
for (let j = 0; j < cookieCrumbs; j++) {
51+
headers.push('cookie', 'a=1');
52+
}
53+
54+
const req = client.request(headers);
55+
req.on('error', () => {});
56+
req.on('close', () => {
57+
if (req.rstCode === NGHTTP2_ENHANCE_YOUR_CALM) {
58+
rejected++;
59+
if (!destroyed) {
60+
destroyed = true;
61+
client.destroy();
62+
}
63+
}
64+
countdown.dec();
65+
});
66+
req.end();
67+
}
68+
}));
69+
}));

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL