Bug 719905: Fix resolution of resource: URLs containing : and / characters. r=valentin draft
authorKris Maglione <maglione.k@gmail.com>
Thu, 04 Feb 2016 20:19:24 -0800
changeset 353780 bb712b53706922c81a17bba04b690994047bba30
parent 330475 b16a5fa1a25e1f531d8b1164c7a1301a79a4f089
child 518875 8ec95044c070cfc0e8303d31750b70e3a0cfd4a0
push id15939
push usermaglione.k@gmail.com
push dateTue, 19 Apr 2016 21:41:30 +0000
reviewersvalentin
bugs719905
milestone47.0a1
Bug 719905: Fix resolution of resource: URLs containing : and / characters. r=valentin MozReview-Commit-ID: Jetsp6LpT7L
netwerk/protocol/res/SubstitutingProtocolHandler.cpp
netwerk/test/unit/test_bug337744.js
--- a/netwerk/protocol/res/SubstitutingProtocolHandler.cpp
+++ b/netwerk/protocol/res/SubstitutingProtocolHandler.cpp
@@ -348,38 +348,50 @@ SubstitutingProtocolHandler::ResolveURI(
 
   rv = uri->GetPath(path);
   if (NS_FAILED(rv)) return rv;
 
   if (ResolveSpecialCases(host, path, result)) {
     return NS_OK;
   }
 
-  // Unescape the path so we can perform some checks on it.
-  nsAutoCString unescapedPath(path);
-  NS_UnescapeURL(unescapedPath);
-
-  // Don't misinterpret the filepath as an absolute URI.
-  if (unescapedPath.FindChar(':') != -1)
-    return NS_ERROR_MALFORMED_URI;
-
-  if (unescapedPath.FindChar('\\') != -1)
-    return NS_ERROR_MALFORMED_URI;
-
-  const char *p = path.get() + 1; // path always starts with a slash
-  NS_ASSERTION(*(p-1) == '/', "Path did not begin with a slash!");
-
-  if (*p == '/')
-    return NS_ERROR_MALFORMED_URI;
-
   nsCOMPtr<nsIURI> baseURI;
   rv = GetSubstitution(host, getter_AddRefs(baseURI));
   if (NS_FAILED(rv)) return rv;
 
-  rv = baseURI->Resolve(nsDependentCString(p, path.Length()-1), result);
+  // Unescape the path so we can perform some checks on it.
+  nsCOMPtr<nsIURL> url = do_QueryInterface(uri);
+  if (!url) {
+    return NS_ERROR_MALFORMED_URI;
+  }
+
+  nsAutoCString unescapedPath;
+  rv = url->GetFilePath(unescapedPath);
+  if (NS_FAILED(rv)) return rv;
+
+  NS_UnescapeURL(unescapedPath);
+  if (unescapedPath.FindChar('\\') != -1) {
+    return NS_ERROR_MALFORMED_URI;
+  }
+
+  // Some code relies on an empty path resolving to a file rather than a
+  // directory.
+  NS_ASSERTION(path.CharAt(0) == '/', "Path must begin with '/'");
+  if (path.Length() == 1) {
+    rv = baseURI->GetSpec(result);
+  } else {
+    // Make sure we always resolve the path as file-relative to our target URI.
+    path.InsertLiteral(".", 0);
+
+    rv = baseURI->Resolve(path, result);
+  }
+
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
 
   if (MOZ_LOG_TEST(gResLog, LogLevel::Debug)) {
     nsAutoCString spec;
     uri->GetAsciiSpec(spec);
     MOZ_LOG(gResLog, LogLevel::Debug, ("%s\n -> %s\n", spec.get(), PromiseFlatCString(result).get()));
   }
   return rv;
 }
--- a/netwerk/test/unit/test_bug337744.js
+++ b/netwerk/test/unit/test_bug337744.js
@@ -1,41 +1,86 @@
 /* verify that certain invalid URIs are not parsed by the resource
    protocol handler */
 
+Cu.import("resource://gre/modules/Services.jsm");
+
 const specs = [
-  "resource:////",
-  "resource:///http://www.mozilla.org/",
-  "resource:///file:///",
-  "resource:///..\\",
-  "resource:///..\\..\\",
-  "resource:///..%5C",
-  "resource:///..%5c"
+  "resource://res-test//",
+  "resource://res-test/?foo=http:",
+  "resource://res-test/?foo=" + encodeURIComponent("http://example.com/"),
+  "resource://res-test/?foo=" + encodeURIComponent("x\\y"),
+  "resource://res-test/..%2F",
+  "resource://res-test/..%2f",
+  "resource://res-test/..%2F..",
+  "resource://res-test/..%2f..",
+  "resource://res-test/../../",
+  "resource://res-test/http://www.mozilla.org/",
+  "resource://res-test/file:///",
+];
+
+const error_specs = [
+  "resource://res-test/..\\",
+  "resource://res-test/..\\..\\",
+  "resource://res-test/..%5C",
+  "resource://res-test/..%5c",
 ];
 
-function check_for_exception(spec)
+function get_channel(spec)
 {
-  var ios =
-    Cc["@mozilla.org/network/io-service;1"].
-    getService(Ci.nsIIOService);
+  return Services.io.newChannel2(spec,
+                                 null,
+                                 null,
+                                 null,      // aLoadingNode
+                                 Services.scriptSecurityManager.getSystemPrincipal(),
+                                 null,      // aTriggeringPrincipal
+                                 Ci.nsILoadInfo.SEC_NORMAL,
+                                 Ci.nsIContentPolicy.TYPE_OTHER);
+}
 
+function check_safe_resolution(spec, rootURI)
+{
+  do_print(`Testing URL "${spec}"`);
+
+  let channel = get_channel(spec);
+
+  ok(channel.name.startsWith(rootURI), `URL resolved safely to ${channel.name}`);
+  ok(!/%2f/i.test(channel.name), `URL contains no escaped / characters`);
+}
+
+function check_resolution_error(spec)
+{
   try {
-    var channel = ios.newChannel2(spec,
-                                  null,
-                                  null,
-                                  null,      // aLoadingNode
-                                  Services.scriptSecurityManager.getSystemPrincipal(),
-                                  null,      // aTriggeringPrincipal
-                                  Ci.nsILoadInfo.SEC_NORMAL,
-                                  Ci.nsIContentPolicy.TYPE_OTHER);
+    get_channel(spec);
+    ok(false, "Expected an error");
+  } catch (e) {
+    equal(e.result, Components.results.NS_ERROR_MALFORMED_URI,
+          "Expected a malformed URI error");
   }
-  catch (e) {
-    return;
-  }
-
-  do_throw("Successfully opened invalid URI: '" + spec + "'");
 }
 
 function run_test() {
+  // resource:/// and resource://gre/ are resolved specially, so we need
+  // to create a temporary resource package to test the standard logic
+  // with.
+
+  let resProto = Cc['@mozilla.org/network/protocol;1?name=resource'].getService(Ci.nsIResProtocolHandler);
+  let rootFile = Services.dirsvc.get("GreD", Ci.nsIFile);
+  let rootURI = Services.io.newFileURI(rootFile);
+
+  resProto.setSubstitution("res-test", rootURI);
+  do_register_cleanup(() => {
+    resProto.setSubstitution("res-test", null);
+  });
+
+  let baseRoot = resProto.resolveURI(Services.io.newURI("resource:///", null, null));
+  let greRoot = resProto.resolveURI(Services.io.newURI("resource://gre/", null, null));
+
   for (var spec of specs) {
-    check_for_exception(spec);
+    check_safe_resolution(spec, rootURI.spec);
+    check_safe_resolution(spec.replace("res-test", ""), baseRoot);
+    check_safe_resolution(spec.replace("res-test", "gre"), greRoot);
+  }
+
+  for (var spec of error_specs) {
+    check_resolution_error(spec);
   }
 }