fix(net): prevent reusing unpatched sockets

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<SocketEventMap> {
 
           return tlsSocket
         }
-      })
+      }),
+      this.#stopReusingUnpatchedSockets()
     )
 
     /**
@@ -175,4 +203,80 @@ export class SocketInterceptor extends Interceptor<SocketEventMap> {
       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')
+      }
+    }
+  }
 }

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)
       }

test/modules/http/compliance/http-socket-reuse.test.ts

@@ -175,3 +175,36 @@ it('allows reusing the same socket for multiple bypassed requests with a body',
     expect(requestListener).toHaveBeenNthCalledWith(2, 'second')
   }
 })
+
+it('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)
+    // Read the response body to free the socket.
+    await response.text()
+    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()
+  }
+})