Bug 719905: Fix resolution of resource: URLs containing : and / characters. r=valentin
authorKris Maglione <maglione.k@gmail.com>
Thu, 04 Feb 2016 20:19:24 -0800
changeset 331646 6741a3b28c55793fc1048afede74a305cc778165
parent 331645 00bb04b8c9a0495fe026f609b18e09835e49c927
child 331647 036c763dedca215c00853146dc050a766dfd1f04
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersvalentin
bugs719905
milestone48.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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,59 +1,114 @@
 /* verify that certain invalid URIs are not parsed by the resource
    protocol handler */
 
 Cu.import("resource://gre/modules/NetUtil.jsm");
+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:///",
 ];
 
-var ssm = Cc["@mozilla.org/scriptsecuritymanager;1"]
-            .getService(Ci.nsIScriptSecurityManager);
-// create some fake principal that has not engough
+const error_specs = [
+  "resource://res-test/..\\",
+  "resource://res-test/..\\..\\",
+  "resource://res-test/..%5C",
+  "resource://res-test/..%5c",
+];
+
+// Create some fake principal that has not enough
 // privileges to access any resource: uri.
 var uri = NetUtil.newURI("http://www.example.com", null, null);
-var principal = ssm.createCodebasePrincipal(uri, {});
+var principal = Services.scriptSecurityManager.createCodebasePrincipal(uri, {});
 
-function check_for_exception(spec)
+function get_channel(spec)
 {
   var channelURI = NetUtil.newURI(spec, null, null);
 
   var channel = NetUtil.newChannel({
     uri: NetUtil.newURI(spec, null, null),
     loadingPrincipal: principal,
     securityFlags: Ci.nsILoadInfo.SEC_ALLOW_CROSS_ORIGIN_DATA_IS_NULL,
     contentPolicyType: Ci.nsIContentPolicy.TYPE_OTHER
   });
 
   try {
     channel.asyncOpen2(null);
-    do_check_true(false, "asyncOpen2() of URI: " + spec + "should throw");
+    ok(false, "asyncOpen2() of URI: " + spec + "should throw");
   }
   catch (e) {
     // make sure we get the right error code in the exception
     // ERROR code for NS_ERROR_DOM_BAD_URI is 1012
-    do_check_eq(e.code, 1012);
+    equal(e.code, 1012);
   }
 
   try {
     channel.open2();
-    do_check_true(false, "Open2() of uri: " + spec + "should throw");
+    ok(false, "Open2() of uri: " + spec + "should throw");
   }
   catch (e) {
     // make sure we get the right error code in the exception
     // ERROR code for NS_ERROR_DOM_BAD_URI is 1012
-    do_check_eq(e.code, 1012);
+    equal(e.code, 1012);
+  }
+
+  return channel;
+}
+
+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 {
+    get_channel(spec);
+    ok(false, "Expected an error");
+  } catch (e) {
+    equal(e.result, Components.results.NS_ERROR_MALFORMED_URI,
+          "Expected a malformed URI error");
   }
 }
 
 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);
   }
 }