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
This commit is contained in:
Max Schmitt 2023-01-30 17:44:26 +01:00 committed by GitHub
parent 03b15f6550
commit 92eb8e5090
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 38 additions and 10 deletions

View file

@ -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(`<ws preparing> 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<string>((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` +

View file

@ -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<net.Socket>();
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<dns.LookupAddress[]> {
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');
}

View file

@ -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.

View file

@ -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']