review follow-ups

This commit is contained in:
Max Schmitt 2024-09-16 17:29:29 +02:00
parent aedf295646
commit 69bc8872d5
4 changed files with 14 additions and 14 deletions

View file

@ -86,6 +86,7 @@ export abstract class Browser extends SdkObject {
let clientCertificatesProxy: ClientCertificatesProxy | undefined; let clientCertificatesProxy: ClientCertificatesProxy | undefined;
if (options.clientCertificates?.length) { if (options.clientCertificates?.length) {
clientCertificatesProxy = new ClientCertificatesProxy(options); clientCertificatesProxy = new ClientCertificatesProxy(options);
options = { ...options };
options.proxyOverride = await clientCertificatesProxy.listen(); options.proxyOverride = await clientCertificatesProxy.listen();
options.internalIgnoreHTTPSErrors = true; options.internalIgnoreHTTPSErrors = true;
} }

View file

@ -93,8 +93,8 @@ export abstract class BrowserType extends SdkObject {
return browser; return browser;
} }
async launchPersistentContext(metadata: CallMetadata, userDataDir: string, persistentContextOptions: channels.BrowserTypeLaunchPersistentContextOptions & { useWebSocket?: boolean, internalIgnoreHTTPSErrors?: boolean }): Promise<BrowserContext> { async launchPersistentContext(metadata: CallMetadata, userDataDir: string, options: channels.BrowserTypeLaunchPersistentContextOptions & { useWebSocket?: boolean, internalIgnoreHTTPSErrors?: boolean }): Promise<BrowserContext> {
const launchOptions = this._validateLaunchOptions(persistentContextOptions); const launchOptions = this._validateLaunchOptions(options);
if (this._useBidi) if (this._useBidi)
launchOptions.useWebSocket = true; launchOptions.useWebSocket = true;
const controller = new ProgressController(metadata, this); const controller = new ProgressController(metadata, this);
@ -102,13 +102,14 @@ export abstract class BrowserType extends SdkObject {
const browser = await controller.run(async progress => { const browser = await controller.run(async progress => {
// Note: Any initial TLS requests will fail since we rely on the Page/Frames initialize which sets ignoreHTTPSErrors. // Note: Any initial TLS requests will fail since we rely on the Page/Frames initialize which sets ignoreHTTPSErrors.
let clientCertificatesProxy: ClientCertificatesProxy | undefined; let clientCertificatesProxy: ClientCertificatesProxy | undefined;
if (persistentContextOptions.clientCertificates?.length) { if (options.clientCertificates?.length) {
clientCertificatesProxy = new ClientCertificatesProxy(persistentContextOptions); clientCertificatesProxy = new ClientCertificatesProxy(options);
launchOptions.proxyOverride = await clientCertificatesProxy?.listen(); launchOptions.proxyOverride = await clientCertificatesProxy?.listen();
persistentContextOptions.internalIgnoreHTTPSErrors = true; options = { ...options };
options.internalIgnoreHTTPSErrors = true;
} }
progress.cleanupWhenAborted(() => clientCertificatesProxy?.close()); progress.cleanupWhenAborted(() => clientCertificatesProxy?.close());
const browser = await this._innerLaunchWithRetries(progress, launchOptions, persistentContextOptions, helper.debugProtocolLogger(), userDataDir).catch(e => { throw this._rewriteStartupLog(e); }); const browser = await this._innerLaunchWithRetries(progress, launchOptions, options, helper.debugProtocolLogger(), userDataDir).catch(e => { throw this._rewriteStartupLog(e); });
browser._defaultContext!._clientCertificatesProxy = clientCertificatesProxy; browser._defaultContext!._clientCertificatesProxy = clientCertificatesProxy;
return browser; return browser;
}, TimeoutSettings.launchTimeout(launchOptions)); }, TimeoutSettings.launchTimeout(launchOptions));

View file

@ -169,9 +169,8 @@ export abstract class APIRequestContext extends SdkObject {
const method = params.method?.toUpperCase() || 'GET'; const method = params.method?.toUpperCase() || 'GET';
const proxy = defaults.proxy; const proxy = defaults.proxy;
let agent; let agent;
// When `clientCertificates` is present, we set the `proxy` property to our own socks proxy // We skip 'per-context' in order to not break existing users. 'per-context' was previously used to
// for the browser to use. However, we don't need it here, because we already respect // workaround an upstream Chromium bug. Can be removed in the future.
// `clientCertificates` when fetching from Node.js.
if (proxy && proxy.server !== 'per-context' && !shouldBypassProxy(requestUrl, proxy.bypass)) if (proxy && proxy.server !== 'per-context' && !shouldBypassProxy(requestUrl, proxy.bypass))
agent = createProxyAgent(proxy); agent = createProxyAgent(proxy);

View file

@ -97,9 +97,8 @@ class SocksProxyConnection {
} }
async connect() { async connect() {
if (this.socksProxy.agent) if (this.socksProxy.proxyAgentFromOptions)
// @ts-expect-error this.target = await this.socksProxy.proxyAgentFromOptions.callback(new EventEmitter() as any, { host: rewriteToLocalhostIfNeeded(this.host), port: this.port, secureEndpoint: false });
this.target = await this.socksProxy.agent.callback(new EventEmitter(), { host: rewriteToLocalhostIfNeeded(this.host), port: this.port });
else else
this.target = await createSocket(rewriteToLocalhostIfNeeded(this.host), this.port); this.target = await createSocket(rewriteToLocalhostIfNeeded(this.host), this.port);
@ -241,7 +240,7 @@ export class ClientCertificatesProxy {
ignoreHTTPSErrors: boolean | undefined; ignoreHTTPSErrors: boolean | undefined;
secureContextMap: Map<string, tls.SecureContext> = new Map(); secureContextMap: Map<string, tls.SecureContext> = new Map();
alpnCache: ALPNCache; alpnCache: ALPNCache;
agent: ReturnType<typeof createProxyAgent> | undefined; proxyAgentFromOptions: ReturnType<typeof createProxyAgent> | undefined;
constructor( constructor(
contextOptions: Pick<types.BrowserContextOptions, 'clientCertificates' | 'ignoreHTTPSErrors' | 'proxy'> contextOptions: Pick<types.BrowserContextOptions, 'clientCertificates' | 'ignoreHTTPSErrors' | 'proxy'>
@ -249,7 +248,7 @@ export class ClientCertificatesProxy {
verifyClientCertificates(contextOptions.clientCertificates); verifyClientCertificates(contextOptions.clientCertificates);
this.alpnCache = new ALPNCache(); this.alpnCache = new ALPNCache();
this.ignoreHTTPSErrors = contextOptions.ignoreHTTPSErrors; this.ignoreHTTPSErrors = contextOptions.ignoreHTTPSErrors;
this.agent = contextOptions.proxy ? createProxyAgent(contextOptions.proxy) : undefined; this.proxyAgentFromOptions = contextOptions.proxy ? createProxyAgent(contextOptions.proxy) : undefined;
this._initSecureContexts(contextOptions.clientCertificates); this._initSecureContexts(contextOptions.clientCertificates);
this._socksProxy = new SocksProxy(); this._socksProxy = new SocksProxy();
this._socksProxy.setPattern('*'); this._socksProxy.setPattern('*');