From 147757e3156d87ccbbe9da0f7276312ab2cddf35 Mon Sep 17 00:00:00 2001 From: Sam Adams Date: Fri, 3 Jan 2025 15:00:08 +0000 Subject: [PATCH 1/2] fix(clientrequestinterceptor): prevent stacked proxies when intercepting node http(s) requests It was possible to end up with the http.request (etc) proxies wrapping existing proxies, which can cause odd behaviour. --- src/interceptors/ClientRequest/index.ts | 127 +++++++++++------------- 1 file changed, 57 insertions(+), 70 deletions(-) diff --git a/src/interceptors/ClientRequest/index.ts b/src/interceptors/ClientRequest/index.ts index c5dac7a3..126fc4d4 100644 --- a/src/interceptors/ClientRequest/index.ts +++ b/src/interceptors/ClientRequest/index.ts @@ -16,6 +16,13 @@ import { recordRawFetchHeaders, restoreHeadersPrototype, } from './utils/recordRawHeaders' +import { types } from 'node:util' + +type MutableReqProxy = T & {original: T, updateCallbacks: (onRequest: MockHttpSocketRequestCallback, onResponse: MockHttpSocketResponseCallback) => void} + +function isMutableReqProxy(target: T): target is MutableReqProxy { + return types.isProxy(target) && 'updateCallbacks' in target && typeof target.updateCallbacks === 'function'; +} export class ClientRequestInterceptor extends Interceptor { static symbol = Symbol('client-request-interceptor') @@ -24,87 +31,73 @@ export class ClientRequestInterceptor extends Interceptor { super(ClientRequestInterceptor.symbol) } - protected setup(): void { - const { get: originalGet, request: originalRequest } = http - const { get: originalHttpsGet, request: originalHttpsRequest } = https - - const onRequest = this.onRequest.bind(this) - const onResponse = this.onResponse.bind(this) - - http.request = new Proxy(http.request, { + protected buildProxy(protocol: 'http:' | 'https:', target: T, onRequest: MockHttpSocketRequestCallback, onResponse: MockHttpSocketResponseCallback): MutableReqProxy { + return Object.assign(new Proxy(target, { apply: (target, thisArg, args: Parameters) => { const [url, options, callback] = normalizeClientRequestArgs( - 'http:', + protocol, args ) - const mockAgent = new MockAgent({ + const agentOpts = { customAgent: options.agent, onRequest, onResponse, - }) + } + const mockAgent = protocol === 'http:' ? new MockAgent(agentOpts) : new MockHttpsAgent(agentOpts); options.agent = mockAgent return Reflect.apply(target, thisArg, [url, options, callback]) }, - }) - - http.get = new Proxy(http.get, { - apply: (target, thisArg, args: Parameters) => { - const [url, options, callback] = normalizeClientRequestArgs( - 'http:', - args - ) - - const mockAgent = new MockAgent({ - customAgent: options.agent, - onRequest, - onResponse, - }) - options.agent = mockAgent - - return Reflect.apply(target, thisArg, [url, options, callback]) + }), { + updateCallbacks: (_onRequest: MockHttpSocketRequestCallback, _onResponse: MockHttpSocketResponseCallback) => { + onRequest = _onRequest; + onResponse = _onResponse; }, - }) - - // - // HTTPS. - // - - https.request = new Proxy(https.request, { - apply: (target, thisArg, args: Parameters) => { - const [url, options, callback] = normalizeClientRequestArgs( - 'https:', - args - ) + original: target, + }); + } + protected setup(): void { + const { get: httpGet, request: httpRequest } = http + const { get: httpsGet, request: httpsRequest } = https - const mockAgent = new MockHttpsAgent({ - customAgent: options.agent, - onRequest, - onResponse, - }) - options.agent = mockAgent + const onRequest = this.onRequest.bind(this) + const onResponse = this.onResponse.bind(this) - return Reflect.apply(target, thisArg, [url, options, callback]) - }, - }) + if (isMutableReqProxy(httpRequest)) { + httpRequest.updateCallbacks(onRequest, onResponse); + } else { + http.request = this.buildProxy('http:', httpRequest, onRequest, onResponse); + this.subscriptions.push(() => { + http.request = httpRequest + }) + } - https.get = new Proxy(https.get, { - apply: (target, thisArg, args: Parameters) => { - const [url, options, callback] = normalizeClientRequestArgs( - 'https:', - args - ) + if (isMutableReqProxy(httpGet)) { + httpGet.updateCallbacks(onRequest, onResponse); + } else { + http.get = this.buildProxy('http:', httpGet, onRequest, onResponse); + this.subscriptions.push(() => { + http.get = httpGet + }) + } - const mockAgent = new MockHttpsAgent({ - customAgent: options.agent, - onRequest, - onResponse, - }) - options.agent = mockAgent + if (isMutableReqProxy(httpsRequest)) { + httpsRequest.updateCallbacks(onRequest, onResponse); + } else { + https.request = this.buildProxy('https:', httpsRequest, onRequest, onResponse); + this.subscriptions.push(() => { + https.request = httpsRequest + }) + } - return Reflect.apply(target, thisArg, [url, options, callback]) - }, - }) + if (isMutableReqProxy(httpsGet)) { + httpsGet.updateCallbacks(onRequest, onResponse); + } else { + https.get = this.buildProxy('https:', httpsGet, onRequest, onResponse); + this.subscriptions.push(() => { + https.get = httpsGet + }) + } // Spy on `Header.prototype.set` and `Header.prototype.append` calls // and record the raw header names provided. This is to support @@ -112,12 +105,6 @@ export class ClientRequestInterceptor extends Interceptor { recordRawFetchHeaders() this.subscriptions.push(() => { - http.get = originalGet - http.request = originalRequest - - https.get = originalHttpsGet - https.request = originalHttpsRequest - restoreHeadersPrototype() }) } From 10819f5028432ca2952dc5bce4f2a8b3db0a066d Mon Sep 17 00:00:00 2001 From: Sam Adams Date: Fri, 3 Jan 2025 15:56:25 +0000 Subject: [PATCH 2/2] chore(clientrequest): add logging when proxy is overridden Add logging so we know when we update an existing proxy and fix some linting (remove semicolons) --- src/interceptors/ClientRequest/index.ts | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/interceptors/ClientRequest/index.ts b/src/interceptors/ClientRequest/index.ts index 126fc4d4..e01a197b 100644 --- a/src/interceptors/ClientRequest/index.ts +++ b/src/interceptors/ClientRequest/index.ts @@ -43,18 +43,18 @@ export class ClientRequestInterceptor extends Interceptor { onRequest, onResponse, } - const mockAgent = protocol === 'http:' ? new MockAgent(agentOpts) : new MockHttpsAgent(agentOpts); + const mockAgent = protocol === 'http:' ? new MockAgent(agentOpts) : new MockHttpsAgent(agentOpts) options.agent = mockAgent return Reflect.apply(target, thisArg, [url, options, callback]) }, }), { updateCallbacks: (_onRequest: MockHttpSocketRequestCallback, _onResponse: MockHttpSocketResponseCallback) => { - onRequest = _onRequest; - onResponse = _onResponse; + onRequest = _onRequest + onResponse = _onResponse }, original: target, - }); + }) } protected setup(): void { const { get: httpGet, request: httpRequest } = http @@ -64,36 +64,37 @@ export class ClientRequestInterceptor extends Interceptor { const onResponse = this.onResponse.bind(this) if (isMutableReqProxy(httpRequest)) { - httpRequest.updateCallbacks(onRequest, onResponse); + httpRequest.updateCallbacks(onRequest, onResponse) + this.logger.info('found existing proxy - updating for new request handlers') } else { - http.request = this.buildProxy('http:', httpRequest, onRequest, onResponse); + http.request = this.buildProxy('http:', httpRequest, onRequest, onResponse) this.subscriptions.push(() => { http.request = httpRequest }) } if (isMutableReqProxy(httpGet)) { - httpGet.updateCallbacks(onRequest, onResponse); + httpGet.updateCallbacks(onRequest, onResponse) } else { - http.get = this.buildProxy('http:', httpGet, onRequest, onResponse); + http.get = this.buildProxy('http:', httpGet, onRequest, onResponse) this.subscriptions.push(() => { http.get = httpGet }) } if (isMutableReqProxy(httpsRequest)) { - httpsRequest.updateCallbacks(onRequest, onResponse); + httpsRequest.updateCallbacks(onRequest, onResponse) } else { - https.request = this.buildProxy('https:', httpsRequest, onRequest, onResponse); + https.request = this.buildProxy('https:', httpsRequest, onRequest, onResponse) this.subscriptions.push(() => { https.request = httpsRequest }) } if (isMutableReqProxy(httpsGet)) { - httpsGet.updateCallbacks(onRequest, onResponse); + httpsGet.updateCallbacks(onRequest, onResponse) } else { - https.get = this.buildProxy('https:', httpsGet, onRequest, onResponse); + https.get = this.buildProxy('https:', httpsGet, onRequest, onResponse) this.subscriptions.push(() => { https.get = httpsGet })