Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(client-certificates): don't use proxy when using BrowserContext.request #31898

Merged
merged 3 commits into from
Jul 29, 2024
Merged

fix(client-certificates): don't use proxy when using BrowserContext.request #31898

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7555c53
fix(client-certificates): don't use proxy when using BrowserContext.r…
mxschmitt Jul 29, 2024
8e9efda
Update packages/playwright-core/src/server/fetch.ts
mxschmitt Jul 29, 2024
0627d8a
make tests pass
mxschmitt Jul 29, 2024
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
2 changes: 1 addition & 1 deletion packages/playwright-core/src/server/fetch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ export abstract class APIRequestContext extends SdkObject {
const method = params.method?.toUpperCase() || 'GET';
const proxy = defaults.proxy;
let agent;
if (proxy && proxy.server !== 'per-context' && !shouldBypassProxy(requestUrl, proxy.bypass)) {
if (proxy && !defaults.clientCertificates?.length && proxy.server !== 'per-context' && !shouldBypassProxy(requestUrl, proxy.bypass)) {
mxschmitt marked this conversation as resolved.
Show resolved Hide resolved
const proxyOpts = url.parse(proxy.server);
if (proxyOpts.protocol?.startsWith('socks')) {
agent = new SocksProxyAgent({
Expand Down
26 changes: 25 additions & 1 deletion tests/library/client-certificates.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const { createHttpsServer, createHttp2Server } = require('../../packages/playwri

type TestOptions = {
startCCServer(options?: {
host?: string;
http2?: boolean;
enableHTTP1FallbackWhenUsingHttp2?: boolean;
useFakeLocalhost?: boolean;
Expand Down Expand Up @@ -63,7 +64,7 @@ const test = base.extend<TestOptions>({
}
res.end(parts.map(({ key, value }) => `<div data-testid="${key}">${value}</div>`).join(''));
});
await new Promise<void>(f => server.listen(0, 'localhost', () => f()));
await new Promise<void>(f => server.listen(0, options.host ?? 'localhost', () => f()));
const host = options?.useFakeLocalhost ? 'local.playwright' : 'localhost';
return `https://${host}:${(server.address() as net.AddressInfo).port}/`;
});
Expand Down Expand Up @@ -251,6 +252,29 @@ test.describe('browser', () => {
await page.close();
});

test('should pass with matching certificates on context APIRequestContext instance', async ({ browser, startCCServer, asset, browserName }) => {
const serverURL = await startCCServer({ host: '127.0.0.1' });
const baseOptions = {
certPath: asset('client-certificates/client/trusted/cert.pem'),
keyPath: asset('client-certificates/client/trusted/key.pem'),
};
const page = await browser.newPage({
clientCertificates: [{
origin: new URL(serverURL).origin,
...baseOptions,
}, {
origin: new URL(serverURL).origin.replace('localhost', '127.0.0.1'),
...baseOptions,
}],
});
for (const url of [serverURL, serverURL.replace('localhost', '127.0.0.1')]) {
const response = await page.request.get(url);
expect(response.status()).toBe(200);
expect(await response.text()).toContain('Hello Alice, your certificate was issued by localhost!');
}
await page.close();
});

test('should pass with matching certificates and trailing slash', async ({ browser, startCCServer, asset, browserName }) => {
const serverURL = await startCCServer({ useFakeLocalhost: browserName === 'webkit' && process.platform === 'darwin' });
const page = await browser.newPage({
Expand Down
Loading