From dfb295e696e37b11f0d4c609751d8dbfc31d0a18 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Thu, 10 Apr 2025 10:39:41 +0100 Subject: [PATCH] chore: refactor some code around proxies Extracted from #35389. No behavior change whatsoever. --- .../playwright-core/src/server/browserType.ts | 2 +- .../src/server/chromium/chromium.ts | 6 +-- .../dispatchers/browserTypeDispatcher.ts | 2 +- .../dispatchers/localUtilsDispatcher.ts | 2 +- packages/playwright-core/src/server/fetch.ts | 44 ++---------------- .../socksClientCertificatesInterceptor.ts | 6 +-- .../playwright-core/src/server/transport.ts | 30 +++++++----- .../src/server/utils/network.ts | 46 ++++++++++++++++++- 8 files changed, 76 insertions(+), 62 deletions(-) diff --git a/packages/playwright-core/src/server/browserType.ts b/packages/playwright-core/src/server/browserType.ts index c4c44d83dec30..208fb55fdca6e 100644 --- a/packages/playwright-core/src/server/browserType.ts +++ b/packages/playwright-core/src/server/browserType.ts @@ -292,7 +292,7 @@ export abstract class BrowserType extends SdkObject { await fs.promises.mkdir(options.tracesDir, { recursive: true }); } - async connectOverCDP(metadata: CallMetadata, endpointURL: string, options: { slowMo?: number }, timeout?: number): Promise { + async connectOverCDP(metadata: CallMetadata, endpointURL: string, options: { slowMo?: number, timeout?: number, headers?: types.HeadersArray }): Promise { throw new Error('CDP connections are only supported by Chromium'); } diff --git a/packages/playwright-core/src/server/chromium/chromium.ts b/packages/playwright-core/src/server/chromium/chromium.ts index 35acc621e29ec..94c9103cee934 100644 --- a/packages/playwright-core/src/server/chromium/chromium.ts +++ b/packages/playwright-core/src/server/chromium/chromium.ts @@ -64,12 +64,12 @@ export class Chromium extends BrowserType { this._devtools = this._createDevTools(); } - override async connectOverCDP(metadata: CallMetadata, endpointURL: string, options: { slowMo?: number, headers?: types.HeadersArray }, timeout?: number) { + override async connectOverCDP(metadata: CallMetadata, endpointURL: string, options: { slowMo?: number, headers?: types.HeadersArray, timeout?: number }) { const controller = new ProgressController(metadata, this); controller.setLogName('browser'); return controller.run(async progress => { return await this._connectOverCDPInternal(progress, endpointURL, options); - }, TimeoutSettings.timeout({ timeout })); + }, TimeoutSettings.timeout(options)); } async _connectOverCDPInternal(progress: Progress, endpointURL: string, options: types.LaunchOptions & { headers?: types.HeadersArray }, onClose?: () => Promise) { @@ -87,7 +87,7 @@ export class Chromium extends BrowserType { const wsEndpoint = await urlToWSEndpoint(progress, endpointURL, headersMap); progress.throwIfAborted(); - const chromeTransport = await WebSocketTransport.connect(progress, wsEndpoint, headersMap); + const chromeTransport = await WebSocketTransport.connect(progress, wsEndpoint, { headers: headersMap }); const cleanedUp = new ManualPromise(); const doCleanup = async () => { await removeFolders([artifactsDir]); diff --git a/packages/playwright-core/src/server/dispatchers/browserTypeDispatcher.ts b/packages/playwright-core/src/server/dispatchers/browserTypeDispatcher.ts index bf21e726bcc01..40bf3eb9d80d7 100644 --- a/packages/playwright-core/src/server/dispatchers/browserTypeDispatcher.ts +++ b/packages/playwright-core/src/server/dispatchers/browserTypeDispatcher.ts @@ -43,7 +43,7 @@ export class BrowserTypeDispatcher extends Dispatcher { - const browser = await this._object.connectOverCDP(metadata, params.endpointURL, params, params.timeout); + const browser = await this._object.connectOverCDP(metadata, params.endpointURL, params); const browserDispatcher = new BrowserDispatcher(this, browser); return { browser: browserDispatcher, diff --git a/packages/playwright-core/src/server/dispatchers/localUtilsDispatcher.ts b/packages/playwright-core/src/server/dispatchers/localUtilsDispatcher.ts index a681b349d325f..48fe42320fa3d 100644 --- a/packages/playwright-core/src/server/dispatchers/localUtilsDispatcher.ts +++ b/packages/playwright-core/src/server/dispatchers/localUtilsDispatcher.ts @@ -92,7 +92,7 @@ export class LocalUtilsDispatcher extends Dispatcher<{ guid: string }, channels. }; const wsEndpoint = await urlToWSEndpoint(progress, params.wsEndpoint); - const transport = await WebSocketTransport.connect(progress, wsEndpoint, wsHeaders, true, 'x-playwright-debug-log'); + const transport = await WebSocketTransport.connect(progress, wsEndpoint, { headers: wsHeaders, followRedirects: true, debugLogHeader: 'x-playwright-debug-log' }); const socksInterceptor = new SocksInterceptor(transport, params.exposeNetwork, params.socksProxyRedirectPortForTest); const pipe = new JsonPipeDispatcher(this); transport.onmessage = json => { diff --git a/packages/playwright-core/src/server/fetch.ts b/packages/playwright-core/src/server/fetch.ts index 9760ea89a1aa5..71f9de0833e0b 100644 --- a/packages/playwright-core/src/server/fetch.ts +++ b/packages/playwright-core/src/server/fetch.ts @@ -18,14 +18,12 @@ import http from 'http'; import https from 'https'; import { Transform, pipeline } from 'stream'; import { TLSSocket } from 'tls'; -import url from 'url'; import * as zlib from 'zlib'; import { TimeoutSettings } from './timeoutSettings'; -import { assert, constructURLBasedOnBaseURL, eventsHelper, monotonicTime } from '../utils'; +import { assert, constructURLBasedOnBaseURL, createProxyAgent, eventsHelper, monotonicTime } from '../utils'; import { createGuid } from './utils/crypto'; import { getUserAgent } from './utils/userAgent'; -import { HttpsProxyAgent, SocksProxyAgent } from '../utilsBundle'; import { BrowserContext, verifyClientCertificates } from './browserContext'; import { CookieStore, domainMatches, parseRawCookie } from './cookieStore'; import { MultipartFormData } from './formData'; @@ -183,8 +181,8 @@ export abstract class APIRequestContext extends SdkObject { 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 && proxy.server !== 'per-context' && !shouldBypassProxy(requestUrl, proxy.bypass)) - agent = createProxyAgent(proxy); + if (proxy?.server !== 'per-context') + agent = createProxyAgent(proxy, requestUrl); let maxRedirects = params.maxRedirects ?? (defaults.maxRedirects ?? 20); maxRedirects = maxRedirects === 0 ? -1 : maxRedirects; @@ -647,13 +645,6 @@ export class GlobalAPIRequestContext extends APIRequestContext { const timeoutSettings = new TimeoutSettings(); if (options.timeout !== undefined) timeoutSettings.setDefaultTimeout(options.timeout); - const proxy = options.proxy; - if (proxy?.server) { - let url = proxy?.server.trim(); - if (!/^\w+:\/\//.test(url)) - url = 'http://' + url; - proxy.server = url; - } if (options.storageState) { this._origins = options.storageState.origins?.map(origin => ({ indexedDB: [], ...origin })); this._cookieStore.addCookies(options.storageState.cookies || []); @@ -668,7 +659,7 @@ export class GlobalAPIRequestContext extends APIRequestContext { maxRedirects: options.maxRedirects, httpCredentials: options.httpCredentials, clientCertificates: options.clientCertificates, - proxy, + proxy: options.proxy, timeoutSettings, }; this._tracing = new Tracing(this, options.tracesDir); @@ -705,20 +696,6 @@ export class GlobalAPIRequestContext extends APIRequestContext { } } -export function createProxyAgent(proxy: types.ProxySettings) { - const proxyOpts = url.parse(proxy.server); - if (proxyOpts.protocol?.startsWith('socks')) { - return new SocksProxyAgent({ - host: proxyOpts.hostname, - port: proxyOpts.port || undefined, - }); - } - if (proxy.username) - proxyOpts.auth = `${proxy.username}:${proxy.password || ''}`; - // TODO: We should use HttpProxyAgent conditional on proxyOpts.protocol instead of always using CONNECT method. - return new HttpsProxyAgent(proxyOpts); -} - function toHeadersArray(rawHeaders: string[]): types.HeadersArray { const result: types.HeadersArray = []; for (let i = 0; i < rawHeaders.length; i += 2) @@ -791,19 +768,6 @@ function removeHeader(headers: { [name: string]: string }, name: string) { delete headers[name]; } -function shouldBypassProxy(url: URL, bypass?: string): boolean { - if (!bypass) - return false; - const domains = bypass.split(',').map(s => { - s = s.trim(); - if (!s.startsWith('.')) - s = '.' + s; - return s; - }); - const domain = '.' + url.hostname; - return domains.some(d => domain.endsWith(d)); -} - function setBasicAuthorizationHeader(headers: { [name: string]: string }, credentials: HTTPCredentials) { const { username, password } = credentials; const encoded = Buffer.from(`${username || ''}:${password || ''}`).toString('base64'); diff --git a/packages/playwright-core/src/server/socksClientCertificatesInterceptor.ts b/packages/playwright-core/src/server/socksClientCertificatesInterceptor.ts index a68cb88abb779..2cb588378208b 100644 --- a/packages/playwright-core/src/server/socksClientCertificatesInterceptor.ts +++ b/packages/playwright-core/src/server/socksClientCertificatesInterceptor.ts @@ -23,7 +23,7 @@ import tls from 'tls'; import { SocksProxy } from './utils/socksProxy'; import { ManualPromise, escapeHTML, generateSelfSignedCertificate, rewriteErrorMessage } from '../utils'; import { verifyClientCertificates } from './browserContext'; -import { createProxyAgent } from './fetch'; +import { createProxyAgent } from './utils/network'; import { debugLogger } from './utils/debugLogger'; import { createSocket, createTLSSocket } from './utils/happyEyeballs'; @@ -242,7 +242,7 @@ export class ClientCertificatesProxy { ignoreHTTPSErrors: boolean | undefined; secureContextMap: Map = new Map(); alpnCache: ALPNCache; - proxyAgentFromOptions: ReturnType | undefined; + proxyAgentFromOptions: ReturnType; constructor( contextOptions: Pick @@ -250,7 +250,7 @@ export class ClientCertificatesProxy { verifyClientCertificates(contextOptions.clientCertificates); this.alpnCache = new ALPNCache(); this.ignoreHTTPSErrors = contextOptions.ignoreHTTPSErrors; - this.proxyAgentFromOptions = contextOptions.proxy ? createProxyAgent(contextOptions.proxy) : undefined; + this.proxyAgentFromOptions = createProxyAgent(contextOptions.proxy); this._initSecureContexts(contextOptions.clientCertificates); this._socksProxy = new SocksProxy(); this._socksProxy.setPattern('*'); diff --git a/packages/playwright-core/src/server/transport.ts b/packages/playwright-core/src/server/transport.ts index ec2caec0c550e..8e87301b78654 100644 --- a/packages/playwright-core/src/server/transport.ts +++ b/packages/playwright-core/src/server/transport.ts @@ -60,6 +60,12 @@ export interface ConnectionTransport { onclose?: (reason?: string) => void, } +type WebSocketTransportOptions = { + headers?: { [key: string]: string; }; + followRedirects?: boolean; + debugLogHeader?: string; +}; + export class WebSocketTransport implements ConnectionTransport { private _ws: WebSocket; private _progress?: Progress; @@ -70,14 +76,14 @@ export class WebSocketTransport implements ConnectionTransport { readonly wsEndpoint: string; readonly headers: HeadersArray = []; - static async connect(progress: (Progress|undefined), url: string, headers?: { [key: string]: string; }, followRedirects?: boolean, debugLogHeader?: string): Promise { - return await WebSocketTransport._connect(progress, url, headers || {}, { follow: !!followRedirects, hadRedirects: false }, debugLogHeader); + static async connect(progress: (Progress|undefined), url: string, options: WebSocketTransportOptions = {}): Promise { + return await WebSocketTransport._connect(progress, url, options, false /* hadRedirects */); } - static async _connect(progress: (Progress|undefined), url: string, headers: { [key: string]: string; }, redirect: { follow: boolean, hadRedirects: boolean }, debugLogHeader?: string): Promise { + static async _connect(progress: (Progress|undefined), url: string, options: WebSocketTransportOptions, hadRedirects: boolean): Promise { const logUrl = stripQueryParams(url); progress?.log(` ${logUrl}`); - const transport = new WebSocketTransport(progress, url, logUrl, headers, redirect.follow && redirect.hadRedirects, debugLogHeader); + const transport = new WebSocketTransport(progress, url, logUrl, { ...options, followRedirects: !!options.followRedirects && hadRedirects }); let success = false; progress?.cleanupWhenAborted(async () => { if (!success) @@ -94,13 +100,13 @@ export class WebSocketTransport implements ConnectionTransport { transport._ws.close(); }); transport._ws.on('unexpected-response', (request: ClientRequest, response: IncomingMessage) => { - if (redirect.follow && !redirect.hadRedirects && (response.statusCode === 301 || response.statusCode === 302 || response.statusCode === 307 || response.statusCode === 308)) { + if (options.followRedirects && !hadRedirects && (response.statusCode === 301 || response.statusCode === 302 || response.statusCode === 307 || response.statusCode === 308)) { fulfill({ redirect: response }); transport._ws.close(); return; } for (let i = 0; i < response.rawHeaders.length; i += 2) { - if (debugLogHeader && response.rawHeaders[i] === debugLogHeader) + if (options.debugLogHeader && response.rawHeaders[i] === options.debugLogHeader) progress?.log(response.rawHeaders[i + 1]); } const chunks: Buffer[] = []; @@ -117,32 +123,32 @@ export class WebSocketTransport implements ConnectionTransport { if (result.redirect) { // Strip authorization headers from the redirected request. - const newHeaders = Object.fromEntries(Object.entries(headers || {}).filter(([name]) => { + const newHeaders = Object.fromEntries(Object.entries(options.headers || {}).filter(([name]) => { return !name.includes('access-key') && name.toLowerCase() !== 'authorization'; })); - return WebSocketTransport._connect(progress, result.redirect.headers.location!, newHeaders, { follow: true, hadRedirects: true }, debugLogHeader); + return WebSocketTransport._connect(progress, result.redirect.headers.location!, { ...options, headers: newHeaders }, true /* hadRedirects */); } success = true; return transport; } - constructor(progress: Progress|undefined, url: string, logUrl: string, headers?: { [key: string]: string; }, followRedirects?: boolean, debugLogHeader?: string) { + constructor(progress: Progress|undefined, url: string, logUrl: string, options: WebSocketTransportOptions) { this.wsEndpoint = url; this._logUrl = logUrl; this._ws = new ws(url, [], { maxPayload: 256 * 1024 * 1024, // 256Mb, // Prevent internal http client error when passing negative timeout. handshakeTimeout: Math.max(progress?.timeUntilDeadline() ?? 30_000, 1), - headers, - followRedirects, + headers: options.headers, + followRedirects: options.followRedirects, agent: (/^(https|wss):\/\//.test(url)) ? httpsHappyEyeballsAgent : httpHappyEyeballsAgent, perMessageDeflate, }); this._ws.on('upgrade', response => { for (let i = 0; i < response.rawHeaders.length; i += 2) { this.headers.push({ name: response.rawHeaders[i], value: response.rawHeaders[i + 1] }); - if (debugLogHeader && response.rawHeaders[i] === debugLogHeader) + if (options.debugLogHeader && response.rawHeaders[i] === options.debugLogHeader) progress?.log(response.rawHeaders[i + 1]); } }); diff --git a/packages/playwright-core/src/server/utils/network.ts b/packages/playwright-core/src/server/utils/network.ts index 8e0357f958122..adc2ad577cc26 100644 --- a/packages/playwright-core/src/server/utils/network.ts +++ b/packages/playwright-core/src/server/utils/network.ts @@ -20,10 +20,11 @@ import http2 from 'http2'; import https from 'https'; import url from 'url'; -import { HttpsProxyAgent, getProxyForUrl } from '../../utilsBundle'; +import { HttpsProxyAgent, SocksProxyAgent, getProxyForUrl } from '../../utilsBundle'; import { httpHappyEyeballsAgent, httpsHappyEyeballsAgent } from './happyEyeballs'; import type net from 'net'; +import type { ProxySettings } from '../types'; export type HTTPRequestParams = { url: string, @@ -109,6 +110,49 @@ export function fetchData(params: HTTPRequestParams, onError?: (params: HTTPRequ }); } +function shouldBypassProxy(url: URL, bypass?: string): boolean { + if (!bypass) + return false; + const domains = bypass.split(',').map(s => { + s = s.trim(); + if (!s.startsWith('.')) + s = '.' + s; + return s; + }); + const domain = '.' + url.hostname; + return domains.some(d => domain.endsWith(d)); +} + +export function createProxyAgent(proxy?: ProxySettings, forUrl?: URL) { + if (!proxy) + return; + if (forUrl && proxy.bypass && shouldBypassProxy(forUrl, proxy.bypass)) + return; + + // Browsers allow to specify proxy without a protocol, defaulting to http. + let proxyServer = proxy.server.trim(); + if (!/^\w+:\/\//.test(proxyServer)) + proxyServer = 'http://' + proxyServer; + + const proxyOpts = url.parse(proxyServer); + if (proxyOpts.protocol?.startsWith('socks')) { + return new SocksProxyAgent({ + host: proxyOpts.hostname, + port: proxyOpts.port || undefined, + }); + } + if (proxy.username) + proxyOpts.auth = `${proxy.username}:${proxy.password || ''}`; + + if (forUrl && ['ws:', 'wss:'].includes(forUrl.protocol)) { + // Force CONNECT method for WebSockets. + return new HttpsProxyAgent(proxyOpts); + } + + // TODO: We should use HttpProxyAgent conditional on proxyOpts.protocol instead of always using CONNECT method. + return new HttpsProxyAgent(proxyOpts); +} + export function createHttpServer(requestListener?: (req: http.IncomingMessage, res: http.ServerResponse) => void): http.Server; export function createHttpServer(options: http.ServerOptions, requestListener?: (req: http.IncomingMessage, res: http.ServerResponse) => void): http.Server; export function createHttpServer(...args: any[]): http.Server {