From 09b1e3ffa91877ce88177adc6a635faed45eb571 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Fri, 16 Jun 2023 20:44:32 -0700 Subject: [PATCH] fix(chromium): response.body() for worker requests should work (#23764) Previously, worker requests used page's session to call `Network.getResponseBody`. Fixes #23750. --- .../src/server/chromium/crNetworkManager.ts | 133 +++++++++--------- .../src/server/chromium/crPage.ts | 2 +- tests/page/workers.spec.ts | 4 +- 3 files changed, 73 insertions(+), 66 deletions(-) diff --git a/packages/playwright-core/src/server/chromium/crNetworkManager.ts b/packages/playwright-core/src/server/chromium/crNetworkManager.ts index caa07046db..6d731cdbe7 100644 --- a/packages/playwright-core/src/server/chromium/crNetworkManager.ts +++ b/packages/playwright-core/src/server/chromium/crNetworkManager.ts @@ -29,8 +29,13 @@ import type { CRPage } from './crPage'; import { assert, headersObjectToArray } from '../../utils'; import type { CRServiceWorker } from './crServiceWorker'; +type SessionInfo = { + session: CRSession; + workerFrame?: frames.Frame; +}; + export class CRNetworkManager { - private _client: CRSession; + private _session: CRSession; private _page: Page | null; private _serviceWorker: CRServiceWorker | null; private _parentManager: CRNetworkManager | null; @@ -44,42 +49,42 @@ export class CRNetworkManager { private _eventListeners: RegisteredListener[]; private _responseExtraInfoTracker = new ResponseExtraInfoTracker(); - constructor(client: CRSession, page: Page | null, serviceWorker: CRServiceWorker | null, parentManager: CRNetworkManager | null) { - this._client = client; + constructor(session: CRSession, page: Page | null, serviceWorker: CRServiceWorker | null, parentManager: CRNetworkManager | null) { + this._session = session; this._page = page; this._serviceWorker = serviceWorker; this._parentManager = parentManager; - this._eventListeners = this.instrumentNetworkEvents(client); + this._eventListeners = this.instrumentNetworkEvents({ session }); } - instrumentNetworkEvents(session: CRSession, workerFrame?: frames.Frame): RegisteredListener[] { + instrumentNetworkEvents(sessionInfo: SessionInfo): RegisteredListener[] { const listeners = [ - eventsHelper.addEventListener(session, 'Fetch.requestPaused', this._onRequestPaused.bind(this, workerFrame)), - eventsHelper.addEventListener(session, 'Fetch.authRequired', this._onAuthRequired.bind(this)), - eventsHelper.addEventListener(session, 'Network.requestWillBeSent', this._onRequestWillBeSent.bind(this, workerFrame)), - eventsHelper.addEventListener(session, 'Network.requestWillBeSentExtraInfo', this._onRequestWillBeSentExtraInfo.bind(this)), - eventsHelper.addEventListener(session, 'Network.requestServedFromCache', this._onRequestServedFromCache.bind(this)), - eventsHelper.addEventListener(session, 'Network.responseReceived', this._onResponseReceived.bind(this)), - eventsHelper.addEventListener(session, 'Network.responseReceivedExtraInfo', this._onResponseReceivedExtraInfo.bind(this)), - eventsHelper.addEventListener(session, 'Network.loadingFinished', this._onLoadingFinished.bind(this)), - eventsHelper.addEventListener(session, 'Network.loadingFailed', this._onLoadingFailed.bind(this, workerFrame)), + eventsHelper.addEventListener(sessionInfo.session, 'Fetch.requestPaused', this._onRequestPaused.bind(this, sessionInfo)), + eventsHelper.addEventListener(sessionInfo.session, 'Fetch.authRequired', this._onAuthRequired.bind(this)), + eventsHelper.addEventListener(sessionInfo.session, 'Network.requestWillBeSent', this._onRequestWillBeSent.bind(this, sessionInfo)), + eventsHelper.addEventListener(sessionInfo.session, 'Network.requestWillBeSentExtraInfo', this._onRequestWillBeSentExtraInfo.bind(this)), + eventsHelper.addEventListener(sessionInfo.session, 'Network.requestServedFromCache', this._onRequestServedFromCache.bind(this)), + eventsHelper.addEventListener(sessionInfo.session, 'Network.responseReceived', this._onResponseReceived.bind(this, sessionInfo)), + eventsHelper.addEventListener(sessionInfo.session, 'Network.responseReceivedExtraInfo', this._onResponseReceivedExtraInfo.bind(this)), + eventsHelper.addEventListener(sessionInfo.session, 'Network.loadingFinished', this._onLoadingFinished.bind(this)), + eventsHelper.addEventListener(sessionInfo.session, 'Network.loadingFailed', this._onLoadingFailed.bind(this, sessionInfo)), ]; if (this._page) { listeners.push(...[ - eventsHelper.addEventListener(session, 'Network.webSocketCreated', e => this._page!._frameManager.onWebSocketCreated(e.requestId, e.url)), - eventsHelper.addEventListener(session, 'Network.webSocketWillSendHandshakeRequest', e => this._page!._frameManager.onWebSocketRequest(e.requestId)), - eventsHelper.addEventListener(session, 'Network.webSocketHandshakeResponseReceived', e => this._page!._frameManager.onWebSocketResponse(e.requestId, e.response.status, e.response.statusText)), - eventsHelper.addEventListener(session, 'Network.webSocketFrameSent', e => e.response.payloadData && this._page!._frameManager.onWebSocketFrameSent(e.requestId, e.response.opcode, e.response.payloadData)), - eventsHelper.addEventListener(session, 'Network.webSocketFrameReceived', e => e.response.payloadData && this._page!._frameManager.webSocketFrameReceived(e.requestId, e.response.opcode, e.response.payloadData)), - eventsHelper.addEventListener(session, 'Network.webSocketClosed', e => this._page!._frameManager.webSocketClosed(e.requestId)), - eventsHelper.addEventListener(session, 'Network.webSocketFrameError', e => this._page!._frameManager.webSocketError(e.requestId, e.errorMessage)), + eventsHelper.addEventListener(sessionInfo.session, 'Network.webSocketCreated', e => this._page!._frameManager.onWebSocketCreated(e.requestId, e.url)), + eventsHelper.addEventListener(sessionInfo.session, 'Network.webSocketWillSendHandshakeRequest', e => this._page!._frameManager.onWebSocketRequest(e.requestId)), + eventsHelper.addEventListener(sessionInfo.session, 'Network.webSocketHandshakeResponseReceived', e => this._page!._frameManager.onWebSocketResponse(e.requestId, e.response.status, e.response.statusText)), + eventsHelper.addEventListener(sessionInfo.session, 'Network.webSocketFrameSent', e => e.response.payloadData && this._page!._frameManager.onWebSocketFrameSent(e.requestId, e.response.opcode, e.response.payloadData)), + eventsHelper.addEventListener(sessionInfo.session, 'Network.webSocketFrameReceived', e => e.response.payloadData && this._page!._frameManager.webSocketFrameReceived(e.requestId, e.response.opcode, e.response.payloadData)), + eventsHelper.addEventListener(sessionInfo.session, 'Network.webSocketClosed', e => this._page!._frameManager.webSocketClosed(e.requestId)), + eventsHelper.addEventListener(sessionInfo.session, 'Network.webSocketFrameError', e => this._page!._frameManager.webSocketError(e.requestId, e.errorMessage)), ]); } return listeners; } async initialize() { - await this._client.send('Network.enable'); + await this._session.send('Network.enable'); } dispose() { @@ -92,7 +97,7 @@ export class CRNetworkManager { } async setOffline(offline: boolean) { - await this._client.send('Network.emulateNetworkConditions', { + await this._session.send('Network.emulateNetworkConditions', { offline, // values of 0 remove any active throttling. crbug.com/456324#c9 latency: 0, @@ -113,41 +118,41 @@ export class CRNetworkManager { this._protocolRequestInterceptionEnabled = enabled; if (enabled) { await Promise.all([ - this._client.send('Network.setCacheDisabled', { cacheDisabled: true }), - this._client.send('Fetch.enable', { + this._session.send('Network.setCacheDisabled', { cacheDisabled: true }), + this._session.send('Fetch.enable', { handleAuthRequests: true, patterns: [{ urlPattern: '*', requestStage: 'Request' }], }), ]); } else { await Promise.all([ - this._client.send('Network.setCacheDisabled', { cacheDisabled: false }), - this._client.send('Fetch.disable') + this._session.send('Network.setCacheDisabled', { cacheDisabled: false }), + this._session.send('Fetch.disable') ]); } } async clearCache() { // Sending 'Network.setCacheDisabled' with 'cacheDisabled = true' will clear the MemoryCache. - await this._client.send('Network.setCacheDisabled', { cacheDisabled: true }); + await this._session.send('Network.setCacheDisabled', { cacheDisabled: true }); if (!this._protocolRequestInterceptionEnabled) - await this._client.send('Network.setCacheDisabled', { cacheDisabled: false }); - await this._client.send('Network.clearBrowserCache'); + await this._session.send('Network.setCacheDisabled', { cacheDisabled: false }); + await this._session.send('Network.clearBrowserCache'); } - _onRequestWillBeSent(workerFrame: frames.Frame | undefined, event: Protocol.Network.requestWillBeSentPayload) { + _onRequestWillBeSent(sessionInfo: SessionInfo, event: Protocol.Network.requestWillBeSentPayload) { // Request interception doesn't happen for data URLs with Network Service. if (this._protocolRequestInterceptionEnabled && !event.request.url.startsWith('data:')) { const requestId = event.requestId; const requestPausedEvent = this._requestIdToRequestPausedEvent.get(requestId); if (requestPausedEvent) { - this._onRequest(workerFrame, event, requestPausedEvent); + this._onRequest(sessionInfo, event, requestPausedEvent); this._requestIdToRequestPausedEvent.delete(requestId); } else { this._requestIdToRequestWillBeSentEvent.set(event.requestId, event); } } else { - this._onRequest(workerFrame, event, null); + this._onRequest(sessionInfo, event, null); } } @@ -169,7 +174,7 @@ export class CRNetworkManager { this._attemptedAuthentications.add(event.requestId); } const { username, password } = shouldProvideCredentials && this._credentials ? this._credentials : { username: undefined, password: undefined }; - this._client._sendMayFail('Fetch.continueWithAuth', { + this._session._sendMayFail('Fetch.continueWithAuth', { requestId: event.requestId, authChallengeResponse: { response, username, password }, }); @@ -181,12 +186,12 @@ export class CRNetworkManager { return !this._credentials.origin || new URL(url).origin.toLowerCase() === this._credentials.origin.toLowerCase(); } - _onRequestPaused(workerFrame: frames.Frame | undefined, event: Protocol.Fetch.requestPausedPayload) { + _onRequestPaused(sessionInfo: SessionInfo, event: Protocol.Fetch.requestPausedPayload) { if (!event.networkId) { // Fetch without networkId means that request was not recongnized by inspector, and // it will never receive Network.requestWillBeSent. Most likely, this is an internal request // that we can safely fail. - this._client._sendMayFail('Fetch.failRequest', { + this._session._sendMayFail('Fetch.failRequest', { requestId: event.requestId, errorReason: 'Aborted', }); @@ -198,14 +203,14 @@ export class CRNetworkManager { const requestId = event.networkId; const requestWillBeSentEvent = this._requestIdToRequestWillBeSentEvent.get(requestId); if (requestWillBeSentEvent) { - this._onRequest(workerFrame, requestWillBeSentEvent, event); + this._onRequest(sessionInfo, requestWillBeSentEvent, event); this._requestIdToRequestWillBeSentEvent.delete(requestId); } else { this._requestIdToRequestPausedEvent.set(requestId, event); } } - _onRequest(workerFrame: frames.Frame | undefined, requestWillBeSentEvent: Protocol.Network.requestWillBeSentPayload, requestPausedEvent: Protocol.Fetch.requestPausedPayload | null) { + _onRequest(sessionInfo: SessionInfo, requestWillBeSentEvent: Protocol.Network.requestWillBeSentPayload, requestPausedEvent: Protocol.Fetch.requestPausedPayload | null) { if (requestWillBeSentEvent.request.url.startsWith('data:')) return; let redirectedFrom: InterceptableRequest | null = null; @@ -217,7 +222,7 @@ export class CRNetworkManager { redirectedFrom = request; } } - let frame = requestWillBeSentEvent.frameId ? this._page?._frameManager.frame(requestWillBeSentEvent.frameId) : workerFrame; + let frame = requestWillBeSentEvent.frameId ? this._page?._frameManager.frame(requestWillBeSentEvent.frameId) : sessionInfo.workerFrame; // Requests from workers lack frameId, because we receive Network.requestWillBeSent // on the worker target. However, we receive Fetch.requestPaused on the page target, // and lack workerFrame there. Luckily, Fetch.requestPaused provides a frameId. @@ -246,7 +251,7 @@ export class CRNetworkManager { ]; if (requestHeaders['Access-Control-Request-Headers']) responseHeaders.push({ name: 'Access-Control-Allow-Headers', value: requestHeaders['Access-Control-Request-Headers'] }); - this._client._sendMayFail('Fetch.fulfillRequest', { + this._session._sendMayFail('Fetch.fulfillRequest', { requestId: requestPausedEvent.requestId, responseCode: 204, responsePhrase: network.STATUS_TEXTS['204'], @@ -259,7 +264,7 @@ export class CRNetworkManager { // Non-service-worker requests MUST have a frame—if they don't, we pretend there was no request if (!frame && !this._serviceWorker) { if (requestPausedEvent) - this._client._sendMayFail('Fetch.continueRequest', { requestId: requestPausedEvent.requestId }); + this._session._sendMayFail('Fetch.continueRequest', { requestId: requestPausedEvent.requestId }); return; } @@ -267,14 +272,14 @@ export class CRNetworkManager { if (requestPausedEvent) { // We do not support intercepting redirects. if (redirectedFrom || (!this._userRequestInterceptionEnabled && this._protocolRequestInterceptionEnabled)) - this._client._sendMayFail('Fetch.continueRequest', { requestId: requestPausedEvent.requestId }); + this._session._sendMayFail('Fetch.continueRequest', { requestId: requestPausedEvent.requestId }); else - route = new RouteImpl(this._client, requestPausedEvent.requestId); + route = new RouteImpl(this._session, requestPausedEvent.requestId); } const isNavigationRequest = requestWillBeSentEvent.requestId === requestWillBeSentEvent.loaderId && requestWillBeSentEvent.type === 'Document'; const documentId = isNavigationRequest ? requestWillBeSentEvent.loaderId : undefined; const request = new InterceptableRequest({ - owningNetworkManager: this, + session: sessionInfo.session, context: (this._page || this._serviceWorker)!._browserContext, frame: frame || null, serviceWorker: this._serviceWorker || null, @@ -300,19 +305,19 @@ export class CRNetworkManager { const contentLengthHeader = Object.entries(responsePayload.headers).find(header => header[0].toLowerCase() === 'content-length'); const expectedLength = contentLengthHeader ? +contentLengthHeader[1] : undefined; - const client = request._owningNetworkManager._client; - const response = await client.send('Network.getResponseBody', { requestId: request._requestId }); + const session = request.session; + const response = await session.send('Network.getResponseBody', { requestId: request._requestId }); if (response.body || !expectedLength) return Buffer.from(response.body, response.base64Encoded ? 'base64' : 'utf8'); // For { // 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._client._sendMayFail('Fetch.continueRequest', { + await this._session._sendMayFail('Fetch.continueRequest', { requestId: this._interceptionId!, url: overrides.url, headers: overrides.headers, @@ -560,7 +565,7 @@ class RouteImpl implements network.RouteDelegate { const responseHeaders = splitSetCookieHeader(response.headers); // 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._client._sendMayFail('Fetch.fulfillRequest', { + await this._session._sendMayFail('Fetch.fulfillRequest', { requestId: this._interceptionId!, responseCode: response.status, responsePhrase: network.STATUS_TEXTS[String(response.status)], @@ -574,7 +579,7 @@ class RouteImpl implements network.RouteDelegate { assert(errorReason, 'Unknown error code: ' + errorCode); // 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._client._sendMayFail('Fetch.failRequest', { + await this._session._sendMayFail('Fetch.failRequest', { requestId: this._interceptionId!, errorReason }); diff --git a/packages/playwright-core/src/server/chromium/crPage.ts b/packages/playwright-core/src/server/chromium/crPage.ts index c992d0632a..f3dfbf94a7 100644 --- a/packages/playwright-core/src/server/chromium/crPage.ts +++ b/packages/playwright-core/src/server/chromium/crPage.ts @@ -758,7 +758,7 @@ class FrameSession { }); session.on('Runtime.exceptionThrown', exception => this._page.emit(Page.Events.PageError, exceptionToError(exception.exceptionDetails))); // TODO: attribute workers to the right frame. - this._networkManager.instrumentNetworkEvents(session, this._page._frameManager.frame(this._targetId)!); + this._networkManager.instrumentNetworkEvents({ session, workerFrame: this._page._frameManager.frame(this._targetId) ?? undefined }); } _onDetachedFromTarget(event: Protocol.Target.detachedFromTargetPayload) { diff --git a/tests/page/workers.spec.ts b/tests/page/workers.spec.ts index bb0e929af8..1222340663 100644 --- a/tests/page/workers.spec.ts +++ b/tests/page/workers.spec.ts @@ -18,6 +18,7 @@ import { test as it, expect } from './pageTest'; import { attachFrame } from '../config/utils'; import type { ConsoleMessage } from 'playwright-core'; +import fs from 'fs'; it('Page.workers @smoke', async function({ page, server }) { await Promise.all([ @@ -143,7 +144,7 @@ it('should attribute network activity for worker inside iframe to the iframe', a expect(request.frame()).toBe(frame); }); -it('should report network activity', async function({ page, server, browserName, browserMajorVersion }) { +it('should report network activity', async function({ page, server, browserName, browserMajorVersion, asset }) { it.skip(browserName === 'firefox' && browserMajorVersion < 114, 'https://github.com/microsoft/playwright/issues/21760'); const [worker] = await Promise.all([ page.waitForEvent('worker'), @@ -158,6 +159,7 @@ it('should report network activity', async function({ page, server, browserName, expect(request.url()).toBe(url); expect(response.request()).toBe(request); expect(response.ok()).toBe(true); + expect(await response.text()).toBe(fs.readFileSync(asset('one-style.css'), 'utf8')); }); it('should report network activity on worker creation', async function({ page, server, browserName, browserMajorVersion }) {