From 33a09fb44a6f593270589acfac482d9b275b389c Mon Sep 17 00:00:00 2001 From: Philip Pfaffe Date: Fri, 17 May 2024 13:18:02 +0000 Subject: [PATCH] Ensure inspectedWindow.reload reloads the correct page This prevents unintended reloads when racing with navigations. Drive-by: Check extension allowlist when loading page resources. Bug: 338248595 Change-Id: Ibbac5bbd45b1db0d05e32fe8e384740933ee4639 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5546062 Reviewed-by: Danil Somsikov Commit-Queue: Philip Pfaffe --- front_end/core/sdk/ResourceTreeModel.test.ts | 28 ++++++++++--- front_end/core/sdk/ResourceTreeModel.ts | 12 ++++-- .../models/extensions/ExtensionServer.test.ts | 40 ++++++++++++++++++ .../models/extensions/ExtensionServer.ts | 41 ++++++++++++------- 4 files changed, 98 insertions(+), 23 deletions(-) diff --git a/front_end/core/sdk/ResourceTreeModel.test.ts b/front_end/core/sdk/ResourceTreeModel.test.ts index 55aa12aa81e..1a3a32d214b 100644 --- a/front_end/core/sdk/ResourceTreeModel.test.ts +++ b/front_end/core/sdk/ResourceTreeModel.test.ts @@ -8,16 +8,22 @@ import { describeWithMockConnection, setMockConnectionResponseHandler, } from '../../testing/MockConnection.js'; -import {addChildFrame, getMainFrame, MAIN_FRAME_ID, navigate} from '../../testing/ResourceTreeHelpers.js'; +import { + addChildFrame, + getInitializedResourceTreeModel, + getMainFrame, + LOADER_ID, + MAIN_FRAME_ID, + navigate, +} from '../../testing/ResourceTreeHelpers.js'; import * as SDK from './sdk.js'; describeWithMockConnection('ResourceTreeModel', () => { - it('calls clearRequests on reloadPage', () => { - const target = createTarget(); - const resourceTreeModel = target.model(SDK.ResourceTreeModel.ResourceTreeModel); + it('calls clearRequests on reloadPage', async () => { + const resourceTreeModel = await getInitializedResourceTreeModel(createTarget()); const clearRequests = sinon.stub(SDK.NetworkManager.NetworkManager.prototype, 'clearRequests'); - resourceTreeModel!.reloadPage(); + resourceTreeModel.reloadPage(); assert.isTrue(clearRequests.calledOnce, 'Not called just once'); }); @@ -102,6 +108,18 @@ describeWithMockConnection('ResourceTreeModel', () => { assert.isTrue(reloadSubframePage.notCalled); }); + it('tags reloads with the targets loaderId', async () => { + const target = createTarget(); + const resourceTreeModel = await getInitializedResourceTreeModel(target); + + const reload = sinon.spy(target.pageAgent(), 'invoke_reload'); + assert.isNotNull(resourceTreeModel.mainFrame); + resourceTreeModel.reloadPage(); + assert.isTrue(reload.calledOnce); + assert.deepStrictEqual( + reload.args[0], [{ignoreCache: undefined, loaderId: LOADER_ID, scriptToEvaluateOnLoad: undefined}]); + }); + it('identifies not top frame', async () => { const tabTarget = createTarget({type: SDK.Target.Type.Tab}); const mainFrameTarget = createTarget({parentTarget: tabTarget}); diff --git a/front_end/core/sdk/ResourceTreeModel.ts b/front_end/core/sdk/ResourceTreeModel.ts index 237dacd53eb..c78faae1302 100644 --- a/front_end/core/sdk/ResourceTreeModel.ts +++ b/front_end/core/sdk/ResourceTreeModel.ts @@ -417,6 +417,10 @@ export class ResourceTreeModel extends SDKModel { } reloadPage(ignoreCache?: boolean, scriptToEvaluateOnLoad?: string): void { + const loaderId = this.mainFrame?.loaderId; + if (!loaderId) { + return; + } // Only dispatch PageReloadRequested upon first reload request to simplify client logic. if (!this.#pendingReloadOptions) { this.dispatchEventToListeners(Events.PageReloadRequested, this); @@ -431,7 +435,7 @@ export class ResourceTreeModel extends SDKModel { networkManager.clearRequests(); } this.dispatchEventToListeners(Events.WillReloadPage); - void this.agent.invoke_reload({ignoreCache, scriptToEvaluateOnLoad}); + void this.agent.invoke_reload({ignoreCache, scriptToEvaluateOnLoad, loaderId}); } navigate(url: Platform.DevToolsPath.UrlString): Promise { @@ -659,7 +663,7 @@ export class ResourceTreeFrame { #sameTargetParentFrameInternal: ResourceTreeFrame|null; readonly #idInternal: Protocol.Page.FrameId; crossTargetParentFrameId: string|null; - #loaderIdInternal: string; + #loaderIdInternal: Protocol.Network.LoaderId; #nameInternal: string|null|undefined; #urlInternal: Platform.DevToolsPath.UrlString; #domainAndRegistryInternal: string; @@ -692,7 +696,7 @@ export class ResourceTreeFrame { this.#idInternal = frameId; this.crossTargetParentFrameId = null; - this.#loaderIdInternal = payload?.loaderId || ''; + this.#loaderIdInternal = payload?.loaderId ?? '' as Protocol.Network.LoaderId; this.#nameInternal = payload && payload.name; this.#urlInternal = payload && payload.url as Platform.DevToolsPath.UrlString || Platform.DevToolsPath.EmptyUrlString; @@ -812,7 +816,7 @@ export class ResourceTreeFrame { return this.#unreachableUrlInternal; } - get loaderId(): string { + get loaderId(): Protocol.Network.LoaderId { return this.#loaderIdInternal; } diff --git a/front_end/models/extensions/ExtensionServer.test.ts b/front_end/models/extensions/ExtensionServer.test.ts index 1bc4e6cb228..e0a7ed7d2f2 100644 --- a/front_end/models/extensions/ExtensionServer.test.ts +++ b/front_end/models/extensions/ExtensionServer.test.ts @@ -52,6 +52,28 @@ describeWithDevtoolsExtension('Extensions', {}, context => { assert.isTrue(addExtensionSpy.calledOnce, 'addExtension called once'); assert.isTrue(addExtensionSpy.returned(undefined), 'addExtension returned undefined'); }); + + it('only returns page resources for allowed targets', async () => { + const urls = ['http://example.com', 'chrome://version'] as Platform.DevToolsPath.UrlString[]; + const targets = urls.map(async url => { + const target = createTarget({url}); + const resourceTreeModel = target.model(SDK.ResourceTreeModel.ResourceTreeModel); + assert.isNotNull(resourceTreeModel); + await resourceTreeModel.once(SDK.ResourceTreeModel.Events.CachedResourcesLoaded); + target.setInspectedURL(url); + resourceTreeModel.mainFrame?.addResource(new SDK.Resource.Resource( + resourceTreeModel, null, url, url, null, null, Common.ResourceType.resourceTypes.Document, 'application/text', + null, null)); + return target; + }); + + await Promise.all(targets); + + const resources = + await new Promise(r => context.chrome.devtools!.inspectedWindow.getResources(r)); + + assert.deepStrictEqual(resources.map(r => r.url), ['https://example.com/', 'http://example.com']); + }); }); describeWithDevtoolsExtension('Extensions', {}, context => { @@ -245,6 +267,24 @@ describeWithDevtoolsExtension('Extensions', {}, context => { await context.chrome.devtools?.recorder.unregisterRecorderExtensionPlugin(extensionPlugin); }); + + it('reload only the main toplevel frame', async () => { + const target = SDK.TargetManager.TargetManager.instance().primaryPageTarget(); + assert.isNotNull(target); + const secondTarget = createTarget(); + + const secondResourceTreeModel = secondTarget.model(SDK.ResourceTreeModel.ResourceTreeModel); + assert.isNotNull(secondResourceTreeModel); + const secondReloadStub = sinon.stub(secondResourceTreeModel, 'reloadPage'); + const resourceTreeModel = target.model(SDK.ResourceTreeModel.ResourceTreeModel); + assert.isNotNull(resourceTreeModel); + const reloadStub = sinon.stub(resourceTreeModel, 'reloadPage'); + const reloadPromise = new Promise(resolve => reloadStub.callsFake(resolve)); + context.chrome.devtools!.inspectedWindow.reload(); + await reloadPromise; + assert.isTrue(reloadStub.calledOnce); + assert.isTrue(secondReloadStub.notCalled); + }); }); const allowedUrl = FRAME_URL; diff --git a/front_end/models/extensions/ExtensionServer.ts b/front_end/models/extensions/ExtensionServer.ts index e7471e04eaa..b5b1d11491e 100644 --- a/front_end/models/extensions/ExtensionServer.ts +++ b/front_end/models/extensions/ExtensionServer.ts @@ -809,7 +809,17 @@ export class ExtensionServer extends Common.ObjectWrapper.ObjectWrapper extension?.isAllowedOnTarget(r.url())); + Logs.NetworkLog.NetworkLog.instance().requests().filter(r => this.extensionAllowedOnURL(r.url(), port)); const harLog = await HAR.Log.Log.build(requests); for (let i = 0; i < harLog.entries.length; ++i) { // @ts-ignore @@ -870,7 +886,7 @@ export class ExtensionServer extends Common.ObjectWrapper.ObjectWrapper { - const url = contentProvider.contentURL(); - const origin = extensionOrigins.get(port); - const extension = origin && this.registeredExtensions.get(origin); - if (!extension?.isAllowedOnTarget(url)) { + if (!this.extensionAllowedOnURL(contentProvider.contentURL(), port)) { this.dispatchCallback(message.requestId, port, this.status.E_FAILED('Permission denied')); return undefined; } @@ -945,9 +960,7 @@ export class ExtensionServer extends Common.ObjectWrapper.ObjectWrapper