Bug 1783497 - Don't throw if chrome URL canonicalization fails r=smaug

Looks like this is not needed and making a web-compat issue. Differential Revision: https://phabricator.services.mozilla.com/D156985
dothq · Oct 19, 2022 · d12fa12 · d12fa12
1 parent 26da71b
commit d12fa12
Show file tree

Hide file tree

Showing 6 changed files with 45 additions and 19 deletions.
diff --git a/chrome/nsChromeProtocolHandler.cpp b/chrome/nsChromeProtocolHandler.cpp
@@ -82,7 +82,7 @@ nsChromeProtocolHandler::GetProtocolFlags(uint32_t* result) {
   // and "chrome://navigator/content/navigator.xul".
 
   rv = nsChromeRegistry::Canonify(surl);
-  if (NS_FAILED(rv)) return rv;
+  mozilla::Unused << NS_WARN_IF(NS_FAILED(rv));
 
   surl.forget(result);
   return NS_OK;
@@ -98,21 +98,9 @@ nsChromeProtocolHandler::NewChannel(nsIURI* aURI, nsILoadInfo* aLoadInfo,
 
   MOZ_ASSERT(aResult, "Null out param");
 
-#ifdef DEBUG
-  // Check that the uri we got is already canonified
-  nsresult debug_rv;
   nsCOMPtr<nsIURI> debugURL = aURI;
-  debug_rv = nsChromeRegistry::Canonify(debugURL);
-  if (NS_SUCCEEDED(debug_rv)) {
-    bool same;
-    debug_rv = aURI->Equals(debugURL, &same);
-    if (NS_SUCCEEDED(debug_rv)) {
-      NS_ASSERTION(same,
-                   "Non-canonified chrome uri passed to "
-                   "nsChromeProtocolHandler::NewChannel!");
-    }
-  }
-#endif
+  rv = nsChromeRegistry::Canonify(debugURL);
+  NS_ENSURE_SUCCESS(rv, rv);
 
   nsCOMPtr<nsIChannel> result;
 

diff --git a/chrome/test/unit/test_bug415367.js b/chrome/test/unit/test_bug415367.js
@@ -31,9 +31,9 @@ function test_uri(obj) {
 function run_test() {
   var tests = [
     { uri: "chrome://blah/content/blah.xul", result: true },
-    { uri: "chrome://blah/content/:/blah/blah.xul", result: false },
-    { uri: "chrome://blah/content/%252e./blah/blah.xul", result: false },
-    { uri: "chrome://blah/content/%252e%252e/blah/blah.xul", result: false },
+    { uri: "chrome://blah/content/:/blah/blah.xul", result: true },
+    { uri: "chrome://blah/content/%252e./blah/blah.xul", result: true },
+    { uri: "chrome://blah/content/%252e%252e/blah/blah.xul", result: true },
     { uri: "chrome://blah/content/blah.xul?param=%252e./blah/", result: true },
     { uri: "chrome://blah/content/blah.xul?param=:/blah/", result: true },
     {

diff --git a/chrome/test/unit/test_create_channel_chrome_url.js b/chrome/test/unit/test_create_channel_chrome_url.js
@@ -0,0 +1,36 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/
+ */
+
+"use strict";
+
+const { NetUtil } = ChromeUtils.import("resource://gre/modules/NetUtil.jsm");
+
+function testURL(url) {
+  Services.io.newChannelFromURI(
+    NetUtil.newURI(url),
+    null, // aLoadingNode
+    Services.scriptSecurityManager.getSystemPrincipal(),
+    null, // aTriggeringPrincipal
+    Ci.nsILoadInfo.SEC_ALLOW_CROSS_ORIGIN_SEC_CONTEXT_IS_NULL,
+    Ci.nsIContentPolicy.TYPE_OTHER
+  );
+}
+
+add_task(async function test_create_channel_with_chrome_url() {
+  try {
+    testURL("chrome://path");
+    Assert.ok(false);
+  } catch (e) {
+    // Chrome url fails canonicalization
+    Assert.equal(e.result, Cr.NS_ERROR_FAILURE);
+  }
+
+  try {
+    testURL("chrome://path/path/path");
+    Assert.ok(false);
+  } catch (e) {
+    // Chrome url passes canonicalization
+    Assert.equal(e.result, Cr.NS_ERROR_FILE_NOT_FOUND);
+  }
+});
diff --git a/chrome/test/unit/xpcshell.ini b/chrome/test/unit/xpcshell.ini
@@ -15,3 +15,4 @@ tags = addons
 [test_data_protocol_registration.js]
 [test_no_remote_registration.js]
 [test_resolve_uris.js]
+[test_create_channel_chrome_url.js]
diff --git a/dom/html/test/forms/test_input_url.html b/dom/html/test/forms/test_input_url.html
@@ -27,6 +27,7 @@
 
 function checkValidURL(element)
 {
+  info(`Checking ${element.value}\n`);
   gInvalid = false;
   ok(!element.validity.typeMismatch,
     "Element should not suffer from type mismatch");
@@ -71,6 +72,7 @@
   [ "http://mózillä.órg", true ],
   [ "ht://mózillä.órg", true ],
   [ "httŭ://mózillä.órg", false ],
+  [ "chrome://bookmarks", true ],
 ];
 
 values.forEach(function([value, valid]) {

diff --git a/services/common/tests/unit/test_utils_makeURI.js b/services/common/tests/unit/test_utils_makeURI.js
@@ -58,6 +58,5 @@ function _test_makeURI() {
 
   _("Invalid uris are undefined");
   Assert.equal(CommonUtils.makeURI("mozillalabs.com"), undefined);
-  Assert.equal(CommonUtils.makeURI("chrome://badstuff"), undefined);
   Assert.equal(CommonUtils.makeURI("this is a test"), undefined);
 }