From b59fee5a82301271c72540798afdc2f4a4f380ca Mon Sep 17 00:00:00 2001 From: Archkon <180910180+Archkon@users.noreply.github.com> Date: Thu, 9 Jul 2026 22:22:32 +0800 Subject: [PATCH] http: avoid aborting IncomingMessage signal on normal close IncomingMessage 'close' is emitted when request handling completes, not only when the underlying connection is closed. Using that event to abort IncomingMessage.signal makes the signal abort after a request body is read normally. Track the underlying socket close instead, and detach the listener when the request or response completes normally. This keeps the signal from being aborted by normal stream completion or later keep-alive socket closure, while still aborting it when the connection closes during the active request lifecycle. Signed-off-by: Archkon <180910180+Archkon@users.noreply.github.com> --- doc/api/http.md | 13 +++- lib/_http_client.js | 3 + lib/_http_incoming.js | 58 +++++++++++++-- lib/_http_server.js | 6 +- test/parallel/test-http-request-signal.js | 86 +++++++++++++++++++++-- 5 files changed, 153 insertions(+), 13 deletions(-) diff --git a/doc/api/http.md b/doc/api/http.md index 3cdcab71361260..92dcee74a6f08a 100644 --- a/doc/api/http.md +++ b/doc/api/http.md @@ -3045,13 +3045,20 @@ Calls `message.socket.setTimeout(msecs, callback)`. added: - v26.1.0 - v24.16.0 +changes: + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/64392 + description: The signal is no longer aborted after the message + completes normally. --> * Type: {AbortSignal} -An {AbortSignal} that is aborted when the underlying socket closes or the -request is destroyed. The signal is created lazily on first access — no -{AbortController} is allocated for requests that never use this property. +An {AbortSignal} that is aborted when the message is destroyed before +completion or when its underlying socket closes before request handling or +response reading completes. +The signal is created lazily on first access — no {AbortController} is allocated +for requests that never use this property. This is useful for cancelling downstream asynchronous work such as database queries or `fetch` calls when a client disconnects mid-request. diff --git a/lib/_http_client.js b/lib/_http_client.js index 891da4e3f9a984..d97336bf92eb25 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -50,6 +50,7 @@ const { prepareError, kSkipPendingData, } = require('_http_common'); +const { kDetachAbortSignal } = require('_http_incoming'); const { kUniqueHeaders, parseUniqueHeadersOption, @@ -999,6 +1000,8 @@ function responseOnEnd() { const req = this.req; const socket = req.socket; + this[kDetachAbortSignal](); + if (socket) { if (req.timeoutCb) socket.removeListener('timeout', emitRequestTimeout); socket.removeListener('timeout', responseOnTimeout); diff --git a/lib/_http_incoming.js b/lib/_http_incoming.js index 6c082454b08a2b..f2823307005c71 100644 --- a/lib/_http_incoming.js +++ b/lib/_http_incoming.js @@ -38,6 +38,11 @@ const kTrailers = Symbol('kTrailers'); const kTrailersDistinct = Symbol('kTrailersDistinct'); const kTrailersCount = Symbol('kTrailersCount'); const kAbortController = Symbol('kAbortController'); +const kAbortSignalSocket = Symbol('kAbortSignalSocket'); +const kAbortSignalListener = Symbol('kAbortSignalListener'); +const kAbortSignalDetached = Symbol('kAbortSignalDetached'); +const kAttachAbortSignal = Symbol('kAttachAbortSignal'); +const kDetachAbortSignal = Symbol('kDetachAbortSignal'); function readStart(socket) { if (socket && !socket._paused && socket.readable) @@ -94,6 +99,9 @@ function IncomingMessage(socket) { // read by the user, so there's no point continuing to handle it. this._dumped = false; this[kAbortController] = null; + this[kAbortSignalSocket] = null; + this[kAbortSignalListener] = null; + this[kAbortSignalDetached] = false; } ObjectSetPrototypeOf(IncomingMessage.prototype, Readable.prototype); ObjectSetPrototypeOf(IncomingMessage, Readable); @@ -195,18 +203,51 @@ ObjectDefineProperty(IncomingMessage.prototype, 'signal', { if (this[kAbortController] === null) { const ac = new AbortController(); this[kAbortController] = ac; - if (this.destroyed) { + if (this.destroyed && (!this.readableEnded || !this.complete)) { ac.abort(); } else { - this.once('close', function() { - ac.abort(); - }); + this[kAttachAbortSignal](); } } return this[kAbortController].signal; }, }); +IncomingMessage.prototype[kAttachAbortSignal] = function() { + if (this[kAbortController].signal.aborted || + this[kAbortSignalDetached] || + this[kAbortSignalListener] !== null) { + return; + } + + const socket = this.socket; + if (!socket) { + return; + } + + if (socket.destroyed) { + abortSignal(this); + return; + } + + this[kAbortSignalSocket] = socket; + this[kAbortSignalListener] = () => { + abortSignal(this); + }; + socket.once('close', this[kAbortSignalListener]); +}; + +IncomingMessage.prototype[kDetachAbortSignal] = function() { + const socket = this[kAbortSignalSocket]; + const listener = this[kAbortSignalListener]; + this[kAbortSignalDetached] = true; + this[kAbortSignalSocket] = null; + this[kAbortSignalListener] = null; + if (socket !== null && listener !== null) { + socket.removeListener('close', listener); + } +}; + IncomingMessage.prototype.setTimeout = function setTimeout(msecs, callback) { if (callback) this.on('timeout', callback); @@ -242,6 +283,7 @@ IncomingMessage.prototype._destroy = function _destroy(err, cb) { if (!this.readableEnded || !this.complete) { this.aborted = true; this.emit('aborted'); + abortSignal(this); } // If aborted and the underlying socket is not already destroyed, @@ -263,6 +305,13 @@ IncomingMessage.prototype._destroy = function _destroy(err, cb) { } }; +function abortSignal(self) { + self[kDetachAbortSignal](); + if (self[kAbortController] !== null) { + self[kAbortController].abort(); + } +} + IncomingMessage.prototype._addHeaderLines = _addHeaderLines; function _addHeaderLines(headers, n) { if (headers?.length) { @@ -480,6 +529,7 @@ function onError(self, error, cb) { module.exports = { IncomingMessage, + kDetachAbortSignal, readStart, readStop, }; diff --git a/lib/_http_server.js b/lib/_http_server.js index 0f5865126689d3..e4e57092ee7a73 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -70,7 +70,10 @@ const { defaultTriggerAsyncIdScope, getOrSetAsyncId, } = require('internal/async_hooks'); -const { IncomingMessage } = require('_http_incoming'); +const { + IncomingMessage, + kDetachAbortSignal, +} = require('_http_incoming'); const { ConnResetException, codes: { @@ -1198,6 +1201,7 @@ function resOnFinish(req, res, socket, state, server) { // array will be empty. assert(state.incoming.length === 0 || state.incoming[0] === req); + req[kDetachAbortSignal](); state.incoming.shift(); // If the user never called req.read(), and didn't pipe() or diff --git a/test/parallel/test-http-request-signal.js b/test/parallel/test-http-request-signal.js index b7c9fdc79e8365..5b9fd17c433e10 100644 --- a/test/parallel/test-http-request-signal.js +++ b/test/parallel/test-http-request-signal.js @@ -4,7 +4,7 @@ const common = require('../common'); const assert = require('assert'); const http = require('http'); -// Test 1: req.signal is an AbortSignal and aborts on 'close' +// Test 1: req.signal is an AbortSignal and aborts on socket close { const server = http.createServer(common.mustCall((req, res) => { assert.ok(req.signal instanceof AbortSignal); @@ -21,21 +21,68 @@ const http = require('http'); })); } -// Test 2: req.signal is aborted if accessed after destroy +// Test 2: req.signal is not aborted when a request body completes normally. +{ + const body = JSON.stringify({ hello: 'world' }); + const server = http.createServer(common.mustCall((req, res) => { + assert.ok(req.signal instanceof AbortSignal); + assert.strictEqual(req.signal.aborted, false); + req.signal.onabort = common.mustNotCall(); + + req.on('close', common.mustCall(() => { + assert.strictEqual(req.aborted, false); + assert.strictEqual(req.complete, true); + assert.strictEqual(req.signal.aborted, false); + })); + + req.on('end', common.mustCall(() => { + setTimeout(common.mustCall(() => { + assert.strictEqual(req.aborted, false); + assert.strictEqual(req.complete, true); + assert.strictEqual(req.signal.aborted, false); + res.end('ok'); + }), 10); + })); + req.resume(); + })); + + server.listen(0, common.mustCall(() => { + const clientReq = http.request( + { + port: server.address().port, + method: 'PATCH', + path: '/tables/1', + headers: { + 'content-type': 'application/json', + 'content-length': Buffer.byteLength(body), + }, + }, + common.mustCall((res) => { + res.resume(); + res.on('end', common.mustCall(() => { + server.close(); + })); + }), + ); + clientReq.end(body); + })); +} + +// Test 3: req.signal is aborted if accessed after destroy { const req = new http.IncomingMessage(null); req.destroy(); assert.strictEqual(req.signal.aborted, true); } -// Test 3: Multiple accesses return the same signal +// Test 4: Multiple accesses return the same signal { const req = new http.IncomingMessage(null); assert.strictEqual(req.signal, req.signal); } -// Test 4: res.signal on a client-side http.request() response (IncomingMessage). +// Test 5: res.signal on a client-side http.request() response (IncomingMessage). { const server = http.createServer(common.mustCall((req, res) => { res.writeHead(200); @@ -61,7 +108,36 @@ const http = require('http'); })); } -// Test 5: Client cancels a pending request. +// Test 6: res.signal is not aborted when a response body completes normally. +{ + const server = http.createServer(common.mustCall((req, res) => { + res.end('ok'); + })); + + server.listen(0, common.mustCall(() => { + const clientReq = http.request( + { port: server.address().port }, + common.mustCall((res) => { + assert.ok(res.signal instanceof AbortSignal); + assert.strictEqual(res.signal.aborted, false); + res.signal.onabort = common.mustNotCall(); + + res.resume(); + res.on('end', common.mustCall(() => { + assert.strictEqual(res.complete, true); + assert.strictEqual(res.signal.aborted, false); + })); + res.on('close', common.mustCall(() => { + assert.strictEqual(res.signal.aborted, false); + server.close(); + })); + }), + ); + clientReq.end(); + })); +} + +// Test 7: Client cancels a pending request. { const server = http.createServer(common.mustCall((req, res) => { req.signal.onabort = common.mustCall(() => {