diff --git a/packages/playwright-core/src/server/fetch.ts b/packages/playwright-core/src/server/fetch.ts index b154ce63079fb..4905f22f8369a 100644 --- a/packages/playwright-core/src/server/fetch.ts +++ b/packages/playwright-core/src/server/fetch.ts @@ -87,7 +87,7 @@ export type APIRequestFinishedEvent = { securityDetails?: har.SecurityDetails; }; -type SendRequestOptions = https.RequestOptions & { +type SendRequestOptions = Omit & { maxRedirects: number, headers: HeadersObject, __testHookLookup?: (hostname: string) => LookupAddress[] @@ -168,6 +168,13 @@ export abstract class APIRequestContext extends SdkObject { this.emit(APIRequestContext.Events.Dispose); } + private _proxyAgentForUrl(url: URL): http.Agent | undefined { + const proxy = this._defaultOptions().proxy; + // We skip 'per-context' in order to not break existing users. 'per-context' was previously used to + // workaround an upstream Chromium bug. Can be removed in the future. + return createProxyAgent(proxy?.server === 'per-context' ? undefined : proxy, url); + } + private _ensureAgent(protocol: string): http.Agent { let agent = this._agentForProtocol.get(protocol); if (!agent) { @@ -221,22 +228,13 @@ export abstract class APIRequestContext extends SdkObject { setBasicAuthorizationHeader(headers, credentials); const method = params.method?.toUpperCase() || 'GET'; - const proxy = defaults.proxy; - let agent; - // We skip 'per-context' in order to not break existing users. 'per-context' was previously used to - // workaround an upstream Chromium bug. Can be removed in the future. - if (proxy?.server !== 'per-context') - agent = createProxyAgent(proxy, requestUrl); - let maxRedirects = params.maxRedirects ?? (defaults.maxRedirects ?? 20); maxRedirects = maxRedirects === 0 ? -1 : maxRedirects; const options: SendRequestOptions = { method, headers, - agent, maxRedirects, - ...getMatchingTLSOptionsForOrigin(this._defaultOptions().clientCertificates, requestUrl.origin), __testHookLookup: (params as any).__testHookLookup, }; // rejectUnauthorized = undefined is treated as true in Node.js 12. @@ -362,11 +360,15 @@ export abstract class APIRequestContext extends SdkObject { const resultPromise = new Promise((fulfill, reject) => { const requestConstructor: ((url: URL, options: http.RequestOptions, callback?: (res: http.IncomingMessage) => void) => http.ClientRequest) = (url.protocol === 'https:' ? https : http).request; - // Without an explicit proxy agent, use this context's own agent, which has - // keep-alive enabled and connects with Happy Eyeballs (autoSelectFamily). - // Resolved per request so that a cross-protocol redirect picks the right agent. - const requestOptions = { ...options, ...happyEyeballsOptions }; - requestOptions.agent = options.agent ?? this._ensureAgent(url.protocol); + // Proxy bypass rules and client certificates are resolved per hop, so that + // a redirect target picks its own agent and TLS options. Without a proxy agent, + // use this context's own agent (keep-alive, Happy Eyeballs). + const requestOptions: https.RequestOptions = { + ...options, + ...happyEyeballsOptions, + ...getMatchingTLSOptionsForOrigin(this._defaultOptions().clientCertificates, url.origin), + agent: this._proxyAgentForUrl(url) ?? this._ensureAgent(url.protocol), + }; if (options.__testHookLookup) requestOptions.lookup = lookupWithTestHook(options.__testHookLookup); @@ -484,7 +486,6 @@ export abstract class APIRequestContext extends SdkObject { const redirectOptions: SendRequestOptions = { method, headers, - agent: options.agent, maxRedirects: options.maxRedirects - 1, __testHookLookup: options.__testHookLookup, }; @@ -512,11 +513,6 @@ export abstract class APIRequestContext extends SdkObject { if (locationURL.origin !== url.origin) removeHeader(headers, 'authorization'); - // Client certificates are origin-scoped — pick them based on the redirect - // target, not the original URL. - Object.assign(redirectOptions, - getMatchingTLSOptionsForOrigin(this._defaultOptions().clientCertificates, locationURL.origin)); - notifyRequestFinished(); fulfill(this._sendRequest(progress, log, locationURL, redirectOptions, postData)); request.destroy(); diff --git a/tests/library/fetch-proxy.spec.ts b/tests/library/fetch-proxy.spec.ts index 5d35bef85b063..a8ec03a7f6247 100644 --- a/tests/library/fetch-proxy.spec.ts +++ b/tests/library/fetch-proxy.spec.ts @@ -171,3 +171,32 @@ it('should send correct ALPN protocol to HTTPS proxy', { annotation: { type: 'is proxy.close(); await request.dispose(); }); + +it('should apply proxy.bypass to redirect targets', { annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright/issues/42815' } }, async ({ contextFactory, server, proxyServer }) => { + const crossProcessHost = new URL(server.CROSS_PROCESS_PREFIX).host; + it.skip(crossProcessHost === server.HOST, 'Needs two different host names for the same server'); + + proxyServer.forwardTo(server.PORT, { allowConnectRequests: true }); + server.setRedirect('/redirect-to-cross-process', server.CROSS_PROCESS_PREFIX + '/simple.json'); + server.setRedirect('/redirect-to-same-origin', server.PREFIX + '/simple.json'); + const context = await contextFactory({ + proxy: { server: `localhost:${proxyServer.PORT}`, bypass: server.HOSTNAME } + }); + + { + // Bypassed first hop redirects to a host that must go through the proxy. + const response = await context.request.get(server.PREFIX + '/redirect-to-cross-process'); + expect(response.url()).toBe(server.CROSS_PROCESS_PREFIX + '/simple.json'); + expect(await response.json()).toEqual({ foo: 'bar' }); + expect(proxyServer.connectHosts).toEqual([crossProcessHost]); + proxyServer.connectHosts = []; + } + + { + // Proxied first hop redirects to a bypassed host. + const response = await context.request.get(server.CROSS_PROCESS_PREFIX + '/redirect-to-same-origin'); + expect(response.url()).toBe(server.PREFIX + '/simple.json'); + expect(await response.json()).toEqual({ foo: 'bar' }); + expect(proxyServer.connectHosts).toEqual([crossProcessHost]); + } +});