diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 04acb5d30f6..66fad34629c 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 a6779610ddb..13bf30b4feb 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 4576d5678f0..9f5dddd183f 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 00000000000..d10eec6f43a --- /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());