Backed out 2 changesets (bug 1855992) for causing talos failures @ mozilla::net::nsAboutProtocolHandler::NewChannel CLOSED TREE

Backed out changeset f287e725a845 (bug 1855992)
Backed out changeset 81236027b7dc (bug 1855992)

browser/base/content/test/tabs/browser_e10s_about_page_triggeringprincipal.js

@@ -8,9 +8,7 @@ const kAboutPagesRegistered = Promise.all([
     registerCleanupFunction,
     "test-about-principal-child",
     kChildPage,
-    Ci.nsIAboutModule.URI_MUST_LOAD_IN_CHILD |
-      Ci.nsIAboutModule.ALLOW_SCRIPT |
-      Ci.nsIAboutModule.URI_SAFE_FOR_UNTRUSTED_CONTENT
+    Ci.nsIAboutModule.URI_MUST_LOAD_IN_CHILD | Ci.nsIAboutModule.ALLOW_SCRIPT
   ),
   BrowserTestUtils.registerAboutPage(
     registerCleanupFunction,

docshell/base/nsAboutRedirector.cpp

@@ -197,7 +197,6 @@ static const RedirEntry kRedirMap[] = {
     {"crashparent", "about:blank", nsIAboutModule::HIDE_FROM_ABOUTABOUT},
     {"crashcontent", "about:blank",
      nsIAboutModule::HIDE_FROM_ABOUTABOUT |
-         nsIAboutModule::URI_SAFE_FOR_UNTRUSTED_CONTENT |
          nsIAboutModule::URI_CAN_LOAD_IN_CHILD |
          nsIAboutModule::URI_MUST_LOAD_IN_CHILD},
     {"crashgpu", "about:blank", nsIAboutModule::HIDE_FROM_ABOUTABOUT},
@@ -222,8 +221,7 @@ nsAboutRedirector::NewChannel(nsIURI* aURI, nsILoadInfo* aLoadInfo,
       path.EqualsASCII("crashgpu") || path.EqualsASCII("crashextensions")) {
     bool isExternal;
     aLoadInfo->GetLoadTriggeredFromExternal(&isExternal);
-    if (isExternal || !aLoadInfo->TriggeringPrincipal() ||
-        !aLoadInfo->TriggeringPrincipal()->IsSystemPrincipal()) {
+    if (isExternal) {
       return NS_ERROR_NOT_AVAILABLE;
     }
 

dom/ipc/BrowserParent.cpp

@@ -1353,8 +1353,10 @@ IPCResult BrowserParent::RecvNewWindowGlobal(
   // the wrong type of webIsolated process
   EnumSet<ContentParent::ValidatePrincipalOptions> validationOptions = {};
   nsCOMPtr<nsIURI> docURI = aInit.documentURI();
-  if (docURI->SchemeIs("blob") || docURI->SchemeIs("chrome")) {
+  if (docURI->SchemeIs("about") || docURI->SchemeIs("blob") ||
+      docURI->SchemeIs("chrome")) {
     // XXXckerschb TODO - Do not use SystemPrincipal for:
+    // Bug 1700639: about:plugins
     // Bug 1699385: Remove allowSystem for blobs
     // Bug 1698087: chrome://devtools/content/shared/webextension-fallback.html
     // chrome reftests, e.g.
@@ -1364,20 +1366,6 @@ IPCResult BrowserParent::RecvNewWindowGlobal(
     validationOptions = {ContentParent::ValidatePrincipalOptions::AllowSystem};
   }
 
-  // Some reftests have frames inside their chrome URIs and those load
-  // about:blank:
-  if (xpc::IsInAutomation() && docURI->SchemeIs("about")) {
-    WindowGlobalParent* wgp = browsingContext->GetParentWindowContext();
-    nsAutoCString spec;
-    NS_ENSURE_SUCCESS(docURI->GetSpec(spec),
-                      IPC_FAIL(this, "Should have spec for about: URI"));
-    if (spec.Equals("about:blank") && wgp &&
-        wgp->DocumentPrincipal()->IsSystemPrincipal()) {
-      validationOptions = {
-          ContentParent::ValidatePrincipalOptions::AllowSystem};
-    }
-  }
-
   if (!mManager->ValidatePrincipal(aInit.principal(), validationOptions)) {
     ContentParent::LogAndAssertFailedPrincipalValidationInfo(aInit.principal(),
                                                              __func__);

netwerk/protocol/about/nsAboutProtocolHandler.cpp

@@ -29,6 +29,14 @@ namespace net {
 
 static NS_DEFINE_CID(kNestedAboutURICID, NS_NESTEDABOUTURI_CID);
 
+static bool IsSafeForUntrustedContent(nsIAboutModule* aModule, nsIURI* aURI) {
+  uint32_t flags;
+  nsresult rv = aModule->GetURIFlags(aURI, &flags);
+  NS_ENSURE_SUCCESS(rv, false);
+
+  return (flags & nsIAboutModule::URI_SAFE_FOR_UNTRUSTED_CONTENT) != 0;
+}
+
 static bool IsSafeToLinkForUntrustedContent(nsIURI* aURI) {
   nsAutoCString path;
   aURI->GetPathQueryRef(path);
@@ -150,93 +158,71 @@ nsAboutProtocolHandler::NewChannel(nsIURI* uri, nsILoadInfo* aLoadInfo,
   nsresult rv = NS_GetAboutModule(uri, getter_AddRefs(aboutMod));
 
   nsAutoCString path;
-  if (NS_SUCCEEDED(NS_GetAboutModuleName(uri, path)) &&
-      path.EqualsLiteral("srcdoc")) {
+  nsresult rv2 = NS_GetAboutModuleName(uri, path);
+  if (NS_SUCCEEDED(rv2) && path.EqualsLiteral("srcdoc")) {
     // about:srcdoc is meant to be unresolvable, yet is included in the
     // about lookup tables so that it can pass security checks when used in
     // a srcdoc iframe.  To ensure that it stays unresolvable, we pretend
     // that it doesn't exist.
-    return NS_ERROR_MALFORMED_URI;
+    rv = NS_ERROR_FACTORY_NOT_REGISTERED;
   }
 
-  if (NS_FAILED(rv)) {
-    if (rv == NS_ERROR_FACTORY_NOT_REGISTERED) {
-      // This looks like an about: we don't know about.  Convert
-      // this to an invalid URI error.
-      return NS_ERROR_MALFORMED_URI;
-    }
+  if (NS_SUCCEEDED(rv)) {
+    // The standard return case:
+    rv = aboutMod->NewChannel(uri, aLoadInfo, result);
+    if (NS_SUCCEEDED(rv)) {
+      // Not all implementations of nsIAboutModule::NewChannel()
+      // set the LoadInfo on the newly created channel yet, as
+      // an interim solution we set the LoadInfo here if not
+      // available on the channel. Bug 1087720
+      nsCOMPtr<nsILoadInfo> loadInfo = (*result)->LoadInfo();
+      if (aLoadInfo != loadInfo) {
+        NS_ASSERTION(false,
+                     "nsIAboutModule->newChannel(aURI, aLoadInfo) needs to "
+                     "set LoadInfo");
+        AutoTArray<nsString, 2> params = {
+            u"nsIAboutModule->newChannel(aURI)"_ns,
+            u"nsIAboutModule->newChannel(aURI, aLoadInfo)"_ns};
+        nsContentUtils::ReportToConsole(
+            nsIScriptError::warningFlag, "Security by Default"_ns,
+            nullptr,  // aDocument
+            nsContentUtils::eNECKO_PROPERTIES, "APIDeprecationWarning", params);
+        (*result)->SetLoadInfo(aLoadInfo);
+      }
 
+      // If this URI is safe for untrusted content, enforce that its
+      // principal be based on the channel's originalURI by setting the
+      // owner to null.
+      // Note: this relies on aboutMod's newChannel implementation
+      // having set the proper originalURI, which probably isn't ideal.
+      if (IsSafeForUntrustedContent(aboutMod, uri)) {
+        (*result)->SetOwner(nullptr);
+      }
+
+      RefPtr<nsNestedAboutURI> aboutURI;
+      nsresult rv2 =
+          uri->QueryInterface(kNestedAboutURICID, getter_AddRefs(aboutURI));
+      if (NS_SUCCEEDED(rv2) && aboutURI->GetBaseURI()) {
+        nsCOMPtr<nsIWritablePropertyBag2> writableBag =
+            do_QueryInterface(*result);
+        if (writableBag) {
+          writableBag->SetPropertyAsInterface(u"baseURI"_ns,
+                                              aboutURI->GetBaseURI());
+        }
+      }
+    }
     return rv;
   }
 
-  uint32_t flags = 0;
-  if (NS_FAILED(aboutMod->GetURIFlags(uri, &flags))) {
-    return NS_ERROR_FAILURE;
+  // mumble...
+
+  if (rv == NS_ERROR_FACTORY_NOT_REGISTERED) {
+    // This looks like an about: we don't know about.  Convert
+    // this to an invalid URI error.
+    rv = NS_ERROR_MALFORMED_URI;
   }
 
-  bool safeForUntrustedContent =
-      (flags & nsIAboutModule::URI_SAFE_FOR_UNTRUSTED_CONTENT) != 0;
-
-  MOZ_DIAGNOSTIC_ASSERT(
-      safeForUntrustedContent ||
-          (flags & (nsIAboutModule::URI_CAN_LOAD_IN_CHILD |
-                    nsIAboutModule::URI_MUST_LOAD_IN_CHILD)) == 0,
-      "Only unprivileged content should be loaded in child processes. (Did "
-      "you forget to add URI_SAFE_FOR_UNTRUSTED_CONTENT to your about: "
-      "page?)");
-
-  // The standard return case:
-  rv = aboutMod->NewChannel(uri, aLoadInfo, result);
-  if (NS_FAILED(rv)) {
-    if (rv == NS_ERROR_FACTORY_NOT_REGISTERED) {
-      // This looks like an about: we don't know about.  Convert
-      // this to an invalid URI error.
-      return NS_ERROR_MALFORMED_URI;
-    }
-
-    return rv;
-  }
-
-  // Not all implementations of nsIAboutModule::NewChannel()
-  // set the LoadInfo on the newly created channel yet, as
-  // an interim solution we set the LoadInfo here if not
-  // available on the channel. Bug 1087720
-  nsCOMPtr<nsILoadInfo> loadInfo = (*result)->LoadInfo();
-  if (aLoadInfo != loadInfo) {
-    NS_ASSERTION(false,
-                 "nsIAboutModule->newChannel(aURI, aLoadInfo) needs to "
-                 "set LoadInfo");
-    AutoTArray<nsString, 2> params = {
-        u"nsIAboutModule->newChannel(aURI)"_ns,
-        u"nsIAboutModule->newChannel(aURI, aLoadInfo)"_ns};
-    nsContentUtils::ReportToConsole(
-        nsIScriptError::warningFlag, "Security by Default"_ns,
-        nullptr,  // aDocument
-        nsContentUtils::eNECKO_PROPERTIES, "APIDeprecationWarning", params);
-    (*result)->SetLoadInfo(aLoadInfo);
-  }
-
-  // If this URI is safe for untrusted content, enforce that its
-  // principal be based on the channel's originalURI by setting the
-  // owner to null.
-  // Note: this relies on aboutMod's newChannel implementation
-  // having set the proper originalURI, which probably isn't ideal.
-  if (safeForUntrustedContent) {
-    (*result)->SetOwner(nullptr);
-  }
-
-  RefPtr<nsNestedAboutURI> aboutURI;
-  if (NS_SUCCEEDED(
-          uri->QueryInterface(kNestedAboutURICID, getter_AddRefs(aboutURI))) &&
-      aboutURI->GetBaseURI()) {
-    nsCOMPtr<nsIWritablePropertyBag2> writableBag = do_QueryInterface(*result);
-    if (writableBag) {
-      writableBag->SetPropertyAsInterface(u"baseURI"_ns,
-                                          aboutURI->GetBaseURI());
-    }
-  }
-
-  return NS_OK;
+  return rv;
 }
 
 NS_IMETHODIMP