Bug 1903780 - Support directory names again in downloads.download r=geckoview-reviewers,NeilDeakin,m_kato

Differential Revision: https://phabricator.services.mozilla.com/D218719

mobile/shared/components/extensions/ext-downloads.js

@@ -201,9 +201,10 @@ this.downloads = class extends ExtensionAPIPersistent {
             }
 
             if (
-              pathComponents.some(component => {
+              pathComponents.some((component, i) => {
                 const sanitized = DownloadPaths.sanitize(component, {
                   compressWhitespaces: false,
+                  allowDirectoryNames: i < pathComponents.length - 1,
                 });
                 return component != sanitized;
               })

netwerk/mime/nsIMIMEService.idl

@@ -175,6 +175,13 @@ interface nsIMIMEService : nsISupports {
      */
     const long VALIDATE_ALLOW_INVALID_FILENAMES = 128;
 
+    /**
+     * Some names are unsafe as a file name, but safe for directory names.
+     * If this is used, the validation is more permissive towards names that
+     * are valid directory names.
+     */
+    const long VALIDATE_ALLOW_DIRECTORY_NAMES = 256;
+
     /**
      * Generate a valid filename from the channel that can be used to save
      * the content of the channel to the local disk.

toolkit/components/downloads/DownloadPaths.sys.mjs

@@ -20,10 +20,16 @@ export var DownloadPaths = {
    *        should be compressed. The default value is true.
    * @param {boolean} [allowInvalidFilenames] Allow invalid and dangerous
    *        filenames and extensions as is.
+   * @param {boolean} [allowDirectoryNames] Allow invalid or dangerous file
+   *        names if the name is a valid and safe directory name.
    */
   sanitize(
     leafName,
-    { compressWhitespaces = true, allowInvalidFilenames = false } = {}
+    {
+      compressWhitespaces = true,
+      allowInvalidFilenames = false,
+      allowDirectoryNames = false,
+    } = {}
   ) {
     const mimeSvc = Cc["@mozilla.org/mime;1"].getService(Ci.nsIMIMEService);
 
@@ -34,6 +40,9 @@ export var DownloadPaths = {
     if (allowInvalidFilenames) {
       flags |= mimeSvc.VALIDATE_ALLOW_INVALID_FILENAMES;
     }
+    if (allowDirectoryNames) {
+      flags |= mimeSvc.VALIDATE_ALLOW_DIRECTORY_NAMES;
+    }
     return mimeSvc.validateFileNameForSaving(leafName, "", flags);
   },
 

toolkit/components/downloads/test/unit/test_DownloadPaths.js

@@ -70,6 +70,14 @@ add_task(async function test_sanitize() {
   testSanitize("\u1680\u180e\u2000\u2008\u200a . txt", "txt");
   testSanitize("\u2028\u2029\u202f\u205f\u3000\ufeff . txt", "txt");
 
+  // Whitespace around dot.
+  testSanitize("1. First", "1.First");
+  testSanitize("1 . First", "1 .First");
+  testSanitize("2. Two. 3rd", "2. Two.3rd");
+  testSanitize("1. First", "1. First", { allowDirectoryNames: true });
+  testSanitize("1 . First", "1 . First", { allowDirectoryNames: true });
+  testSanitize("2. Two. 3rd", "2. Two. 3rd", { allowDirectoryNames: true });
+
   // Strings with whitespace and dots only.
   testSanitize(".", "");
   testSanitize("..", "");

toolkit/components/extensions/parent/ext-downloads.js

@@ -701,9 +701,10 @@ this.downloads = class extends ExtensionAPIPersistent {
             }
 
             if (
-              pathComponents.some(component => {
+              pathComponents.some((component, i) => {
                 let sanitized = DownloadPaths.sanitize(component, {
                   compressWhitespaces: false,
+                  allowDirectoryNames: i < pathComponents.length - 1,
                 });
                 return component != sanitized;
               })

toolkit/components/extensions/test/mochitest/test_ext_downloads_download.html

@@ -68,6 +68,12 @@ async function background() {
     "Should fail with a dot in the filename"
   );
 
+  await browser.test.assertRejects(
+    browser.downloads.download({url, filename: "123. file"}),
+    /filename must not contain illegal characters/,
+    "Should fail with a space after the trailing dot in the filename"
+  );
+
   await browser.test.assertRejects(
     browser.downloads.download({url, filename: "../file.gif"}),
     /filename must not contain back-references/,

toolkit/components/extensions/test/xpcshell/test_ext_downloads_download.js

@@ -192,6 +192,17 @@ add_task(async function test_downloads() {
     "source and filename with existing subdirs"
   );
 
+  // Regression test for https://bugzilla.mozilla.org/show_bug.cgi?id=1903780
+  await testDownload(
+    {
+      url: FILE_URL,
+      filename: "sub/1. organized/file2",
+    },
+    ["sub", "1. organized", "file2"],
+    FILE_LEN,
+    "Directory containing invalid file name"
+  );
+
   // Only run Windows path separator test on Windows.
   if (WINDOWS) {
     // Call download() with a filename with Windows path separator.

uriloader/exthandler/nsExternalHelperAppService.cpp

@@ -3732,12 +3732,14 @@ void nsExternalHelperAppService::SanitizeFileName(nsAString& aFileName,
     outFileName.Truncate(lastNonTrimmable);
   }
 
-  nsAutoString extension;
-  int32_t dotidx = outFileName.RFind(u".");
-  if (dotidx != -1) {
-    extension = Substring(outFileName, dotidx + 1);
-    extension.StripWhitespace();
-    outFileName = Substring(outFileName, 0, dotidx + 1) + extension;
+  if (!(aFlags & VALIDATE_ALLOW_DIRECTORY_NAMES)) {
+    nsAutoString extension;
+    int32_t dotidx = outFileName.RFind(u".");
+    if (dotidx != -1) {
+      extension = Substring(outFileName, dotidx + 1);
+      extension.StripWhitespace();
+      outFileName = Substring(outFileName, 0, dotidx + 1) + extension;
+    }
   }
 
 #ifdef XP_WIN

uriloader/exthandler/tests/unit/test_filename_sanitize.js

@@ -279,6 +279,22 @@ add_task(async function validate_filename_method() {
     "very filename with extension only"
   );
 
+  Assert.equal(
+    mimeService.validateFileNameForSaving("1. First", "text/unknown", 0),
+    "1.First",
+    "1. First"
+  );
+
+  Assert.equal(
+    mimeService.validateFileNameForSaving(
+      "1. First",
+      "text/unknown",
+      mimeService.VALIDATE_ALLOW_DIRECTORY_NAMES
+    ),
+    "1. First",
+    "1. First with VALIDATE_ALLOW_DIRECTORY_NAMES"
+  );
+
   Assert.equal(
     mimeService.validateFileNameForSaving("filename.LNK", "text/unknown", 0),
     "filename.LNK.download",