From fde427d8905db6a9dda7b02be36c9308e7f13263 Mon Sep 17 00:00:00 2001 From: Ross Wollman Date: Fri, 10 Dec 2021 14:01:56 -0800 Subject: [PATCH] feat(proxy): unify local network proxy behavior (#10719) When configuring a proxy, Chromium requires a magic tokens to get some local network requests to go through the proxy. This has tripped up a few users, so we make the behavior default to the expected: proxy everything including the local requests. This matches the other vendors as well. NB: This can be disabled via `PLAYWRIGHT_DISABLE_FORCED_CHROMIUM_PROXIED_LOOPBACK=1` Supercedes: #8345 Fixes: #10631 --- .../src/server/chromium/chromium.ts | 2 + .../src/server/chromium/crBrowser.ts | 11 +++- tests/browsercontext-proxy.spec.ts | 50 +++++++++++++++++++ tests/config/proxy.ts | 9 +++- tests/proxy.spec.ts | 49 ++++++++++++++++++ 5 files changed, 119 insertions(+), 2 deletions(-) diff --git a/packages/playwright-core/src/server/chromium/chromium.ts b/packages/playwright-core/src/server/chromium/chromium.ts index ca5c2516944a1..c654c2a85468b 100644 --- a/packages/playwright-core/src/server/chromium/chromium.ts +++ b/packages/playwright-core/src/server/chromium/chromium.ts @@ -277,6 +277,8 @@ export class Chromium extends BrowserType { proxyBypassRules.push('<-loopback>'); if (proxy.bypass) proxyBypassRules.push(...proxy.bypass.split(',').map(t => t.trim()).map(t => t.startsWith('.') ? '*' + t : t)); + if (!process.env.PLAYWRIGHT_DISABLE_FORCED_CHROMIUM_PROXIED_LOOPBACK && !proxyBypassRules.includes('<-loopback>')) + proxyBypassRules.push('<-loopback>'); if (proxyBypassRules.length > 0) chromeArguments.push(`--proxy-bypass-list=${proxyBypassRules.join(';')}`); } diff --git a/packages/playwright-core/src/server/chromium/crBrowser.ts b/packages/playwright-core/src/server/chromium/crBrowser.ts index df4ce6bff0094..aca10d61021d7 100644 --- a/packages/playwright-core/src/server/chromium/crBrowser.ts +++ b/packages/playwright-core/src/server/chromium/crBrowser.ts @@ -90,10 +90,19 @@ export class CRBrowser extends Browser { async newContext(options: types.BrowserContextOptions): Promise { validateBrowserContextOptions(options, this.options); + + let proxyBypassList = undefined; + if (options.proxy) { + if (process.env.PLAYWRIGHT_DISABLE_FORCED_CHROMIUM_PROXIED_LOOPBACK) + proxyBypassList = options.proxy.bypass; + else + proxyBypassList = '<-loopback>' + (options.proxy.bypass ? `,${options.proxy.bypass}` : ''); + } + const { browserContextId } = await this._session.send('Target.createBrowserContext', { disposeOnDetach: true, proxyServer: options.proxy ? options.proxy.server : undefined, - proxyBypassList: options.proxy ? options.proxy.bypass : undefined, + proxyBypassList, }); const context = new CRBrowserContext(this, browserContextId, options); await context._initialize(); diff --git a/tests/browsercontext-proxy.spec.ts b/tests/browsercontext-proxy.spec.ts index dc06d3eee40d4..3c7c730ed15df 100644 --- a/tests/browsercontext-proxy.spec.ts +++ b/tests/browsercontext-proxy.spec.ts @@ -91,6 +91,56 @@ it('should use proxy', async ({ contextFactory, server, proxyServer }) => { await context.close(); }); +it.describe('should proxy local network requests', () => { + for (const additionalBypass of [false, true]) { + it.describe(additionalBypass ? 'with other bypasses' : 'by default', () => { + for (const params of [ + { + target: 'localhost', + description: 'localhost', + }, + { + target: '127.0.0.1', + description: 'loopback address', + }, + { + target: '169.254.3.4', + description: 'link-local' + } + ]) { + it(`${params.description}`, async ({ platform, browserName, contextFactory, server, proxyServer }) => { + it.fail(browserName === 'webkit' && platform === 'darwin' && additionalBypass && ['localhost', '127.0.0.1'].includes(params.target), 'WK fails to proxy 127.0.0.1 and localhost if additional bypasses are present'); + + const path = `/target-${additionalBypass}-${params.target}.html`; + server.setRoute(path, async (req, res) => { + res.end('Served by the proxy'); + }); + + const url = `http://${params.target}:${server.PORT}${path}`; + proxyServer.forwardTo(server.PORT); + const context = await contextFactory({ + proxy: { server: `localhost:${proxyServer.PORT}`, bypass: additionalBypass ? '1.non.existent.domain.for.the.test' : undefined } + }); + + const page = await context.newPage(); + await page.goto(url); + expect(proxyServer.requestUrls).toContain(url); + expect(await page.title()).toBe('Served by the proxy'); + + await page.goto('http://1.non.existent.domain.for.the.test/foo.html').catch(() => {}); + if (additionalBypass) + expect(proxyServer.requestUrls).not.toContain('http://1.non.existent.domain.for.the.test/foo.html'); + else + expect(proxyServer.requestUrls).toContain('http://1.non.existent.domain.for.the.test/foo.html'); + + await context.close(); + }); + } + }); + } +}); + + it('should use ipv6 proxy', async ({ contextFactory, server, proxyServer, browserName }) => { it.fail(browserName === 'firefox', 'page.goto: NS_ERROR_UNKNOWN_HOST'); it.fail(!!process.env.INSIDE_DOCKER, 'docker does not support IPv6 by default'); diff --git a/tests/config/proxy.ts b/tests/config/proxy.ts index 1b5d953423a27..3a445439c8d90 100644 --- a/tests/config/proxy.ts +++ b/tests/config/proxy.ts @@ -50,7 +50,7 @@ export class TestProxy { await new Promise(x => this._server.close(x)); } - forwardTo(port: number) { + forwardTo(port: number, options?: { skipConnectRequests: boolean }) { this._prependHandler('request', (req: IncomingMessage) => { this.requestUrls.push(req.url); const url = new URL(req.url); @@ -58,6 +58,13 @@ export class TestProxy { req.url = url.toString(); }); this._prependHandler('connect', (req: IncomingMessage) => { + // If using this proxy at the browser-level, you'll want to skip trying to + // MITM connect requests otherwise, unless the system/browser is configured + // to ignore HTTPS errors (or the host has been configured to trust the test + // certs), Playwright will crash in funny ways. (e.g. CR Headful tries to connect + // to accounts.google.com as part of its starup routine and fatally complains of "Invalid method encountered".) + if (options?.skipConnectRequests) + return; this.connectHosts.push(req.url); req.url = `localhost:${port}`; }); diff --git a/tests/proxy.spec.ts b/tests/proxy.spec.ts index cae931521be9f..3fc6a868728e4 100644 --- a/tests/proxy.spec.ts +++ b/tests/proxy.spec.ts @@ -73,6 +73,55 @@ it('should work with IP:PORT notion', async ({ browserType, server }) => { await browser.close(); }); +it.describe('should proxy local network requests', () => { + for (const additionalBypass of [false, true]) { + it.describe(additionalBypass ? 'with other bypasses' : 'by default', () => { + for (const params of [ + { + target: 'localhost', + description: 'localhost', + }, + { + target: '127.0.0.1', + description: 'loopback address', + }, + { + target: '169.254.3.4', + description: 'link-local' + } + ]) { + it(`${params.description}`, async ({ platform, browserName, browserType, server, proxyServer }) => { + it.fail(browserName === 'webkit' && platform === 'darwin' && additionalBypass && ['localhost', '127.0.0.1'].includes(params.target), 'WK fails to proxy 127.0.0.1 and localhost if additional bypasses are present'); + + const path = `/target-${additionalBypass}-${params.target}.html`; + server.setRoute(path, async (req, res) => { + res.end('Served by the proxy'); + }); + + const url = `http://${params.target}:${server.PORT}${path}`; + proxyServer.forwardTo(server.PORT, { skipConnectRequests: true }); + const browser = await browserType.launch({ + proxy: { server: `localhost:${proxyServer.PORT}`, bypass: additionalBypass ? '1.non.existent.domain.for.the.test' : undefined } + }); + + const page = await browser.newPage(); + await page.goto(url); + expect(proxyServer.requestUrls).toContain(url); + expect(await page.title()).toBe('Served by the proxy'); + + await page.goto('http://1.non.existent.domain.for.the.test/foo.html').catch(() => {}); + if (additionalBypass) + expect(proxyServer.requestUrls).not.toContain('http://1.non.existent.domain.for.the.test/foo.html'); + else + expect(proxyServer.requestUrls).toContain('http://1.non.existent.domain.for.the.test/foo.html'); + + await browser.close(); + }); + } + }); + } +}); + it('should authenticate', async ({ browserType, server }) => { server.setRoute('/target.html', async (req, res) => { const auth = req.headers['proxy-authorization'];