review 2

docs/src/api/params.md

@@ -521,7 +521,7 @@ Does not enforce fixed viewport, allows resizing window in the headed mode.
     - `cert` ?<[string]> Path to the file with the certificate in PEM format.
     - `key` ?<[string]> Path to the file with the private key in PEM format.
     - `passphrase` ?<[string]> Passphrase for the private key (PEM or PFX).
-    - `pfx` ?<[string]> PFX or PKCS12 encoded private key and certificate chain.
+    - `pfx` ?<[string]> Path to the PFX or PKCS12 encoded private key and certificate chain.
 
 An array of client certificates to be used. Each certificate object must have `cert` and `key` or `pfx` to load the client certificate. Optionally, `passphrase` property should be provided if the private key is encrypted. If the certificate is issued by a custom certificate authority, the `ignoreHTTPSErrors` needs to be set. If the certificate is valid only for specific URLs, the `url` property should be provided with a glob pattern to match the URLs that the certificate is valid for.
 

packages/playwright-core/src/client/browserContext.ts

@@ -529,7 +529,7 @@ export async function prepareBrowserContextParams(options: BrowserContextOptions
     reducedMotion: options.reducedMotion === null ? 'no-override' : options.reducedMotion,
     forcedColors: options.forcedColors === null ? 'no-override' : options.forcedColors,
     acceptDownloads: toAcceptDownloadsProtocol(options.acceptDownloads),
-    clientCertificates: await toClientCertificatesProtocol(options.proxy, options.clientCertificates),
+    clientCertificates: await toClientCertificatesProtocol(options.clientCertificates),
   };
   if (!contextParams.recordVideo && options.videosPath) {
     contextParams.recordVideo = {
@@ -550,25 +550,13 @@ function toAcceptDownloadsProtocol(acceptDownloads?: boolean) {
   return 'deny';
 }
 
-export async function toClientCertificatesProtocol(proxy: BrowserContextOptions['proxy'], clientCertificates?: BrowserContextOptions['clientCertificates']): Promise<channels.PlaywrightNewRequestParams['clientCertificates']> {
+export async function toClientCertificatesProtocol(clientCertificates?: BrowserContextOptions['clientCertificates']): Promise<channels.PlaywrightNewRequestParams['clientCertificates']> {
   if (!clientCertificates)
     return undefined;
-  if (proxy)
-    throw new Error('Cannot specify both proxy and clientCertificates');
   return await Promise.all(clientCertificates.map(async clientCertificate => {
-    if (clientCertificate.certs.length === 0)
-      throw new Error('No certs specified for url: ' + clientCertificate.url);
     return {
       url: clientCertificate.url,
       certs: await Promise.all(clientCertificate.certs.map(async cert => {
-        if (!cert.cert && !cert.key && !cert.passphrase && !cert.pfx)
-          throw new Error('None of cert, key, passphrase or pfx is specified');
-        if (cert.cert && !cert.key)
-          throw new Error('cert is specified without key');
-        if (!cert.cert && cert.key)
-          throw new Error('key is specified without cert');
-        if (cert.pfx && (cert.cert || cert.key || cert.passphrase))
-          throw new Error('pfx is specified together with cert, key or passphrase');
         return {
           cert: cert.cert ? await fs.promises.readFile(cert.cert) : undefined,
           key: cert.key ? await fs.promises.readFile(cert.key) : undefined,

packages/playwright-core/src/client/fetch.ts

@@ -76,7 +76,7 @@ export class APIRequest implements api.APIRequest {
       extraHTTPHeaders: options.extraHTTPHeaders ? headersObjectToArray(options.extraHTTPHeaders) : undefined,
       storageState,
       tracesDir,
-      clientCertificates: await toClientCertificatesProtocol(options.proxy, options.clientCertificates),
+      clientCertificates: await toClientCertificatesProtocol(options.clientCertificates),
     })).request);
     this._contexts.add(context);
     context._request = this;

packages/playwright-core/src/server/browserContext.ts

@@ -689,11 +689,14 @@ export function validateBrowserContextOptions(options: channels.BrowserNewContex
   if (options.proxy) {
     if (!browserOptions.proxy && browserOptions.isChromium && os.platform() === 'win32')
       throw new Error(`Browser needs to be launched with the global proxy. If all contexts override the proxy, global proxy will be never used and can be any string, for example "launch({ proxy: { server: 'http://per-context' } })"`);
+    if (options.clientCertificates)
+      throw new Error('Cannot specify both proxy and clientCertificates');
     options.proxy = normalizeProxySettings(options.proxy);
   }
   if (options.clientCertificates?.length)
     options.ignoreHTTPSErrors = true;
   verifyGeolocation(options.geolocation);
+  verifyClientCertificates(options.clientCertificates);
 }
 
 export function verifyGeolocation(geolocation?: types.Geolocation) {
@@ -709,6 +712,27 @@ export function verifyGeolocation(geolocation?: types.Geolocation) {
     throw new Error(`geolocation.accuracy: precondition 0 <= ACCURACY failed.`);
 }
 
+export function verifyClientCertificates(clientCertificates?: channels.BrowserNewContextParams['clientCertificates']) {
+  if (!clientCertificates)
+    return;
+  for (const { url, certs } of clientCertificates) {
+    if (!url)
+      throw new Error(`clientCertificates.url is required`);
+    if (!certs.length)
+      throw new Error('No certs specified for url: ' + url);
+    for (const cert of certs) {
+      if (!cert.cert && !cert.key && !cert.passphrase && !cert.pfx)
+        throw new Error('None of cert, key, passphrase or pfx is specified');
+      if (cert.cert && !cert.key)
+        throw new Error('cert is specified without key');
+      if (!cert.cert && cert.key)
+        throw new Error('key is specified without cert');
+      if (cert.pfx && (cert.cert || cert.key || cert.passphrase))
+        throw new Error('pfx is specified together with cert, key or passphrase');
+    }
+  }
+}
+
 export function normalizeProxySettings(proxy: types.ProxySettings): types.ProxySettings {
   let { server, bypass } = proxy;
   let url;

packages/playwright-core/src/server/fetch.ts

@@ -27,7 +27,7 @@ import { TimeoutSettings } from '../common/timeoutSettings';
 import { getUserAgent } from '../utils/userAgent';
 import { assert, createGuid, monotonicTime } from '../utils';
 import { HttpsProxyAgent, SocksProxyAgent } from '../utilsBundle';
-import { BrowserContext } from './browserContext';
+import { BrowserContext, verifyClientCertificates } from './browserContext';
 import { CookieStore, domainMatches } from './cookieStore';
 import { MultipartFormData } from './formData';
 import { httpHappyEyeballsAgent, httpsHappyEyeballsAgent } from '../utils/happy-eyeballs';
@@ -562,11 +562,14 @@ export class GlobalAPIRequestContext extends APIRequestContext {
       if (!/^\w+:\/\//.test(url))
         url = 'http://' + url;
       proxy.server = url;
+      if (options.clientCertificates)
+        throw new Error('Cannot specify both proxy and clientCertificates');
     }
     if (options.storageState) {
       this._origins = options.storageState.origins;
       this._cookieStore.addCookies(options.storageState.cookies || []);
     }
+    verifyClientCertificates(options.clientCertificates);
     this._options = {
       baseURL: options.baseURL,
       userAgent: options.userAgent || getUserAgent(),

packages/playwright-core/types/types.d.ts

@@ -15625,7 +15625,7 @@ export interface APIRequest {
         passphrase?: string;
 
         /**
-         * PFX or PKCS12 encoded private key and certificate chain.
+         * Path to the PFX or PKCS12 encoded private key and certificate chain.
          */
         pfx?: string;
       }>;
@@ -16817,7 +16817,7 @@ export interface Browser extends EventEmitter {
         passphrase?: string;
 
         /**
-         * PFX or PKCS12 encoded private key and certificate chain.
+         * Path to the PFX or PKCS12 encoded private key and certificate chain.
          */
         pfx?: string;
       }>;
@@ -20290,7 +20290,7 @@ export interface BrowserContextOptions {
       passphrase?: string;
 
       /**
-       * PFX or PKCS12 encoded private key and certificate chain.
+       * Path to the PFX or PKCS12 encoded private key and certificate chain.
        */
       pfx?: string;
     }>;

tests/library/client-certificates.spec.ts

@@ -59,16 +59,17 @@ const test = base.extend<{ serverURL: string, serverURLRewrittenToLocalhost: str
 test.skip(({ mode }) => mode !== 'default');
 
 const kClientCertificatesDir = path.join(__dirname, '../config/testserver/client-certificates');
+const kDummyFileName = __filename;
 const kValidationSubTests: [BrowserContextOptions, string ][] = [
   [{ clientCertificates: [{ url: 'test', certs: [] }] }, 'No certs specified for url: test'],
   [{ clientCertificates: [{ url: 'test', certs: [{}] }] }, 'None of cert, key, passphrase or pfx is specified'],
   [{ clientCertificates: [{
     url: 'test',
     certs: [{
-      cert: 'foo',
-      key: 'foo',
-      passphrase: 'foo',
-      pfx: 'foo',
+      cert: kDummyFileName,
+      key: kDummyFileName,
+      passphrase: kDummyFileName,
+      pfx: kDummyFileName,
     }]
   }] }, 'pfx is specified together with cert, key or passphrase'],
   [{
@@ -76,10 +77,10 @@ const kValidationSubTests: [BrowserContextOptions, string ][] = [
     clientCertificates: [{
       url: 'test',
       certs: [{
-        cert: 'foo',
-        key: 'foo',
-        passphrase: 'foo',
-        pfx: 'foo',
+        cert: kDummyFileName,
+        key: kDummyFileName,
+        passphrase: kDummyFileName,
+        pfx: kDummyFileName,
       }]
     }] }, 'Cannot specify both proxy and clientCertificates'],
 ];