From dddcd071784a67199585e48e8da356bc52514834 Mon Sep 17 00:00:00 2001 From: Artem Zakharchenko Date: Fri, 29 Nov 2024 19:16:52 +0100 Subject: [PATCH 1/4] test: add failing test --- .../ClientRequest/MockHttpSocket.ts | 4 ++ .../http/compliance/http-upgrade.test.ts | 68 +++++++++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 test/modules/http/compliance/http-upgrade.test.ts diff --git a/src/interceptors/ClientRequest/MockHttpSocket.ts b/src/interceptors/ClientRequest/MockHttpSocket.ts index 2d9e963d..aa1ab8a9 100644 --- a/src/interceptors/ClientRequest/MockHttpSocket.ts +++ b/src/interceptors/ClientRequest/MockHttpSocket.ts @@ -157,6 +157,8 @@ export class MockHttpSocket extends MockSocket { const socket = this.createConnection() + console.log('MockHttpSocket.passthrough()', this.writeBuffer) + // If the developer destroys the socket, destroy the original connection. this.once('error', (error) => { socket.destroy(error) @@ -443,6 +445,8 @@ export class MockHttpSocket extends MockSocket { ) => { this.shouldKeepAlive = shouldKeepAlive + console.log({ _, __, ___, ____ }) + const url = new URL(path, this.baseUrl) const method = this.connectionOptions.method?.toUpperCase() || 'GET' const headers = parseRawHeaders(rawHeaders) diff --git a/test/modules/http/compliance/http-upgrade.test.ts b/test/modules/http/compliance/http-upgrade.test.ts new file mode 100644 index 00000000..68e102ed --- /dev/null +++ b/test/modules/http/compliance/http-upgrade.test.ts @@ -0,0 +1,68 @@ +/** + * @see https://github.com/mswjs/interceptors/issues/682 + */ +// @vitest-environment node-with-websocket +import { vi, it, expect, beforeAll, afterEach, afterAll } from 'vitest' +import net from 'node:net' +import { Server } from 'socket.io' +import { io } from 'socket.io-client' +import { ClientRequestInterceptor } from '../../../../src/interceptors/ClientRequest' + +const interceptor = new ClientRequestInterceptor() +const server = new Server(51678) + +beforeAll(() => { + interceptor.apply() +}) + +afterEach(() => { + interceptor.removeAllListeners() +}) + +afterAll(async () => { + interceptor.dispose() + await new Promise((resolve, reject) => { + server.disconnectSockets() + server.close((error) => { + if (error) reject(error) + resolve() + }) + }) +}) + +it('bypasses a WebSocket upgrade request', async () => { + net.Socket.prototype.emit = new Proxy(net.Socket.prototype.emit, { + apply(target, thisArg, args) { + console.log('EMIT!', args[0]) + + if (args[0] === 'data') { + console.log(args[1].toString(), '\n\n') + } + + return Reflect.apply(target, thisArg, args) + }, + }) + net.Socket.prototype.write = new Proxy(net.Socket.prototype.write, { + apply(target, thisArg, args) { + console.log('WRITE!', args[0].toString()) + + return Reflect.apply(target, thisArg, args) + }, + }) + + interceptor.on('request', ({ request }) => { + console.log(request.method, request.url, Array.from(request.headers)) + }) + + // http.get('http://example.com/') + + const client = io(`http://localhost:51678`, { + transports: ['websocket'], + }) + + await vi.waitFor(async () => { + expect(client.connected).toBe(true) + }) + + console.log('----') +}) From adfeb5eb1b5fb4d20a085fc4c9f71d5bed13207b Mon Sep 17 00:00:00 2001 From: Artem Zakharchenko Date: Mon, 2 Dec 2024 18:54:13 +0100 Subject: [PATCH 2/4] fix(MockHttpSocket): forward writes to the original socket --- .../ClientRequest/MockHttpSocket.ts | 11 ++- src/interceptors/Socket/MockSocket.ts | 1 + .../compliance/http-upgrade-works.test.ts | 85 +++++++++++++++++++ .../http/compliance/http-upgrade.test.ts | 28 ------ 4 files changed, 94 insertions(+), 31 deletions(-) create mode 100644 test/modules/http/compliance/http-upgrade-works.test.ts diff --git a/src/interceptors/ClientRequest/MockHttpSocket.ts b/src/interceptors/ClientRequest/MockHttpSocket.ts index 7759583f..8352ed52 100644 --- a/src/interceptors/ClientRequest/MockHttpSocket.ts +++ b/src/interceptors/ClientRequest/MockHttpSocket.ts @@ -157,7 +157,14 @@ export class MockHttpSocket extends MockSocket { const socket = this.createConnection() - console.log('MockHttpSocket.passthrough()', this.writeBuffer) + /** + * Forward any writes to the mock socket to the underlying original socket. + * This ensures functional duplex connections, like WebSocket. + * @see https://github.com/mswjs/interceptors/issues/682 + */ + this._write = (chunk, encoding, callback) => { + socket.write(chunk, encoding, callback) + } // If the developer destroys the socket, destroy the original connection. this.once('error', (error) => { @@ -449,8 +456,6 @@ export class MockHttpSocket extends MockSocket { ) => { this.shouldKeepAlive = shouldKeepAlive - console.log({ _, __, ___, ____ }) - const url = new URL(path, this.baseUrl) const method = this.connectionOptions.method?.toUpperCase() || 'GET' const headers = parseRawHeaders(rawHeaders) diff --git a/src/interceptors/Socket/MockSocket.ts b/src/interceptors/Socket/MockSocket.ts index 412961ed..fd88b29e 100644 --- a/src/interceptors/Socket/MockSocket.ts +++ b/src/interceptors/Socket/MockSocket.ts @@ -40,6 +40,7 @@ export class MockSocket extends net.Socket { args as WriteArgs ) this.options.write(chunk, encoding, callback) + this._write?.apply(this, args as any) return true } diff --git a/test/modules/http/compliance/http-upgrade-works.test.ts b/test/modules/http/compliance/http-upgrade-works.test.ts new file mode 100644 index 00000000..cb55b429 --- /dev/null +++ b/test/modules/http/compliance/http-upgrade-works.test.ts @@ -0,0 +1,85 @@ +/** + * @see https://github.com/mswjs/interceptors/issues/682 + */ +// @vitest-environment node-with-websocket +import { vi, it, expect, beforeAll, afterEach, afterAll } from 'vitest' +import net from 'node:net' +import { Server } from 'socket.io' +import { io } from 'socket.io-client' +// import { ClientRequestInterceptor } from '../../../../src/interceptors/ClientRequest' + +// const interceptor = new ClientRequestInterceptor() +const server = new Server(51679) + +beforeAll(() => { + // interceptor.apply() +}) + +afterEach(() => { + // interceptor.removeAllListeners() +}) + +afterAll(async () => { + // interceptor.dispose() + await new Promise((resolve, reject) => { + server.disconnectSockets() + server.close((error) => { + if (error) reject(error) + resolve() + }) + }) +}) + +it('bypasses a WebSocket upgrade request', async () => { + net.Socket.prototype.emit = new Proxy(net.Socket.prototype.emit, { + apply(target, thisArg, args) { + console.log('EMIT!', args[0]) + + if (args[0] === 'agentRemove') { + debugger + } + + if (args[0] === 'data') { + // if (args[1].toString().includes('HTTP/1.1 101 Switching Protocols')) { + // debugger + // } + + console.log( + ` + UTF: ${args[1].toString()} + HEX: ${args[1].toString('hex')} + `, + '\n\n' + ) + } + + return Reflect.apply(target, thisArg, args) + }, + }) + + // net.Socket.prototype.write = new Proxy(net.Socket.prototype.write, { + // apply(target, thisArg, args) { + // console.trace(`WRITE! + // UTF: ${args[0].toString()} + // HEX: ${args[0].toString('hex')} + // `) + + // return Reflect.apply(target, thisArg, args) + // }, + // }) + + const client = io(`http://localhost:51679`, { + transports: ['websocket'], + reconnection: false, + retries: 0, + }) + + await vi.waitFor( + async () => { + expect(client.connected).toBe(true) + }, + { timeout: 100_000 } + ) + + console.log('----') +}, 100_000) diff --git a/test/modules/http/compliance/http-upgrade.test.ts b/test/modules/http/compliance/http-upgrade.test.ts index 68e102ed..ea286ae3 100644 --- a/test/modules/http/compliance/http-upgrade.test.ts +++ b/test/modules/http/compliance/http-upgrade.test.ts @@ -3,7 +3,6 @@ */ // @vitest-environment node-with-websocket import { vi, it, expect, beforeAll, afterEach, afterAll } from 'vitest' -import net from 'node:net' import { Server } from 'socket.io' import { io } from 'socket.io-client' import { ClientRequestInterceptor } from '../../../../src/interceptors/ClientRequest' @@ -31,31 +30,6 @@ afterAll(async () => { }) it('bypasses a WebSocket upgrade request', async () => { - net.Socket.prototype.emit = new Proxy(net.Socket.prototype.emit, { - apply(target, thisArg, args) { - console.log('EMIT!', args[0]) - - if (args[0] === 'data') { - console.log(args[1].toString(), '\n\n') - } - - return Reflect.apply(target, thisArg, args) - }, - }) - net.Socket.prototype.write = new Proxy(net.Socket.prototype.write, { - apply(target, thisArg, args) { - console.log('WRITE!', args[0].toString()) - - return Reflect.apply(target, thisArg, args) - }, - }) - - interceptor.on('request', ({ request }) => { - console.log(request.method, request.url, Array.from(request.headers)) - }) - - // http.get('http://example.com/') - const client = io(`http://localhost:51678`, { transports: ['websocket'], }) @@ -63,6 +37,4 @@ it('bypasses a WebSocket upgrade request', async () => { await vi.waitFor(async () => { expect(client.connected).toBe(true) }) - - console.log('----') }) From 0b9a56e651eb3c1fc70d49a6e1138fdd51e5d3fd Mon Sep 17 00:00:00 2001 From: Artem Zakharchenko Date: Mon, 2 Dec 2024 19:25:50 +0100 Subject: [PATCH 3/4] fix(MockHttpSocket): move write forwarding to "write" of MockSocket --- .../ClientRequest/MockHttpSocket.ts | 40 +++++++++++++------ src/interceptors/Socket/MockSocket.ts | 2 - 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/src/interceptors/ClientRequest/MockHttpSocket.ts b/src/interceptors/ClientRequest/MockHttpSocket.ts index 8352ed52..d759ebc1 100644 --- a/src/interceptors/ClientRequest/MockHttpSocket.ts +++ b/src/interceptors/ClientRequest/MockHttpSocket.ts @@ -58,16 +58,31 @@ export class MockHttpSocket extends MockSocket { private requestStream?: Readable private shouldKeepAlive?: boolean - private responseType: 'mock' | 'bypassed' = 'bypassed' + private socketState: 'unknown' | 'mock' | 'passthrough' = 'unknown' private responseParser: HTTPParser<1> private responseStream?: Readable + private originalSocket?: net.Socket constructor(options: MockHttpSocketOptions) { super({ write: (chunk, encoding, callback) => { - this.writeBuffer.push([chunk, encoding, callback]) + // Buffer the writes so they can be flushed in case of the original connection + // and when reading the request body in the interceptor. If the connection has + // been established, no need to buffer the chunks anymore, they will be forwarded. + if (this.socketState !== 'passthrough') { + this.writeBuffer.push([chunk, encoding, callback]) + } if (chunk) { + /** + * Forward any writes to the mock socket to the underlying original socket. + * This ensures functional duplex connections, like WebSocket. + * @see https://github.com/mswjs/interceptors/issues/682 + */ + if (this.socketState === 'passthrough') { + this.originalSocket?.write(chunk, encoding, callback) + } + this.requestParser.execute( Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk, encoding) ) @@ -75,6 +90,11 @@ export class MockHttpSocket extends MockSocket { }, read: (chunk) => { if (chunk !== null) { + /** + * @todo We need to free the parser if the connection has been + * upgraded to a non-HTTP protocol. It won't be able to parse data + * from that point onward anyway. No need to keep it in memory. + */ this.responseParser.execute( Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk) ) @@ -151,20 +171,14 @@ export class MockHttpSocket extends MockSocket { * its data/events through this Socket. */ public passthrough(): void { + this.socketState = 'passthrough' + if (this.destroyed) { return } const socket = this.createConnection() - - /** - * Forward any writes to the mock socket to the underlying original socket. - * This ensures functional duplex connections, like WebSocket. - * @see https://github.com/mswjs/interceptors/issues/682 - */ - this._write = (chunk, encoding, callback) => { - socket.write(chunk, encoding, callback) - } + this.originalSocket = socket // If the developer destroys the socket, destroy the original connection. this.once('error', (error) => { @@ -285,7 +299,7 @@ export class MockHttpSocket extends MockSocket { // First, emit all the connection events // to emulate a successful connection. this.mockConnect() - this.responseType = 'mock' + this.socketState = 'mock' // Flush the write buffer to trigger write callbacks // if it hasn't been flushed already (e.g. someone started reading request stream). @@ -590,7 +604,7 @@ export class MockHttpSocket extends MockSocket { this.responseListenersPromise = this.onResponse({ response, - isMockedResponse: this.responseType === 'mock', + isMockedResponse: this.socketState === 'mock', requestId: Reflect.get(this.request, kRequestId), request: this.request, socket: this, diff --git a/src/interceptors/Socket/MockSocket.ts b/src/interceptors/Socket/MockSocket.ts index fd88b29e..5bcc42ee 100644 --- a/src/interceptors/Socket/MockSocket.ts +++ b/src/interceptors/Socket/MockSocket.ts @@ -40,7 +40,6 @@ export class MockSocket extends net.Socket { args as WriteArgs ) this.options.write(chunk, encoding, callback) - this._write?.apply(this, args as any) return true } @@ -49,7 +48,6 @@ export class MockSocket extends net.Socket { args as WriteArgs ) this.options.write(chunk, encoding, callback) - return super.end.apply(this, args as any) } From cbb18ce0aa2cec15d75af9e01cdb6745a33ce26f Mon Sep 17 00:00:00 2001 From: Artem Zakharchenko Date: Mon, 2 Dec 2024 19:30:10 +0100 Subject: [PATCH 4/4] chore: remove unused test --- .../compliance/http-upgrade-works.test.ts | 85 ------------------- 1 file changed, 85 deletions(-) delete mode 100644 test/modules/http/compliance/http-upgrade-works.test.ts diff --git a/test/modules/http/compliance/http-upgrade-works.test.ts b/test/modules/http/compliance/http-upgrade-works.test.ts deleted file mode 100644 index cb55b429..00000000 --- a/test/modules/http/compliance/http-upgrade-works.test.ts +++ /dev/null @@ -1,85 +0,0 @@ -/** - * @see https://github.com/mswjs/interceptors/issues/682 - */ -// @vitest-environment node-with-websocket -import { vi, it, expect, beforeAll, afterEach, afterAll } from 'vitest' -import net from 'node:net' -import { Server } from 'socket.io' -import { io } from 'socket.io-client' -// import { ClientRequestInterceptor } from '../../../../src/interceptors/ClientRequest' - -// const interceptor = new ClientRequestInterceptor() -const server = new Server(51679) - -beforeAll(() => { - // interceptor.apply() -}) - -afterEach(() => { - // interceptor.removeAllListeners() -}) - -afterAll(async () => { - // interceptor.dispose() - await new Promise((resolve, reject) => { - server.disconnectSockets() - server.close((error) => { - if (error) reject(error) - resolve() - }) - }) -}) - -it('bypasses a WebSocket upgrade request', async () => { - net.Socket.prototype.emit = new Proxy(net.Socket.prototype.emit, { - apply(target, thisArg, args) { - console.log('EMIT!', args[0]) - - if (args[0] === 'agentRemove') { - debugger - } - - if (args[0] === 'data') { - // if (args[1].toString().includes('HTTP/1.1 101 Switching Protocols')) { - // debugger - // } - - console.log( - ` - UTF: ${args[1].toString()} - HEX: ${args[1].toString('hex')} - `, - '\n\n' - ) - } - - return Reflect.apply(target, thisArg, args) - }, - }) - - // net.Socket.prototype.write = new Proxy(net.Socket.prototype.write, { - // apply(target, thisArg, args) { - // console.trace(`WRITE! - // UTF: ${args[0].toString()} - // HEX: ${args[0].toString('hex')} - // `) - - // return Reflect.apply(target, thisArg, args) - // }, - // }) - - const client = io(`http://localhost:51679`, { - transports: ['websocket'], - reconnection: false, - retries: 0, - }) - - await vi.waitFor( - async () => { - expect(client.connected).toBe(true) - }, - { timeout: 100_000 } - ) - - console.log('----') -}, 100_000)