From e6ccba3e9b0d8d57066ba8bff4444841acdce564 Mon Sep 17 00:00:00 2001 From: Michael Solomon Date: Tue, 14 Jul 2026 00:16:17 +0300 Subject: [PATCH 1/6] failing test --- .../http/compliance/http-socket-tttt.test.ts | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 test/modules/http/compliance/http-socket-tttt.test.ts diff --git a/test/modules/http/compliance/http-socket-tttt.test.ts b/test/modules/http/compliance/http-socket-tttt.test.ts new file mode 100644 index 00000000..09f3bade --- /dev/null +++ b/test/modules/http/compliance/http-socket-tttt.test.ts @@ -0,0 +1,47 @@ +// @vitest-environment node +import http from 'node:http' +import { HttpServer } from '@open-draft/test-server/http' +import { HttpRequestInterceptor } from '#/src/interceptors/http' +import { toWebResponse } from '#/test/helpers' + +const httpServer = new HttpServer((app) => { + app.get('/resource/*', (req, res) => { + res.status(200).send('original') + }) +}) + +const interceptor = new HttpRequestInterceptor() + +beforeAll(async () => { + await httpServer.listen() +}) + +afterAll(async () => { + await httpServer.close() +}) + +it.only('unpatched socket', async () => { + const requestListener = vi.fn() + + interceptor.on('request', async ({ request }) => { + requestListener(await request.text()) + }) + + { + const request = http.get(httpServer.http.url('/resource/one')) + const [response] = await toWebResponse(request) + + expect.soft(response.status).toBe(200) + await response.text() // must read the response body to free the socket + expect(requestListener).not.toHaveBeenCalled() + } + + interceptor.apply() + { + const request = http.get(httpServer.http.url('/resource/one')) + const [response] = await toWebResponse(request) + + expect.soft(response.status).toBe(200) + expect(requestListener).toHaveBeenCalledOnce() + } +}) \ No newline at end of file From f8752d58e51c29cf9ee58fb8d9983609ec020f9e Mon Sep 17 00:00:00 2001 From: Michael Solomon Date: Tue, 14 Jul 2026 00:23:07 +0300 Subject: [PATCH 2/6] fix --- ...et-tttt.test.ts => http-socket-created-before-apply.test.ts} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename test/modules/http/compliance/{http-socket-tttt.test.ts => http-socket-created-before-apply.test.ts} (91%) diff --git a/test/modules/http/compliance/http-socket-tttt.test.ts b/test/modules/http/compliance/http-socket-created-before-apply.test.ts similarity index 91% rename from test/modules/http/compliance/http-socket-tttt.test.ts rename to test/modules/http/compliance/http-socket-created-before-apply.test.ts index 09f3bade..16acfe12 100644 --- a/test/modules/http/compliance/http-socket-tttt.test.ts +++ b/test/modules/http/compliance/http-socket-created-before-apply.test.ts @@ -20,7 +20,7 @@ afterAll(async () => { await httpServer.close() }) -it.only('unpatched socket', async () => { +it.only('intercepts a request after interceptor.apply(), even if it reuse a socket created beforehand', async () => { const requestListener = vi.fn() interceptor.on('request', async ({ request }) => { From e7f72267e3e70cbd0840bc04a616e6466c85411a Mon Sep 17 00:00:00 2001 From: Michael Solomon Date: Tue, 14 Jul 2026 00:25:00 +0300 Subject: [PATCH 3/6] remove only --- .../http/compliance/http-socket-created-before-apply.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/modules/http/compliance/http-socket-created-before-apply.test.ts b/test/modules/http/compliance/http-socket-created-before-apply.test.ts index 16acfe12..fb05d3f7 100644 --- a/test/modules/http/compliance/http-socket-created-before-apply.test.ts +++ b/test/modules/http/compliance/http-socket-created-before-apply.test.ts @@ -20,7 +20,7 @@ afterAll(async () => { await httpServer.close() }) -it.only('intercepts a request after interceptor.apply(), even if it reuse a socket created beforehand', async () => { +it('intercepts a request after interceptor.apply(), even if it reuse a socket created beforehand', async () => { const requestListener = vi.fn() interceptor.on('request', async ({ request }) => { From 79796d924c782bd4953515cada8b4207749bc8bd Mon Sep 17 00:00:00 2001 From: Michael Solomon Date: Tue, 14 Jul 2026 00:31:43 +0300 Subject: [PATCH 4/6] fix --- .../http-socket-created-before-apply.test.ts | 47 ------------------- .../http/compliance/http-socket-reuse.test.ts | 31 ++++++++++++ 2 files changed, 31 insertions(+), 47 deletions(-) delete mode 100644 test/modules/http/compliance/http-socket-created-before-apply.test.ts diff --git a/test/modules/http/compliance/http-socket-created-before-apply.test.ts b/test/modules/http/compliance/http-socket-created-before-apply.test.ts deleted file mode 100644 index fb05d3f7..00000000 --- a/test/modules/http/compliance/http-socket-created-before-apply.test.ts +++ /dev/null @@ -1,47 +0,0 @@ -// @vitest-environment node -import http from 'node:http' -import { HttpServer } from '@open-draft/test-server/http' -import { HttpRequestInterceptor } from '#/src/interceptors/http' -import { toWebResponse } from '#/test/helpers' - -const httpServer = new HttpServer((app) => { - app.get('/resource/*', (req, res) => { - res.status(200).send('original') - }) -}) - -const interceptor = new HttpRequestInterceptor() - -beforeAll(async () => { - await httpServer.listen() -}) - -afterAll(async () => { - await httpServer.close() -}) - -it('intercepts a request after interceptor.apply(), even if it reuse a socket created beforehand', async () => { - const requestListener = vi.fn() - - interceptor.on('request', async ({ request }) => { - requestListener(await request.text()) - }) - - { - const request = http.get(httpServer.http.url('/resource/one')) - const [response] = await toWebResponse(request) - - expect.soft(response.status).toBe(200) - await response.text() // must read the response body to free the socket - expect(requestListener).not.toHaveBeenCalled() - } - - interceptor.apply() - { - const request = http.get(httpServer.http.url('/resource/one')) - const [response] = await toWebResponse(request) - - expect.soft(response.status).toBe(200) - expect(requestListener).toHaveBeenCalledOnce() - } -}) \ No newline at end of file diff --git a/test/modules/http/compliance/http-socket-reuse.test.ts b/test/modules/http/compliance/http-socket-reuse.test.ts index 8bb45ea6..50f96eb6 100644 --- a/test/modules/http/compliance/http-socket-reuse.test.ts +++ b/test/modules/http/compliance/http-socket-reuse.test.ts @@ -175,3 +175,34 @@ it('allows reusing the same socket for multiple bypassed requests with a body', expect(requestListener).toHaveBeenNthCalledWith(2, 'second') } }) + +it.only('intercepts a request after interceptor.apply(), even if it reuse a socket created beforehand', async () => { + const requestListener = vi.fn() + + interceptor.dispose() + interceptor.on('request', async ({ request }) => { + requestListener(await request.text()) + }) + + { + const request = https.get(httpServer.https.url('/resource/one'), { + rejectUnauthorized: false, + }) + const [response] = await toWebResponse(request) + + expect.soft(response.status).toBe(200) + await response.text() // must read the response body to free the socket + expect(requestListener).not.toHaveBeenCalled() + } + + interceptor.apply() + { + const request = https.get(httpServer.https.url('/resource/one'), { + rejectUnauthorized: false, + }) + const [response] = await toWebResponse(request) + + expect.soft(response.status).toBe(200) + expect(requestListener).toHaveBeenCalledOnce() + } +}) \ No newline at end of file From a5adc8d8f7248c922f9336d7de24936b6aff3c1b Mon Sep 17 00:00:00 2001 From: Artem Zakharchenko Date: Tue, 14 Jul 2026 15:36:06 +0200 Subject: [PATCH 5/6] chore: test formatting --- test/modules/http/compliance/http-socket-reuse.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/modules/http/compliance/http-socket-reuse.test.ts b/test/modules/http/compliance/http-socket-reuse.test.ts index 50f96eb6..5e2e2e1d 100644 --- a/test/modules/http/compliance/http-socket-reuse.test.ts +++ b/test/modules/http/compliance/http-socket-reuse.test.ts @@ -196,6 +196,7 @@ it.only('intercepts a request after interceptor.apply(), even if it reuse a sock } interceptor.apply() + { const request = https.get(httpServer.https.url('/resource/one'), { rejectUnauthorized: false, @@ -205,4 +206,4 @@ it.only('intercepts a request after interceptor.apply(), even if it reuse a sock expect.soft(response.status).toBe(200) expect(requestListener).toHaveBeenCalledOnce() } -}) \ No newline at end of file +}) From 9ac7fe69870ee874deb33351276c7d832e9dcca1 Mon Sep 17 00:00:00 2001 From: Artem Zakharchenko Date: Tue, 14 Jul 2026 16:42:43 +0200 Subject: [PATCH 6/6] fix(net): prevent reusing unpatched sockets --- src/interceptors/net/index.ts | 110 +++++++++++++++++- src/interceptors/net/socket-controller.ts | 21 +++- .../http/compliance/http-socket-reuse.test.ts | 5 +- 3 files changed, 129 insertions(+), 7 deletions(-) diff --git a/src/interceptors/net/index.ts b/src/interceptors/net/index.ts index eba3a0a6..ed9a46cd 100644 --- a/src/interceptors/net/index.ts +++ b/src/interceptors/net/index.ts @@ -5,10 +5,18 @@ import { type NetworkConnectionOptions, normalizeNetConnectArgs, } from './utils/normalize-net-connect-args' -import { TcpSocketController, TlsSocketController } from './socket-controller' +import { + kPatched, + TcpSocketController, + TlsSocketController, + type TcpHandle, +} from './socket-controller' import { normalizeTlsConnectArgs } from './utils/normalize-tls-connect-args' import { createLogger } from '../../utils/logger' -import { patchesRegistry } from '../../utils/patchesRegistry' +import { + patchesRegistry, + getDeepPropertyDescriptor, +} from '../../utils/patchesRegistry' import { Interceptor } from '#/src/interceptor' interface SocketConnectionEventData { @@ -39,6 +47,25 @@ type SocketEventMap = { const logger = createLogger('socket') +/** + * Returns true if the given socket has been explicitly unrefed. + * The ref state lives on the socket's handle. Some handles (e.g. `TLSWrap`) + * don't implement it themselves and defer to their parent handle (`TCP`). + */ +function isSocketUnrefed(socket: net.Socket): boolean { + let handle: TcpHandle | undefined = socket._handle + + while (handle) { + if (typeof handle.hasRef === 'function') { + return !handle.hasRef() + } + + handle = handle._parent + } + + return false +} + /** * Interceptor for `net.Socket` connections. */ @@ -161,7 +188,8 @@ export class SocketInterceptor extends Interceptor { return tlsSocket } - }) + }), + this.#stopReusingUnpatchedSockets() ) /** @@ -175,4 +203,80 @@ export class SocketInterceptor extends Interceptor { net.createConnection = realNetCreateConnection }) } + + /** + * Prevent the `net.Socket` instances created before this interceptor was applied + * from getting reused by the `Agent` after the interceptor is applied. + */ + #stopReusingUnpatchedSockets(): () => void { + const match = getDeepPropertyDescriptor(net.Socket.prototype, 'destroyed') + + if ( + match == null || + match.descriptor.get == null || + match.descriptor.set == null + ) { + return () => {} + } + + const { get: realDestroyedGetter, set: realDestroyedSetter } = + match.descriptor + + Object.defineProperty(net.Socket.prototype, 'destroyed', { + configurable: true, + get(this: net.Socket) { + const realDestroyed = realDestroyedGetter.call(this) + + if (realDestroyed || this[kPatched]) { + return realDestroyed + } + + /** + * @note Agent pings every free socket via "socket.destroyed" to see + * if they can be reused. If such a ping is detected on an unpatched socket, + * destroy it. That forces the agent to issue a new socket, which will call + * "net.connect" again, falling through the interception. + * + * A socket idle in an `Agent` keep-alive pool is detected via + * documented agent contracts rather than internal listener names: + * - Pooled sockets listen to the "agentRemove" event (documented as + * the way to remove a socket from the agent); + * - `agent.keepSocketAlive()` unrefs a socket when pooling it, and + * `agent.reuseSocket()` refs it on reuse (both are the documented + * default implementations); + * - A socket detached from a request has no `_httpMessage`. + */ + if ( + !this.connecting && + this.listenerCount('agentRemove') > 0 && + !this._httpMessage && + isSocketUnrefed(this) + ) { + this.destroy() + return true + } + + return realDestroyed + }, + // Attach a setter for behavior parity. + set(this: net.Socket, value: boolean) { + realDestroyedSetter.call(this, value) + }, + }) + + return () => { + if (match.owner === net.Socket.prototype) { + // The real descriptor was an own property, restore it in-place. + Object.defineProperty( + net.Socket.prototype, + 'destroyed', + match.descriptor + ) + } else { + // The real descriptor lives up the prototype chain (stream.Duplex), + // deleting the shadow restores the original lookup. + Reflect.deleteProperty(net.Socket.prototype, 'destroyed') + } + } + } } diff --git a/src/interceptors/net/socket-controller.ts b/src/interceptors/net/socket-controller.ts index ff8ba7bd..5350b04d 100644 --- a/src/interceptors/net/socket-controller.ts +++ b/src/interceptors/net/socket-controller.ts @@ -10,6 +10,7 @@ import { getAddressInfoByConnectionOptions } from './utils/address-info' const kListenerWrap = Symbol('kListenerWrap') export const kRawSocket = Symbol('kRawSocket') +export const kPatched = Symbol('kPatched') const logger = createLogger('socket') @@ -19,6 +20,8 @@ type OperationStatus = 0 | 1 declare module 'node:net' { interface Socket { + [kPatched]?: boolean + _httpMessage?: object | null _pendingData: | string | Buffer @@ -70,6 +73,7 @@ export interface TcpHandle { bytesWritten: number ref?: () => void unref?: () => void + hasRef?: () => boolean fchmod: (mode: number) => void setBlocking: (blocking: boolean) => OperationStatus setNoDelay?: (noDelay: boolean) => void @@ -205,6 +209,10 @@ export abstract class SocketController { constructor(socket: net.Socket) { this[kRawSocket] = socket + // Mark this socket as patched so socket-related patches + // (e.g. the "destroyed" getter) can tell it apart from + // the sockets created before the interception was applied. + socket[kPatched] = true this.readyState = SocketController.PENDING } @@ -340,7 +348,9 @@ export class TcpSocketController extends SocketController { this.#bufferedWrites.length === 0 && this.socket.listenerCount('connect') > 0 ) { - logger.verbose('assume connect->write socket, calling "connect" listeners...') + logger.verbose( + 'assume connect->write socket, calling "connect" listeners...' + ) this.emulateConnect() } }) @@ -390,7 +400,9 @@ export class TcpSocketController extends SocketController { // The server socket will NEVER have any "data" listeners attached // becuase the "connection" interceptor event emits on the next tick. if (this.socket.listenerCount('internal:write') === 0) { - logger.verbose('no server data listeners, scheduling to the next tick...') + logger.verbose( + 'no server data listeners, scheduling to the next tick...' + ) process.nextTick(() => { logger.verbose( @@ -642,6 +654,11 @@ export class TcpSocketController extends SocketController { const createRealSocket = () => { const realSocket = this.createConnection() + // Mark the passthrough socket as patched so it's exempt from + // the unpatched socket detection (it never enters agent pools, + // but this skips the detection cost on its every "destroyed" read). + realSocket[kPatched] = true + if (this.socket.timeout != null) { realSocket.setTimeout(this.socket.timeout) } diff --git a/test/modules/http/compliance/http-socket-reuse.test.ts b/test/modules/http/compliance/http-socket-reuse.test.ts index 5e2e2e1d..a9e764da 100644 --- a/test/modules/http/compliance/http-socket-reuse.test.ts +++ b/test/modules/http/compliance/http-socket-reuse.test.ts @@ -176,7 +176,7 @@ it('allows reusing the same socket for multiple bypassed requests with a body', } }) -it.only('intercepts a request after interceptor.apply(), even if it reuse a socket created beforehand', async () => { +it('intercepts a request after interceptor.apply(), even if it reuse a socket created beforehand', async () => { const requestListener = vi.fn() interceptor.dispose() @@ -191,7 +191,8 @@ it.only('intercepts a request after interceptor.apply(), even if it reuse a sock const [response] = await toWebResponse(request) expect.soft(response.status).toBe(200) - await response.text() // must read the response body to free the socket + // Read the response body to free the socket. + await response.text() expect(requestListener).not.toHaveBeenCalled() }