diff --git a/src/interceptors/ClientRequest/MockHttpSocket.ts b/src/interceptors/ClientRequest/MockHttpSocket.ts index 9a9f4d497..851be1199 100644 --- a/src/interceptors/ClientRequest/MockHttpSocket.ts +++ b/src/interceptors/ClientRequest/MockHttpSocket.ts @@ -63,6 +63,9 @@ export class MockHttpSocket extends MockSocket { private shouldKeepAlive?: boolean private socketState: 'unknown' | 'mock' | 'passthrough' = 'unknown' + private hasEmittedEarlyConnect = false + private hasEmittedEarlySecureConnect = false + private isInternalCall = false private responseParser: HTTPParser<1> private responseStream?: Readable private originalSocket?: net.Socket @@ -149,6 +152,23 @@ export class MockHttpSocket extends MockSocket { } } + /** + * Override _read to track when Node.js internals are making calls. + * Without this, calls to `_read` on the socket will set up listeners + * to `connect` events which will never actually get called because + * `hasEmittedEarlyConnect` will skip emitting `connect` once we've + * detected that a user has tried listening to `connect` events before + * the socket was ready for it. + */ + public _read(size: number): void { + this.isInternalCall = true + try { + super._read(size) + } finally { + this.isInternalCall = false + } + } + public emit(event: string | symbol, ...args: any[]): boolean { const emitEvent = super.emit.bind(this, event as any, ...args) @@ -159,6 +179,46 @@ export class MockHttpSocket extends MockSocket { return emitEvent() } + + + /** + * Override the 'once' method to detect when clients are waiting for connection events. + * This prevents deadlock when clients wait for 'secureConnect' before writing data. Some + * HTTP clients like the Stripe SDK like to do this. Since the interceptor waits for + * `write` to be called before it knows if something is to be mocked or bypassed, this + * causes a deadlock because it'll never emit `secureConnect` until it knows. + */ + public once(event: string, listener: (...args: any[]) => void): this { + // Only emit early connection events for user code, not Node.js internals + // Node.js calls once('connect') from Socket._read which sets isInternalCall flag + if (!this.isInternalCall && this.connecting && this.socketState === 'unknown') { + if (event === 'secureConnect' && this.baseUrl.protocol === 'https:' && !this.hasEmittedEarlySecureConnect) { + this.hasEmittedEarlySecureConnect = true + // Schedule the emission for the next tick to allow the listener to be attached first + setImmediate(() => { + if (this.socketState === 'unknown') { + this.connecting = false // Mark as connected + // Emit connect first if not already emitted + if (!this.hasEmittedEarlyConnect) { + this.hasEmittedEarlyConnect = true + this.emit('connect') + } + this.emit('secureConnect') + } + }) + } else if (event === 'connect' && !this.hasEmittedEarlyConnect) { + this.hasEmittedEarlyConnect = true + // Schedule the emission for the next tick to allow the listener to be attached first + setImmediate(() => { + if (this.socketState === 'unknown') { + this.connecting = false // Mark as connected + this.emit('connect') + } + }) + } + } + return super.once(event as any, listener as any) + } public destroy(error?: Error | undefined): this { // Destroy the response parser when the socket gets destroyed. @@ -178,6 +238,13 @@ export class MockHttpSocket extends MockSocket { * its data/events through this Socket. */ public passthrough(): void { + // If we scheduled an early connect event but haven't emitted it yet, + // emit it now before going to passthrough mode so Node.js can apply setTimeout + if (this.hasEmittedEarlyConnect && this.connecting) { + this.connecting = false + this.emit('connect') + } + this.socketState = 'passthrough' if (this.destroyed) { @@ -186,6 +253,11 @@ export class MockHttpSocket extends MockSocket { const socket = this.createConnection() this.originalSocket = socket + + // If a timeout was set on this socket before passthrough, apply it to the original socket + if (this.timeout !== undefined && this.timeout > 0) { + socket.setTimeout(this.timeout) + } /** * @note Inherit the original socket's connection handle. @@ -202,12 +274,10 @@ export class MockHttpSocket extends MockSocket { } // If the developer destroys the socket, destroy the original connection. - this.once('error', (error) => { + this.once('error', (error: Error) => { socket.destroy(error) }) - this.address = socket.address.bind(socket) - // Flush the buffered "socket.write()" calls onto // the original socket instance (i.e. write request body). // Exhaust the "requestBuffer" in case this Socket @@ -274,10 +344,20 @@ export class MockHttpSocket extends MockSocket { socket .on('lookup', (...args) => this.emit('lookup', ...args)) .on('connect', () => { + this.connecting = socket.connecting - this.emit('connect') + + // Don't re-emit 'connect' if already emitted early + if (!this.hasEmittedEarlyConnect) { + this.emit('connect') + } + }) + .on('secureConnect', () => { + // Don't re-emit 'secureConnect' if already emitted early + if (!this.hasEmittedEarlySecureConnect) { + this.emit('secureConnect') + } }) - .on('secureConnect', () => this.emit('secureConnect')) .on('secure', () => this.emit('secure')) .on('session', (session) => this.emit('session', session)) .on('ready', () => this.emit('ready')) @@ -300,6 +380,22 @@ export class MockHttpSocket extends MockSocket { .on('end', () => this.emit('end')) } + // If address is called on a passthrough socket before the original socket is set, it won't + // return anything. This can happen because sockets fire `connect` early to avoid a causing + // a deadlock with some HTTP clients that like to wait for `connect` or `secureConnect` + // before calling `write`. + public address(): net.AddressInfo | {} { + if (this.originalSocket) { + return this.originalSocket.address() + } + + return { + address: this.connectionOptions.hostname || this.connectionOptions.host || '127.0.0.1', + family: this.connectionOptions.family === 6 ? 'IPv6' : 'IPv4', + port: this.connectionOptions.port || 80 + } + } + /** * Convert the given Fetch API `Response` instance to an * HTTP message and push it to the socket. @@ -509,8 +605,8 @@ export class MockHttpSocket extends MockSocket { } private onRequestStart: RequestHeadersCompleteCallback = ( - versionMajor, - versionMinor, + _versionMajor, + _versionMinor, rawHeaders, _, path, @@ -631,10 +727,10 @@ export class MockHttpSocket extends MockSocket { } private onResponseStart: ResponseHeadersCompleteCallback = ( - versionMajor, - versionMinor, + _versionMajor, + _versionMinor, rawHeaders, - method, + _method, url, status, statusText diff --git a/test/modules/http/regressions/http-socket-secure-connect-passthrough.test.ts b/test/modules/http/regressions/http-socket-secure-connect-passthrough.test.ts new file mode 100644 index 000000000..28ced338f --- /dev/null +++ b/test/modules/http/regressions/http-socket-secure-connect-passthrough.test.ts @@ -0,0 +1,225 @@ +/** + * @vitest-environment node + * @see https://github.com/mswjs/interceptors/issues/XXX + */ +import { vi, it, expect, beforeAll, afterAll, afterEach } from 'vitest' +import http from 'node:http' +import https from 'node:https' +import type { TLSSocket } from 'node:tls' +import { HttpServer } from '@open-draft/test-server/http' +import { ClientRequestInterceptor } from '../../../../src/interceptors/ClientRequest/index' + +const httpServer = new HttpServer((app) => { + app.get('/', (_req, res) => { + res.status(200).send('original-response') + }) + app.post('/echo', (req, res) => { + let body = '' + req.on('data', chunk => body += chunk) + req.on('end', () => { + res.status(200).json({ received: body }) + }) + }) +}) + +const interceptor = new ClientRequestInterceptor() + +beforeAll(async () => { + interceptor.apply() + await httpServer.listen() +}) + +afterEach(() => { + interceptor.removeAllListeners() +}) + +afterAll(async () => { + interceptor.dispose() + await httpServer.close() +}) + +it('emits "secureConnect" event for passthrough HTTPS requests when client waits for it before writing', async () => { + // This test reproduces the Stripe SDK pattern where the client waits + // for secureConnect before writing data to ensure TLS handshake is complete. + // The issue is that when a request is NOT mocked (passthrough), the MockHttpSocket + // doesn't emit secureConnect at the right time, causing clients to hang. + + const secureConnectListener = vi.fn() + const responseListener = vi.fn() + const errorListener = vi.fn() + + const requestData = JSON.stringify({ test: 'data' }) + + const request = https.request(httpServer.https.url('/echo'), { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'Content-Length': Buffer.byteLength(requestData) + }, + rejectUnauthorized: false // Allow self-signed cert from test server + }) + + request.on('error', errorListener) + + request.on('response', (res) => { + responseListener() + + let body = '' + res.on('data', chunk => body += chunk) + res.on('end', () => { + expect(JSON.parse(body)).toEqual({ received: requestData }) + }) + }) + + // This is the critical pattern from Stripe SDK: + // Wait for the socket and then wait for secureConnect before writing + request.once('socket', (socket) => { + expect(socket).toBeDefined() + + // The socket should indicate it's a TLS socket + const tlsSocket = socket as TLSSocket + expect(tlsSocket.encrypted).toBe(true) + + if (socket.connecting) { + // Client waits for secureConnect before writing data + // This is where the bug occurs - secureConnect never fires for passthrough + socket.once('secureConnect', () => { + secureConnectListener() + request.write(requestData) + request.end() + }) + } else { + // Socket is already connected + secureConnectListener() + request.write(requestData) + request.end() + } + }) + + // Wait for the request to complete + // This will timeout if secureConnect is never emitted + await vi.waitFor(() => { + expect(responseListener).toHaveBeenCalledTimes(1) + }, { timeout: 5000 }) + + // Verify the secureConnect event was handled + expect(secureConnectListener).toHaveBeenCalledTimes(1) + expect(errorListener).not.toHaveBeenCalled() +}) + +it('emits "connect" event for passthrough HTTP requests when client waits for it before writing', async () => { + // Similar test for HTTP to ensure connect event works correctly + + const connectListener = vi.fn() + const responseListener = vi.fn() + const errorListener = vi.fn() + + const requestData = JSON.stringify({ test: 'data' }) + + const request = http.request(httpServer.http.url('/echo'), { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'Content-Length': Buffer.byteLength(requestData) + } + }) + + request.on('error', errorListener) + + request.on('response', (res) => { + responseListener() + + let body = '' + res.on('data', chunk => body += chunk) + res.on('end', () => { + expect(JSON.parse(body)).toEqual({ received: requestData }) + }) + }) + + // Pattern similar to Stripe SDK but for HTTP + request.once('socket', (socket) => { + expect(socket).toBeDefined() + + if (socket.connecting) { + // Client waits for connect before writing data + socket.once('connect', () => { + connectListener() + request.write(requestData) + request.end() + }) + } else { + // Socket is already connected + connectListener() + request.write(requestData) + request.end() + } + }) + + // Wait for the request to complete + await vi.waitFor(() => { + expect(responseListener).toHaveBeenCalledTimes(1) + }, { timeout: 5000 }) + + // Verify the connect event was handled + expect(connectListener).toHaveBeenCalledTimes(1) + expect(errorListener).not.toHaveBeenCalled() +}) + +it('emits "secureConnect" for mocked HTTPS requests when client waits for it before writing', async () => { + // Verify that mocked responses work correctly (this should already work) + + interceptor.on('request', ({ request, controller }) => { + if (request.url.includes('/mocked')) { + controller.respondWith(new Response(JSON.stringify({ mocked: true }), { + status: 200, + headers: { 'Content-Type': 'application/json' } + })) + } + }) + + const secureConnectListener = vi.fn() + const responseListener = vi.fn() + + const requestData = JSON.stringify({ test: 'data' }) + + const request = https.request(httpServer.https.url('/mocked'), { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'Content-Length': Buffer.byteLength(requestData) + }, + rejectUnauthorized: false + }) + + request.on('response', (res) => { + responseListener() + + let body = '' + res.on('data', chunk => body += chunk) + res.on('end', () => { + expect(JSON.parse(body)).toEqual({ mocked: true }) + }) + }) + + request.once('socket', (socket) => { + if (socket.connecting) { + socket.once('secureConnect', () => { + secureConnectListener() + request.write(requestData) + request.end() + }) + } else { + secureConnectListener() + request.write(requestData) + request.end() + } + }) + + // Wait for the request to complete + await vi.waitFor(() => { + expect(responseListener).toHaveBeenCalledTimes(1) + }, { timeout: 5000 }) + + // This should work for mocked responses + expect(secureConnectListener).toHaveBeenCalledTimes(1) +}) \ No newline at end of file