From f667de2a9be51cab3bbc3e9de469d9b2f3742a93 Mon Sep 17 00:00:00 2001 From: Yurii Bliuchak <1957659+bliuchak@users.noreply.github.com> Date: Mon, 27 Jul 2026 21:56:16 +0200 Subject: [PATCH 1/2] fix(forward): destroy outbound socket when client disconnects early forward.ts (and forward_socks.ts) never destroyed the outbound request when the client-facing response closed first while the upstream target had accepted the connection but never responded - e.g. during server.close(true) shutdown, or the browser disconnecting. chain.ts already had this symmetry (sourceSocket.on('close', () => targetSocket .destroy())); forward.ts/forward_socks.ts didn't, leaving the outbound socket open indefinitely. The originally reported diagnosis (missing client.destroy() in client.on('error')) turned out to be a no-op - Node already destroys the socket there internally - so the fix targets the actual gap instead. Fixes #670 --- src/forward.ts | 12 +++++ src/forward_socks.ts | 9 ++++ test/e2e/forward-socket-cleanup.js | 82 ++++++++++++++++++++++++++++++ 3 files changed, 103 insertions(+) create mode 100644 test/e2e/forward-socket-cleanup.js diff --git a/src/forward.ts b/src/forward.ts index fb4d6ee6..97dce759 100644 --- a/src/forward.ts +++ b/src/forward.ts @@ -135,6 +135,18 @@ export const forward = async ( // Can't use pipeline here as it automatically destroys the streams request.pipe(client); + + // Mirrors chain.ts: if the client-facing side goes away before the + // upstream request/response completes (e.g. server.close(true) during + // shutdown, or the client disconnecting early), destroy the outbound + // request/socket too, so it isn't left dangling indefinitely. + // This runs before the byte-counting `close` handler registered above + // (it's added later, asynchronously, once a socket exists) - that's fine, + // since destroy() doesn't affect the already-recorded bytesRead/bytesWritten. + response.on('close', () => { + client.destroy(); + }); + client.on('error', (error: NodeJS.ErrnoException) => { if (response.headersSent) { resolve(); diff --git a/src/forward_socks.ts b/src/forward_socks.ts index 60a0f0b4..aecedee0 100644 --- a/src/forward_socks.ts +++ b/src/forward_socks.ts @@ -91,6 +91,15 @@ export const forwardSocks = async ( // Can't use pipeline here as it automatically destroys the streams request.pipe(client); + + // Mirrors chain.ts: if the client-facing side goes away before the + // upstream request/response completes (e.g. server.close(true) during + // shutdown, or the client disconnecting early), destroy the outbound + // request/socket too, so it isn't left dangling indefinitely. + response.on('close', () => { + client.destroy(); + }); + client.on('error', (error: NodeJS.ErrnoException) => { if (response.headersSent) { resolve(); diff --git a/test/e2e/forward-socket-cleanup.js b/test/e2e/forward-socket-cleanup.js new file mode 100644 index 00000000..fffd0fe9 --- /dev/null +++ b/test/e2e/forward-socket-cleanup.js @@ -0,0 +1,82 @@ +import http from 'node:http'; +import net from 'node:net'; +import { expect } from 'chai'; + +import { Server } from '../../src/index.js'; + +describe('forward() socket cleanup', () => { + let target; + let targetPort; + let httpAgent; + let proxyServer; + + beforeEach(async () => { + // Target that accepts the request but never responds, so the outbound + // socket stays open only for as long as something keeps it open. + target = http.createServer(() => {}); + await new Promise((resolve) => target.listen(0, resolve)); + targetPort = target.address().port; + + httpAgent = new http.Agent({ keepAlive: true }); + }); + + afterEach(async () => { + if (proxyServer) await proxyServer.close(true); + httpAgent.destroy(); + await new Promise((resolve) => target.close(resolve)); + }); + + it('destroys the outbound socket when the client disconnects before the upstream responds', async () => { + let targetSocket; + const originalCreateConnection = httpAgent.createConnection.bind(httpAgent); + httpAgent.createConnection = (options, callback) => { + const socket = originalCreateConnection(options, callback); + targetSocket = socket; + return socket; + }; + + proxyServer = new Server({ + port: 0, + prepareRequestFunction: () => ({ httpAgent }), + }); + await proxyServer.listen(); + const proxyPort = proxyServer.server.address().port; + + const client = net.connect({ host: '127.0.0.1', port: proxyPort }); + await new Promise((resolve, reject) => { + client.once('connect', resolve); + client.once('error', reject); + }); + + client.write( + `GET http://127.0.0.1:${targetPort}/ HTTP/1.1\r\n` + + `host: 127.0.0.1:${targetPort}\r\n` + + `connection: keep-alive\r\n\r\n`, + ); + + // Wait until the outbound socket to the target actually exists. + await new Promise((resolve) => { + const interval = setInterval(() => { + if (targetSocket) { + clearInterval(interval); + resolve(); + } + }, 5); + }); + + expect(targetSocket.destroyed).to.be.false; + + // Simulate the client (browser) disappearing while the target is + // still hanging - e.g. graceful shutdown via server.close(true), + // without the caller separately destroying their custom httpAgent. + await proxyServer.close(true); + + // The outbound socket must be cleaned up as a result, not left dangling. + await new Promise((resolve) => { + if (targetSocket.destroyed) return resolve(); + targetSocket.once('close', resolve); + }); + + expect(targetSocket.destroyed).to.be.true; + }); +}); From 1e3fd03da0f213d22a531ddde85582f677001149 Mon Sep 17 00:00:00 2001 From: Yurii Bliuchak <1957659+bliuchak@users.noreply.github.com> Date: Tue, 28 Jul 2026 11:03:35 +0200 Subject: [PATCH 2/2] test(forward): add explicit timeout to socket-creation poll The polling wait for the outbound socket to exist had no rejection path, so a regression that stopped httpAgent.createConnection() from being called would surface as a generic mocha timeout instead of a clear failure message. Address Copilot's review comment on #671. --- test/e2e/forward-socket-cleanup.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/e2e/forward-socket-cleanup.js b/test/e2e/forward-socket-cleanup.js index fffd0fe9..803980b6 100644 --- a/test/e2e/forward-socket-cleanup.js +++ b/test/e2e/forward-socket-cleanup.js @@ -55,13 +55,18 @@ describe('forward() socket cleanup', () => { ); // Wait until the outbound socket to the target actually exists. - await new Promise((resolve) => { + await new Promise((resolve, reject) => { const interval = setInterval(() => { if (targetSocket) { clearInterval(interval); + clearTimeout(timeout); resolve(); } }, 5); + const timeout = setTimeout(() => { + clearInterval(interval); + reject(new Error('Timed out waiting for httpAgent.createConnection() to be called - the outbound socket was never created.')); + }, 2000); }); expect(targetSocket.destroyed).to.be.false;