diff --git a/doc/api/http2.md b/doc/api/http2.md index ac6e1eb9afef05..3027a7cfc1b12d 100644 --- a/doc/api/http2.md +++ b/doc/api/http2.md @@ -2874,9 +2874,10 @@ changes: This is a credit based limit, existing `Http2Stream`s may cause this limit to be exceeded, but new `Http2Stream` instances will be rejected while this limit is exceeded. The current number of `Http2Stream` sessions, - the current memory use of the header compression tables, current data - queued to be sent, and unacknowledged `PING` and `SETTINGS` frames are all - counted towards the current limit. **Default:** `10`. + the current memory use of the header compression tables, header blocks + retained by open streams, current data queued to be sent, and + unacknowledged `PING` and `SETTINGS` frames are all counted towards the + current limit. **Default:** `10`. * `maxHeaderListPairs` {number} Sets the maximum number of header entries. This is similar to [`server.maxHeadersCount`][] or [`request.maxHeadersCount`][] in the `node:http` module. The minimum value @@ -3095,9 +3096,10 @@ changes: credit based limit, existing `Http2Stream`s may cause this limit to be exceeded, but new `Http2Stream` instances will be rejected while this limit is exceeded. The current number of `Http2Stream` sessions, - the current memory use of the header compression tables, current data - queued to be sent, and unacknowledged `PING` and `SETTINGS` frames are all - counted towards the current limit. **Default:** `10`. + the current memory use of the header compression tables, header blocks + retained by open streams, current data queued to be sent, and + unacknowledged `PING` and `SETTINGS` frames are all counted towards the + current limit. **Default:** `10`. * `maxHeaderListPairs` {number} Sets the maximum number of header entries. This is similar to [`server.maxHeadersCount`][] or [`request.maxHeadersCount`][] in the `node:http` module. The minimum value @@ -3275,9 +3277,10 @@ changes: This is a credit based limit, existing `Http2Stream`s may cause this limit to be exceeded, but new `Http2Stream` instances will be rejected while this limit is exceeded. The current number of `Http2Stream` sessions, - the current memory use of the header compression tables, current data - queued to be sent, and unacknowledged `PING` and `SETTINGS` frames are all - counted towards the current limit. **Default:** `10`. + the current memory use of the header compression tables, header blocks + retained by open streams, current data queued to be sent, and + unacknowledged `PING` and `SETTINGS` frames are all counted towards the + current limit. **Default:** `10`. * `maxHeaderListPairs` {number} Sets the maximum number of header entries. This is similar to [`server.maxHeadersCount`][] or [`request.maxHeadersCount`][] in the `node:http` module. The minimum value diff --git a/src/node_http2.cc b/src/node_http2.cc index 55bca557a81e4d..591798a6371595 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -902,6 +902,14 @@ BaseObjectPtr Http2Session::RemoveStream(int32_t id) { stream = FindStream(id); if (stream) { streams_.erase(id); + if (stream->current_headers_length_ > 0) { + DecrementCurrentSessionMemory(stream->current_headers_length_); + stream->current_headers_length_ = 0; + } + if (stream->retained_headers_length_ > 0) { + DecrementCurrentSessionMemory(stream->retained_headers_length_); + stream->retained_headers_length_ = 0; + } DecrementCurrentSessionMemory(sizeof(*stream)); } return stream; @@ -1548,7 +1556,10 @@ void Http2Session::HandleHeadersFrame(const nghttp2_frame* frame) { }); CHECK_EQ(stream->headers_count(), 0); - DecrementCurrentSessionMemory(stream->current_headers_length_); + // Keep the header block charged against maxSessionMemory while the + // corresponding JS objects can still keep it alive for the lifetime of + // the stream. + stream->retained_headers_length_ += stream->current_headers_length_; stream->current_headers_length_ = 0; Local args[] = { diff --git a/src/node_http2.h b/src/node_http2.h index 28245d7b98e06b..0bb2b0cb2891c8 100644 --- a/src/node_http2.h +++ b/src/node_http2.h @@ -490,9 +490,11 @@ class Http2Stream : public AsyncWrap, // The Current Headers block... As headers are received for this stream, // they are temporarily stored here until the OnFrameReceived is called - // signalling the end of the HEADERS frame + // signalling the end of the HEADERS frame. nghttp2_headers_category current_headers_category_ = NGHTTP2_HCAT_HEADERS; uint32_t current_headers_length_ = 0; // total number of octets + // Charged against maxSessionMemory while headers stay alive in JS. + uint64_t retained_headers_length_ = 0; std::vector current_headers_; // This keeps track of the amount of data read from the socket while the diff --git a/test/parallel/test-http2-max-session-memory-stalled-headers.js b/test/parallel/test-http2-max-session-memory-stalled-headers.js new file mode 100644 index 00000000000000..7c3ddf7b353207 --- /dev/null +++ b/test/parallel/test-http2-max-session-memory-stalled-headers.js @@ -0,0 +1,62 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); +const http2 = require('http2'); + +const { + NGHTTP2_ENHANCE_YOUR_CALM, +} = http2.constants; + +// Regression test: header blocks retained by stalled streams should continue +// to count against maxSessionMemory after they have been handed to JS. +const maxSessionMemory = 1; +const totalRequests = 400; +const cookieCrumbs = 120; + +let accepted = 0; +let rejected = 0; + +const server = http2.createServer({ maxSessionMemory }); +server.on('stream', (stream) => { + accepted++; + stream.on('error', () => {}); + stream.respond(); + stream.write('x'); +}); + +server.listen(0, common.mustCall(() => { + const client = http2.connect(`http://localhost:${server.address().port}`, { + settings: { + initialWindowSize: 0, + }, + }); + client.on('error', () => {}); + + client.on('remoteSettings', common.mustCall(() => { + for (let i = 0; i < totalRequests; i++) { + const headers = [':path', '/']; + for (let j = 0; j < cookieCrumbs; j++) { + headers.push('cookie', 'a=1'); + } + + const req = client.request(headers); + req.on('error', () => {}); + req.on('close', () => { + if (req.rstCode === NGHTTP2_ENHANCE_YOUR_CALM) + rejected++; + }); + req.end(); + } + + setTimeout(common.mustCall(() => { + assert(rejected > 0); + assert(accepted < totalRequests); + client.destroy(); + server.close(); + }), common.platformTimeout(3000)); + })); +}));