Reapply "fix(har timing): record connect timing for proxied connections" (#32855)

packages/playwright-core/bundles/utils/package.json

@@ -14,7 +14,7 @@
     "debug": "^4.3.4",
     "dotenv": "^16.4.5",
     "graceful-fs": "4.2.10",
-    "https-proxy-agent": "5.0.0",
+    "https-proxy-agent": "7.0.5",
     "jpeg-js": "0.4.4",
     "mime": "^3.0.0",
     "minimatch": "^3.1.2",
@@ -24,7 +24,7 @@
     "proxy-from-env": "1.1.0",
     "retry": "0.12.0",
     "signal-exit": "3.0.7",
-    "socks-proxy-agent": "6.1.1",
+    "socks-proxy-agent": "8.0.4",
     "stack-utils": "2.0.5",
     "ws": "8.17.1"
   },

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

@@ -20,7 +20,6 @@ import http from 'http';
 import https from 'https';
 import type { Readable, TransformCallback } from 'stream';
 import { pipeline, Transform } from 'stream';
-import url from 'url';
 import zlib from 'zlib';
 import type { HTTPCredentials } from '../../types/types';
 import { TimeoutSettings } from '../common/timeoutSettings';
@@ -493,12 +492,12 @@ export abstract class APIRequestContext extends SdkObject {
         // happy eyeballs don't emit lookup and connect events, so we use our custom ones
         const happyEyeBallsTimings = timingForSocket(socket);
         dnsLookupAt = happyEyeBallsTimings.dnsLookupAt;
-        tcpConnectionAt = happyEyeBallsTimings.tcpConnectionAt;
+        tcpConnectionAt ??= happyEyeBallsTimings.tcpConnectionAt;
 
         // non-happy-eyeballs sockets
         listeners.push(
             eventsHelper.addEventListener(socket, 'lookup', () => { dnsLookupAt = monotonicTime(); }),
-            eventsHelper.addEventListener(socket, 'connect', () => { tcpConnectionAt = monotonicTime(); }),
+            eventsHelper.addEventListener(socket, 'connect', () => { tcpConnectionAt ??= monotonicTime(); }),
             eventsHelper.addEventListener(socket, 'secureConnect', () => {
               tlsHandshakeAt = monotonicTime();
 
@@ -515,11 +514,21 @@ export abstract class APIRequestContext extends SdkObject {
             }),
         );
 
+        // when using socks proxy, having the socket means the connection got established
+        if (agent instanceof SocksProxyAgent)
+          tcpConnectionAt ??= monotonicTime();
+
         serverIPAddress = socket.remoteAddress;
         serverPort = socket.remotePort;
       });
       request.on('finish', () => { requestFinishAt = monotonicTime(); });
 
+      // http proxy
+      request.on('proxyConnect', () => {
+        tcpConnectionAt ??= monotonicTime();
+      });
+
+
       progress.log(`→ ${options.method} ${url.toString()}`);
       if (options.headers) {
         for (const [name, value] of Object.entries(options.headers))
@@ -686,17 +695,16 @@ 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,
-    });
-  }
+  const proxyURL = new URL(proxy.server);
+  if (proxyURL.protocol?.startsWith('socks'))
+    return new SocksProxyAgent(proxyURL);
+
   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);
+    proxyURL.username = proxy.username;
+  if (proxy.password)
+    proxyURL.password = proxy.password;
+  // TODO: We should use HttpProxyAgent conditional on proxyURL.protocol instead of always using CONNECT method.
+  return new HttpsProxyAgent(proxyURL);
 }
 
 function toHeadersArray(rawHeaders: string[]): types.HeadersArray {

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

@@ -98,7 +98,7 @@ class SocksProxyConnection {
 
   async connect() {
     if (this.socksProxy.proxyAgentFromOptions)
-      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.proxyAgentFromOptions.connect(new EventEmitter() as any, { host: rewriteToLocalhostIfNeeded(this.host), port: this.port, secureEndpoint: false });
     else
       this.target = await createSocket(rewriteToLocalhostIfNeeded(this.host), this.port);
 

packages/playwright-core/src/utils/network.ts

@@ -50,7 +50,7 @@ export function httpRequest(params: HTTPRequestParams, onResponse: (r: http.Inco
 
   const proxyURL = getProxyForUrl(params.url);
   if (proxyURL) {
-    const parsedProxyURL = url.parse(proxyURL);
+    const parsedProxyURL = new URL(proxyURL);
     if (params.url.startsWith('http:')) {
       options = {
         path: parsedUrl.href,

tests/config/proxy.ts

@@ -114,7 +114,7 @@ export async function setupSocksForwardingServer({
   const socksProxy = new SocksProxy();
   socksProxy.setPattern('*');
   socksProxy.addListener(SocksProxy.Events.SocksRequested, async (payload: SocksSocketRequestedPayload) => {
-    if (!['127.0.0.1', 'fake-localhost-127-0-0-1.nip.io', 'localhost'].includes(payload.host) || payload.port !== allowedTargetPort) {
+    if (!['127.0.0.1', '0:0:0:0:0:0:0:1', 'fake-localhost-127-0-0-1.nip.io', 'localhost'].includes(payload.host) || payload.port !== allowedTargetPort) {
       socksProxy.sendSocketError({ uid: payload.uid, error: 'ECONNREFUSED' });
       return;
     }

tests/config/serverFixtures.ts

@@ -51,7 +51,7 @@ export const serverFixtures: Fixtures<ServerFixtures, ServerWorkerOptions> = {
 
     const socksServer = new MockSocksServer();
     const socksPort = port + 2;
-    await socksServer.listen(socksPort, 'localhost');
+    await socksServer.listen(socksPort, loopback);
 
     const proxyPort = port + 3;
     const proxyServer = await TestProxy.create(proxyPort);

tests/library/client-certificates.spec.ts

@@ -390,7 +390,7 @@ test.describe('browser', () => {
     });
     expect(connectHosts).toEqual([]);
     await page.goto(serverURL);
-    const host = browserName === 'webkit' && isMac ? 'localhost' : '127.0.0.1';
+    const host = browserName === 'webkit' && isMac ? '0:0:0:0:0:0:0:1' : '127.0.0.1';
     expect(connectHosts).toEqual([`${host}:${serverPort}`]);
     await expect(page.getByTestId('message')).toHaveText('Hello Alice, your certificate was issued by localhost!');
     await page.close();

tests/library/har.spec.ts

@@ -24,9 +24,9 @@ import type { Log } from '../../packages/trace/src/har';
 import { parseHar } from '../config/utils';
 const { createHttp2Server } = require('../../packages/playwright-core/lib/utils');
 
-async function pageWithHar(contextFactory: (options?: BrowserContextOptions) => Promise<BrowserContext>, testInfo: any, options: { outputPath?: string } & Partial<Pick<BrowserContextOptions['recordHar'], 'content' | 'omitContent' | 'mode'>> = {}) {
+async function pageWithHar(contextFactory: (options?: BrowserContextOptions) => Promise<BrowserContext>, testInfo: any, options: { outputPath?: string, proxy?: BrowserContextOptions['proxy'] } & Partial<Pick<BrowserContextOptions['recordHar'], 'content' | 'omitContent' | 'mode'>> = {}) {
   const harPath = testInfo.outputPath(options.outputPath || 'test.har');
-  const context = await contextFactory({ recordHar: { path: harPath, ...options }, ignoreHTTPSErrors: true });
+  const context = await contextFactory({ recordHar: { path: harPath, ...options }, ignoreHTTPSErrors: true, proxy: options.proxy });
   const page = await context.newPage();
   return {
     page,
@@ -858,6 +858,25 @@ it('should respect minimal mode for API Requests', async ({ contextFactory, serv
   expect(entry.response.bodySize).toBe(-1);
 });
 
+it('should include timings when using http proxy', async ({ contextFactory, server, proxyServer }, testInfo) => {
+  proxyServer.forwardTo(server.PORT, { allowConnectRequests: true });
+  const { page, getLog } = await pageWithHar(contextFactory, testInfo, { proxy: { server: `localhost:${proxyServer.PORT}` } });
+  const response = await page.request.get(server.EMPTY_PAGE);
+  expect(proxyServer.connectHosts).toEqual([`localhost:${server.PORT}`]);
+  await expect(response).toBeOK();
+  const log = await getLog();
+  expect(log.entries[0].timings.connect).toBeGreaterThan(0);
+});
+
+it('should include timings when using socks proxy', async ({ contextFactory, server, socksPort }, testInfo) => {
+  const { page, getLog } = await pageWithHar(contextFactory, testInfo, { proxy: { server: `socks5://localhost:${socksPort}` } });
+  const response = await page.request.get(server.EMPTY_PAGE);
+  expect(await response.text()).toContain('Served by the SOCKS proxy');
+  await expect(response).toBeOK();
+  const log = await getLog();
+  expect(log.entries[0].timings.connect).toBeGreaterThan(0);
+});
+
 it('should include redirects from API request', async ({ contextFactory, server }, testInfo) => {
   server.setRedirect('/redirect-me', '/simple.json');
   const { page, getLog } = await pageWithHar(contextFactory, testInfo);