Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 17 additions & 21 deletions packages/playwright-core/src/server/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ export type APIRequestFinishedEvent = {
securityDetails?: har.SecurityDetails;
};

type SendRequestOptions = https.RequestOptions & {
type SendRequestOptions = Omit<https.RequestOptions, 'agent'> & {
maxRedirects: number,
headers: HeadersObject,
__testHookLookup?: (hostname: string) => LookupAddress[]
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -362,11 +360,15 @@ export abstract class APIRequestContext extends SdkObject {
const resultPromise = new Promise<SendRequestResult>((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);

Expand Down Expand Up @@ -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,
};
Expand Down Expand Up @@ -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();
Expand Down
29 changes: 29 additions & 0 deletions tests/library/fetch-proxy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
}
});
Loading