Bug 1405174 - Fix resolution of resource: URLs when the target of the substitution doesn't end with a /. r=valentin
☠☠ backed out by 3087a8c87fe4 ☠ ☠
authorMike Hommey <mh+mozilla@glandium.org>
Sat, 30 Sep 2017 08:49:28 +0900
changeset 384577 8aeb6541520a4fcd1db36f959826e35cec0a8a1e
parent 384576 fe8db1b3e36127c4fca87124519b4fd6b74363dc
child 384578 6673c2269926768eee8218a4d3ed122baa1f344f
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.appendRelativePath("this/directory/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);
   }
 }