From a4e28529aae0502cd0f970d74fae94b09d6c7c81 Mon Sep 17 00:00:00 2001 From: Max Schmitt Date: Mon, 17 May 2021 15:11:46 +0200 Subject: [PATCH] fixed client side error handling --- src/client/tcpSocket.ts | 30 ++++++++- src/dispatchers/tcpSocketDispatcher.ts | 10 ++- src/protocol/channels.ts | 12 ++++ src/protocol/protocol.yml | 11 ++++ src/protocol/validator.ts | 4 ++ src/server/socksServer.ts | 85 ++++++++++++++++++++------ src/server/tcpSocket.ts | 20 +++--- tests/portForwardingServer.spec.ts | 45 ++++++++++++-- 8 files changed, 183 insertions(+), 34 deletions(-) diff --git a/src/client/tcpSocket.ts b/src/client/tcpSocket.ts index 95388d263afe6..b7ebf4c33cec4 100644 --- a/src/client/tcpSocket.ts +++ b/src/client/tcpSocket.ts @@ -35,8 +35,11 @@ export class TCPSocket extends ChannelOwner this.end().catch(() => {})); - this._socket.on('data', data => this.write(data).catch(() => {})); + this._socket.on('error', (err: NodeJS.ErrnoException) => this._handleError(err)); + this._socket.on('connect', () => { + this.connected().catch(() => {}); + this._socket.on('data', data => this.write(data).catch(() => {})); + }); this._socket.on('close', () => { this.end().catch(() => {}); }); @@ -56,4 +59,27 @@ export class TCPSocket extends ChannelOwner { await this._channel.end(); } + + _handleError(err: NodeJS.ErrnoException) { + let code: undefined | 'hostUnreachable' | 'networkUnreachable' | 'connectionRefused'; + switch (err.code) { + case 'ENOENT': + case 'ENOTFOUND': + case 'ETIMEDOUT': + case 'EHOSTUNREACH': + code = 'hostUnreachable'; + break; + case 'ENETUNREACH': + code = 'networkUnreachable'; + break; + case 'ECONNREFUSED': + code = 'connectionRefused'; + break; + } + this._channel.error({code}); + } + + async connected(): Promise { + await this._channel.connected(); + } } diff --git a/src/dispatchers/tcpSocketDispatcher.ts b/src/dispatchers/tcpSocketDispatcher.ts index 13753c7f43bc5..e96ef927053fc 100644 --- a/src/dispatchers/tcpSocketDispatcher.ts +++ b/src/dispatchers/tcpSocketDispatcher.ts @@ -30,12 +30,18 @@ export class TCPSocketDispatcher extends Dispatcher { + this._object._socketHandler.connected(); + } + async error(params: channels.TCPSocketErrorOptions): Promise { + this._object._socketHandler.error(params.code); + } async write(params: channels.AndroidSocketWriteParams): Promise { - this._object._socket.write(Buffer.from(params.data, 'base64')); + this._object._socketHandler.write(Buffer.from(params.data, 'base64')); } async end(): Promise { - this._object._socket.end(); + this._object._socketHandler.end(); } } diff --git a/src/protocol/channels.ts b/src/protocol/channels.ts index b9c0fa6ae4abe..04ddf82bf4f08 100644 --- a/src/protocol/channels.ts +++ b/src/protocol/channels.ts @@ -3071,6 +3071,8 @@ export interface TCPSocketChannel extends Channel { on(event: 'data', callback: (params: TCPSocketDataEvent) => void): this; on(event: 'close', callback: (params: TCPSocketCloseEvent) => void): this; write(params: TCPSocketWriteParams, metadata?: Metadata): Promise; + error(params: TCPSocketErrorParams, metadata?: Metadata): Promise; + connected(params?: TCPSocketConnectedParams, metadata?: Metadata): Promise; end(params?: TCPSocketEndParams, metadata?: Metadata): Promise; } export type TCPSocketDataEvent = { @@ -3084,6 +3086,16 @@ export type TCPSocketWriteOptions = { }; export type TCPSocketWriteResult = void; +export type TCPSocketErrorParams = { + code?: 'connectionRefused' | 'networkUnreachable' | 'hostUnreachable', +}; +export type TCPSocketErrorOptions = { + code?: 'connectionRefused' | 'networkUnreachable' | 'hostUnreachable', +}; +export type TCPSocketErrorResult = void; +export type TCPSocketConnectedParams = {}; +export type TCPSocketConnectedOptions = {}; +export type TCPSocketConnectedResult = void; export type TCPSocketEndParams = {}; export type TCPSocketEndOptions = {}; export type TCPSocketEndResult = void; diff --git a/src/protocol/protocol.yml b/src/protocol/protocol.yml index 722985e2642ad..f9af78ae744d5 100644 --- a/src/protocol/protocol.yml +++ b/src/protocol/protocol.yml @@ -2492,6 +2492,17 @@ TCPSocket: parameters: data: binary + error: + parameters: + code: + type: enum? + literals: + - connectionRefused + - networkUnreachable + - hostUnreachable + + connected: + end: events: diff --git a/src/protocol/validator.ts b/src/protocol/validator.ts index 951ce8cd90b5c..60d5f6c1ef9f2 100644 --- a/src/protocol/validator.ts +++ b/src/protocol/validator.ts @@ -1195,6 +1195,10 @@ export function createScheme(tChannel: (name: string) => Validator): Scheme { scheme.TCPSocketWriteParams = tObject({ data: tBinary, }); + scheme.TCPSocketErrorParams = tObject({ + code: tOptional(tEnum(['connectionRefused', 'networkUnreachable', 'hostUnreachable'])), + }); + scheme.TCPSocketConnectedParams = tOptional(tObject({})); scheme.TCPSocketEndParams = tOptional(tObject({})); return scheme; diff --git a/src/server/socksServer.ts b/src/server/socksServer.ts index b0a4c9203b264..3cc564bc9f407 100644 --- a/src/server/socksServer.ts +++ b/src/server/socksServer.ts @@ -53,7 +53,7 @@ enum SOCKS_ATYP { IPv6 = 0x04 } -enum SOCKS_REP { +enum SOCKS_REPLY { SUCCESS = 0x00, GENFAIL = 0x01, DISALLOW = 0x02, @@ -67,7 +67,7 @@ enum SOCKS_REP { const BUF_REP_INTR_SUCCESS = Buffer.from([ 0x05, - SOCKS_REP.SUCCESS, + SOCKS_REPLY.SUCCESS, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, @@ -228,7 +228,7 @@ class SocksV5ServerParser { socket.write(Buffer.from([0x05, 0x00])); } - async ready(): Promise<{ info: SocksConnectionInfo, forward: () => void, intercept: () => net.Socket }> { + async ready(): Promise<{ info: SocksConnectionInfo, forward: () => void, intercept: () => SocksInterceptedHandler }> { await this._ready; return { info: this._info, @@ -237,13 +237,13 @@ class SocksV5ServerParser { this._socket.on('close', () => dstSocket.end()); this._socket.on('end', () => dstSocket.end()); dstSocket.setKeepAlive(false); - dstSocket.on('error', (err: NodeJS.ErrnoException) => handleProxyError(this._socket, err)); + dstSocket.on('error', (err: NodeJS.ErrnoException) => handleProxyError(this._socket, err.code)); dstSocket.on('connect', () => { const localbytes = [127, 0, 0, 1]; const bufrep = Buffer.alloc(6 + localbytes.length); let p = 4; bufrep[0] = 0x05; - bufrep[1] = SOCKS_REP.SUCCESS; + bufrep[1] = SOCKS_REPLY.SUCCESS; bufrep[2] = 0x00; bufrep[3] = SOCKS_ATYP.IPv4; for (let i = 0; i < localbytes.length; ++i, ++p) @@ -256,31 +256,60 @@ class SocksV5ServerParser { }).connect(this._info.dstPort, this._info.dstAddr); }, - intercept: (): net.Socket => { - this._socket.write(BUF_REP_INTR_SUCCESS); - this._socket.resume(); - return this._socket; + intercept: (): SocksInterceptedHandler => { + return new SocksInterceptedHandler(this._socket); }, }; } } -function handleProxyError(socket: net.Socket, err: NodeJS.ErrnoException): void { +export type SOCKS_SOCKET_ERRORS = 'connectionRefused' | 'networkUnreachable' | 'hostUnreachable' + +const ERROR_2_SOCKS_REPLY = new Map([ + ['connectionRefused', SOCKS_REPLY.CONNREFUSED], + ['networkUnreachable', SOCKS_REPLY.CONNREFUSED], + ['hostUnreachable', SOCKS_REPLY.CONNREFUSED], +]); + +export class SocksInterceptedHandler { + socket: net.Socket; + constructor(socket: net.Socket) { + this.socket = socket; + } + connected() { + this.socket.write(BUF_REP_INTR_SUCCESS); + this.socket.resume(); + } + error(status?: SOCKS_SOCKET_ERRORS) { + let reply = SOCKS_REPLY.GENFAIL; + if (status && ERROR_2_SOCKS_REPLY.has(status)) + reply = ERROR_2_SOCKS_REPLY.get(status)!; + this.socket.end(Buffer.from([0x05, reply])); + } + write(data: Buffer) { + this.socket.write(data); + } + end() { + this.socket.end(); + } +} + +function handleProxyError(socket: net.Socket, errCode?: string): void { if (socket.writable) { - const errbuf = Buffer.from([0x05, SOCKS_REP.GENFAIL]); - if (err.code) { - switch (err.code) { + const errbuf = Buffer.from([0x05, SOCKS_REPLY.GENFAIL]); + if (errCode) { + switch (errCode) { case 'ENOENT': case 'ENOTFOUND': case 'ETIMEDOUT': case 'EHOSTUNREACH': - errbuf[1] = SOCKS_REP.HOSTUNREACH; + errbuf[1] = SOCKS_REPLY.HOSTUNREACH; break; case 'ENETUNREACH': - errbuf[1] = SOCKS_REP.NETUNREACH; + errbuf[1] = SOCKS_REPLY.NETUNREACH; break; case 'ECONNREFUSED': - errbuf[1] = SOCKS_REP.CONNREFUSED; + errbuf[1] = SOCKS_REPLY.CONNREFUSED; break; } } @@ -288,7 +317,7 @@ function handleProxyError(socket: net.Socket, err: NodeJS.ErrnoException): void } } -type IncomingProxyRequestHandler = (info: SocksConnectionInfo, forward: () => void, intercept: () => net.Socket) => void +type IncomingProxyRequestHandler = (info: SocksConnectionInfo, forward: () => void, intercept: () => SocksInterceptedHandler) => void export class SocksProxyServer { public server: net.Server; @@ -310,3 +339,25 @@ export class SocksProxyServer { this.server.close(); } } + + +/** s + * const server = new SocksProxyServer((info: SocksConnectionInfo, forward: () => void, intercept: () => net.Socket) => { + console.log(info) + if (info.dstAddr === '1s27.0.0.1') { + const socket = intercept(); + const body = 'Hello ' + info.srcAddr + '!\n\nToday is: ' + (new Date()); + socket.end([ + 'HTTP/1.1 200 OK', + 'Connection: close', + 'Content-Type: text/plain', + 'Content-Length: ' + Buffer.byteLength(body), + '', + body + ].join('\r\n')); + return; + } + forward(); +}); +server.listen(1080, 'localhost'); + */ \ No newline at end of file diff --git a/src/server/tcpSocket.ts b/src/server/tcpSocket.ts index 8139b7861c4ee..6bbda4fe2fe54 100644 --- a/src/server/tcpSocket.ts +++ b/src/server/tcpSocket.ts @@ -20,7 +20,7 @@ import { EventEmitter } from 'events'; import { SdkObject } from './instrumentation'; import { debugLogger } from '../utils/debugLogger'; import { isLocalIpAddress } from '../utils/utils'; -import { SocksProxyServer, SocksConnectionInfo } from './socksServer'; +import { SocksProxyServer, SocksConnectionInfo, SocksInterceptedHandler, SOCKS_SOCKET_ERRORS } from './socksServer'; import { LaunchOptions } from './types'; export class BrowserServerPortForwardingServer extends EventEmitter { @@ -54,7 +54,7 @@ export class BrowserServerPortForwardingServer extends EventEmitter { }; } - private _handler = (info: SocksConnectionInfo, forward: () => void, intercept: () => net.Socket): void => { + private _handler = (info: SocksConnectionInfo, forward: () => void, intercept: () => SocksInterceptedHandler): void => { const shouldProxyRequestToClient = isLocalIpAddress(info.dstAddr) && this._forwardPorts.includes(info.dstPort); debugLogger.log('proxy', `incoming connection from ${info.srcAddr}:${info.srcPort} to ${info.dstAddr}:${info.dstPort} shouldProxyRequestToClient=${shouldProxyRequestToClient}`); if (!shouldProxyRequestToClient) { @@ -79,15 +79,21 @@ export class BrowserServerPortForwardingServer extends EventEmitter { } export class TCPSocket extends SdkObject { - _socket: net.Socket + _socketHandler: SocksInterceptedHandler _dstAddr: string _dstPort: number - constructor(parent: SdkObject, socket: net.Socket, dstAddr: string, dstPort: number) { + constructor(parent: SdkObject, handler: SocksInterceptedHandler, dstAddr: string, dstPort: number) { super(parent, 'TCPSocket'); - this._socket = socket; + this._socketHandler = handler; this._dstAddr = dstAddr; this._dstPort = dstPort; - socket.on('data', data => this.emit('data', data)); - socket.on('close', data => this.emit('close', data)); + handler.socket.on('data', data => this.emit('data', data)); + handler.socket.on('close', data => this.emit('close', data)); + } + connected() { + this._socketHandler.connected(); + } + error(error: SOCKS_SOCKET_ERRORS) { + this._socketHandler.error(error); } } diff --git a/tests/portForwardingServer.spec.ts b/tests/portForwardingServer.spec.ts index 4266e87c3fc2f..14d34136c6bef 100644 --- a/tests/portForwardingServer.spec.ts +++ b/tests/portForwardingServer.spec.ts @@ -85,7 +85,34 @@ it.describe('forwarding proxy', () => { await browserServer.close(); }); - it('should lead to a request failure if the proxied target will timeout', async ({ browserType, browserOptions }, workerInfo) => { + it('should lead to a connection refused error for forwarded requests', async ({ browserType, browserOptions, browserName, isWindows}, workerInfo) => { + const examplePort = 20_000 + workerInfo.workerIndex * 3; + const browserServer = await browserType.launchServer({ + ...browserOptions, + _acceptForwardedPorts: true + } as LaunchOptions); + const browser = await browserType.connect({ + wsEndpoint: browserServer.wsEndpoint(), + _forwardPorts: [examplePort] + } as ConnectOptions); + const page = await browser.newPage(); + let error: Error; + await page.goto(`http://localhost:${examplePort}`).catch(e => error = e); + + if (browserName === 'chromium') + expect(error.message).toContain('net::ERR_SOCKS_CONNECTION_FAILED'); + else if (browserName === 'webkit' && isWindows) + expect(error.message).toContain(`Couldn\'t connect to server`); + else if (browserName === 'webkit') + expect(error.message).toContain('Could not connect'); + else + expect(error.message).toContain('NS_ERROR_CONNECTION_REFUSED'); + await browserServer.close(); + }); + + it('should lead to a request failure if the proxied target will timeout', async ({ browserName, browserType, browserOptions, isWindows}, workerInfo) => { + if (browserName === 'webkit') + it.fixme(); // https://github.com/microsoft/playwright/issues/6613 process.env.PW_TEST_PROXY_TARGET = '50001'; const browserServer = await browserType.launchServer({ ...browserOptions, @@ -97,11 +124,17 @@ it.describe('forwarding proxy', () => { _forwardPorts: [examplePort] } as ConnectOptions); const page = await browser.newPage(); - const failedRequests = []; - page.on('requestfailed', request => failedRequests.push(request)); - await expect(page.goto(`http://localhost:${examplePort}`)).rejects.toThrowError(); - expect(failedRequests.length).toBe(1); - expect(failedRequests[0].failure().errorText).toBeTruthy(); + let error: Error; + await page.goto(`http://localhost:44123/non-existing-url`).catch(e => error = e); + + if (browserName === 'chromium') + expect(error.message).toContain('net::ERR_SOCKS_CONNECTION_FAILED'); + else if (browserName === 'webkit' && isWindows) + expect(error.message).toContain(`Couldn\'t connect to server`); + else if (browserName === 'webkit') + expect(error.message).toContain('Could not connect'); + else + expect(error.message).toContain('NS_ERROR_CONNECTION_REFUSED'); await browserServer.close(); });