Skip to content

Commit

Permalink
feat(proxy): unify local network proxy behavior (#10719)
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@rwoll
rwoll committed Dec 10, 2021
1 parent c463af4 commit fde427d
Show file tree
Hide file tree
Showing 5 changed files with 119 additions and 2 deletions.
2 changes: 2 additions & 0 deletions packages/playwright-core/src/server/chromium/chromium.ts
View file
Expand Up @@ -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(';')}`);
}
Expand Down
11 changes: 10 additions & 1 deletion packages/playwright-core/src/server/chromium/crBrowser.ts
View file
Expand Up @@ -90,10 +90,19 @@ export class CRBrowser extends Browser {

async newContext(options: types.BrowserContextOptions): Promise<BrowserContext> {
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();
Expand Down
50 changes: 50 additions & 0 deletions tests/browsercontext-proxy.spec.ts
View file
Expand Up @@ -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('<html><title>Served by the proxy</title></html>');
});

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');
Expand Down
9 changes: 8 additions & 1 deletion tests/config/proxy.ts
View file
Expand Up @@ -50,14 +50,21 @@ 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);
url.host = `localhost:${port}`;
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}`;
});
Expand Down
49 changes: 49 additions & 0 deletions tests/proxy.spec.ts
View file
Expand Up @@ -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('<html><title>Served by the proxy</title></html>');
});

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'];
Expand Down

0 comments on commit fde427d

Please sign in to comment.