-
Notifications
You must be signed in to change notification settings - Fork 5.7k
fix(webkit): support Cross-Origin-Opener-Policy navigation #9120
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
4e008e6
ef66621
3f5117d
1f7fbb1
e958b2e
1500b60
d51b9ee
554db2d
67c548c
6c32eff
d3b004f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -76,6 +76,7 @@ export class WKPage implements PageDelegate { | |
| private _nextWindowOpenPopupFeatures?: string[]; | ||
| private _recordingVideoFile: string | null = null; | ||
| private _screencastGeneration: number = 0; | ||
| private _currentMainFrameNavigation: WKNavigation | undefined; | ||
|
|
||
| constructor(browserContext: WKBrowserContext, pageProxySession: WKSession, opener: WKPage | null) { | ||
| this._pageProxySession = pageProxySession; | ||
|
|
@@ -411,6 +412,8 @@ export class WKPage implements PageDelegate { | |
| } | ||
|
|
||
| private _onFrameStoppedLoading(frameId: string) { | ||
| if (this._currentMainFrameNavigation?.shouldIgnoreFrameStoppedLoading(frameId)) | ||
| return; | ||
| this._page._frameManager.frameStoppedLoading(frameId); | ||
| } | ||
|
|
||
|
|
@@ -940,7 +943,26 @@ export class WKPage implements PageDelegate { | |
| return result.handle; | ||
| } | ||
|
|
||
| _onRequestWillBeSent(session: WKSession, event: Protocol.Network.requestWillBeSentPayload) { | ||
| _onRequestWillBeSentProvisional(session: WKSession, event: Protocol.Network.requestWillBeSentPayload) { | ||
| if (!this._currentMainFrameNavigation?.checkIfSameNavigationInNewProcess(event)) { | ||
| this._onRequestWillBeSent(session, event); | ||
| return; | ||
| } | ||
| const oldId = this._currentMainFrameNavigation._originalRequestId; | ||
| const request = this._requestIdToRequest.get(oldId); | ||
| if (!request) | ||
| return; | ||
| const payload = this._requestIdToResponseReceivedPayloadEvent.get(oldId); | ||
| this._requestIdToRequest.delete(oldId); | ||
| this._requestIdToResponseReceivedPayloadEvent.delete(oldId); | ||
| const newId = event.requestId; | ||
| request._requestId = newId; | ||
| this._requestIdToRequest.set(newId, request); | ||
| if (payload) | ||
| this._requestIdToResponseReceivedPayloadEvent.set(newId, payload); | ||
| } | ||
|
|
||
| private _onRequestWillBeSent(session: WKSession, event: Protocol.Network.requestWillBeSentPayload) { | ||
| if (event.request.url.startsWith('data:')) | ||
| return; | ||
| let redirectedFrom: WKInterceptableRequest | null = null; | ||
|
|
@@ -961,6 +983,8 @@ export class WKPage implements PageDelegate { | |
| // TODO(einbinder) this will fail if we are an XHR document request | ||
| const isNavigationRequest = event.type === 'Document'; | ||
| const documentId = isNavigationRequest ? event.loaderId : undefined; | ||
| if (isNavigationRequest && frame.isMainFrame()) | ||
| this._currentMainFrameNavigation = new WKNavigation(this, event); | ||
| let route = null; | ||
| // We do not support intercepting redirects. | ||
| if (this._page._needsRequestInterception() && !redirectedFrom) | ||
|
|
@@ -986,7 +1010,7 @@ export class WKPage implements PageDelegate { | |
| session.sendMayFail('Network.interceptRequestWithError', { errorType: 'Cancellation', requestId: event.requestId }); | ||
| return; | ||
| } | ||
| if (!request._route) { | ||
| if (!request._route || this._currentMainFrameNavigation?.shoudIgnoreRequestInterception(event)) { | ||
| // Intercepted, although we do not intend to allow interception. | ||
| // Just continue. | ||
| session.sendMayFail('Network.interceptWithRequest', { requestId: request._requestId }); | ||
|
|
@@ -1006,6 +1030,8 @@ export class WKPage implements PageDelegate { | |
| } | ||
|
|
||
| _onResponseReceived(event: Protocol.Network.responseReceivedPayload) { | ||
| if (this._currentMainFrameNavigation?.checkIfDuplicateResponseEvent(event)) | ||
| return; | ||
| const request = this._requestIdToRequest.get(event.requestId); | ||
| // FileUpload sends a response without a matching request. | ||
| if (!request) | ||
|
|
@@ -1063,7 +1089,12 @@ export class WKPage implements PageDelegate { | |
| this._page._frameManager.reportRequestFinished(request.request, response); | ||
| } | ||
|
|
||
| _onLoadingFailed(event: Protocol.Network.loadingFailedPayload) { | ||
| private _onLoadingFailed(event: Protocol.Network.loadingFailedPayload) { | ||
| if (!this._currentMainFrameNavigation?.shouldIgnoreLoadingFailedEvent(event)) | ||
| this._onLoadingFailedShared(event); | ||
| } | ||
|
|
||
| _onLoadingFailedShared(event: Protocol.Network.loadingFailedPayload) { | ||
| const request = this._requestIdToRequest.get(event.requestId); | ||
| // For certain requestIds we never receive requestWillBeSent event. | ||
| // @see https://crbug.com/750469 | ||
|
|
@@ -1099,6 +1130,10 @@ export class WKPage implements PageDelegate { | |
| async _clearPermissions() { | ||
| await this._pageProxySession.send('Emulation.resetPermissions', {}); | ||
| } | ||
|
|
||
| hasProvisionalPage() { | ||
| return !!this._provisionalPage; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -1160,3 +1195,68 @@ function isLoadedSecurely(url: string, timing: network.ResourceTiming) { | |
| return true; | ||
| } catch (_) {} | ||
| } | ||
|
|
||
|
|
||
| class WKNavigation { | ||
| private readonly _page: WKPage; | ||
| // loaderId is a navigation id which is unique within browser and persists during cross-process navigation. | ||
| private readonly _loaderId: string; | ||
| readonly _originalRequestId: Protocol.Network.RequestId; | ||
| private readonly _frameId: string; | ||
| private _responseReceived: boolean = false; | ||
| private _didCheckFirstRequestInProvisionalPage: boolean = false; | ||
| private _provisionalPageRequestId: string | undefined; | ||
|
|
||
| constructor(page: WKPage, event: Protocol.Network.requestWillBeSentPayload) { | ||
| this._page = page; | ||
| this._loaderId = event.loaderId; | ||
| this._originalRequestId = event.requestId; | ||
| this._frameId = event.frameId; | ||
| } | ||
|
|
||
| shouldIgnoreFrameStoppedLoading(frameId: string): boolean { | ||
| if (frameId !== this._frameId) | ||
| return false; | ||
| // Navigation in the original frame is canceled and actually continues in the | ||
| // provisional page, so we ignore failure events from the original page. | ||
| return this._page.hasProvisionalPage(); | ||
| } | ||
|
|
||
| shouldIgnoreLoadingFailedEvent(event: Protocol.Network.loadingFailedPayload): boolean { | ||
| if (event.requestId !== this._originalRequestId) | ||
| return false; | ||
| if (!event.canceled) | ||
| return false; | ||
| // Navigation in the original frame is canceled and actually continues in the | ||
| // provisional page, so we ignore failure events from the original page. | ||
| return this._page.hasProvisionalPage(); | ||
| } | ||
|
|
||
| checkIfSameNavigationInNewProcess(event: Protocol.Network.requestWillBeSentPayload): boolean { | ||
| if (this._didCheckFirstRequestInProvisionalPage) | ||
| return false; | ||
| this._didCheckFirstRequestInProvisionalPage = true; | ||
| if (event.loaderId !== this._loaderId) | ||
| return false; | ||
| this._provisionalPageRequestId = event.requestId; | ||
| return true; | ||
| } | ||
|
|
||
| checkIfDuplicateResponseEvent(event: Protocol.Network.responseReceivedPayload): boolean { | ||
| if (this._originalRequestId === event.requestId || this._provisionalPageRequestId === event.requestId) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we ignore responses from the old web process only? This ensures that we get correct response from the new web process, in case WebKit fixes more security holes in the old web process and send bogus response from there (if any).
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At the moment response notification comes from old process we don't know if it's gonna be a cross-process or same process navigation. In case of same-process navigation we have to dispatch messages from the old process. Unless we introduce a separate notification that the navigation turned into cross-process and buffer up some messages from old web process before the notification arrives we cannot safely ignore responses from the old web process . |
||
| // In case of cross-process navigation caused by Cross-Origin-Opener-Policy response header | ||
| // WebKit sends two responseReceived events from the old web process and one such event from the | ||
| // new provisonal web process. | ||
| if (this._responseReceived) | ||
| return true; | ||
| this._responseReceived = true; | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| shoudIgnoreRequestInterception(event: Protocol.Network.requestInterceptedPayload): boolean { | ||
| // It only makes sense to intercept request in the old process before it is sent to | ||
| // the server (in the new process the response is already received and will be replayed) | ||
| return this._provisionalPageRequestId === event.requestId; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am afraid this might not be always the case. For example, we navigate to cross-origin 204 page that creates a provisional page but never commits?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Coincidentally that works because we generate loadingFinished event in WebKit ourselves when 204 response comes:
playwright/src/server/webkit/wkPage.ts
Lines 1023 to 1030 in 45b365d
With network failures we will receive
Playwright.provisionalLoadFailedand that will abort the navigation, so we are good in that case too. Added a test.