Bug 1402279 - Part 2 - Unify the usage of the DownloadPaths module. r=mak,aswan draft
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Tue, 26 Sep 2017 16:03:24 +0100
changeset 670528 d17ef98796d5a4fcac899c6319045eec02b0633e
parent 670527 7505715770e8fa383a42e0f8a18258c08e8e0f3a
child 733255 22a18540f4d55d27aed5d8000f67ccfa25573414
push id81653
push userpaolo.mozmail@amadzone.org
push dateTue, 26 Sep 2017 15:05:31 +0000
reviewersmak, aswan
bugs1402279
milestone58.0a1
Bug 1402279 - Part 2 - Unify the usage of the DownloadPaths module. r=mak,aswan MozReview-Commit-ID: HEhwkyxtYTP
toolkit/components/extensions/ext-downloads.js
toolkit/components/jsdownloads/src/DownloadPaths.jsm
toolkit/components/jsdownloads/test/unit/test_DownloadPaths.js
toolkit/content/contentAreaUtils.js
toolkit/mozapps/downloads/nsHelperAppDlg.js
--- a/toolkit/components/extensions/ext-downloads.js
+++ b/toolkit/components/extensions/ext-downloads.js
@@ -424,17 +424,17 @@ this.downloads = class extends Extension
             if (path.absolute) {
               return Promise.reject({message: "filename must not be an absolute path"});
             }
 
             if (path.components.some(component => component == "..")) {
               return Promise.reject({message: "filename must not contain back-references (..)"});
             }
 
-            if (AppConstants.platform === "win" && /[|"*?:<>]/.test(filename)) {
+            if (path.components.some(component => component != DownloadPaths.sanitize(component))) {
               return Promise.reject({message: "filename must not contain illegal characters"});
             }
           }
 
           if (options.conflictAction == "prompt") {
             // TODO
             return Promise.reject({message: "conflictAction prompt not yet implemented"});
           }
@@ -467,28 +467,24 @@ this.downloads = class extends Extension
                 channel.QueryInterface(Ci.nsIUploadChannel2);
                 channel.explicitSetUploadStream(stream, null, -1, method, false);
               }
             }
             return Promise.resolve();
           }
 
           async function createTarget(downloadsDir) {
-            let target;
-            if (filename) {
-              target = OS.Path.join(downloadsDir, filename);
-            } else {
+            if (!filename) {
               let uri = Services.io.newURI(options.url);
+              if (uri instanceof Ci.nsIURL) {
+                filename = DownloadPaths.sanitize(uri.fileName);
+              }
+            }
 
-              let remote;
-              if (uri instanceof Ci.nsIURL) {
-                remote = uri.fileName;
-              }
-              target = OS.Path.join(downloadsDir, remote || "download");
-            }
+            let target = OS.Path.join(downloadsDir, filename || "download");
 
             // Create any needed subdirectories if required by filename.
             const dir = OS.Path.dirname(target);
             await OS.File.makeDir(dir, {from: downloadsDir});
 
             if (await OS.File.exists(target)) {
               // This has a race, something else could come along and create
               // the file between this test and them time the download code
--- a/toolkit/components/jsdownloads/src/DownloadPaths.jsm
+++ b/toolkit/components/jsdownloads/src/DownloadPaths.jsm
@@ -9,18 +9,81 @@
 "use strict";
 
 this.EXPORTED_SYMBOLS = [
   "DownloadPaths",
 ];
 
 const { classes: Cc, interfaces: Ci, utils: Cu, results: Cr } = Components;
 
+Cu.import("resource://gre/modules/XPCOMUtils.jsm");
+
+XPCOMUtils.defineLazyModuleGetter(this, "AppConstants",
+                                  "resource://gre/modules/AppConstants.jsm");
+
+/**
+ * Platform-dependent regular expression used by the "sanitize" method.
+ */
+XPCOMUtils.defineLazyGetter(this, "gConvertToSpaceRegExp", () => {
+  /* eslint-disable no-control-regex */
+  switch (AppConstants.platform) {
+    // On mobile devices, the file system may be very limited in what it
+    // considers valid characters. To avoid errors, sanitize conservatively.
+    case "android":
+      return /[\x00-\x1f\x7f-\x9f:*?|"<>;,+=\[\]]+/g;
+    case "win":
+      return /[\x00-\x1f\x7f-\x9f:*?|]+/g;
+    case "macosx":
+      return /[\x00-\x1f\x7f-\x9f:]+/g;
+    default:
+      return /[\x00-\x1f\x7f-\x9f]+/g;
+  }
+  /* eslint-enable no-control-regex */
+});
+
 this.DownloadPaths = {
   /**
+   * Sanitizes an arbitrary string for use as the local file name of a download.
+   * The input is often a document title or a manually edited name. The output
+   * can be an empty string if the input does not include any valid character.
+   *
+   * The length of the resulting string is not limited, because restrictions
+   * apply to the full path name after the target folder has been added.
+   *
+   * Splitting the base name and extension to add a counter or to identify the
+   * file type should only be done after the sanitization process, because it
+   * can alter the final part of the string or remove leading dots.
+   *
+   * Runs of slashes and backslashes are replaced with an underscore.
+   *
+   * On Windows, the angular brackets `<` and `>` are replaced with parentheses,
+   * and double quotes are replaced with single quotes.
+   *
+   * Runs of control characters are replaced with a space. On Mac, colons are
+   * also included in this group. On Windows, stars, question marks, and pipes
+   * are additionally included. On Android, semicolons, commas, plus signs,
+   * equal signs, and brackets are additionally included.
+   *
+   * Leading and trailing dots and whitespace are removed on all platforms. This
+   * avoids the accidental creation of hidden files on Unix and invalid or
+   * inaccessible file names on Windows. These characters are not removed when
+   * located at the end of the base name or at the beginning of the extension.
+   */
+  sanitize(leafName) {
+    if (AppConstants.platform == "win") {
+      leafName = leafName.replace(/</g, "(")
+                         .replace(/>/g, ")")
+                         .replace(/"/g, "'");
+    }
+    return leafName.replace(/[\\/]+/g, "_")
+                   .replace(gConvertToSpaceRegExp, " ")
+                   .replace(/^[\s\u180e.]+|[\s\u180e.]+$/g, "");
+  },
+
+  /**
    * Creates a uniquely-named file starting from the name of the provided file.
    * If a file with the provided name already exists, the function attempts to
    * create nice alternatives, like "base(1).ext" (instead of "base-1.ext").
    *
    * If a unique name cannot be found, the function throws the XPCOM exception
    * NS_ERROR_FILE_TOO_BIG. Other exceptions, like NS_ERROR_FILE_ACCESS_DENIED,
    * can also be expected.
    *
--- a/toolkit/components/jsdownloads/test/unit/test_DownloadPaths.js
+++ b/toolkit/components/jsdownloads/test/unit/test_DownloadPaths.js
@@ -1,30 +1,36 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
 /**
  * Tests for the "DownloadPaths.jsm" JavaScript module.
  */
 
+Cu.import("resource://gre/modules/AppConstants.jsm");
+
 /**
  * Provides a temporary save directory.
  *
  * @returns nsIFile pointing to the new or existing directory.
  */
 function createTemporarySaveDirectory() {
   var saveDir = Cc["@mozilla.org/file/directory_service;1"].
                 getService(Ci.nsIProperties).get("TmpD", Ci.nsIFile);
   saveDir.append("testsavedir");
   if (!saveDir.exists()) {
     saveDir.create(Ci.nsIFile.DIRECTORY_TYPE, 0o755);
   }
   return saveDir;
 }
 
+function testSanitize(leafName, expectedLeafName) {
+  do_check_eq(DownloadPaths.sanitize(leafName), expectedLeafName);
+}
+
 function testSplitBaseNameAndExtension(aLeafName, [aBase, aExt]) {
   var [base, ext] = DownloadPaths.splitBaseNameAndExtension(aLeafName);
   do_check_eq(base, aBase);
   do_check_eq(ext, aExt);
 
   // If we modify the base name and concatenate it with the extension again,
   // another roundtrip through the function should give a consistent result.
   // The only exception is when we introduce an extension in a file name that
@@ -36,16 +42,63 @@ function testSplitBaseNameAndExtension(a
   do_check_eq(ext, aExt);
 }
 
 function testCreateNiceUniqueFile(aTempFile, aExpectedLeafName) {
   var createdFile = DownloadPaths.createNiceUniqueFile(aTempFile);
   do_check_eq(createdFile.leafName, aExpectedLeafName);
 }
 
+add_task(async function test_sanitize() {
+  // Platform-dependent conversion of special characters to spaces.
+  const kSpecialChars = "A:*?|\"\"<<>>;,+=[]B][=+,;>><<\"\"|?*:C";
+  if (AppConstants.platform == "android") {
+    testSanitize(kSpecialChars, "A B C");
+    testSanitize(" :: Website :: ", "Website");
+    testSanitize("* Website!", "Website!");
+    testSanitize("Website | Page!", "Website   Page!");
+    testSanitize("Directory Listing: /a/b/", "Directory Listing  _a_b_");
+  } else if (AppConstants.platform == "win") {
+    testSanitize(kSpecialChars, "A ''(());,+=[]B][=+,;))(('' C");
+    testSanitize(" :: Website :: ", "Website");
+    testSanitize("* Website!", "Website!");
+    testSanitize("Website | Page!", "Website   Page!");
+    testSanitize("Directory Listing: /a/b/", "Directory Listing  _a_b_");
+  } else if (AppConstants.platform == "macosx") {
+    testSanitize(kSpecialChars, "A *?|\"\"<<>>;,+=[]B][=+,;>><<\"\"|?* C");
+    testSanitize(" :: Website :: ", "Website");
+    testSanitize("* Website!", "* Website!");
+    testSanitize("Website | Page!", "Website | Page!");
+    testSanitize("Directory Listing: /a/b/", "Directory Listing  _a_b_");
+  } else {
+    testSanitize(kSpecialChars, kSpecialChars);
+    testSanitize(" :: Website :: ", ":: Website ::");
+    testSanitize("* Website!", "* Website!");
+    testSanitize("Website | Page!", "Website | Page!");
+    testSanitize("Directory Listing: /a/b/", "Directory Listing: _a_b_");
+  }
+
+  // Conversion of consecutive runs of slashes and backslashes to underscores.
+  testSanitize("\\ \\\\Website\\/Page// /", "_ _Website_Page_ _");
+
+  // Removal of leading and trailing whitespace and dots after conversion.
+  testSanitize("  Website  ", "Website");
+  testSanitize(". . Website . Page . .", "Website . Page");
+  testSanitize(" File . txt ", "File . txt");
+  testSanitize("\f\n\r\t\v\x00\x1f\x7f\x80\x9f\xa0 . txt", "txt");
+  testSanitize("\u1680\u180e\u2000\u2008\u200a . txt", "txt");
+  testSanitize("\u2028\u2029\u202f\u205f\u3000\ufeff . txt", "txt");
+
+  // Strings with whitespace and dots only.
+  testSanitize(".", "");
+  testSanitize("..", "");
+  testSanitize(" ", "");
+  testSanitize(" . ", "");
+});
+
 add_task(async function test_splitBaseNameAndExtension() {
   // Usual file names.
   testSplitBaseNameAndExtension("base", ["base", ""]);
   testSplitBaseNameAndExtension("base.ext", ["base", ".ext"]);
   testSplitBaseNameAndExtension("base.application", ["base", ".application"]);
   testSplitBaseNameAndExtension("base.x.Z", ["base", ".x.Z"]);
   testSplitBaseNameAndExtension("base.ext.Z", ["base", ".ext.Z"]);
   testSplitBaseNameAndExtension("base.ext.gz", ["base", ".ext.gz"]);
--- a/toolkit/content/contentAreaUtils.js
+++ b/toolkit/content/contentAreaUtils.js
@@ -2,16 +2,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
 
 XPCOMUtils.defineLazyModuleGetters(this, {
   BrowserUtils: "resource://gre/modules/BrowserUtils.jsm",
   Downloads: "resource://gre/modules/Downloads.jsm",
+  DownloadPaths: "resource://gre/modules/DownloadPaths.jsm",
   DownloadLastDir: "resource://gre/modules/DownloadLastDir.jsm",
   FileUtils: "resource://gre/modules/FileUtils.jsm",
   OS: "resource://gre/modules/osfile.jsm",
   PrivateBrowsingUtils: "resource://gre/modules/PrivateBrowsingUtils.jsm",
   Services: "resource://gre/modules/Services.jsm",
   Deprecated: "resource://gre/modules/Deprecated.jsm",
   AppConstants: "resource://gre/modules/AppConstants.jsm",
   NetUtil: "resource://gre/modules/NetUtil.jsm",
@@ -1054,39 +1055,18 @@ function getDefaultFileName(aDefaultFile
   } catch (e) {
     // in case localized string cannot be found
   }
   // 9) If all else fails, use "index"
   return "index";
 }
 
 function validateFileName(aFileName) {
-  var re = /[\/]+/g;
-  if (navigator.appVersion.indexOf("Windows") != -1) {
-    re = /[\\\/\|]+/g;
-    aFileName = aFileName.replace(/[\"]+/g, "'");
-    aFileName = aFileName.replace(/[\*\:\?]+/g, " ");
-    aFileName = aFileName.replace(/[\<]+/g, "(");
-    aFileName = aFileName.replace(/[\>]+/g, ")");
-  } else if (navigator.appVersion.indexOf("Macintosh") != -1)
-    re = /[\:\/]+/g;
-  else if (navigator.appVersion.indexOf("Android") != -1) {
-    // On mobile devices, the filesystem may be very limited in what
-    // it considers valid characters. To avoid errors, we sanitize
-    // conservatively.
-    const dangerousChars = "*?<>|\":/\\[];,+=";
-    var processed = "";
-    for (var i = 0; i < aFileName.length; i++)
-      processed += aFileName.charCodeAt(i) >= 32 &&
-                   !(dangerousChars.indexOf(aFileName[i]) >= 0) ? aFileName[i]
-                                                                : "_";
-
-    // Last character should not be a space
-    processed = processed.trim();
-
+  let processed = DownloadPaths.sanitize(aFileName) || "_";
+  if (AppConstants.platform == "android") {
     // If a large part of the filename has been sanitized, then we
     // will use a default filename instead
     if (processed.replace(/_/g, "").length <= processed.length / 2) {
       // We purposefully do not use a localized default filename,
       // which we could have done using
       // ContentAreaUtils.stringBundle.GetStringFromName("DefaultSaveFileName")
       // since it may contain invalid characters.
       var original = processed;
@@ -1094,20 +1074,18 @@ function validateFileName(aFileName) {
 
       // Preserve a suffix, if there is one
       if (original.indexOf(".") >= 0) {
         var suffix = original.split(".").slice(-1)[0];
         if (suffix && suffix.indexOf("_") < 0)
           processed += "." + suffix;
       }
     }
-    return processed;
   }
-
-  return aFileName.replace(re, "_");
+  return processed;
 }
 
 function getNormalizedLeafName(aFile, aDefaultExtension) {
   if (!aDefaultExtension)
     return aFile;
 
   if (AppConstants.platform == "win") {
     // Remove trailing dots and spaces on windows
--- a/toolkit/mozapps/downloads/nsHelperAppDlg.js
+++ b/toolkit/mozapps/downloads/nsHelperAppDlg.js
@@ -365,24 +365,18 @@ nsUnknownContentTypeDialog.prototype = {
           aLauncher.saveDestinationAvailable(result);
         });
       });
     })().catch(Components.utils.reportError);
   },
 
   getFinalLeafName: function (aLeafName, aFileExt)
   {
-    // Remove any leading periods, since we don't want to save hidden files
-    // automatically.
-    aLeafName = aLeafName.replace(/^\.+/, "");
-
-    if (aLeafName == "")
-      aLeafName = "unnamed" + (aFileExt ? "." + aFileExt : "");
-
-    return aLeafName;
+    return DownloadPaths.sanitize(aLeafName) ||
+           "unnamed" + (aFileExt ? "." + aFileExt : "");
   },
 
   /**
    * Ensures that a local folder/file combination does not already exist in
    * the file system (or finds such a combination with a reasonably similar
    * leaf name), creates the corresponding file, and returns it.
    *
    * @param   aLocalFolder