From 5262e5ab3584ac811d39f0abd4e3a523f98bcb7a Mon Sep 17 00:00:00 2001 From: Playwright Service <89237858+playwrightmachine@users.noreply.github.com> Date: Tue, 17 Oct 2023 13:41:23 -0700 Subject: [PATCH] feat(chromium-tip-of-tree): roll to r1159 (#27605) --- packages/playwright-core/browsers.json | 4 ++-- .../src/server/chromium/chromiumSwitches.ts | 3 ++- packages/playwright-core/src/server/fetch.ts | 1 + tests/config/proxy.ts | 9 ++------- tests/config/testserver/index.ts | 4 +++- tests/library/browsercontext-proxy.spec.ts | 4 ++-- tests/library/fetch-proxy.spec.ts | 8 ++++---- tests/library/proxy.spec.ts | 4 +++- 8 files changed, 19 insertions(+), 18 deletions(-) diff --git a/packages/playwright-core/browsers.json b/packages/playwright-core/browsers.json index ad302cb88d..39380fc448 100644 --- a/packages/playwright-core/browsers.json +++ b/packages/playwright-core/browsers.json @@ -15,9 +15,9 @@ }, { "name": "chromium-tip-of-tree", - "revision": "1158", + "revision": "1159", "installByDefault": false, - "browserVersion": "120.0.6057.0" + "browserVersion": "120.0.6062.0" }, { "name": "firefox", diff --git a/packages/playwright-core/src/server/chromium/chromiumSwitches.ts b/packages/playwright-core/src/server/chromium/chromiumSwitches.ts index 63de54cb99..65e458e81f 100644 --- a/packages/playwright-core/src/server/chromium/chromiumSwitches.ts +++ b/packages/playwright-core/src/server/chromium/chromiumSwitches.ts @@ -34,7 +34,8 @@ export const chromiumSwitches = [ '--disable-extensions', // AvoidUnnecessaryBeforeUnloadCheckSync - https://github.com/microsoft/playwright/issues/14047 // Translate - https://github.com/microsoft/playwright/issues/16126 - '--disable-features=ImprovedCookieControls,LazyFrameLoading,GlobalMediaControls,DestroyProfileOnBrowserClose,MediaRouter,DialMediaRouteProvider,AcceptCHFrame,AutoExpandDetailsElement,CertificateTransparencyComponentUpdater,AvoidUnnecessaryBeforeUnloadCheckSync,Translate', + // HttpsUpgrades - https://github.com/microsoft/playwright/pull/27605 + '--disable-features=ImprovedCookieControls,LazyFrameLoading,GlobalMediaControls,DestroyProfileOnBrowserClose,MediaRouter,DialMediaRouteProvider,AcceptCHFrame,AutoExpandDetailsElement,CertificateTransparencyComponentUpdater,AvoidUnnecessaryBeforeUnloadCheckSync,Translate,HttpsUpgrades', '--allow-pre-commit-input', '--disable-hang-monitor', '--disable-ipc-flooding-protection', diff --git a/packages/playwright-core/src/server/fetch.ts b/packages/playwright-core/src/server/fetch.ts index 7ffb330b83..b7af65fe51 100644 --- a/packages/playwright-core/src/server/fetch.ts +++ b/packages/playwright-core/src/server/fetch.ts @@ -171,6 +171,7 @@ export abstract class APIRequestContext extends SdkObject { } else { if (proxy.username) proxyOpts.auth = `${proxy.username}:${proxy.password || ''}`; + // TODO: We should use HttpProxyAgent conditional on proxyOpts.protocol instead of always using CONNECT method. agent = new HttpsProxyAgent(proxyOpts); } } diff --git a/tests/config/proxy.ts b/tests/config/proxy.ts index 9c45d8480b..b3eeb558a8 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, options?: { skipConnectRequests: boolean }) { + forwardTo(port: number, options?: { allowConnectRequests: boolean }) { this._prependHandler('request', (req: IncomingMessage) => { this.requestUrls.push(req.url); const url = new URL(req.url); @@ -58,12 +58,7 @@ 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 startup routine and fatally complains of "Invalid method encountered".) - if (options?.skipConnectRequests) + if (!options?.allowConnectRequests) return; this.connectHosts.push(req.url); req.url = `localhost:${port}`; diff --git a/tests/config/testserver/index.ts b/tests/config/testserver/index.ts index 855f4ceec1..de440a1491 100644 --- a/tests/config/testserver/index.ts +++ b/tests/config/testserver/index.ts @@ -110,8 +110,10 @@ export class TestServer { _onSocket(socket: net.Socket) { // ECONNRESET and HPE_INVALID_EOF_STATE are legit errors given // that tab closing aborts outgoing connections to the server. + // HPE_INVALID_METHOD is a legit error when a client (e.g. Chromium which + // makes https requests to http sites) makes a https connection to a http server. socket.on('error', error => { - if ((error as any).code !== 'ECONNRESET' && (error as any).code !== 'HPE_INVALID_EOF_STATE') + if (!['ECONNRESET', 'HPE_INVALID_EOF_STATE', 'HPE_INVALID_METHOD'].includes((error as any).code)) throw error; }); } diff --git a/tests/library/browsercontext-proxy.spec.ts b/tests/library/browsercontext-proxy.spec.ts index f9f724d3e9..3335d9edfb 100644 --- a/tests/library/browsercontext-proxy.spec.ts +++ b/tests/library/browsercontext-proxy.spec.ts @@ -97,7 +97,7 @@ it('should use proxy', async ({ contextFactory, server, proxyServer }) => { it('should set cookie for top-level domain', async ({ contextFactory, server, proxyServer, browserName, isLinux }) => { it.fixme(browserName === 'webkit' && isLinux); - proxyServer.forwardTo(server.PORT); + proxyServer.forwardTo(server.PORT, { allowConnectRequests: true }); const context = await contextFactory({ proxy: { server: `localhost:${proxyServer.PORT}` } }); @@ -216,7 +216,7 @@ it('should use proxy for https urls', async ({ contextFactory, httpsServer, prox httpsServer.setRoute('/target.html', async (req, res) => { res.end('Served by https server via proxy'); }); - proxyServer.forwardTo(httpsServer.PORT); + proxyServer.forwardTo(httpsServer.PORT, { allowConnectRequests: true }); const context = await contextFactory({ ignoreHTTPSErrors: true, proxy: { server: `localhost:${proxyServer.PORT}` } diff --git a/tests/library/fetch-proxy.spec.ts b/tests/library/fetch-proxy.spec.ts index 9377bbdc06..90b50b9f1f 100644 --- a/tests/library/fetch-proxy.spec.ts +++ b/tests/library/fetch-proxy.spec.ts @@ -28,7 +28,7 @@ it.use({ it.skip(({ mode }) => mode !== 'default'); it('context request should pick up proxy credentials', async ({ browserType, server, proxyServer }) => { - proxyServer.forwardTo(server.PORT); + proxyServer.forwardTo(server.PORT, { allowConnectRequests: true }); let auth; proxyServer.setAuthHandler(req => { auth = req.headers['proxy-authorization']; @@ -46,7 +46,7 @@ it('context request should pick up proxy credentials', async ({ browserType, ser }); it('global request should pick up proxy credentials', async ({ playwright, server, proxyServer }) => { - proxyServer.forwardTo(server.PORT); + proxyServer.forwardTo(server.PORT, { allowConnectRequests: true }); let auth; proxyServer.setAuthHandler(req => { auth = req.headers['proxy-authorization']; @@ -67,7 +67,7 @@ it('should work with context level proxy', async ({ contextFactory, contextOptio res.end('Served by the proxy'); }); - proxyServer.forwardTo(server.PORT); + proxyServer.forwardTo(server.PORT, { allowConnectRequests: true }); const context = await contextFactory({ proxy: { server: `localhost:${proxyServer.PORT}` } }); @@ -88,7 +88,7 @@ it(`should support proxy.bypass`, async ({ contextFactory, contextOptions, serve // that resolves everything to some weird search results page. // // @see https://gist.github.com/CollinChaffin/24f6c9652efb3d6d5ef2f5502720ef00 - proxyServer.forwardTo(server.PORT); + proxyServer.forwardTo(server.PORT, { allowConnectRequests: true }); const context = await contextFactory({ ...contextOptions, proxy: { server: `localhost:${proxyServer.PORT}`, bypass: `1.non.existent.domain.for.the.test, 2.non.existent.domain.for.the.test, .another.test` } diff --git a/tests/library/proxy.spec.ts b/tests/library/proxy.spec.ts index e783c4c53a..b69098422c 100644 --- a/tests/library/proxy.spec.ts +++ b/tests/library/proxy.spec.ts @@ -99,7 +99,7 @@ it.describe('should proxy local network requests', () => { }); const url = `http://${params.target}:${server.PORT}${path}`; - proxyServer.forwardTo(server.PORT, { skipConnectRequests: true }); + proxyServer.forwardTo(server.PORT); const browser = await browserType.launch({ proxy: { server: `localhost:${proxyServer.PORT}`, bypass: additionalBypass ? '1.non.existent.domain.for.the.test' : undefined } }); @@ -300,6 +300,8 @@ async function setupSocksForwardingServer(port: number, forwardPort: number){ socket.pipe(dstSock).pipe(socket); socket.on('close', () => dstSock.end()); socket.on('end', () => dstSock.end()); + dstSock.on('error', () => socket.end()); + dstSock.on('end', () => socket.end()); dstSock.setKeepAlive(false); dstSock.connect(forwardPort, '127.0.0.1'); }