From 92eb8e5090037ff35385f4655178c06bb2969a0a Mon Sep 17 00:00:00 2001 From: Max Schmitt Date: Mon, 30 Jan 2023 17:44:26 +0100 Subject: [PATCH] chore: make connectOverCDP work with localhost (#20396) This wraps happy eyeballs in two places, the place where we make the JSON request to Chromium and the actual CDP WebSocket request. It required changes inside our happy eyeballs implementation since the [websocket library does not set](https://github.com/websockets/ws/blob/master/lib/websocket.js#L714) the `clientRequestOptions.hostname` field, it just sets the `host` field where we then fall back to when its not set. This test would pass before Node.js 18 and fail after Node.js 18 without my changes. Fixes https://github.com/microsoft/playwright/issues/20364 --- .../src/server/chromium/chromium.ts | 6 +++-- .../src/server/happy-eyeballs.ts | 25 +++++++++++++------ .../playwright-core/src/server/transport.ts | 2 ++ tests/library/chromium/chromium.spec.ts | 15 +++++++++++ 4 files changed, 38 insertions(+), 10 deletions(-) diff --git a/packages/playwright-core/src/server/chromium/chromium.ts b/packages/playwright-core/src/server/chromium/chromium.ts index d1239b7d6f..f8c5b8675e 100644 --- a/packages/playwright-core/src/server/chromium/chromium.ts +++ b/packages/playwright-core/src/server/chromium/chromium.ts @@ -51,6 +51,7 @@ import { registry } from '../registry'; import { ManualPromise } from '../../utils/manualPromise'; import { validateBrowserContextOptions } from '../browserContext'; import { chromiumSwitches } from './chromiumSwitches'; +import { httpHappyEyeballsAgent, httpsHappyEyeballsAgent } from '../happy-eyeballs'; const ARTIFACTS_FOLDER = path.join(os.tmpdir(), 'playwright-artifacts-'); @@ -337,10 +338,11 @@ async function urlToWSEndpoint(progress: Progress, endpointURL: string) { return endpointURL; progress.log(` retrieving websocket url from ${endpointURL}`); const httpURL = endpointURL.endsWith('/') ? `${endpointURL}json/version/` : `${endpointURL}/json/version/`; - const request = endpointURL.startsWith('https') ? https : http; + const isHTTPS = endpointURL.startsWith('https://'); const json = await new Promise((resolve, reject) => { - request.get(httpURL, { + (isHTTPS ? https : http).get(httpURL, { timeout: NET_DEFAULT_TIMEOUT, + agent: isHTTPS ? httpsHappyEyeballsAgent : httpHappyEyeballsAgent, }, resp => { if (resp.statusCode! < 200 || resp.statusCode! >= 400) { reject(new Error(`Unexpected status ${resp.statusCode} when connecting to ${httpURL}.\n` + diff --git a/packages/playwright-core/src/server/happy-eyeballs.ts b/packages/playwright-core/src/server/happy-eyeballs.ts index e346b6411c..4b80c010a4 100644 --- a/packages/playwright-core/src/server/happy-eyeballs.ts +++ b/packages/playwright-core/src/server/happy-eyeballs.ts @@ -31,27 +31,28 @@ const connectionAttemptDelayMs = 300; class HttpHappyEyeballsAgent extends http.Agent { createConnection(options: http.ClientRequestArgs, oncreate?: (err: Error | null, socket?: net.Socket) => void): net.Socket | undefined { // There is no ambiguity in case of IP address. - if (net.isIP(options.hostname!)) + if (net.isIP(clientRequestArgsToHostName(options))) return net.createConnection(options as net.NetConnectOpts); - createConnectionAsync(options, oncreate).catch(err => oncreate?.(err)); + createConnectionAsync(options, oncreate, /* useTLS */ false).catch(err => oncreate?.(err)); } } class HttpsHappyEyeballsAgent extends https.Agent { createConnection(options: http.ClientRequestArgs, oncreate?: (err: Error | null, socket?: net.Socket) => void): net.Socket | undefined { // There is no ambiguity in case of IP address. - if (net.isIP(options.hostname!)) + if (net.isIP(clientRequestArgsToHostName(options))) return tls.connect(options as tls.ConnectionOptions); - createConnectionAsync(options, oncreate).catch(err => oncreate?.(err)); + createConnectionAsync(options, oncreate, /* useTLS */ true).catch(err => oncreate?.(err)); } } export const httpsHappyEyeballsAgent = new HttpsHappyEyeballsAgent(); export const httpHappyEyeballsAgent = new HttpHappyEyeballsAgent(); -async function createConnectionAsync(options: http.ClientRequestArgs, oncreate?: (err: Error | null, socket?: net.Socket) => void) { +async function createConnectionAsync(options: http.ClientRequestArgs, oncreate: ((err: Error | null, socket?: net.Socket) => void) | undefined, useTLS: boolean) { const lookup = (options as SendRequestOptions).__testHookLookup || lookupAddresses; - const addresses = await lookup(options.hostname!); + const hostname = clientRequestArgsToHostName(options); + const addresses = await lookup(hostname); const sockets = new Set(); let firstError; let errorCount = 0; @@ -66,12 +67,12 @@ async function createConnectionAsync(options: http.ClientRequestArgs, oncreate?: const connected = new ManualPromise(); for (const { address } of addresses) { - const socket = options.protocol === 'https:' ? + const socket = useTLS ? tls.connect({ ...(options as tls.ConnectionOptions), port: options.port as number, host: address, - servername: options.hostname || undefined }) : + servername: hostname }) : net.createConnection({ ...options, port: options.port as number, @@ -126,3 +127,11 @@ async function lookupAddresses(hostname: string): Promise { return result; } +function clientRequestArgsToHostName(options: http.ClientRequestArgs): string { + if (options.hostname) + return options.hostname; + if (options.host) + return options.host.split(':')[0]; + throw new Error('Either options.hostname or options.host must be provided'); +} + diff --git a/packages/playwright-core/src/server/transport.ts b/packages/playwright-core/src/server/transport.ts index 9bc213267d..e20b1ec085 100644 --- a/packages/playwright-core/src/server/transport.ts +++ b/packages/playwright-core/src/server/transport.ts @@ -20,6 +20,7 @@ import type { WebSocket } from '../utilsBundle'; import type { ClientRequest, IncomingMessage } from 'http'; import type { Progress } from './progress'; import { makeWaitForNextTask } from '../utils'; +import { httpHappyEyeballsAgent, httpsHappyEyeballsAgent } from './happy-eyeballs'; export type ProtocolRequest = { id: number; @@ -100,6 +101,7 @@ export class WebSocketTransport implements ConnectionTransport { handshakeTimeout: Math.max(progress?.timeUntilDeadline() ?? 30_000, 1), headers, followRedirects, + agent: (/^(https|wss):\/\//.test(url)) ? httpsHappyEyeballsAgent : httpHappyEyeballsAgent }); this._progress = progress; // The 'ws' module in node sometimes sends us multiple messages in a single task. diff --git a/tests/library/chromium/chromium.spec.ts b/tests/library/chromium/chromium.spec.ts index 05cd1fee44..c3e1ab4939 100644 --- a/tests/library/chromium/chromium.spec.ts +++ b/tests/library/chromium/chromium.spec.ts @@ -548,6 +548,21 @@ playwrightTest('should use proxy with connectOverCDP', async ({ browserType, ser } }); +playwrightTest('should be able to connect via localhost', async ({ browserType }, testInfo) => { + const port = 9339 + testInfo.workerIndex; + const browserServer = await browserType.launch({ + args: ['--remote-debugging-port=' + port] + }); + try { + const cdpBrowser = await browserType.connectOverCDP(`http://localhost:${port}`); + const contexts = cdpBrowser.contexts(); + expect(contexts.length).toBe(1); + await cdpBrowser.close(); + } finally { + await browserServer.close(); + } +}); + playwrightTest('should pass args with spaces', async ({ browserType, createUserDataDir }, testInfo) => { const browser = await browserType.launchPersistentContext(await createUserDataDir(), { args: ['--user-agent=I am Foo']