cherry-pick(#31898): fix(client-certificates): don't use proxy when u…

…sing BrowserContext.request
microsoft · Jul 29, 2024 · 446de52 · 446de52
1 parent 2ea14ca
commit 446de52
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 2 deletions.
diff --git a/packages/playwright-core/src/server/fetch.ts b/packages/playwright-core/src/server/fetch.ts
@@ -168,7 +168,10 @@ 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)) {
+    // When `clientCertificates` is present, we set the `proxy` property to our own socks proxy
+    // for the browser to use. However, we don't need it here, because we already respect
+    // `clientCertificates` when fetching from Node.js.
+    if (proxy && !defaults.clientCertificates?.length && proxy.server !== 'per-context' && !shouldBypassProxy(requestUrl, proxy.bypass)) {
       const proxyOpts = url.parse(proxy.server);
       if (proxyOpts.protocol?.startsWith('socks')) {
         agent = new SocksProxyAgent({

diff --git a/tests/library/client-certificates.spec.ts b/tests/library/client-certificates.spec.ts
@@ -24,6 +24,7 @@ const { createHttpsServer, createHttp2Server } = require('../../packages/playwri
 
 type TestOptions = {
   startCCServer(options?: {
+    host?: string;
     http2?: boolean;
     enableHTTP1FallbackWhenUsingHttp2?: boolean;
     useFakeLocalhost?: boolean;
@@ -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}/`;
     });
@@ -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({