Bug 1678255 - prompt for external protocol links whose loads were also triggered externally, instead of looping forever, r=pbz,nika

This passes around the "are we external" bit of load information a bunch,
such that the external protocol handling code has access to it.

In this bug and bug 1667468, I think ideally I would have used a check
if we're the OS default for a given protocol before continuing. However,
this information is currently unavailable on Linux (bug 1599713), and
worse, I believe is likely to remain unavailable in flatpak and other
such restricted environments (cf. bug 1618094 - we aren't able to find
out anything about protocol handlers from the OS).

So instead, we prompt the user if we are about to open a link passed
to us externally. There is a small chance this will be Breaking People's
Workflows, where I don't know whether anyone relies on Firefox happily
passing these URIs along to the relevant application (more convenient
than doing all the registry/API work yourself in scripts!) or anything
like that. To help with that, there's a pref,
`network.protocol-handler.prompt-from-external`, that can be created and
set to false to avoid prompting in this case.

Differential Revision: https://phabricator.services.mozilla.com/D103967

docshell/base/nsDocShell.cpp

@@ -12773,7 +12773,8 @@ nsresult nsDocShell::OnLinkClickSync(nsIContent* aContent,
             extProtService->IsExposedProtocol(scheme.get(), &isExposed);
         if (NS_SUCCEEDED(rv) && !isExposed) {
           return extProtService->LoadURI(aLoadState->URI(), triggeringPrincipal,
-                                         mBrowsingContext);
+                                         mBrowsingContext,
+                                         /* aTriggeredExternally */ false);
         }
       }
     }

dom/ipc/ContentParent.cpp

@@ -4473,7 +4473,8 @@ mozilla::ipc::IPCResult ContentParent::RecvAccumulateMixedContentHSTS(
 
 mozilla::ipc::IPCResult ContentParent::RecvLoadURIExternal(
     nsIURI* uri, nsIPrincipal* aTriggeringPrincipal,
-    const MaybeDiscarded<BrowsingContext>& aContext) {
+    const MaybeDiscarded<BrowsingContext>& aContext,
+    bool aWasExternallyTriggered) {
   if (aContext.IsDiscarded()) {
     return IPC_OK();
   }
@@ -4489,7 +4490,8 @@ mozilla::ipc::IPCResult ContentParent::RecvLoadURIExternal(
   }
 
   BrowsingContext* bc = aContext.get();
-  extProtService->LoadURI(uri, aTriggeringPrincipal, bc);
+  extProtService->LoadURI(uri, aTriggeringPrincipal, bc,
+                          aWasExternallyTriggered);
   return IPC_OK();
 }
 

dom/ipc/ContentParent.h

@@ -1087,7 +1087,8 @@ class ContentParent final
 
   mozilla::ipc::IPCResult RecvLoadURIExternal(
       nsIURI* uri, nsIPrincipal* triggeringPrincipal,
-      const MaybeDiscarded<BrowsingContext>& aContext);
+      const MaybeDiscarded<BrowsingContext>& aContext,
+      bool aWasExternallyTriggered);
   mozilla::ipc::IPCResult RecvExtProtocolChannelConnectParent(
       const uint64_t& registrarId);
 

dom/ipc/PContent.ipdl

@@ -1047,7 +1047,10 @@ parent:
     async StartVisitedQueries(nsIURI[] uri);
     async SetURITitle(nsIURI uri, nsString title);
 
-    async LoadURIExternal(nsIURI uri, nsIPrincipal triggeringPrincipal, MaybeDiscardedBrowsingContext browsingContext);
+    async LoadURIExternal(nsIURI uri,
+                          nsIPrincipal triggeringPrincipal,
+                          MaybeDiscardedBrowsingContext browsingContext,
+                          bool wasExternallyTriggered);
     async ExtProtocolChannelConnectParent(uint64_t registrarId);
 
     // PrefService message

toolkit/mozapps/handling/ContentDispatchChooser.jsm

@@ -19,6 +19,14 @@ var EXPORTED_SYMBOLS = [
   "ContentDispatchChooserTelemetry",
 ];
 
+const gPrefs = {};
+XPCOMUtils.defineLazyPreferenceGetter(
+  gPrefs,
+  "promptForExternal",
+  "network.protocol-handler.prompt-from-external",
+  true
+);
+
 const PROTOCOL_HANDLER_OPEN_PERM_KEY = "open-protocol-handler";
 const PERMISSION_KEY_DELIMITER = "^";
 
@@ -238,13 +246,27 @@ class nsContentDispatchChooser {
    * @param {nsIURI} aURI - URI to be handled.
    * @param {nsIPrincipal} [aPrincipal] - Principal which triggered the load.
    * @param {BrowsingContext} [aBrowsingContext] - Context of the load.
+   * @param {bool} [aTriggeredExternally] - Whether the load came from outside
+   * this application.
    */
-  async handleURI(aHandler, aURI, aPrincipal, aBrowsingContext) {
+  async handleURI(
+    aHandler,
+    aURI,
+    aPrincipal,
+    aBrowsingContext,
+    aTriggeredExternally = false
+  ) {
     let callerHasPermission = this._hasProtocolHandlerPermission(
       aHandler.type,
       aPrincipal
     );
 
+    // Force showing the dialog for links passed from outside the application.
+    // This avoids infinite loops, see bug 1678255, bug 1667468, etc.
+    if (aTriggeredExternally && gPrefs.promptForExternal) {
+      aHandler.alwaysAskBeforeHandling = true;
+    }
+
     // Skip the dialog if a preferred application is set and the caller has
     // permission.
     if (

uriloader/exthandler/nsExternalHelperAppService.cpp

@@ -989,12 +989,13 @@ static const char kExternalProtocolDefaultPref[] =
 NS_IMETHODIMP
 nsExternalHelperAppService::LoadURI(nsIURI* aURI,
                                     nsIPrincipal* aTriggeringPrincipal,
-                                    BrowsingContext* aBrowsingContext) {
+                                    BrowsingContext* aBrowsingContext,
+                                    bool aTriggeredExternally) {
   NS_ENSURE_ARG_POINTER(aURI);
 
   if (XRE_IsContentProcess()) {
     mozilla::dom::ContentChild::GetSingleton()->SendLoadURIExternal(
-        aURI, aTriggeringPrincipal, aBrowsingContext);
+        aURI, aTriggeringPrincipal, aBrowsingContext, aTriggeredExternally);
     return NS_OK;
   }
 
@@ -1096,7 +1097,7 @@ nsExternalHelperAppService::LoadURI(nsIURI* aURI,
   NS_ENSURE_SUCCESS(rv, rv);
 
   return chooser->HandleURI(handler, uri, aTriggeringPrincipal,
-                            aBrowsingContext);
+                            aBrowsingContext, aTriggeredExternally);
 }
 
 //////////////////////////////////////////////////////////////////////////////////////////////////////

uriloader/exthandler/nsExternalHelperAppService.h

@@ -82,7 +82,8 @@ class nsExternalHelperAppService : public nsIExternalHelperAppService,
   NS_IMETHOD GetProtocolHandlerInfo(const nsACString& aScheme,
                                     nsIHandlerInfo** aHandlerInfo) override;
   NS_IMETHOD LoadURI(nsIURI* aURI, nsIPrincipal* aTriggeringPrincipal,
-                     mozilla::dom::BrowsingContext* aBrowsingContext) override;
+                     mozilla::dom::BrowsingContext* aBrowsingContext,
+                     bool aWasTriggeredExternally) override;
   NS_IMETHOD SetProtocolHandlerDefaults(nsIHandlerInfo* aHandlerInfo,
                                         bool aOSHandlerExists) override;
 

uriloader/exthandler/nsExternalProtocolHandler.cpp

@@ -165,7 +165,8 @@ nsresult nsExtProtocolChannel::OpenURL() {
     }
 
     RefPtr<nsIPrincipal> principal = mLoadInfo->TriggeringPrincipal();
-    rv = extProtService->LoadURI(mUrl, principal, ctx);
+    rv = extProtService->LoadURI(mUrl, principal, ctx,
+                                 mLoadInfo->GetLoadTriggeredFromExternal());
 
     if (NS_SUCCEEDED(rv) && mListener) {
       mStatus = NS_ERROR_NO_CONTENT;

uriloader/exthandler/nsIContentDispatchChooser.idl

@@ -29,10 +29,13 @@ interface nsIContentDispatchChooser : nsISupports {
   *        The principal making the request.
   * @param aBrowsingContext
   *        The browsing context where the load should happen.
+  * @param aWasTriggeredExternally
+  *        True if the load was tripped by an external app.
   */
   void handleURI(in nsIHandlerInfo aHandler,
            in nsIURI aURI,
            in nsIPrincipal aTriggeringPrincipal,
-           in BrowsingContext aBrowsingContext);
+           in BrowsingContext aBrowsingContext,
+           [optional] in bool aWasTriggeredExternally);
 };
 

uriloader/exthandler/nsIExternalProtocolService.idl

@@ -109,13 +109,17 @@ interface nsIExternalProtocolService : nsISupports
    *        handlers will fail.  We need to do better than that; bug 394483
    *        filed in order to track.
    *
+   * @param aWasTriggeredExternally
+   *        If true, indicates the load was initiated by an external app.
+   *
    * @note  Embedders that do not expose the http protocol should not currently
    *        use web-based protocol handlers, as handoff won't work correctly
    *        (bug 394479).
    */
   void loadURI(in nsIURI aURI,
                [optional] in nsIPrincipal aTriggeringPrincipal,
-               [optional] in BrowsingContext aBrowsingContext);
+               [optional] in BrowsingContext aBrowsingContext,
+               [optional] in bool aWasTriggeredExternally);
 
   /**
    * Gets a human-readable description for the application responsible for

uriloader/exthandler/tests/mochitest/browser.ini

@@ -45,6 +45,7 @@ support-files =
 [browser_first_prompt_not_blocked_without_user_interaction.js]
 support-files =
   file_external_protocol_iframe.html
+[browser_protocol_ask_dialog_external.js]
 [browser_protocol_ask_dialog_permission.js]
 [browser_protocolhandler_loop.js]
 [browser_remember_download_option.js]

uriloader/exthandler/tests/mochitest/browser_protocol_ask_dialog_external.js

@@ -0,0 +1,106 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+ChromeUtils.import(
+  "resource://testing-common/HandlerServiceTestUtils.jsm",
+  this
+);
+
+let gHandlerService = Cc["@mozilla.org/uriloader/handler-service;1"].getService(
+  Ci.nsIHandlerService
+);
+
+/**
+ * Creates dummy protocol handler
+ */
+function initTestHandlers() {
+  let handlerInfo = HandlerServiceTestUtils.getBlankHandlerInfo("yoink");
+
+  let appHandler = Cc[
+    "@mozilla.org/uriloader/local-handler-app;1"
+  ].createInstance(Ci.nsILocalHandlerApp);
+  // This is a dir and not executable, but that's enough for here.
+  appHandler.executable = Services.dirsvc.get("XCurProcD", Ci.nsIFile);
+  handlerInfo.possibleApplicationHandlers.appendElement(appHandler);
+  handlerInfo.preferredApplicationHandler = appHandler;
+  handlerInfo.preferredAction = handlerInfo.useHelperApp;
+  handlerInfo.alwaysAskBeforeHandling = false;
+  gHandlerService.store(handlerInfo);
+  registerCleanupFunction(() => {
+    gHandlerService.remove(handlerInfo);
+  });
+}
+
+/**
+ * Check that if we get a direct request from another app / the OS to open a
+ * link, we always prompt, even if we think we know what the correct answer
+ * is. This is to avoid infinite loops in such situations where the OS and
+ * Firefox have conflicting ideas about the default handler, or where our
+ * checks with the OS don't work (Linux and/or Snap, at time of this comment).
+ */
+add_task(async function test_external_asks_anyway() {
+  await SpecialPowers.pushPrefEnv({
+    set: [["network.protocol-handler.prompt-from-external", true]],
+  });
+  initTestHandlers();
+
+  let cmdLineHandler = Cc["@mozilla.org/browser/final-clh;1"].getService(
+    Ci.nsICommandLineHandler
+  );
+  let fakeCmdLine = {
+    length: 1,
+    _arg: "yoink:yoink",
+
+    getArgument(aIndex) {
+      if (aIndex == 0) {
+        return this._arg;
+      }
+      throw Components.Exception("", Cr.NS_ERROR_INVALID_ARG);
+    },
+
+    findFlag() {
+      return -1;
+    },
+
+    handleFlagWithParam() {
+      if (this._argCount) {
+        this._argCount = 0;
+        return this._arg;
+      }
+
+      return "";
+    },
+
+    state: 2,
+
+    STATE_INITIAL_LAUNCH: 0,
+    STATE_REMOTE_AUTO: 1,
+    STATE_REMOTE_EXPLICIT: 2,
+
+    preventDefault: false,
+
+    resolveURI() {
+      return Services.io.newURI(this._arg);
+    },
+    QueryInterface: ChromeUtils.generateQI(["nsICommandLine"]),
+  };
+  let chooserDialogOpenPromise = waitForProtocolAppChooserDialog(
+    gBrowser,
+    true
+  );
+  cmdLineHandler.handle(fakeCmdLine);
+  let dialog = await chooserDialogOpenPromise;
+  ok(dialog, "Should have prompted.");
+
+  let dialogClosedPromise = waitForProtocolAppChooserDialog(
+    gBrowser.selectedBrowser,
+    false
+  );
+  let dialogEl = dialog._frame.contentDocument.querySelector("dialog");
+  dialogEl.cancelDialog();
+  await dialogClosedPromise;
+  // We will have opened a tab; close it.
+  BrowserTestUtils.removeTab(gBrowser.selectedTab);
+});

uriloader/exthandler/tests/mochitest/head.js

@@ -125,11 +125,26 @@ async function openHelperAppDialog(launcher) {
   return dlg;
 }
 
+/**
+ * Wait for a subdialog event indicating a dialog either opened
+ * or was closed.
+ *
+ * First argument is the browser in which to listen. If a tabbrowser,
+ * we listen to subdialogs for any tab of that browser.
+ */
 async function waitForSubDialog(browser, url, state) {
   let eventStr = state ? "dialogopen" : "dialogclose";
 
-  let tabDialogBox = gBrowser.getTabDialogBox(browser);
-  let dialogStack = tabDialogBox.getTabDialogManager()._dialogStack;
+  let eventTarget;
+
+  // Tabbrowser?
+  if (browser.tabContainer) {
+    eventTarget = browser.tabContainer.ownerDocument.documentElement;
+  } else {
+    // Individual browser. Get its box:
+    let tabDialogBox = browser.ownerGlobal.gBrowser.getTabDialogBox(browser);
+    eventTarget = tabDialogBox.getTabDialogManager()._dialogStack;
+  }
 
   let checkFn;
 
@@ -138,7 +153,7 @@ async function waitForSubDialog(browser, url, state) {
   }
 
   let event = await BrowserTestUtils.waitForEvent(
-    dialogStack,
+    eventTarget,
     eventStr,
     true,
     checkFn