Bug 1405174 - Fix resolution of resource: URLs when the target of the substitution doesn't end with a /. r=valentin
authorMike Hommey <mh+mozilla@glandium.org>
Sat, 30 Sep 2017 08:49:28 +0900
changeset 384581 32c618d07fe1a1c78de3aacaedcf2a7781a50d28
parent 384580 3087a8c87fe4f39ddfe81834418434554b5355a9
child 384582 b04ebc4bcd3d59bee31d363ac34b55bc8e71c8f7
push id32631
push userarchaeopteryx@coole-files.de
push dateThu, 05 Oct 2017 08:51:33 +0000
treeherdermozilla-central@66042a706980 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersvalentin
bugs1405174
milestone58.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 1405174 - Fix resolution of resource: URLs when the target of the substitution doesn't end with a /. r=valentin When for some reason the target of a resource: substitution doesn't end with a / (which can happen when e.g. building a FileURI with a path that doesn't exist), relative path resolution of the resource URLs end up rooted under the parent of the non-existing directory. e.g resource://foo/bar is substituted with /path/for/bar if resource://foo/ is registered for file:///path/for/foo (instead of file:///path/for/foo/)
netwerk/protocol/res/SubstitutingProtocolHandler.cpp
netwerk/test/unit/test_bug337744.js
--- a/netwerk/protocol/res/SubstitutingProtocolHandler.cpp
+++ b/netwerk/protocol/res/SubstitutingProtocolHandler.cpp
@@ -420,17 +420,37 @@ SubstitutingProtocolHandler::ResolveURI(
 
   // 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.Insert('.', 0);
+    // When the baseURI is a nsIFileURL, and the directory it points to doesn't
+    // exist, it doesn't end with a /. In that case, a file-relative resolution
+    // is going to pick something in the parent directory, so we resolve using
+    // an absolute path derived from the full path in that case.
+    nsCOMPtr<nsIFileURL> baseDir = do_QueryInterface(baseURI);
+    if (baseDir) {
+      nsAutoCString basePath;
+      rv = baseURI->GetFilePath(basePath);
+      if (NS_SUCCEEDED(rv) && !StringEndsWith(basePath, NS_LITERAL_CSTRING("/"))) {
+        // Cf. the assertion above, path already starts with a /, so prefixing
+        // with a string that doesn't end with one will leave us wit the right
+        // amount of /.
+        path.Insert(basePath, 0);
+      } else {
+        // Allow to fall through below.
+        baseDir = nullptr;
+      }
+    }
+    if (!baseDir) {
+      path.Insert('.', 0);
+    }
     rv = baseURI->Resolve(path, result);
   }
 
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   if (MOZ_LOG_TEST(gResLog, LogLevel::Debug)) {
--- a/netwerk/test/unit/test_bug337744.js
+++ b/netwerk/test/unit/test_bug337744.js
@@ -89,26 +89,32 @@ 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);
 
+  rootFile.append("directory-that-does-not-exist");
+  let inexistentURI = Services.io.newFileURI(rootFile);
+
   resProto.setSubstitution("res-test", rootURI);
+  resProto.setSubstitution("res-inexistent", inexistentURI);
   do_register_cleanup(() => {
     resProto.setSubstitution("res-test", null);
+    resProto.setSubstitution("res-inexistent", null);
   });
 
   let baseRoot = resProto.resolveURI(Services.io.newURI("resource:///"));
   let greRoot = resProto.resolveURI(Services.io.newURI("resource://gre/"));
 
   for (var spec of specs) {
     check_safe_resolution(spec, rootURI.spec);
+    check_safe_resolution(spec.replace("res-test", "res-inexistent"), inexistentURI.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);
   }
 }