From 9e2e2fa7166c6767c292e54c13d4f922439dd253 Mon Sep 17 00:00:00 2001 From: gdemesa Date: Mon, 9 Nov 2026 11:18:32 -0800 Subject: [PATCH] http2: enforce maxSessionMemory for client requests The documentation for `maxSessionMemory` says new streams are rejected while the limit is exceeded. That was only enforced for streams opened by the peer (`Http2Session::CanAddStream()`); `session.request()` went straight to `nghttp2_submit_request()`. A client whose socket stopped draining could therefore keep submitting requests and queue frames in `outgoing_buffers_` without bound. Check the session memory before submitting a request and fail only that request with `ERR_HTTP2_STREAM_ERROR` (`NGHTTP2_ENHANCE_YOUR_CALM`), leaving the session usable. As with response headers, request bodies queued for flow control are not counted, so large uploads do not block unrelated requests on the same session. Also count the `outgoing_buffers_` descriptors towards the session memory; each queued write costs 32 bytes regardless of its length. Refs: https://github.com/nodejs/undici/issues/6181 Co-Authored-By: Claude Sonnet 5.5 Signed-off-by: gdemesa --- lib/internal/http2/core.js | 8 ++ src/node_http2.cc | 13 +++ src/node_http2.h | 1 + ...http2-max-session-memory-client-request.js | 95 +++++++++++++++++++ 4 files changed, 117 insertions(+) create mode 100644 test/parallel/test-http2-max-session-memory-client-request.js diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 04acb5d30f63..66fad34629c2 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -295,6 +295,7 @@ const { const { NGHTTP2_CANCEL, + NGHTTP2_ENHANCE_YOUR_CALM, NGHTTP2_REFUSED_STREAM, NGHTTP2_DEFAULT_WEIGHT, NGHTTP2_FLAG_END_STREAM, @@ -886,6 +887,13 @@ function requestOnConnect(headersList, options) { // For the first two, emit the error on the session, // For the third, emit the error on the stream, it will bubble up to the // session if not handled. + if (ret === false) { + // The session is over its `maxSessionMemory` budget. Only this request is + // rejected; the session itself stays usable. + this.destroy(new ERR_HTTP2_STREAM_ERROR( + nameForErrorCode[NGHTTP2_ENHANCE_YOUR_CALM])); + return; + } if (typeof ret === 'number') { let err; switch (ret) { diff --git a/src/node_http2.cc b/src/node_http2.cc index a6779610ddb4..13bf30b4feb9 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -3073,6 +3073,19 @@ void Http2Session::Request(const FunctionCallbackInfo& args) { return; } + // maxSessionMemory is documented to reject new streams while the limit is + // exceeded. CanAddStream() enforces that for peer-initiated streams; apply + // the same memory check to locally initiated requests so that a session + // whose socket is not draining cannot queue frames without bound. Like the + // response header check, queued request bodies are not counted, so large + // uploads do not prevent unrelated requests on the same session. + if (!session->has_available_session_memory(sizeof(Http2Stream), + /* exclude_outbound */ true)) + [[unlikely]] { + Debug(session, "not submitting request: session memory limit exceeded"); + return args.GetReturnValue().Set(false); + } + Debug(session, "request submitted"); int32_t ret = 0; diff --git a/src/node_http2.h b/src/node_http2.h index 4576d5678f0b..9f5dddd183f0 100644 --- a/src/node_http2.h +++ b/src/node_http2.h @@ -832,6 +832,7 @@ class Http2Session : public AsyncWrap, uint64_t total = current_session_memory_ + sizeof(Http2Session); total += current_nghttp2_memory_; total += outgoing_storage_.size(); + total += outgoing_buffers_.size() * sizeof(NgHttp2StreamWrite); return total; } diff --git a/test/parallel/test-http2-max-session-memory-client-request.js b/test/parallel/test-http2-max-session-memory-client-request.js new file mode 100644 index 000000000000..d10eec6f43a2 --- /dev/null +++ b/test/parallel/test-http2-max-session-memory-client-request.js @@ -0,0 +1,95 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); +const http2 = require('http2'); +const { once } = require('events'); + +const { NGHTTP2_ENHANCE_YOUR_CALM } = http2.constants; + +// maxSessionMemory is documented to reject new streams while the limit is +// exceeded. Make sure that also holds for requests initiated by a client +// session, so that a peer that does not read cannot make the client queue +// frames without bound. As for response headers, request bodies that are +// queued waiting for flow control are not counted, so a large upload does not +// prevent unrelated requests on the same session. + +async function queuedUploadDoesNotBlockRequests() { + // The server never opens its flow-control window, so the upload stays queued + // in the client session. + const server = http2.createServer({ settings: { initialWindowSize: 0 } }); + server.on('stream', common.mustCall((stream) => { + stream.on('error', () => {}); + }, 2)); + server.listen(0); + await once(server, 'listening'); + + const client = http2.connect(`http://localhost:${server.address().port}`, { + maxSessionMemory: 1, + }); + client.on('error', common.mustNotCall()); + await once(client, 'remoteSettings'); + + const uploadArrived = once(server, 'stream'); + const upload = client.request({ ':method': 'POST', ':path': '/upload' }); + upload.on('error', () => {}); + upload.end(Buffer.alloc(4 * 1024 * 1024)); + await uploadArrived; + + const other = client.request({ ':path': '/other' }); + other.on('error', () => {}); + await once(server, 'stream'); + + other.destroy(); + upload.destroy(); + client.destroy(); + server.close(); +} + +async function manyRequestsAreBounded() { + // A peer that accepts the connection and then stops reading. Before the + // session memory check, every request was queued regardless of the limit. + const server = http2.createServer(); + server.on('connection', (socket) => setTimeout(() => socket.pause(), 30)); + server.on('stream', (stream) => stream.on('error', () => {})); + server.listen(0); + await once(server, 'listening'); + + const client = http2.connect(`http://localhost:${server.address().port}`, { + maxSessionMemory: 1, + }); + client.on('error', () => {}); + await once(client, 'connect'); + + const total = 30_000; + let rejected = 0; + for (let i = 0; i < total; i++) { + const req = client.request({ ':path': `/${i}` }); + req.on('error', (err) => { + if (err.code === 'ERR_HTTP2_STREAM_ERROR' && + err.message.includes('NGHTTP2_ENHANCE_YOUR_CALM')) { + rejected++; + } + }); + req.end(); + } + await new Promise(setImmediate); + + assert.ok(rejected > 0, 'expected some requests to be rejected'); + // Without the check all of the requests (plus the connection preface) were + // queued: the number of queued frames grew with the number submitted. + assert.ok(client.state.outboundQueueSize < total, + `queued ${client.state.outboundQueueSize} of ${total}`); + assert.strictEqual(NGHTTP2_ENHANCE_YOUR_CALM, 11); + + client.destroy(); + server.close(); +} + +(async () => { + await queuedUploadDoesNotBlockRequests(); + await manyRequestsAreBounded(); +})().then(common.mustCall());