From 58155de0647a9edffc48f552e4b1c060a96bcce6 Mon Sep 17 00:00:00 2001 From: Michael Solomon Date: Sat, 6 Jul 2024 03:10:04 +0300 Subject: [PATCH 1/5] before cleanup --- .../ClientRequest/MockHttpSocket.ts | 62 ++++++------------- .../http/response/http-empty-response.test.ts | 14 +++++ 2 files changed, 32 insertions(+), 44 deletions(-) diff --git a/src/interceptors/ClientRequest/MockHttpSocket.ts b/src/interceptors/ClientRequest/MockHttpSocket.ts index 5f7314c8..b440615b 100644 --- a/src/interceptors/ClientRequest/MockHttpSocket.ts +++ b/src/interceptors/ClientRequest/MockHttpSocket.ts @@ -4,8 +4,8 @@ import { type RequestHeadersCompleteCallback, type ResponseHeadersCompleteCallback, } from '_http_common' -import { STATUS_CODES } from 'node:http' -import { Readable } from 'node:stream' +import { IncomingMessage, ServerResponse } from 'node:http' +import { Readable, Writable } from 'node:stream' import { invariant } from 'outvariant' import { INTERNAL_REQUEST_ID_HEADER_NAME } from '../../Interceptor' import { MockSocket } from '../Socket/MockSocket' @@ -272,36 +272,22 @@ export class MockHttpSocket extends MockSocket { // if it hasn't been flushed already (e.g. someone started reading request stream). this.flushWriteBuffer() - const httpHeaders: Array = [] - - httpHeaders.push( - Buffer.from( - `HTTP/1.1 ${response.status} ${ - response.statusText || STATUS_CODES[response.status] - }\r\n` - ) - ) + const w = new Writable({ + write: (chunk, encoding, callback) => { + this.push(chunk, encoding); + callback() + }, + }) + const fakeResponse = new ServerResponse(new IncomingMessage(new net.Socket())) + // @ts-expect-error Node has test for this: https://github.com/nodejs/node/blob/10099bb3f7fd97bb9dd9667188426866b3098e07/test/parallel/test-http-server-response-standalone.js#L32 + fakeResponse.assignSocket(w) + fakeResponse.statusCode = response.status; + fakeResponse.statusMessage = response.statusText; // Get the raw headers stored behind the symbol to preserve name casing. const headers = getRawFetchHeaders(response.headers) || response.headers for (const [name, value] of headers) { - httpHeaders.push(Buffer.from(`${name}: ${value}\r\n`)) - } - - // An empty line separating headers from the body. - httpHeaders.push(Buffer.from('\r\n')) - - const flushHeaders = (value?: Uint8Array) => { - if (httpHeaders.length === 0) { - return - } - - if (typeof value !== 'undefined') { - httpHeaders.push(Buffer.from(value)) - } - - this.push(Buffer.concat(httpHeaders)) - httpHeaders.length = 0 + fakeResponse.setHeader(name, value); } if (response.body) { @@ -312,19 +298,10 @@ export class MockHttpSocket extends MockSocket { const { done, value } = await reader.read() if (done) { + fakeResponse.end() break } - - // Flush the headers upon the first chunk in the stream. - // This ensures the consumer will start receiving the response - // as it streams in (subsequent chunks are pushed). - if (httpHeaders.length > 0) { - flushHeaders(value) - continue - } - - // Subsequent body chukns are push to the stream. - this.push(value) + fakeResponse.write(value); } } catch (error) { // Coerce response stream errors to 500 responses. @@ -334,13 +311,10 @@ export class MockHttpSocket extends MockSocket { return } + } else { + fakeResponse.end() } - // If the headers were not flushed up to this point, - // this means the response either had no body or had - // an empty body stream. Flush the headers. - flushHeaders() - // Close the socket if the connection wasn't marked as keep-alive. if (!this.shouldKeepAlive) { this.emit('readable') diff --git a/test/modules/http/response/http-empty-response.test.ts b/test/modules/http/response/http-empty-response.test.ts index 3069b6a0..bc6c04fe 100644 --- a/test/modules/http/response/http-empty-response.test.ts +++ b/test/modules/http/response/http-empty-response.test.ts @@ -29,3 +29,17 @@ it('supports responding with an empty mocked response', async () => { expect(res.statusCode).toBe(200) expect(await text()).toBe('') }) + +it('support transfer-encoding: chunked', async () => { + interceptor.once('request', ({ request }) => { + request.respondWith(new Response('OK', { + headers: { 'Transfer-Encoding': 'chunked' } + })) + }) + + const request = http.get('http://localhost') + const { res, text } = await waitForClientRequest(request) + + expect(res.statusCode).toBe(200) + expect(await text()).toBe('OK') +}) From dc400ceb15e98e8e79198265c312fafad1960e7e Mon Sep 17 00:00:00 2001 From: Artem Zakharchenko Date: Sat, 6 Jul 2024 12:42:02 +0200 Subject: [PATCH 2/5] chore: create a new test suite --- .../http/response/http-empty-response.test.ts | 14 ------- .../http-response-transfer-encoding.test.ts | 39 +++++++++++++++++++ 2 files changed, 39 insertions(+), 14 deletions(-) create mode 100644 test/modules/http/response/http-response-transfer-encoding.test.ts diff --git a/test/modules/http/response/http-empty-response.test.ts b/test/modules/http/response/http-empty-response.test.ts index bc6c04fe..3069b6a0 100644 --- a/test/modules/http/response/http-empty-response.test.ts +++ b/test/modules/http/response/http-empty-response.test.ts @@ -29,17 +29,3 @@ it('supports responding with an empty mocked response', async () => { expect(res.statusCode).toBe(200) expect(await text()).toBe('') }) - -it('support transfer-encoding: chunked', async () => { - interceptor.once('request', ({ request }) => { - request.respondWith(new Response('OK', { - headers: { 'Transfer-Encoding': 'chunked' } - })) - }) - - const request = http.get('http://localhost') - const { res, text } = await waitForClientRequest(request) - - expect(res.statusCode).toBe(200) - expect(await text()).toBe('OK') -}) diff --git a/test/modules/http/response/http-response-transfer-encoding.test.ts b/test/modules/http/response/http-response-transfer-encoding.test.ts new file mode 100644 index 00000000..75fdc75f --- /dev/null +++ b/test/modules/http/response/http-response-transfer-encoding.test.ts @@ -0,0 +1,39 @@ +/** + * @vitest-environment node + */ +import { it, expect, beforeAll, afterEach, afterAll } from 'vitest' +import http from 'node:http' +import { waitForClientRequest } from '../../../helpers' +import { ClientRequestInterceptor } from '../../../../src/interceptors/ClientRequest' + +const interceptor = new ClientRequestInterceptor() + +beforeAll(() => { + interceptor.apply() +}) + +afterEach(() => { + interceptor.removeAllListeners() +}) + +afterAll(() => { + interceptor.dispose() +}) + +it('responds with a mocked "transfer-encoding: chunked" response', async () => { + interceptor.on('request', ({ request }) => { + request.respondWith( + new Response('mock', { + headers: { 'Transfer-Encoding': 'chunked' }, + }) + ) + }) + + const request = http.get('http://localhost') + const { res, text } = await waitForClientRequest(request) + + expect(res.statusCode).toBe(200) + expect(res.headers).toHaveProperty('transfer-encoding', 'chunked') + expect(res.rawHeaders).toContain('Transfer-Encoding') + expect(await text()).toBe('mock') +}) From fe8dbe65632c20d804fa3f28fac2c2db9e3eb915 Mon Sep 17 00:00:00 2001 From: Artem Zakharchenko Date: Sat, 6 Jul 2024 12:42:21 +0200 Subject: [PATCH 3/5] fix(MockHttpSocket): use MockSocket vs raw Writable --- .../ClientRequest/MockHttpSocket.ts | 31 ++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/src/interceptors/ClientRequest/MockHttpSocket.ts b/src/interceptors/ClientRequest/MockHttpSocket.ts index b440615b..fa212e87 100644 --- a/src/interceptors/ClientRequest/MockHttpSocket.ts +++ b/src/interceptors/ClientRequest/MockHttpSocket.ts @@ -5,7 +5,7 @@ import { type ResponseHeadersCompleteCallback, } from '_http_common' import { IncomingMessage, ServerResponse } from 'node:http' -import { Readable, Writable } from 'node:stream' +import { Readable } from 'node:stream' import { invariant } from 'outvariant' import { INTERNAL_REQUEST_ID_HEADER_NAME } from '../../Interceptor' import { MockSocket } from '../Socket/MockSocket' @@ -272,22 +272,25 @@ export class MockHttpSocket extends MockSocket { // if it hasn't been flushed already (e.g. someone started reading request stream). this.flushWriteBuffer() - const w = new Writable({ - write: (chunk, encoding, callback) => { - this.push(chunk, encoding); - callback() - }, - }) - const fakeResponse = new ServerResponse(new IncomingMessage(new net.Socket())) - // @ts-expect-error Node has test for this: https://github.com/nodejs/node/blob/10099bb3f7fd97bb9dd9667188426866b3098e07/test/parallel/test-http-server-response-standalone.js#L32 - fakeResponse.assignSocket(w) - fakeResponse.statusCode = response.status; - fakeResponse.statusMessage = response.statusText; + const fakeResponse = new ServerResponse(new IncomingMessage(this)) + + // Node has test for this: https://github.com/nodejs/node/blob/10099bb3f7fd97bb9dd9667188426866b3098e07/test/parallel/test-http-server-response-standalone.js#L32 + fakeResponse.assignSocket( + new MockSocket({ + write: (chunk, encoding, callback) => { + this.push(chunk, encoding) + callback?.() + }, + read() {}, + }) + ) + fakeResponse.statusCode = response.status + fakeResponse.statusMessage = response.statusText // Get the raw headers stored behind the symbol to preserve name casing. const headers = getRawFetchHeaders(response.headers) || response.headers for (const [name, value] of headers) { - fakeResponse.setHeader(name, value); + fakeResponse.setHeader(name, value) } if (response.body) { @@ -301,7 +304,7 @@ export class MockHttpSocket extends MockSocket { fakeResponse.end() break } - fakeResponse.write(value); + fakeResponse.write(value) } } catch (error) { // Coerce response stream errors to 500 responses. From 11bd42de069ac6f83dd2123c5eba5a38554e31d8 Mon Sep 17 00:00:00 2001 From: Artem Zakharchenko Date: Sat, 6 Jul 2024 12:47:33 +0200 Subject: [PATCH 4/5] fix(MockHttpSocket): improve naming --- .../ClientRequest/MockHttpSocket.ts | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/src/interceptors/ClientRequest/MockHttpSocket.ts b/src/interceptors/ClientRequest/MockHttpSocket.ts index fa212e87..7e823608 100644 --- a/src/interceptors/ClientRequest/MockHttpSocket.ts +++ b/src/interceptors/ClientRequest/MockHttpSocket.ts @@ -272,10 +272,19 @@ export class MockHttpSocket extends MockSocket { // if it hasn't been flushed already (e.g. someone started reading request stream). this.flushWriteBuffer() - const fakeResponse = new ServerResponse(new IncomingMessage(this)) + // Create a `ServerResponse` instance to delegate HTTP message parsing, + // Transfer-Encoding, and other things to Node.js internals. + const serverResponse = new ServerResponse(new IncomingMessage(this)) - // Node has test for this: https://github.com/nodejs/node/blob/10099bb3f7fd97bb9dd9667188426866b3098e07/test/parallel/test-http-server-response-standalone.js#L32 - fakeResponse.assignSocket( + /** + * Assign a mock socket instance to the server response to + * spy on the response chunk writes. Push the transformed response chunks + * to this `MockHttpSocket` instance to trigger the "data" event. + * @note Providing the same `MockSocket` instance when creating `ServerResponse` + * does not have the same effect. + * @see https://github.com/nodejs/node/blob/10099bb3f7fd97bb9dd9667188426866b3098e07/test/parallel/test-http-server-response-standalone.js#L32 + */ + serverResponse.assignSocket( new MockSocket({ write: (chunk, encoding, callback) => { this.push(chunk, encoding) @@ -284,13 +293,13 @@ export class MockHttpSocket extends MockSocket { read() {}, }) ) - fakeResponse.statusCode = response.status - fakeResponse.statusMessage = response.statusText + serverResponse.statusCode = response.status + serverResponse.statusMessage = response.statusText // Get the raw headers stored behind the symbol to preserve name casing. const headers = getRawFetchHeaders(response.headers) || response.headers for (const [name, value] of headers) { - fakeResponse.setHeader(name, value) + serverResponse.setHeader(name, value) } if (response.body) { @@ -301,21 +310,19 @@ export class MockHttpSocket extends MockSocket { const { done, value } = await reader.read() if (done) { - fakeResponse.end() + serverResponse.end() break } - fakeResponse.write(value) + + serverResponse.write(value) } } catch (error) { // Coerce response stream errors to 500 responses. - // Don't flush the original response headers because - // unhandled errors translate to 500 error responses forcefully. this.respondWith(createServerErrorResponse(error)) - return } } else { - fakeResponse.end() + serverResponse.end() } // Close the socket if the connection wasn't marked as keep-alive. From ffffde796966bb1dc94d6399fc4bb7114efbd171 Mon Sep 17 00:00:00 2001 From: Artem Zakharchenko Date: Sat, 6 Jul 2024 13:05:15 +0200 Subject: [PATCH 5/5] fix(MockHttpSocket): remove server "connection" and "date" headers --- src/interceptors/ClientRequest/MockHttpSocket.ts | 12 ++++++++++++ .../http/response/http-empty-response.test.ts | 4 ++++ 2 files changed, 16 insertions(+) diff --git a/src/interceptors/ClientRequest/MockHttpSocket.ts b/src/interceptors/ClientRequest/MockHttpSocket.ts index 7e823608..2e98cfff 100644 --- a/src/interceptors/ClientRequest/MockHttpSocket.ts +++ b/src/interceptors/ClientRequest/MockHttpSocket.ts @@ -296,6 +296,18 @@ export class MockHttpSocket extends MockSocket { serverResponse.statusCode = response.status serverResponse.statusMessage = response.statusText + /** + * @note Remove the `Connection` and `Date` response headers + * injected by `ServerResponse` by default. Those are required + * from the server but the interceptor is NOT technically a server. + * It's confusing to add response headers that the developer didn't + * specify themselves. They can always add these if they wish. + * @see https://www.rfc-editor.org/rfc/rfc9110#field.date + * @see https://www.rfc-editor.org/rfc/rfc9110#field.connection + */ + serverResponse.removeHeader('connection') + serverResponse.removeHeader('date') + // Get the raw headers stored behind the symbol to preserve name casing. const headers = getRawFetchHeaders(response.headers) || response.headers for (const [name, value] of headers) { diff --git a/test/modules/http/response/http-empty-response.test.ts b/test/modules/http/response/http-empty-response.test.ts index 3069b6a0..92b3a701 100644 --- a/test/modules/http/response/http-empty-response.test.ts +++ b/test/modules/http/response/http-empty-response.test.ts @@ -27,5 +27,9 @@ it('supports responding with an empty mocked response', async () => { const { res, text } = await waitForClientRequest(request) expect(res.statusCode).toBe(200) + // Must not set any response headers that were not + // explicitly provided in the mocked response. + expect(res.headers).toEqual({}) + expect(res.rawHeaders).toEqual([]) expect(await text()).toBe('') })