diff --git a/CHANGELOG.md b/CHANGELOG.md index 97bf376..073c08a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,10 @@ # Change Log Notable changes will be documented here. +## [0.41.0] +- Surface network errors with proxies ([microsoft/vscode#298236](https://github.com/microsoft/vscode/issues/298236)) +- Support `testCertificates` request option for adding test certificates via request options instead of global state + ## [0.40.0] - Surface `responseHeaders`, `responseStatusCode`, and `responseStatusText` on patched WebSocket for all response types (success, non-upgrade, and proxy errors) ([microsoft/vscode-proxy-agent#94](https://github.com/microsoft/vscode-proxy-agent/pull/94)) diff --git a/package.json b/package.json index e61c529..3df900d 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@vscode/proxy-agent", - "version": "0.40.0", + "version": "0.41.0", "description": "NodeJS http(s) agent implementation for VS Code", "main": "out/index.js", "types": "out/index.d.ts", diff --git a/src/agent.ts b/src/agent.ts index 5deaae7..37da0ab 100644 --- a/src/agent.ts +++ b/src/agent.ts @@ -269,4 +269,5 @@ type PacProxyAgentOptions = fallbackToDirect?: boolean; originalAgent?: false | http.Agent; _vscodeTestReplaceCaCerts?: boolean; + testCertificates?: (string | Buffer)[]; } diff --git a/src/index.ts b/src/index.ts index 3a5405a..bec5320 100644 --- a/src/index.ts +++ b/src/index.ts @@ -162,7 +162,7 @@ export function createProxyResolver(params: ProxyAgentParams) { results = []; } - function resolveProxyWithRequest(flags: { useProxySettings: boolean, addCertificatesV1: boolean }, req: http.ClientRequest, opts: http.RequestOptions, url: string, callback: (proxy?: string) => void) { + function resolveProxyWithRequest(flags: { useProxySettings: boolean, addCertificatesV1: boolean, testCertificates?: (string | Buffer)[] }, req: http.ClientRequest, opts: http.RequestOptions, url: string, callback: (proxy?: string) => void) { if (!timeout) { timeout = setTimeout(logEvent, 10 * 60 * 1000); } @@ -175,7 +175,7 @@ export function createProxyResolver(params: ProxyAgentParams) { return; } useProxySettings(url, req, stackText, callback); - }); + }, flags.testCertificates); } function useProxySettings(url: string, req: http.ClientRequest | undefined, stackText: string, callback: (proxy?: string) => void) { @@ -364,7 +364,7 @@ function noProxyFromConfig(noProxy: string[]) { export type ProxySupportSetting = 'override' | 'fallback' | 'on' | 'off'; -export type ResolveProxyWithRequest = (flags: { useProxySettings: boolean, addCertificatesV1: boolean }, req: http.ClientRequest, opts: http.RequestOptions, url: string, callback: (proxy?: string) => void) => void; +export type ResolveProxyWithRequest = (flags: { useProxySettings: boolean, addCertificatesV1: boolean, testCertificates?: (string | Buffer)[] }, req: http.ClientRequest, opts: http.RequestOptions, url: string, callback: (proxy?: string) => void) => void; export function createHttpPatch(params: ProxyAgentParams, originals: typeof http | typeof https, resolveProxy: ResolveProxyWithRequest) { return { @@ -419,7 +419,7 @@ export function createHttpPatch(params: ProxyAgentParams, originals: typeof http } else { options = { ...options }; } - const resolveP = (req: http.ClientRequest, opts: http.RequestOptions, url: string): Promise => new Promise(resolve => resolveProxy({ useProxySettings, addCertificatesV1 }, req, opts, url, resolve)); + const resolveP = (req: http.ClientRequest, opts: http.RequestOptions, url: string): Promise => new Promise(resolve => resolveProxy({ useProxySettings, addCertificatesV1, testCertificates: (options as SecureContextOptionsPatch).testCertificates }, req, opts, url, resolve)); const host = options.hostname || options.host; const isLocalhost = !host || host === 'localhost' || host === '127.0.0.1'; // Avoiding https://github.com/microsoft/vscode/issues/120354 const agent = createPacProxyAgent(resolveP, { @@ -427,7 +427,7 @@ export function createHttpPatch(params: ProxyAgentParams, originals: typeof http lookupProxyAuthorization: params.lookupProxyAuthorization, // keepAlive: ((originalAgent || originals.globalAgent) as { keepAlive?: boolean }).keepAlive, // Skipping due to https://github.com/microsoft/vscode/issues/228872. _vscodeTestReplaceCaCerts: (options as SecureContextOptionsPatch)._vscodeTestReplaceCaCerts, - }, opts => new Promise(resolve => addCertificatesToOptionsV1(params, params.addCertificatesV1(), opts, resolve))); + }, opts => new Promise(resolve => addCertificatesToOptionsV1(params, params.addCertificatesV1(), opts, resolve, (options as SecureContextOptionsPatch).testCertificates))); agent.protocol = isHttps ? 'https:' : 'http:'; options.agent = agent if (isHttps) { @@ -445,6 +445,7 @@ export function createHttpPatch(params: ProxyAgentParams, originals: typeof http export interface SecureContextOptionsPatch { _vscodeAdditionalCaCerts?: (string | Buffer)[]; _vscodeTestReplaceCaCerts?: boolean; + testCertificates?: (string | Buffer)[]; } export function createNetPatch(params: ProxyAgentParams, originals: typeof net) { @@ -501,6 +502,7 @@ function patchTlsConnect(params: ProxyAgentParams, original: typeof tls.connect) if (!params.addCertificatesV2() || options?.ca) { return original.apply(null, arguments as any); } + const testCerts = (options as SecureContextOptionsPatch | undefined)?.testCertificates; let secureConnectListener: (() => void) | undefined = args.find(arg => typeof arg === 'function'); if (!options) { options = {}; @@ -532,12 +534,22 @@ function patchTlsConnect(params: ProxyAgentParams, original: typeof tls.connect) for (const cert of _certs.get(!!params.loadSystemCertificatesFromNode())?.result || []) { options!.secureContext!.context.addCACert(cert); } + if (testCerts) { + for (const cert of testCerts) { + options!.secureContext!.context.addCACert(cert); + } + } }); } else { params.log.trace('ProxyResolver#tls.connect existing socket already connected - adding certs'); for (const cert of certificates) { options!.secureContext!.context.addCACert(cert); } + if (testCerts) { + for (const cert of testCerts) { + options!.secureContext!.context.addCACert(cert); + } + } } } else { if (!options.secureContext) { @@ -552,6 +564,11 @@ function patchTlsConnect(params: ProxyAgentParams, original: typeof tls.connect) for (const cert of caCertificates) { options!.secureContext!.context.addCACert(cert); } + if (testCerts) { + for (const cert of testCerts) { + options!.secureContext!.context.addCACert(cert); + } + } if (options?.timeout) { socket.setTimeout(options.timeout); socket.once('timeout', () => { @@ -825,6 +842,11 @@ export function createWebSocketPatch(params: ProxyAgentParams, originalWebSocket enumerable: true, configurable: true, }); + Object.defineProperty(ws, 'networkError', { + get() { return proxyDispatcher.networkError; }, + enumerable: true, + configurable: true, + }); return ws; } as unknown as typeof globalThis.WebSocket; @@ -843,6 +865,7 @@ class ProxyDispatcher extends undici.Dispatcher { responseHeaders?: Record; responseStatusCode?: number; responseStatusText?: string; + networkError?: Error; constructor( private params: ProxyAgentParams, @@ -856,31 +879,53 @@ class ProxyDispatcher extends undici.Dispatcher { dispatch(opts: undici.Dispatcher.DispatchOptions, handler: undici.Dispatcher.DispatchHandler): boolean { const self = this; + function captureError(err: Error): void { + const connectHeaders = (err as any).connectResponseHeaders as IncomingHttpHeaders | undefined; + if (connectHeaders) { + self.responseHeaders = convertHeaders(connectHeaders); + } + if ((err as any).connectResponseStatusCode !== undefined) { + self.responseStatusCode = (err as any).connectResponseStatusCode; + } + if (!self.networkError) { + self.networkError = err; + } + } const wrappedHandler: undici.Dispatcher.DispatchHandler = { ...handler, - // Deprecated API used by undici's internal WebSocket/fetch flow. + // Current API. + onResponseStart(controller: undici.Dispatcher.DispatchController, statusCode: number, headers: IncomingHttpHeaders, statusMessage?: string): void { + self.responseStatusCode = statusCode; + self.responseStatusText = statusMessage; + self.responseHeaders = convertHeaders(headers); + return handler.onResponseStart?.(controller, statusCode, headers, statusMessage); + }, + onResponseError(controller: undici.Dispatcher.DispatchController, err: Error): void { + captureError(err); + return handler.onResponseError?.(controller, err); + }, + onRequestUpgrade(controller: undici.Dispatcher.DispatchController, statusCode: number, headers: IncomingHttpHeaders, socket: stream.Duplex): void { + self.responseStatusCode = statusCode; + self.responseHeaders = convertHeaders(headers); + return handler.onRequestUpgrade?.(controller, statusCode, headers, socket); + }, + // Deprecated API still used by undici's internal core. onHeaders(statusCode: number, rawHeaders: Buffer[], resume: () => void, statusText: string): boolean { self.responseStatusCode = statusCode; self.responseStatusText = statusText; self.responseHeaders = parseRawHeaders(rawHeaders); - return handler.onHeaders?.(statusCode, rawHeaders, resume, statusText) ?? true; + return (handler as any).onHeaders?.(statusCode, rawHeaders, resume, statusText) ?? true; }, onError(err: Error): void { - const connectHeaders = (err as any).connectResponseHeaders as IncomingHttpHeaders | undefined; - if (connectHeaders) { - self.responseHeaders = convertHeaders(connectHeaders); - } - if ((err as any).connectResponseStatusCode !== undefined) { - self.responseStatusCode = (err as any).connectResponseStatusCode; - } - return handler.onError?.(err); + captureError(err); + return (handler as any).onError?.(err); }, onUpgrade(statusCode: number, rawHeaders: Buffer[] | string[] | null, socket: stream.Duplex): void { self.responseStatusCode = statusCode; if (rawHeaders) { self.responseHeaders = parseRawHeaders(rawHeaders); } - return handler.onUpgrade?.(statusCode, rawHeaders, socket); + return (handler as any).onUpgrade?.(statusCode, rawHeaders, socket); }, }; const url = opts.origin?.toString(); @@ -908,11 +953,7 @@ class ProxyDispatcher extends undici.Dispatcher { } dispatcher.dispatch(opts, wrappedHandler); } catch (err: any) { - if (typeof (handler as any).onResponseError === 'function') { - (handler as any).onResponseError(null, err); - } else if (typeof (handler as any).onError === 'function') { - (handler as any).onError(err); - } + handler.onResponseError?.(null as any, err); } })(); return true; @@ -1068,14 +1109,15 @@ function getAgentOptions(requestInit: RequestInit | undefined) { return { dispatcher, allowH2, requestCA, proxyCA, socketPath }; } -function addCertificatesToOptionsV1(params: ProxyAgentParams, addCertificatesV1: boolean, opts: http.RequestOptions | tls.ConnectionOptions, callback: () => void) { +function addCertificatesToOptionsV1(params: ProxyAgentParams, addCertificatesV1: boolean, opts: http.RequestOptions | tls.ConnectionOptions, callback: () => void, testCertificates?: (string | Buffer)[]) { if (addCertificatesV1) { getOrLoadAdditionalCertificates(params) .then(caCertificates => { + const allCerts: (string | Buffer)[] = testCertificates?.length ? [...caCertificates, ...testCertificates] : caCertificates; if ((opts as SecureContextOptionsPatch)._vscodeTestReplaceCaCerts) { - (opts as https.RequestOptions).ca = caCertificates; + (opts as https.RequestOptions).ca = allCerts; } else { - (opts as SecureContextOptionsPatch)._vscodeAdditionalCaCerts = caCertificates; + (opts as SecureContextOptionsPatch)._vscodeAdditionalCaCerts = allCerts; } callback(); }) @@ -1242,7 +1284,7 @@ export function toLogString(args: any[]) { const t = typeof value; if (t === 'object') { if (key) { - if ((key === 'ca' || key === '_vscodeAdditionalCaCerts') && Array.isArray(value)) { + if ((key === 'ca' || key === '_vscodeAdditionalCaCerts' || key === 'testCertificates') && Array.isArray(value)) { return `[${value.length} certs]`; } if (key === 'ca' && (typeof value === 'string' || Buffer.isBuffer(value))) { @@ -1267,9 +1309,4 @@ export function toLogString(args: any[]) { })).join(', ')}]`; } -/** - * Certificates for testing. These are not automatically used, but can be added in - * ProxyAgentParams#loadAdditionalCertificates(). This just provides a shared array - * between production code and tests. - */ -export const testCertificates: string[] = []; + diff --git a/tests/test-client/src/direct.test.ts b/tests/test-client/src/direct.test.ts index a2375a4..dc0065a 100644 --- a/tests/test-client/src/direct.test.ts +++ b/tests/test-client/src/direct.test.ts @@ -194,6 +194,28 @@ describe('Direct client', function () { _vscodeTestReplaceCaCerts: true, }); }); + it('should use testCertificates request option', async function () { + const noAdditionalCertsParams: vpa.ProxyAgentParams = { + ...directProxyAgentParamsV1, + loadAdditionalCertificates: async () => [], + }; + vpa.resetCaches(); + try { + const { resolveProxyWithRequest: resolveProxy } = vpa.createProxyResolver(noAdditionalCertsParams); + const patchedHttps: typeof https = { + ...https, + ...vpa.createHttpPatch(noAdditionalCertsParams, https, resolveProxy), + } as any; + await testRequest(patchedHttps, { + hostname: 'test-https-server', + path: '/test-path', + _vscodeTestReplaceCaCerts: true, + testCertificates: ca, + }); + } finally { + vpa.resetCaches(); + } + }); it('should use ca request option', async function () { const { resolveProxyWithRequest: resolveProxy } = vpa.createProxyResolver(directProxyAgentParamsV1); const patchedHttps: typeof https = { diff --git a/tests/test-client/src/websocket.test.ts b/tests/test-client/src/websocket.test.ts index 55c09ed..51c86a3 100644 --- a/tests/test-client/src/websocket.test.ts +++ b/tests/test-client/src/websocket.test.ts @@ -164,4 +164,48 @@ describe('WebSocket proxied', function () { }; }); }); + + it('should capture networkError on HTTP proxy connection refused', async function () { + const params: vpa.ProxyAgentParams = { + ...directProxyAgentParams, + resolveProxy: async () => 'PROXY 127.0.0.1:1', + }; + const { resolveProxyURL } = vpa.createProxyResolver(params); + const PatchedWebSocket = vpa.createWebSocketPatch(params, WebSocket as any, resolveProxyURL); + const ws = new PatchedWebSocket('wss://test-wss-server'); + await new Promise((resolve, reject) => { + ws.onopen = () => { + ws.close(); + reject(new Error('WebSocket should not have connected')); + }; + ws.onerror = () => { + const networkError: Error | undefined = (ws as any).networkError; + assert.ok(networkError, 'networkError should be defined'); + assert.ok(networkError instanceof Error, 'networkError should be an Error'); + resolve(); + }; + }); + }); + + it('should capture networkError on HTTPS proxy connection refused', async function () { + const params: vpa.ProxyAgentParams = { + ...directProxyAgentParams, + resolveProxy: async () => 'HTTPS 127.0.0.1:1', + }; + const { resolveProxyURL } = vpa.createProxyResolver(params); + const PatchedWebSocket = vpa.createWebSocketPatch(params, WebSocket as any, resolveProxyURL); + const ws = new PatchedWebSocket('wss://test-wss-server'); + await new Promise((resolve, reject) => { + ws.onopen = () => { + ws.close(); + reject(new Error('WebSocket should not have connected')); + }; + ws.onerror = () => { + const networkError: Error | undefined = (ws as any).networkError; + assert.ok(networkError, 'networkError should be defined'); + assert.ok(networkError instanceof Error, 'networkError should be an Error'); + resolve(); + }; + }); + }); });