From 8b6f2a74f210a7f3a8efee79a3733fce51be2738 Mon Sep 17 00:00:00 2001 From: Rob Wu Date: Sat, 8 Feb 2025 13:31:59 +0000 Subject: [PATCH] Bug 1826867 - Use NS_GetFinalChannelURI instead of GetURI in SessionHistoryEntry a=dmeehan When redirects are involved, `DocumentLoadListener::DoOnStartRequest` may call `ReplaceLoadingSessionHistoryEntryForLoad`, which updates the history entry with the destination of a redirect. But if the redirection target is a `moz-extension:`-URL, the URL becomes a jar:file:/file: URL. This is because SessionHistoryInfo (in SessionHistoryEntry.cpp) looks up the URL with `nsIChannel::GetURI`. For `moz-extension:`-URLs, the underlying channel has a `jar:file:` or `file:` URL, as provided by ExtensionProtocolHandler (via SubstitutingProtocolHandler::NewChannel). For details, see https://bugzilla.mozilla.org/show_bug.cgi?id=1826867#c7 To fix this, this patch switches to `NS_GetFinalChannelURI` instead. For more history on this type of bug and SessionHistoryInfo, see https://bugzilla.mozilla.org/show_bug.cgi?id=1826867#c9 Original Revision: https://phabricator.services.mozilla.com/D234333 Differential Revision: https://phabricator.services.mozilla.com/D236901 --- docshell/shistory/SessionHistoryEntry.cpp | 5 +- .../test_ext_dnr_redirect_main_frame.js | 78 +++++++++++++++++++ .../test/xpcshell/xpcshell-common.toml | 2 + 3 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 toolkit/components/extensions/test/xpcshell/test_ext_dnr_redirect_main_frame.js diff --git a/docshell/shistory/SessionHistoryEntry.cpp b/docshell/shistory/SessionHistoryEntry.cpp index 4f5b760c8a1c..078d4ac3a757 100644 --- a/docshell/shistory/SessionHistoryEntry.cpp +++ b/docshell/shistory/SessionHistoryEntry.cpp @@ -97,7 +97,10 @@ SessionHistoryInfo::SessionHistoryInfo( nsIChannel* aChannel, uint32_t aLoadType, nsIPrincipal* aPartitionedPrincipalToInherit, nsIContentSecurityPolicy* aCsp) { - aChannel->GetURI(getter_AddRefs(mURI)); + if (NS_FAILED(NS_GetFinalChannelURI(aChannel, getter_AddRefs(mURI)))) { + NS_WARNING("NS_GetFinalChannelURI somehow failed in SessionHistoryInfo?"); + aChannel->GetURI(getter_AddRefs(mURI)); + } mLoadType = aLoadType; nsCOMPtr loadInfo; diff --git a/toolkit/components/extensions/test/xpcshell/test_ext_dnr_redirect_main_frame.js b/toolkit/components/extensions/test/xpcshell/test_ext_dnr_redirect_main_frame.js new file mode 100644 index 000000000000..4aab21434f60 --- /dev/null +++ b/toolkit/components/extensions/test/xpcshell/test_ext_dnr_redirect_main_frame.js @@ -0,0 +1,78 @@ +"use strict"; + +const server = createHttpServer({ hosts: ["example.com"] }); +server.registerPathHandler("/from", () => { + Assert.ok(false, "Test should have redirected /from elsewhere"); +}); + +add_task(async function test_navigate_main_frame() { + let extension = ExtensionTestUtils.loadExtension({ + async background() { + await browser.declarativeNetRequest.updateSessionRules({ + addRules: [ + { + id: 1, + condition: { urlFilter: "from", resourceTypes: ["main_frame"] }, + action: { type: "redirect", redirect: { extensionPath: "/t.htm" } }, + }, + ], + }); + browser.test.sendMessage("redirect_setup"); + }, + manifest: { + manifest_version: 3, + permissions: ["declarativeNetRequest"], + host_permissions: ["*://example.com/*"], + granted_host_permissions: true, + web_accessible_resources: [ + { resources: ["t.htm"], matches: ["*://example.com/*"] }, + ], + }, + temporarilyInstalled: true, // <-- for granted_host_permissions + files: { + "t.htm": ``, + "to.js": () => { + browser.test.sendMessage("redirect_target_loaded", location.href); + }, + }, + }); + await extension.startup(); + await extension.awaitMessage("redirect_setup"); + const expectedFinalUrl = `moz-extension://${extension.uuid}/t.htm`; + let contentPage = await ExtensionTestUtils.loadContentPage( + "http://example.com/from", + { redirectUrl: expectedFinalUrl } + ); + equal( + await extension.awaitMessage("redirect_target_loaded"), + expectedFinalUrl, + "Page at expected URL" + ); + + // Regression test for bug 1940339 / bug 1826867: we should see the + // moz-extension:-URL here, and not the underlying jar:file:-URL. + if (Services.appinfo.sessionHistoryInParent) { + let sh = contentPage.browsingContext.sessionHistory; + let she = sh.getEntryAtIndex(sh.index); + equal(she.URI.spec, expectedFinalUrl, "SessionHistoryEntry url is correct"); + // Extra sanity check: when extensions trigger redirects, the history is + // not populated with pre-redirect URLs. + equal(sh.index, 0, "No pre-redirect history entries"); + } else { + await contentPage.spawn([expectedFinalUrl], expectedFinalUrl => { + let sh = this.docShell.QueryInterface(Ci.nsIWebNavigation).sessionHistory; + let she = sh.legacySHistory.getEntryAtIndex(sh.index); + Assert.equal( + she.URI.spec, + expectedFinalUrl, + "SessionHistoryEntry url is correct (with SHIP disabled)" + ); + // Extra sanity check: when extensions trigger redirects, the history is + // not populated with pre-redirect URLs. + Assert.equal(sh.index, 0, "No pre-redirect history entries"); + }); + } + + await contentPage.close(); + await extension.unload(); +}); diff --git a/toolkit/components/extensions/test/xpcshell/xpcshell-common.toml b/toolkit/components/extensions/test/xpcshell/xpcshell-common.toml index c60056f28417..b5b9c2b6a4e3 100644 --- a/toolkit/components/extensions/test/xpcshell/xpcshell-common.toml +++ b/toolkit/components/extensions/test/xpcshell/xpcshell-common.toml @@ -229,6 +229,8 @@ skip-if = ["os == 'android'"] # Android: downloads.download goes through the emb ["test_ext_dnr_private_browsing.js"] +["test_ext_dnr_redirect_main_frame.js"] + ["test_ext_dnr_redirect_transform.js"] ["test_ext_dnr_regexFilter.js"]