Skip to content

Commit

Permalink
fix(webkit): delay request event until requestIntercepted is received (…
Browse files Browse the repository at this point in the history 
…#28484)

Previously we were wrongly firing `route` event for the request which
are not in fact intercepted (e.g. requests from service worker).

Related #28414
Reference #23781
  • Loading branch information
@yury-s
yury-s committed Dec 4, 2023
1 parent 9a95d9a commit ab68d7b
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 35 deletions.
9 changes: 1 addition & 8 deletions packages/playwright-core/src/server/webkit/wkInterceptableRequest.ts
View file
Expand Up @@ -21,7 +21,6 @@ import type * as types from '../types';
import type { Protocol } from './protocol';
import type { WKSession } from './wkConnection';
import { assert, headersObjectToArray, headersArrayToObject } from '../../utils';
import { ManualPromise } from '../../utils/manualPromise';

const errorReasons: { [reason: string]: Protocol.Network.ResourceErrorType } = {
'aborted': 'Cancellation',
Expand All @@ -46,12 +45,10 @@ export class WKInterceptableRequest {
readonly _requestId: string;
_timestamp: number;
_wallTime: number;
readonly _route: WKRouteImpl | null;

constructor(session: WKSession, route: WKRouteImpl | null, frame: frames.Frame, event: Protocol.Network.requestWillBeSentPayload, redirectedFrom: WKInterceptableRequest | null, documentId: string | undefined) {
constructor(session: WKSession, frame: frames.Frame, event: Protocol.Network.requestWillBeSentPayload, redirectedFrom: WKInterceptableRequest | null, documentId: string | undefined) {
this._session = session;
this._requestId = event.requestId;
this._route = route;
const resourceType = event.type ? event.type.toLowerCase() : (redirectedFrom ? redirectedFrom.request.resourceType() : 'other');
let postDataBuffer = null;
this._timestamp = event.timestamp;
Expand Down Expand Up @@ -102,7 +99,6 @@ export class WKInterceptableRequest {
export class WKRouteImpl implements network.RouteDelegate {
private readonly _session: WKSession;
private readonly _requestId: string;
readonly _requestInterceptedPromise = new ManualPromise<void>();

constructor(session: WKSession, requestId: string) {
this._session = session;
Expand All @@ -112,7 +108,6 @@ export class WKRouteImpl implements network.RouteDelegate {
async abort(errorCode: string) {
const errorType = errorReasons[errorCode];
assert(errorType, 'Unknown error code: ' + errorCode);
await this._requestInterceptedPromise;
// In certain cases, protocol will return error if the request was already canceled
// or the page was closed. We should tolerate these errors.
await this._session.sendMayFail('Network.interceptRequestWithError', { requestId: this._requestId, errorType });
Expand All @@ -122,7 +117,6 @@ export class WKRouteImpl implements network.RouteDelegate {
if (300 <= response.status && response.status < 400)
throw new Error('Cannot fulfill with redirect status: ' + response.status);

await this._requestInterceptedPromise;
// In certain cases, protocol will return error if the request was already canceled
// or the page was closed. We should tolerate these errors.
let mimeType = response.isBase64 ? 'application/octet-stream' : 'text/plain';
Expand All @@ -143,7 +137,6 @@ export class WKRouteImpl implements network.RouteDelegate {
}

async continue(request: network.Request, overrides: types.NormalizedContinueOverrides) {
await this._requestInterceptedPromise;
// In certain cases, protocol will return error if the request was already canceled
// or the page was closed. We should tolerate these errors.
await this._session.sendMayFail('Network.interceptWithRequest', {
Expand Down
69 changes: 46 additions & 23 deletions packages/playwright-core/src/server/webkit/wkPage.ts
View file
Expand Up @@ -60,6 +60,7 @@ export class WKPage implements PageDelegate {
private readonly _pageProxySession: WKSession;
readonly _opener: WKPage | null;
private readonly _requestIdToRequest = new Map<string, WKInterceptableRequest>();
private readonly _requestIdToRequestWillBeSentEvent = new Map<string, Protocol.Network.requestWillBeSentPayload>();
private readonly _workers: WKWorkers;
private readonly _contextIdToContext: Map<number, dom.FrameExecutionContext>;
private _sessionListeners: RegisteredListener[] = [];
Expand Down Expand Up @@ -388,9 +389,9 @@ export class WKPage implements PageDelegate {
eventsHelper.addEventListener(this._session, 'Page.fileChooserOpened', event => this._onFileChooserOpened(event)),
eventsHelper.addEventListener(this._session, 'Network.requestWillBeSent', e => this._onRequestWillBeSent(this._session, e)),
eventsHelper.addEventListener(this._session, 'Network.requestIntercepted', e => this._onRequestIntercepted(this._session, e)),
eventsHelper.addEventListener(this._session, 'Network.responseReceived', e => this._onResponseReceived(e)),
eventsHelper.addEventListener(this._session, 'Network.responseReceived', e => this._onResponseReceived(this._session, e)),
eventsHelper.addEventListener(this._session, 'Network.loadingFinished', e => this._onLoadingFinished(e)),
eventsHelper.addEventListener(this._session, 'Network.loadingFailed', e => this._onLoadingFailed(e)),
eventsHelper.addEventListener(this._session, 'Network.loadingFailed', e => this._onLoadingFailed(this._session, e)),
eventsHelper.addEventListener(this._session, 'Network.webSocketCreated', e => this._page._frameManager.onWebSocketCreated(e.requestId, e.url)),
eventsHelper.addEventListener(this._session, 'Network.webSocketWillSendHandshakeRequest', e => this._page._frameManager.onWebSocketRequest(e.requestId)),
eventsHelper.addEventListener(this._session, 'Network.webSocketHandshakeResponseReceived', e => this._page._frameManager.onWebSocketResponse(e.requestId, e.response.status, e.response.statusText)),
Expand Down Expand Up @@ -1011,6 +1012,15 @@ export class WKPage implements PageDelegate {
_onRequestWillBeSent(session: WKSession, event: Protocol.Network.requestWillBeSentPayload) {
if (event.request.url.startsWith('data:'))
return;

// We do not support intercepting redirects.
if (this._page.needsRequestInterception() && !event.redirectResponse)
this._requestIdToRequestWillBeSentEvent.set(event.requestId, event);
else
this._onRequest(session, event, false);
}

private _onRequest(session: WKSession, event: Protocol.Network.requestWillBeSentPayload, intercepted: boolean) {
let redirectedFrom: WKInterceptableRequest | null = null;
if (event.redirectResponse) {
const request = this._requestIdToRequest.get(event.requestId);
Expand All @@ -1029,13 +1039,16 @@ 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;
let route = null;
// We do not support intercepting redirects.
if (this._page.needsRequestInterception() && !redirectedFrom)
route = new WKRouteImpl(session, event.requestId);
const request = new WKInterceptableRequest(session, route, frame, event, redirectedFrom, documentId);
const request = new WKInterceptableRequest(session, frame, event, redirectedFrom, documentId);
let route;
if (intercepted) {
route = new WKRouteImpl(session, request._requestId);
// There is no point in waiting for the raw headers in Network.responseReceived when intercepting.
// Use provisional headers as raw headers, so that client can call allHeaders() from the route handler.
request.request.setRawRequestHeaders(null);
}
this._requestIdToRequest.set(event.requestId, request);
this._page._frameManager.requestStarted(request.request, route || undefined);
this._page._frameManager.requestStarted(request.request, route);
}

private _handleRequestRedirect(request: WKInterceptableRequest, responsePayload: Protocol.Network.Response, timestamp: number) {
Expand All @@ -1051,34 +1064,36 @@ export class WKPage implements PageDelegate {
}

_onRequestIntercepted(session: WKSession, event: Protocol.Network.requestInterceptedPayload) {
const request = this._requestIdToRequest.get(event.requestId);
if (!request) {
session.sendMayFail('Network.interceptRequestWithError', { errorType: 'Cancellation', requestId: event.requestId });
return;
}
// There is no point in waiting for the raw headers in Network.responseReceived when intercepting.
// Use provisional headers as raw headers, so that client can call allHeaders() from the route handler.
request.request.setRawRequestHeaders(null);
if (!request._route) {
const requestWillBeSentEvent = this._requestIdToRequestWillBeSentEvent.get(event.requestId);
if (!requestWillBeSentEvent) {
// Intercepted, although we do not intend to allow interception.
// Just continue.
session.sendMayFail('Network.interceptWithRequest', { requestId: request._requestId });
} else {
request._route._requestInterceptedPromise.resolve();
session.sendMayFail('Network.interceptWithRequest', { requestId: event.requestId });
return;
}
this._requestIdToRequestWillBeSentEvent.delete(event.requestId);
this._onRequest(session, requestWillBeSentEvent, true);
}

_onResponseReceived(event: Protocol.Network.responseReceivedPayload) {
_onResponseReceived(session: WKSession, event: Protocol.Network.responseReceivedPayload) {
const requestWillBeSentEvent = this._requestIdToRequestWillBeSentEvent.get(event.requestId);
if (requestWillBeSentEvent) {
this._requestIdToRequestWillBeSentEvent.delete(event.requestId);
// We received a response, so the request won't be intercepted (e.g. it was handled by a
// service worker and we don't intercept service workers).
this._onRequest(session, requestWillBeSentEvent, false);
}
const request = this._requestIdToRequest.get(event.requestId);
// FileUpload sends a response without a matching request.
if (!request)
return;

this._requestIdToResponseReceivedPayloadEvent.set(request._requestId, event);
const response = request.createResponse(event.response);
this._page._frameManager.requestReceivedResponse(response);

if (response.status() === 204) {
this._onLoadingFailed({
this._onLoadingFailed(session, {
requestId: event.requestId,
errorText: 'Aborted: 204 No Content',
timestamp: event.timestamp
Expand Down Expand Up @@ -1121,7 +1136,15 @@ export class WKPage implements PageDelegate {
this._page._frameManager.reportRequestFinished(request.request, response);
}

_onLoadingFailed(event: Protocol.Network.loadingFailedPayload) {
_onLoadingFailed(session: WKSession, event: Protocol.Network.loadingFailedPayload) {
const requestWillBeSentEvent = this._requestIdToRequestWillBeSentEvent.get(event.requestId);
if (requestWillBeSentEvent) {
this._requestIdToRequestWillBeSentEvent.delete(event.requestId);
// If loading failed, the request won't be intercepted (e.g. it was handled by a
// service worker and we don't intercept service workers).
this._onRequest(session, requestWillBeSentEvent, false);
}

const request = this._requestIdToRequest.get(event.requestId);
// For certain requestIds we never receive requestWillBeSent event.
// @see https://crbug.com/750469
Expand Down
4 changes: 2 additions & 2 deletions packages/playwright-core/src/server/webkit/wkProvisionalPage.ts
View file
Expand Up @@ -45,9 +45,9 @@ export class WKProvisionalPage {
this._sessionListeners = [
eventsHelper.addEventListener(session, 'Network.requestWillBeSent', overrideFrameId(e => wkPage._onRequestWillBeSent(session, e))),
eventsHelper.addEventListener(session, 'Network.requestIntercepted', overrideFrameId(e => wkPage._onRequestIntercepted(session, e))),
eventsHelper.addEventListener(session, 'Network.responseReceived', overrideFrameId(e => wkPage._onResponseReceived(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(e))),
eventsHelper.addEventListener(session, 'Network.loadingFailed', overrideFrameId(e => wkPage._onLoadingFailed(session, e))),
];

this.initializationPromise = this._wkPage._initializeSession(session, true, ({ frameTree }) => this._handleFrameTree(frameTree));
Expand Down
13 changes: 11 additions & 2 deletions tests/page/page-event-request.spec.ts
View file
Expand Up @@ -69,12 +69,16 @@ it('should report requests and responses handled by service worker', async ({ pa
expect(await failedRequest.response()).toBe(null);
});

it('should report requests and responses handled by service worker with routing', async ({ page, server, isAndroid, isElectron, mode, platform }) => {
it('should report requests and responses handled by service worker with routing', async ({ page, server, isAndroid, isElectron, mode, browserName, platform }) => {
it.fixme(isAndroid);
it.fixme(isElectron);
it.fixme(mode.startsWith('service') && platform === 'linux', 'Times out for no clear reason');

await page.route('**/*', route => route.continue());
const interceptedUrls = [];
await page.route('**/*', route => {
interceptedUrls.push(route.request().url());
void route.continue();
});
await page.goto(server.PREFIX + '/serviceworkers/fetchdummy/sw.html');
await page.evaluate(() => window['activationPromise']);
const [swResponse, request] = await Promise.all([
Expand All @@ -96,6 +100,11 @@ it('should report requests and responses handled by service worker with routing'
expect(failedRequest.failure()).not.toBe(null);
expect(failedRequest.serviceWorker()).toBe(null);
expect(await failedRequest.response()).toBe(null);

const expectedUrls = [server.PREFIX + '/serviceworkers/fetchdummy/sw.html'];
if (browserName === 'webkit')
expectedUrls.push(server.PREFIX + '/serviceworkers/fetchdummy/sw.js');
expect(interceptedUrls).toEqual(expectedUrls);
});

it('should report navigation requests and responses handled by service worker', async ({ page, server, isAndroid, isElectron, browserName }) => {
Expand Down

0 comments on commit ab68d7b

Please sign in to comment.