From a5190f778649c02e70fca43a3e2abc3e5b34a862 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Mon, 14 Sep 2026 09:51:06 +0200 Subject: [PATCH] fix(h2): validate response content-length Signed-off-by: Matteo Collina --- lib/dispatcher/client-h2.js | 34 +++++++++++++++-- test/fetch/http2.js | 75 +++++++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 3 deletions(-) diff --git a/lib/dispatcher/client-h2.js b/lib/dispatcher/client-h2.js index 24f47de23d9..8be753bb50a 100644 --- a/lib/dispatcher/client-h2.js +++ b/lib/dispatcher/client-h2.js @@ -5,6 +5,7 @@ const { pipeline } = require('node:stream') const util = require('../core/util.js') const { RequestContentLengthMismatchError, + ResponseContentLengthMismatchError, RequestAbortedError, SocketError, InformationalError, @@ -1047,6 +1048,7 @@ function writeH2 (client, request) { headersTimeout, bodyTimeout, requestFinalized: false, + responseContentLength: null, responseReceived: false, bodySent: false, pendingEnd: false, @@ -1342,12 +1344,17 @@ function onData (chunk) { return } - const { request, maxResponseSize } = state + const { request, maxResponseSize, responseContentLength } = state if (request.aborted || request.completed) { return } + if (responseContentLength != null && state.bytesRead + chunk.length > responseContentLength) { + state.abort(new ResponseContentLengthMismatchError()) + return + } + if (maxResponseSize > -1 && state.bytesRead + chunk.length > maxResponseSize) { // Unlike HTTP/1.1, which destroys the socket because it cannot abandon one // response without losing framing, resetting the offending stream leaves @@ -1406,11 +1413,20 @@ function onResponse (headers) { stream.end() } - const statusCode = headers[HTTP2_HEADER_STATUS] + const statusCode = Number(headers[HTTP2_HEADER_STATUS]) delete headers[HTTP2_HEADER_STATUS] request.onResponseStarted() state.responseReceived = true + // A Content-Length in HEAD and 304 responses describes the selected + // representation rather than DATA on this stream. Successful CONNECT uses + // the upgrade path above; all other final responses use Content-Length as + // their DATA payload length. + if (request.method !== 'HEAD' && statusCode !== 304) { + const contentLength = headers[HTTP2_HEADER_CONTENT_LENGTH] + state.responseContentLength = contentLength == null ? null : Number(contentLength) + } + if (state.headersTimeout || state.bodyTimeout) { stream.setTimeout(state.bodyTimeout) } @@ -1428,7 +1444,7 @@ function onResponse (headers) { return } - if (request.onResponseStart(Number(statusCode), headers, stream.resume.bind(stream), '') === false) { + if (request.onResponseStart(statusCode, headers, stream.resume.bind(stream), '') === false) { stream.pause() } @@ -1451,6 +1467,11 @@ function onEnd () { // trailers on the state by now, so completing here still delivers them. if (state.responseReceived) { if (!request.aborted && !request.completed) { + if (state.responseContentLength != null && state.bytesRead !== state.responseContentLength) { + state.abort(new ResponseContentLengthMismatchError()) + return + } + state.pendingEnd = true // Complete on 'end': a blocked event loop can keep the stream's 'close' @@ -1507,6 +1528,13 @@ function onError (err) { stream.off('error', onError) + // Node's HTTP/2 implementation can turn an incomplete Content-Length body + // into a protocol stream error instead of emitting 'end'. Prefer the + // content-length mismatch error when the received byte count proves it. + if (state.responseContentLength != null && state.bytesRead !== state.responseContentLength) { + err = new ResponseContentLengthMismatchError() + } + if (typeof stream.rstCode === 'number' && stream.rstCode !== NGHTTP2_NO_ERROR) { err.http2ErrorCode = stream.rstCode } diff --git a/test/fetch/http2.js b/test/fetch/http2.js index 6ee14aabd7a..f3afdfc3a03 100644 --- a/test/fetch/http2.js +++ b/test/fetch/http2.js @@ -123,6 +123,81 @@ test('[Fetch] Simple GET with h2', async (t) => { t.assert.strictEqual(response.statusText, '') }) +test('[Fetch] HTTP/2 response content-length mismatch rejects the body without closing the session', async (t) => { + const server = createSecureServer(await pem.generate({ opts: { keySize: 2048 } })) + let sessions = 0 + + server.on('session', () => { + sessions++ + }) + + server.on('stream', (stream, headers) => { + switch (headers[':path']) { + case '/truncated': + stream.respond({ ':status': 200, 'content-length': 10 }) + stream.write('123', () => stream.destroy()) + break + case '/oversized': + stream.respond({ ':status': 200, 'content-length': 3 }) + stream.end('1234') + break + case '/head': + stream.respond({ ':status': 200, 'content-length': 10 }) + stream.end() + break + case '/not-modified': + stream.respond({ ':status': 304, 'content-length': 10 }) + stream.end() + break + default: + stream.respond({ ':status': 200, 'content-length': 2 }) + stream.end('ok') + } + }) + + server.listen() + await once(server, 'listening') + + const origin = `https://localhost:${server.address().port}` + const client = new Client(origin, { + connect: { + rejectUnauthorized: false + }, + allowH2: true + }) + + t.after(closeClientAndServerAsPromise(client, server)) + + const assertResponseContentLengthMismatch = async (path) => { + const response = await fetch(`${origin}${path}`, { dispatcher: client }) + + await t.assert.rejects(response.text(), (err) => { + t.assert.ok(err instanceof TypeError) + t.assert.strictEqual(err.cause?.code, 'UND_ERR_RES_CONTENT_LENGTH_MISMATCH') + return true + }) + } + + await assertResponseContentLengthMismatch('/truncated') + + const headResponse = await fetch(`${origin}/head`, { + dispatcher: client, + method: 'HEAD' + }) + t.assert.strictEqual(await headResponse.text(), '') + + const notModifiedResponse = await fetch(`${origin}/not-modified`, { dispatcher: client }) + t.assert.strictEqual(await notModifiedResponse.text(), '') + + const validResponse = await fetch(`${origin}/valid`, { dispatcher: client }) + t.assert.strictEqual(await validResponse.text(), 'ok') + t.assert.strictEqual(sessions, 1) + + // Node's test server closes its session after sending more DATA than its own + // Content-Length, so exercise the oversized case after the reuse assertion. + await assertResponseContentLengthMismatch('/oversized') +}) + test('[Fetch] Should handle h2 request with body (string or buffer)', async (t) => { const server = createSecureServer(await pem.generate({ opts: { keySize: 2048 } })) const expectedBody = 'hello from client!'