Filip/playwright
1
0
Fork 0
Code Issues Pull requests Actions Packages Projects Releases Wiki Activity

fix(connect): do not log ws query params (#20047)

Browse source
This commit is contained in:
Dmitry Gozman 2023-01-11 12:27:27 -08:00 committed by GitHub
parent bd4efa9dc1
commit 4f837690e2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 32 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

32
packages/playwright-core/src/server/transport.ts
View file

@ -49,14 +49,16 @@ export interface ConnectionTransport {
export class WebSocketTransport implements ConnectionTransport { export class WebSocketTransport implements ConnectionTransport {
private _ws: WebSocket; private _ws: WebSocket;
private _progress?: Progress; private _progress?: Progress;
private _logUrl: string;
onmessage?: (message: ProtocolResponse) => void; onmessage?: (message: ProtocolResponse) => void;
onclose?: () => void; onclose?: () => void;
readonly wsEndpoint: string; readonly wsEndpoint: string;
static async connect(progress: (Progress|undefined), url: string, headers?: { [key: string]: string; }, followRedirects?: boolean): Promise<WebSocketTransport> { static async connect(progress: (Progress|undefined), url: string, headers?: { [key: string]: string; }, followRedirects?: boolean): Promise<WebSocketTransport> {
progress?.log(`<ws connecting> ${url}`); const logUrl = stripQueryParams(url);
const transport = new WebSocketTransport(progress, url, headers, followRedirects); progress?.log(`<ws connecting> ${logUrl}`);
const transport = new WebSocketTransport(progress, url, logUrl, headers, followRedirects);
let success = false; let success = false;
progress?.cleanupWhenAborted(async () => { progress?.cleanupWhenAborted(async () => {
if (!success) if (!success)
@ -64,17 +66,17 @@ export class WebSocketTransport implements ConnectionTransport {
}); });
await new Promise<WebSocketTransport>((fulfill, reject) => { await new Promise<WebSocketTransport>((fulfill, reject) => {
transport._ws.on('open', async () => { transport._ws.on('open', async () => {
progress?.log(`<ws connected> ${url}`); progress?.log(`<ws connected> ${logUrl}`);
fulfill(transport); fulfill(transport);
}); });
transport._ws.on('error', event => { transport._ws.on('error', event => {
progress?.log(`<ws connect error> ${url} ${event.message}`); progress?.log(`<ws connect error> ${logUrl} ${event.message}`);
reject(new Error('WebSocket error: ' + event.message)); reject(new Error('WebSocket error: ' + event.message));
transport._ws.close(); transport._ws.close();
}); });
transport._ws.on('unexpected-response', (request: ClientRequest, response: IncomingMessage) => { transport._ws.on('unexpected-response', (request: ClientRequest, response: IncomingMessage) => {
const chunks: Buffer[] = []; const chunks: Buffer[] = [];
const errorPrefix = `${url} ${response.statusCode} ${response.statusMessage}`; const errorPrefix = `${logUrl} ${response.statusCode} ${response.statusMessage}`;
response.on('data', chunk => chunks.push(chunk)); response.on('data', chunk => chunks.push(chunk));
response.on('close', () => { response.on('close', () => {
const error = chunks.length ? `${errorPrefix}\n${Buffer.concat(chunks)}` : errorPrefix; const error = chunks.length ? `${errorPrefix}\n${Buffer.concat(chunks)}` : errorPrefix;
@ -88,8 +90,9 @@ export class WebSocketTransport implements ConnectionTransport {
return transport; return transport;
} }
constructor(progress: Progress|undefined, url: string, headers?: { [key: string]: string; }, followRedirects?: boolean) { constructor(progress: Progress|undefined, url: string, logUrl: string, headers?: { [key: string]: string; }, followRedirects?: boolean) {
this.wsEndpoint = url; this.wsEndpoint = url;
this._logUrl = logUrl;
this._ws = new ws(url, [], { this._ws = new ws(url, [], {
perMessageDeflate: false, perMessageDeflate: false,
maxPayload: 256 * 1024 * 1024, // 256Mb, maxPayload: 256 * 1024 * 1024, // 256Mb,
@ -117,12 +120,12 @@ export class WebSocketTransport implements ConnectionTransport {
}); });
this._ws.addEventListener('close', event => { this._ws.addEventListener('close', event => {
this._progress?.log(`<ws disconnected> ${url} code=${event.code} reason=${event.reason}`); this._progress?.log(`<ws disconnected> ${logUrl} code=${event.code} reason=${event.reason}`);
if (this.onclose) if (this.onclose)
this.onclose.call(null); this.onclose.call(null);
}); });
// Prevent Error: read ECONNRESET. // Prevent Error: read ECONNRESET.
this._ws.addEventListener('error', error => this._progress?.log(`<ws error> ${error.type} ${error.message}`)); this._ws.addEventListener('error', error => this._progress?.log(`<ws error> ${logUrl} ${error.type} ${error.message}`));
} }
send(message: ProtocolRequest) { send(message: ProtocolRequest) {
@ -130,7 +133,7 @@ export class WebSocketTransport implements ConnectionTransport {
} }
close() { close() {
this._progress?.log(`<ws disconnecting> ${this._ws.url}`); this._progress?.log(`<ws disconnecting> ${this._logUrl}`);
this._ws.close(); this._ws.close();
} }
@ -142,3 +145,14 @@ export class WebSocketTransport implements ConnectionTransport {
await promise; // Make sure to await the actual disconnect. await promise; // Make sure to await the actual disconnect.
} }
} }
function stripQueryParams(url: string): string {
try {
const u = new URL(url);
u.search = '';
u.hash = '';
return u.toString();
} catch {
return url;
}
}

11
tests/library/browsertype-connect.spec.ts
View file

@ -110,13 +110,20 @@ for (const kind of ['launchServer', 'run-server'] as const) {
} }
}); });
test('should print HTTP error', async ({ connect, server, mode }) => { test('should print HTTP error', async ({ connect, server }) => {
test.skip(mode !== 'default'); // Out of process transport does not allow us to set env vars dynamically.
const error = await connect(`ws://localhost:${server.PORT}/ws-401`).catch(e => e); const error = await connect(`ws://localhost:${server.PORT}/ws-401`).catch(e => e);
expect(error.message).toContain('401'); expect(error.message).toContain('401');
expect(error.message).toContain('Unauthorized body'); expect(error.message).toContain('Unauthorized body');
}); });
test('should print ws error', async ({ connect, server }) => {
const error = await connect(`ws://does-not-exist.problem-domain:10987?secret=MYSECRET`).catch(e => e);
expect(error.message).toContain('<ws connecting> ws://does-not-exist.problem-domain:10987/');
expect(error.message).toContain('<ws error>');
expect(error.message).toContain('getaddrinfo');
expect(error.message).not.toContain('secret=MYSECRET');
});
test('should be able to reconnect to a browser', async ({ connect, startRemoteServer, server }) => { test('should be able to reconnect to a browser', async ({ connect, startRemoteServer, server }) => {
const remoteServer = await startRemoteServer(kind); const remoteServer = await startRemoteServer(kind);
{ {