Bug 1685801: Part 12 - Remove BrowserUtils.urlSecurityCheck. r=mccr8

This moves the exception prettifying to the script security manager for all JS
callers, where it is much cheaper and more consistently applied.

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

browser/actors/ClickHandlerChild.jsm

@@ -7,11 +7,6 @@ var EXPORTED_SYMBOLS = ["ClickHandlerChild"];
 
 const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
 
-ChromeUtils.defineModuleGetter(
-  this,
-  "BrowserUtils",
-  "resource://gre/modules/BrowserUtils.jsm"
-);
 ChromeUtils.defineModuleGetter(
   this,
   "PrivateBrowsingUtils",
@@ -102,7 +97,10 @@ class ClickHandlerChild extends JSWindowActorChild {
 
     if (href) {
       try {
-        BrowserUtils.urlSecurityCheck(href, principal);
+        Services.scriptSecurityManager.checkLoadURIStrWithPrincipal(
+          principal,
+          href
+        );
       } catch (e) {
         return;
       }

browser/actors/ContextMenuChild.jsm

@@ -17,7 +17,6 @@ XPCOMUtils.defineLazyGlobalGetters(this, ["URL"]);
 
 XPCOMUtils.defineLazyModuleGetters(this, {
   E10SUtils: "resource://gre/modules/E10SUtils.jsm",
-  BrowserUtils: "resource://gre/modules/BrowserUtils.jsm",
   SpellCheckHelper: "resource://gre/modules/InlineSpellChecker.jsm",
   LoginManagerChild: "resource://gre/modules/LoginManagerChild.jsm",
   WebNavigationFrames: "resource://gre/modules/WebNavigationFrames.jsm",
@@ -238,9 +237,9 @@ class ContextMenuChild extends JSWindowActorChild {
 
         if (!disable) {
           try {
-            BrowserUtils.urlSecurityCheck(
+            Services.scriptSecurityManager.checkLoadURIWithPrincipal(
-              target.currentURI.spec,
+              target.ownerDocument.nodePrincipal,
-              target.ownerDocument.nodePrincipal
+              target.currentURI
             );
             let canvas = this.document.createElement("canvas");
             canvas.width = target.naturalWidth;

browser/components/urlbar/UrlbarInput.jsm

@@ -14,7 +14,6 @@ XPCOMUtils.defineLazyModuleGetters(this, {
   AppConstants: "resource://gre/modules/AppConstants.jsm",
   BrowserSearchTelemetry: "resource:///modules/BrowserSearchTelemetry.jsm",
   BrowserUIUtils: "resource:///modules/BrowserUIUtils.jsm",
-  BrowserUtils: "resource://gre/modules/BrowserUtils.jsm",
   ExtensionSearchHandler: "resource://gre/modules/ExtensionSearchHandler.jsm",
   ObjectUtils: "resource://gre/modules/ObjectUtils.jsm",
   PrivateBrowsingUtils: "resource://gre/modules/PrivateBrowsingUtils.jsm",
@@ -3258,10 +3257,11 @@ function getDroppableData(event) {
     }
 
     try {
-      // If this throws, urlSecurityCheck would also throw, as that's what it
+      // If this throws, checkLoadURStrWithPrincipal would also throw,
-      // does with things that don't pass the IO service's newURI constructor
+      // as that's what it does with things that don't pass the IO
-      // without fixup. It's conceivable we may want to relax this check in
+      // service's newURI constructor without fixup. It's conceivable we
-      // the future (so e.g. www.foo.com gets fixed up), but not right now.
+      // may want to relax this check in the future (so e.g. www.foo.com
+      // gets fixed up), but not right now.
       let url = new URL(href);
       // If we succeed, try to pass security checks. If this works, return the
       // URL object. If the *security checks* fail, return null.
@@ -3269,9 +3269,9 @@ function getDroppableData(event) {
         let principal = Services.droppedLinkHandler.getTriggeringPrincipal(
           event
         );
-        BrowserUtils.urlSecurityCheck(
+        Services.scriptSecurityManager.checkLoadURIStrWithPrincipal(
-          url,
           principal,
+          url.href,
           Ci.nsIScriptSecurityManager.DISALLOW_INHERIT_PRINCIPAL
         );
         return url;

caps/nsIScriptSecurityManager.idl

@@ -113,11 +113,22 @@ interface nsIScriptSecurityManager : nsISupports
      *        will only appear in the browser console, not window-associated
      *        consoles like the web console.
      */
-    void checkLoadURIWithPrincipal(in nsIPrincipal aPrincipal,
+    [binaryname(CheckLoadURIWithPrincipal)]
+    void checkLoadURIWithPrincipalXPCOM(in nsIPrincipal aPrincipal,
                                         in nsIURI uri,
                                         in unsigned long flags,
                                         [optional] in unsigned long long innerWindowID);
 
+    /**
+     * Same as the above, but when called from JS, raises exceptions with more
+     * useful messages, including both the tested URI and the principal string.
+     */
+    [implicit_jscontext, binaryname(CheckLoadURIWithPrincipalFromJS)]
+    void checkLoadURIWithPrincipal(in nsIPrincipal aPrincipal,
+                                   in nsIURI uri,
+                                   [optional] in unsigned long flags,
+                                   [optional] in unsigned long long innerWindowID);
+
     /**
      * Similar to checkLoadURIWithPrincipal but there are two differences:
      *
@@ -127,10 +138,20 @@ interface nsIScriptSecurityManager : nsISupports
      * load as well); if any of the versions of this URI is not allowed, this
      * function will return error code NS_ERROR_DOM_BAD_URI.
      */
-    void checkLoadURIStrWithPrincipal(in nsIPrincipal aPrincipal,
+    [binaryname(CheckLoadURIStrWithPrincipal)]
+    void checkLoadURIStrWithPrincipalXPCOM(in nsIPrincipal aPrincipal,
                                            in AUTF8String uri,
                                            in unsigned long flags);
 
+    /**
+     * Same as the above, but when called from JS, raises exceptions with more
+     * useful messages, including both the tested URI and the principal string.
+     */
+    [implicit_jscontext, binaryname(CheckLoadURIStrWithPrincipalFromJS)]
+    void checkLoadURIStrWithPrincipal(in nsIPrincipal aPrincipal,
+                                      in AUTF8String uri,
+                                      [optional] in unsigned long flags);
+
     /**
      * Returns true if the URI is from a domain that is allow-listed through
      * prefs to be allowed to use file:// URIs.

caps/nsScriptSecurityManager.cpp

@@ -57,9 +57,11 @@
 #include <stdint.h>
 #include "mozilla/dom/ContentChild.h"
 #include "mozilla/dom/ContentParent.h"
+#include "mozilla/dom/Exceptions.h"
 #include "mozilla/dom/nsCSPContext.h"
 #include "mozilla/dom/ScriptSettings.h"
 #include "mozilla/ClearOnShutdown.h"
+#include "mozilla/ResultExtensions.h"
 #include "mozilla/StaticPtr.h"
 #include "mozilla/dom/WorkerCommon.h"
 #include "mozilla/dom/WorkerPrivate.h"
@@ -1116,6 +1118,49 @@ nsScriptSecurityManager::CheckLoadURIStrWithPrincipal(
   return rv;
 }
 
+NS_IMETHODIMP
+nsScriptSecurityManager::CheckLoadURIWithPrincipalFromJS(nsIPrincipal* aPrincipal,
+                                                         nsIURI* aTargetURI,
+                                                         uint32_t aFlags,
+                                                         uint64_t aInnerWindowID,
+                                                         JSContext* aCx) {
+  NS_ENSURE_ARG_POINTER(aTargetURI);
+
+  nsresult rv = CheckLoadURIWithPrincipal(aPrincipal,
+                                          aTargetURI,
+                                          aFlags,
+                                          aInnerWindowID);
+  if (NS_FAILED(rv)) {
+    nsAutoCString uriStr;
+    Unused << aTargetURI->GetSpec(uriStr);
+
+    nsAutoCString message("Load of ");
+    message.Append(uriStr);
+
+    nsAutoCString principalStr;
+    Unused << aPrincipal->GetSpec(principalStr);
+    if (!principalStr.IsEmpty()) {
+      message.AppendPrintf(" from %s", principalStr.get());
+    }
+
+    message.Append(" denied");
+
+    dom::Throw(aCx, rv, message);
+  }
+
+  return rv;
+}
+
+NS_IMETHODIMP
+nsScriptSecurityManager::CheckLoadURIStrWithPrincipalFromJS(
+    nsIPrincipal* aPrincipal, const nsACString& aTargetURIStr,
+    uint32_t aFlags, JSContext* aCx) {
+  nsCOMPtr<nsIURI> targetURI;
+  MOZ_TRY(NS_NewURI(getter_AddRefs(targetURI), aTargetURIStr));
+
+  return CheckLoadURIWithPrincipalFromJS(aPrincipal, targetURI, aFlags, 0, aCx);
+}
+
 NS_IMETHODIMP
 nsScriptSecurityManager::InFileURIAllowlist(nsIURI* aUri, bool* aResult) {
   MOZ_ASSERT(aUri);

toolkit/content/contentAreaUtils.js

@@ -31,8 +31,24 @@ var ContentAreaUtils = {
   },
 };
 
-function urlSecurityCheck(aURL, aPrincipal, aFlags) {
+function urlSecurityCheck(
-  return BrowserUtils.urlSecurityCheck(aURL, aPrincipal, aFlags);
+  aURL,
+  aPrincipal,
+  aFlags = Services.scriptSecurityManager
+) {
+  if (aURL instanceof Ci.nsIURI) {
+    Services.scriptSecurityManager.checkLoadURIWithPrincipal(
+      aPrincipal,
+      aURL,
+      aFlags
+    );
+  } else {
+    Services.scriptSecurityManager.checkLoadURIStrWithPrincipal(
+      aPrincipal,
+      aURL,
+      aFlags
+    );
+  }
 }
 
 // Clientele: (Make sure you don't break any of these)

toolkit/modules/BrowserUtils.jsm

@@ -13,43 +13,6 @@ const { XPCOMUtils } = ChromeUtils.import(
 );
 
 var BrowserUtils = {
-  /**
-   * urlSecurityCheck: JavaScript wrapper for checkLoadURIWithPrincipal
-   * and checkLoadURIStrWithPrincipal.
-   * If |aPrincipal| is not allowed to link to |aURL|, this function throws with
-   * an error message.
-   *
-   * @param aURL
-   *        The URL a page has linked to. This could be passed either as a string
-   *        or as a nsIURI object.
-   * @param aPrincipal
-   *        The principal of the document from which aURL came.
-   * @param aFlags
-   *        Flags to be passed to checkLoadURIStr. If undefined,
-   *        nsIScriptSecurityManager.STANDARD will be passed.
-   */
-  urlSecurityCheck(aURL, aPrincipal, aFlags) {
-    var secMan = Services.scriptSecurityManager;
-    if (aFlags === undefined) {
-      aFlags = secMan.STANDARD;
-    }
-
-    try {
-      if (aURL instanceof Ci.nsIURI) {
-        secMan.checkLoadURIWithPrincipal(aPrincipal, aURL, aFlags);
-      } else {
-        secMan.checkLoadURIStrWithPrincipal(aPrincipal, aURL, aFlags);
-      }
-    } catch (e) {
-      let principalStr = "";
-      try {
-        principalStr = " from " + aPrincipal.spec;
-      } catch (e2) {}
-
-      throw new Error(`Load of ${aURL + principalStr} denied.`);
-    }
-  },
-
   /**
    * Return or create a principal with the content of one, and the originAttributes
    * of an existing principal (e.g. on a docshell, where the originAttributes ought