From 912f569e8b7675612fe83a1e8f276b6dce2c60dd Mon Sep 17 00:00:00 2001 From: Tim Perry Date: Tue, 14 Jul 2026 22:16:52 +0200 Subject: [PATCH 1/2] http: fix just-merged change that could silently truncate responses Signed-off-by: Tim Perry --- lib/_http_client.js | 9 ++- ...t-http-client-incomplete-response-reset.js | 57 +++++++++++++++++++ 2 files changed, 61 insertions(+), 5 deletions(-) create mode 100644 test/parallel/test-http-client-incomplete-response-reset.js diff --git a/lib/_http_client.js b/lib/_http_client.js index d202a28f03502f..46983a58b6af7c 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -758,11 +758,10 @@ function socketErrorListener(err) { // For Safety. Some additional errors might fire later on // and we need to make sure we don't double-fire the error event. socket._hadError = true; - // Before a response exists, the request itself failed. Once a response - // exists, socket teardown belongs to the IncomingMessage and is finalized - // by socketCloseListener. Preserve errors explicitly used to destroy the - // request or response, which have historically been emitted on the request. - if (!res || isUserDestroyError) { + // Ignore late socket errors only after a complete response. For incomplete + // responses, the error signals that the response body was truncated. + // User-provided destroy errors are always emitted on the request. + if (!res || !res.complete || isUserDestroyError) { emitErrorEvent(req, err); } diff --git a/test/parallel/test-http-client-incomplete-response-reset.js b/test/parallel/test-http-client-incomplete-response-reset.js new file mode 100644 index 00000000000000..e48564c6151e25 --- /dev/null +++ b/test/parallel/test-http-client-incomplete-response-reset.js @@ -0,0 +1,57 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const http = require('http'); + +// A response that is cut short by a connection reset must still surface the +// socket error on the request. The response exists, but it is not complete, so +// swallowing the error would leave the application with a silently truncated +// body it believes is intact. + +const LENGTH = 1024; + +let serverSocket; + +const server = http.createServer(common.mustCall((request, response) => { + serverSocket = request.socket; + // Promise more body than is ever sent. + response.writeHead(200, { 'content-length': LENGTH }); + response.write('hello'); +})); + +server.on('clientError', common.mustNotCall()); + +server.listen(0, common.mustCall(() => { + const req = http.request({ port: server.address().port }, common.mustCall((res) => { + assert.strictEqual(res.statusCode, 200); + + let received = 0; + let reset = false; + + res.on('data', common.mustCallAtLeast((chunk) => { + received += chunk.length; + if (reset) return; + reset = true; + // The headers and part of the body have arrived. Reset from the server + // side so the client sees a genuine inbound RST mid-body. + serverSocket.resetAndDestroy(); + }, 1)); + + res.on('end', common.mustNotCall()); + res.on('close', common.mustCall(() => { + assert.strictEqual(res.complete, false); + assert.strictEqual(res.errored.code, 'ECONNRESET'); + assert.strictEqual(res.errored.message, 'aborted'); + assert.ok(received > 0 && received < LENGTH, + `expected a truncated body, got ${received} of ${LENGTH}`); + server.close(); + })); + })); + + req.on('error', common.mustCall((err) => { + assert.strictEqual(err.code, 'ECONNRESET'); + })); + + req.end(); +})); From 30901e24cd5865e301fc283a05858e9417c44c90 Mon Sep 17 00:00:00 2001 From: Tim Perry Date: Wed, 15 Jul 2026 00:10:36 +0200 Subject: [PATCH 2/2] Fix the request-not-finished case as well Signed-off-by: Tim Perry --- lib/_http_client.js | 8 ++-- ...nt-complete-response-open-request-reset.js | 42 +++++++++++++++++++ ...est-http-client-complete-response-reset.js | 20 ++++----- 3 files changed, 56 insertions(+), 14 deletions(-) create mode 100644 test/parallel/test-http-client-complete-response-open-request-reset.js diff --git a/lib/_http_client.js b/lib/_http_client.js index 46983a58b6af7c..f02f5084881c8c 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -753,15 +753,15 @@ function socketErrorListener(err) { if (req) { const res = req.res; + const exchangeComplete = req.writableFinished && res?.complete; const isUserDestroyError = err === req[kError] || err === res?.errored; // For Safety. Some additional errors might fire later on // and we need to make sure we don't double-fire the error event. socket._hadError = true; - // Ignore late socket errors only after a complete response. For incomplete - // responses, the error signals that the response body was truncated. - // User-provided destroy errors are always emitted on the request. - if (!res || !res.complete || isUserDestroyError) { + // Once both messages are complete, a transport error cannot change the + // result of the exchange. User-provided destroy errors are always emitted. + if (!exchangeComplete || isUserDestroyError) { emitErrorEvent(req, err); } diff --git a/test/parallel/test-http-client-complete-response-open-request-reset.js b/test/parallel/test-http-client-complete-response-open-request-reset.js new file mode 100644 index 00000000000000..51a3c9afaa7ca9 --- /dev/null +++ b/test/parallel/test-http-client-complete-response-open-request-reset.js @@ -0,0 +1,42 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const http = require('http'); + +const BODY = Buffer.alloc(1024 * 1024); + +const server = http.createServer(common.mustCall((request, response) => { + response.writeHead(202, { 'content-length': 0 }); + response.end(); + + request.once('data', common.mustCall(() => { + request.socket.resetAndDestroy(); + })); +})); + +server.on('clientError', common.mustNotCall()); + +server.listen(0, common.mustCall(() => { + let response; + const req = http.request({ + method: 'POST', + port: server.address().port, + headers: { 'content-length': BODY.length }, + }, common.mustCall((res) => { + response = res; + assert.strictEqual(res.statusCode, 202); + assert.strictEqual(req.writableEnded, false); + req.write(BODY); + })); + + req.on('error', common.mustCall((err) => { + assert(response); + assert.strictEqual(response.complete, true); + assert.strictEqual(req.writableFinished, false); + assert.strictEqual(err.code, 'ECONNRESET'); + })); + + req.on('close', common.mustCall(() => server.close())); + req.flushHeaders(); +})); diff --git a/test/parallel/test-http-client-complete-response-reset.js b/test/parallel/test-http-client-complete-response-reset.js index aa9337dbcc5206..4efd59f552033d 100644 --- a/test/parallel/test-http-client-complete-response-reset.js +++ b/test/parallel/test-http-client-complete-response-reset.js @@ -11,13 +11,14 @@ const server = http.createServer(common.mustCall((request, response) => { request.on('data', function onData(chunk) { received += chunk.length; - request.off('data', onData); - request.destroy(); + if (received > BODY.length / 2) { + request.off('data', onData); + response.writeHead(413, { 'content-length': 0 }); + response.end(); + request.destroy(); + } }); - response.writeHead(413, { 'content-length': 0 }); - response.end(); - request.on('end', common.mustNotCall()); request.on('close', common.mustCall(() => { assert.strictEqual(request.complete, false); @@ -38,18 +39,17 @@ server.listen(0, common.mustCall(() => { response = res; assert.strictEqual(res.statusCode, 413); // Deliberately do not consume the response body. A response that has - // already been received should not be followed by a late ClientRequest - // socket error. - req.write(BODY); - req.end(); + // completed after the request finished should not be followed by a late + // ClientRequest socket error. })); req.on('error', common.mustNotCall()); req.on('close', common.mustCall(() => { assert(response); + assert.strictEqual(req.writableFinished, true); assert.strictEqual(response.complete, true); server.close(); })); - req.flushHeaders(); + req.end(BODY); }));