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

@@ -97,7 +97,10 @@ SessionHistoryInfo::SessionHistoryInfo(
     nsIChannel* aChannel, uint32_t aLoadType,
     nsIPrincipal* aPartitionedPrincipalToInherit,
     nsIContentSecurityPolicy* aCsp) {
+  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<nsILoadInfo> loadInfo;

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": `<script src="to.js"></script>`,
+      "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();
+});

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"]