Bug 1044700 - Make OS.Path.join and OS.File.makeDir more resilient to denormalized paths. r=froydnj

toolkit/components/osfile/modules/osfile_shared_front.jsm

@@ -549,21 +549,29 @@ AbstractFile.removeRecursive = function(path, options = {}) {
  * does not support security descriptors.
  */
 AbstractFile.makeDir = function(path, options = {}) {
-  if (!options.from) {
+  let from = options.from;
+  if (!from) {
     OS.File._makeDir(path, options);
     return;
   }
-  if (!path.startsWith(options.from)) {
-    throw new Error("Incorrect use of option |from|: " + path + " is not a descendant of " + options.from);
+  if (!path.startsWith(from)) {
+    // Apparently, `from` is not a parent of `path`. However, we may
+    // have false negatives due to non-normalized paths, e.g.
+    // "foo//bar" is a parent of "foo/bar/sna".
+    path = Path.normalize(path);
+    from = Path.normalize(from);
+    if (!path.startsWith(from)) {
+      throw new Error("Incorrect use of option |from|: " + path + " is not a descendant of " + from);
+    }
   }
   let innerOptions = Object.create(options, {
     ignoreExisting: {
       value: true
     }
   });
-  // Compute the elements that appear in |path| but not in |options.from|.
-  let items = Path.split(path).components.slice(Path.split(options.from).components.length);
-  let current = options.from;
+  // Compute the elements that appear in |path| but not in |from|.
+  let items = Path.split(path).components.slice(Path.split(from).components.length);
+  let current = from;
   for (let item of items) {
     current = Path.join(current, item);
     OS.File._makeDir(current, innerOptions);

toolkit/components/osfile/modules/ospath_unix.jsm

@@ -78,6 +78,9 @@ exports.dirname = dirname;
  *  var path = OS.Path.join(tmpDir, "foo", "bar");
  *
  * Under Unix, this will return "/tmp/foo/bar".
+ *
+ * Empty components are ignored, i.e. `OS.Path.join("foo", "", "bar)` is the
+ * same as `OS.Path.join("foo", "bar")`.
  */
 let join = function(...path) {
   // If there is a path that starts with a "/", eliminate everything before
@@ -86,7 +89,9 @@ let join = function(...path) {
     if (subpath == null) {
       throw new TypeError("invalid path component");
     }
-    if (subpath.length != 0 && subpath[0] == "/") {
+    if (subpath.length == 0) {
+      continue;
+    } else if (subpath[0] == "/") {
       paths = [subpath];
     } else {
       paths.push(subpath);

toolkit/components/osfile/modules/ospath_win.jsm

@@ -135,6 +135,9 @@ exports.dirname = dirname;
  *  var path = OS.Path.join(tmpDir, "foo", "bar");
  *
  * Under Windows, this will return "$TMP\foo\bar".
+ *
+ * Empty components are ignored, i.e. `OS.Path.join("foo", "", "bar)` is the
+ * same as `OS.Path.join("foo", "bar")`.
  */
 let join = function(...path) {
   let paths = [];
@@ -144,6 +147,9 @@ let join = function(...path) {
     if (subpath == null) {
       throw new TypeError("invalid path component");
     }
+    if (subpath == "") {
+      continue;
+    }
     let drive = this.winGetDrive(subpath);
     if (drive) {
       root = drive;

toolkit/components/osfile/tests/xpcshell/test_makeDir.js

@@ -120,4 +120,23 @@ add_task(function test_option_from() {
   do_check_true(!!exception);
   do_check_true(exception instanceof OS.File.Error);
   do_check_true(exception.becauseNoSuchFile);
+
+  // Test edge cases on paths
+
+  let dir3 = Path.join(profileDir, "d", "", "e", "f");
+  do_check_false((yield OS.File.exists(dir3)));
+  yield OS.File.makeDir(dir3, {from: profileDir});
+  do_check_true((yield OS.File.exists(dir3)));
+
+  let dir4;
+  if (OS.Constants.Win) {
+    // Test that we can create a directory recursively even
+    // if we have too many "\\".
+    dir4 = profileDir + "\\\\g";
+  } else {
+    dir4 = profileDir + "////g";
+  }
+  do_check_false((yield OS.File.exists(dir4)));
+  yield OS.File.makeDir(dir4, {from: profileDir});
+  do_check_true((yield OS.File.exists(dir4)));
 });