From 578847d5251ac19b95c4dcb734432c402bc25215 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Thu, 18 Jul 2024 14:32:08 -0700 Subject: [PATCH] fix(webkit): reenable CrossOriginOpenerPolicy --- .../server/webkit/wkInterceptableRequest.ts | 6 +- .../src/server/webkit/wkPage.ts | 39 +++++++++++++ .../src/server/webkit/wkProvisionalPage.ts | 43 +++++++++++++- tests/page/page-goto.spec.ts | 56 ++++++++++++++++++- 4 files changed, 138 insertions(+), 6 deletions(-) diff --git a/packages/playwright-core/src/server/webkit/wkInterceptableRequest.ts b/packages/playwright-core/src/server/webkit/wkInterceptableRequest.ts index 8941f18b70..eed433886d 100644 --- a/packages/playwright-core/src/server/webkit/wkInterceptableRequest.ts +++ b/packages/playwright-core/src/server/webkit/wkInterceptableRequest.ts @@ -42,7 +42,7 @@ const errorReasons: { [reason: string]: Protocol.Network.ResourceErrorType } = { export class WKInterceptableRequest { private readonly _session: WKSession; readonly request: network.Request; - private readonly _requestId: string; + private _requestId: string; _timestamp: number; _wallTime: number; @@ -59,6 +59,10 @@ export class WKInterceptableRequest { resourceType, event.request.method, postDataBuffer, headersObjectToArray(event.request.headers)); } + changeRequestId(requestId: string) { + this._requestId = requestId; + } + createResponse(responsePayload: Protocol.Network.Response): network.Response { const getResponseBody = async () => { const response = await this._session.send('Network.getResponseBody', { requestId: this._requestId }); diff --git a/packages/playwright-core/src/server/webkit/wkPage.ts b/packages/playwright-core/src/server/webkit/wkPage.ts index 3cd454deb2..171d3d9bdd 100644 --- a/packages/playwright-core/src/server/webkit/wkPage.ts +++ b/packages/playwright-core/src/server/webkit/wkPage.ts @@ -252,7 +252,9 @@ export class WKPage implements PageDelegate { if (this._provisionalPage && this._provisionalPage._session.sessionId === targetId) { this._provisionalPage._session.dispose(); this._provisionalPage.dispose(); + const provisionalPage = this._provisionalPage; this._provisionalPage = null; + this._maybeCancelNavigationRequest(provisionalPage); } else if (this._session.sessionId === targetId) { this._session.dispose(); eventsHelper.removeEventListeners(this._sessionListeners); @@ -1015,10 +1017,41 @@ export class WKPage implements PageDelegate { return context.createHandle(result.object) as dom.ElementHandle; } + private _maybeCancelNavigationRequest(provisionalPage: WKProvisionalPage) { + const navigationRequest = provisionalPage.pendingNavigationRequest(); + for (const [requestId, request] of this._requestIdToRequest) { + if (request.request === navigationRequest) { + // Make sure the request completes if the provisional navigation is canceled. + this._onLoadingFailed(provisionalPage._session, { + requestId: requestId, + errorText: 'Provisiolal navigation canceled.', + timestamp: 0, + canceled: true + }); + return; + } + } + } + + _changeRequestId(navigationRequest: network.Request, newRequestId: string) { + for (const [requestId, request] of this._requestIdToRequest) { + if (request.request === navigationRequest) { + this._requestIdToRequest.delete(requestId); + request.changeRequestId(newRequestId); + this._requestIdToRequest.set(newRequestId, request); + // Simply ignore this event as it has already been dispatched from the original process + // and there will ne no requestIntercepted event from the provisional process as it resumes + // existing network load (that has already received reponse headers). + return; + } + } + } + _onRequestWillBeSent(session: WKSession, event: Protocol.Network.requestWillBeSentPayload) { if (event.request.url.startsWith('data:')) return; + // navigation too? // We do not support intercepting redirects. if (this._page.needsRequestInterception() && !event.redirectResponse) this._requestIdToRequestWillBeSentEvent.set(event.requestId, event); @@ -1157,6 +1190,12 @@ export class WKPage implements PageDelegate { if (!request) return; + // If a provisional page was created after Cross-Origin-Opener-Policy headers were received, it + // will take over the request (with a new id). We'll only fail it if the provisional navigation + // fails/get canceled. + if (this._provisionalPage?.pendingNavigationRequest() === request.request) + return; + const response = request.request._existingResponse(); if (response) { response._serverAddrFinished(); diff --git a/packages/playwright-core/src/server/webkit/wkProvisionalPage.ts b/packages/playwright-core/src/server/webkit/wkProvisionalPage.ts index 0b19ca7a17..117ba28417 100644 --- a/packages/playwright-core/src/server/webkit/wkProvisionalPage.ts +++ b/packages/playwright-core/src/server/webkit/wkProvisionalPage.ts @@ -20,10 +20,13 @@ import type { RegisteredListener } from '../../utils/eventsHelper'; import { eventsHelper } from '../../utils/eventsHelper'; import type { Protocol } from './protocol'; import { assert } from '../../utils'; +import type * as network from '../network'; + export class WKProvisionalPage { readonly _session: WKSession; private readonly _wkPage: WKPage; + private _existingNavigationRequest: network.Request | undefined; private _sessionListeners: RegisteredListener[] = []; private _mainFrameId: string | null = null; readonly initializationPromise: Promise; @@ -31,6 +34,13 @@ export class WKProvisionalPage { constructor(session: WKSession, page: WKPage) { this._session = session; this._wkPage = page; + // Network.requestWillBeSent and requestIntercepted (if intercepting) from the original web process + // will always come before a provisional page is created based on the response COOP headers. + // Thereafter we'll receive targetCreated (provisional) and later on in some order loadingFailed from the + // original process and requestWillBeSent from the provisional one. We should ignore loadingFailed + // as the original request continues in the provisional process. But if the provisional load is later + // canceled we should dispatch loadingFailed to the client. + this._existingNavigationRequest = page._page.mainFrame().pendingDocument()?.request; const overrideFrameId = (handler: (p: any) => void) => { return (payload: any) => { @@ -43,16 +53,20 @@ export class WKProvisionalPage { const wkPage = this._wkPage; this._sessionListeners = [ - eventsHelper.addEventListener(session, 'Network.requestWillBeSent', overrideFrameId(e => wkPage._onRequestWillBeSent(session, e))), + eventsHelper.addEventListener(session, 'Network.requestWillBeSent', overrideFrameId(e => this._onRequestWillBeSent(e))), eventsHelper.addEventListener(session, 'Network.requestIntercepted', overrideFrameId(e => wkPage._onRequestIntercepted(session, e))), eventsHelper.addEventListener(session, 'Network.responseReceived', overrideFrameId(e => wkPage._onResponseReceived(session, e))), - eventsHelper.addEventListener(session, 'Network.loadingFinished', overrideFrameId(e => wkPage._onLoadingFinished(e))), - eventsHelper.addEventListener(session, 'Network.loadingFailed', overrideFrameId(e => wkPage._onLoadingFailed(session, e))), + eventsHelper.addEventListener(session, 'Network.loadingFinished', overrideFrameId(e => this._onLoadingFinished(e))), + eventsHelper.addEventListener(session, 'Network.loadingFailed', overrideFrameId(e => this._onLoadingFailed(e))), ]; this.initializationPromise = this._wkPage._initializeSession(session, true, ({ frameTree }) => this._handleFrameTree(frameTree)); } + pendingNavigationRequest(): network.Request | undefined { + return this._existingNavigationRequest; + } + dispose() { eventsHelper.removeEventListeners(this._sessionListeners); } @@ -62,6 +76,29 @@ export class WKProvisionalPage { this._wkPage._onFrameAttached(this._mainFrameId, null); } + private _onRequestWillBeSent(event: Protocol.Network.requestWillBeSentPayload) { + if (this._existingNavigationRequest && this._existingNavigationRequest.url() === event.request.url) { + // If it's a continuation of the main frame navigation request after COOP headers were received, + // take over original request, and replace its request id with the new one. + this._wkPage._changeRequestId(this._existingNavigationRequest, event.requestId); + // Simply ignore this event as it has already been dispatched from the original process + // and there will ne no requestIntercepted event from the provisional process as it resumes + // existing network load (that has already received reponse headers). + return; + } + this._wkPage._onRequestWillBeSent(this._session, event); + } + + private _onLoadingFinished(event: Protocol.Network.loadingFinishedPayload): void { + this._existingNavigationRequest = undefined; + this._wkPage._onLoadingFinished(event); + } + + private _onLoadingFailed(event: Protocol.Network.loadingFailedPayload) { + this._existingNavigationRequest = undefined; + this._wkPage._onLoadingFailed(this._session, event); + } + private _handleFrameTree(frameTree: Protocol.Page.FrameResourceTree) { assert(!frameTree.frame.parentId); this._mainFrameId = frameTree.frame.id; diff --git a/tests/page/page-goto.spec.ts b/tests/page/page-goto.spec.ts index 5ca9ee6e46..830a25e203 100644 --- a/tests/page/page-goto.spec.ts +++ b/tests/page/page-goto.spec.ts @@ -78,7 +78,7 @@ it('should work with cross-process that fails before committing', async ({ page, expect(error instanceof Error).toBeTruthy(); }); -it('should work with Cross-Origin-Opener-Policy', async ({ page, server, browserName }) => { +it('should work with Cross-Origin-Opener-Policy', async ({ page, server }) => { server.setRoute('/empty.html', (req, res) => { res.setHeader('Cross-Origin-Opener-Policy', 'same-origin'); res.end(); @@ -109,7 +109,42 @@ it('should work with Cross-Origin-Opener-Policy', async ({ page, server, browser expect(response.request().failure()).toBeNull(); }); -it('should work with Cross-Origin-Opener-Policy after redirect', async ({ page, server, browserName }) => { +it('should work with Cross-Origin-Opener-Policy and interception', async ({ page, server }) => { + server.setRoute('/empty.html', (req, res) => { + res.setHeader('Cross-Origin-Opener-Policy', 'same-origin'); + res.end(); + }); + const requests = new Set(); + const events = []; + page.on('request', r => { + events.push('request'); + requests.add(r); + }); + page.on('requestfailed', r => { + events.push('requestfailed'); + requests.add(r); + }); + page.on('requestfinished', r => { + events.push('requestfinished'); + requests.add(r); + }); + page.on('response', r => { + events.push('response'); + requests.add(r.request()); + }); + await page.route('**/*', async route => { + await new Promise(f => setTimeout(f, 100)); + await route.continue(); + }); + const response = await page.goto(server.EMPTY_PAGE); + expect(page.url()).toBe(server.EMPTY_PAGE); + await response.finished(); + expect(events).toEqual(['request', 'response', 'requestfinished']); + expect(requests.size).toBe(1); + expect(response.request().failure()).toBeNull(); +}); + +it('should work with Cross-Origin-Opener-Policy after redirect', async ({ page, server }) => { server.setRedirect('/redirect', '/empty.html'); server.setRoute('/empty.html', (req, res) => { res.setHeader('Cross-Origin-Opener-Policy', 'same-origin'); @@ -144,6 +179,23 @@ it('should work with Cross-Origin-Opener-Policy after redirect', async ({ page, expect(firstRequest.url()).toBe(server.PREFIX + '/redirect'); }); +it('should properly cancel Cross-Origin-Opener-Policy navigation', async ({ page, server }) => { + server.setRoute('/empty.html', (req, res) => { + res.setHeader('Cross-Origin-Opener-Policy', 'same-origin'); + res.end(); + }); + const requestPromise = page.waitForRequest(server.EMPTY_PAGE); + page.goto(server.EMPTY_PAGE).catch(() => {}); + await new Promise(f => setTimeout(f, 50)); + // Non COOP response. + await page.goto(server.CROSS_PROCESS_PREFIX + '/error.html'); + const req = await requestPromise; + const response = await Promise.race([req.response(), new Promise(f => setTimeout(() => f('timeout'), 5_000))]); + // First navigation request should either receive response or be canceled by the second + // navigation, but never hang unresolved. + expect(response).not.toBe('timeout'); +}); + it('should capture iframe navigation request', async ({ page, server }) => { await page.goto(server.EMPTY_PAGE); expect(page.url()).toBe(server.EMPTY_PAGE);