Bug 1802885, improve filename sanitization when filename begins with a period, r=mak

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

uriloader/exthandler/nsExternalHelperAppService.cpp

@@ -3281,6 +3281,13 @@ nsExternalHelperAppService::ValidateFileNameForSaving(
                       aMimeType.EqualsLiteral(BINARY_OCTET_STREAM) ||
                       aMimeType.EqualsLiteral("application/x-msdownload");
 
+  // We don't want to save hidden files starting with a dot, so remove any
+  // leading periods. This is done first, so that the remainder will be
+  // treated as the filename, and not an extension.
+  // Also, Windows ignores terminating dots. So we have to as well, so
+  // that our security checks do "the right thing"
+  fileName.Trim(".");
+
   // We get the mime service here even though we're the default implementation
   // of it, so it's possible to override only the mime service and not need to
   // reimplement the whole external helper app service itself.
@@ -3294,8 +3301,9 @@ nsExternalHelperAppService::ValidateFileNameForSaving(
         nsAutoCString leafName;
         url->GetFileName(leafName);
         if (!leafName.IsEmpty()) {
-          if (NS_SUCCEEDED(UnescapeFragment(leafName, url, fileName))) {
-            CopyUTF8toUTF16(leafName, aFileName);  // use escaped name
+          if (NS_FAILED(UnescapeFragment(leafName, url, fileName))) {
+            CopyUTF8toUTF16(leafName, fileName);  // use escaped name instead
+            fileName.Trim(".");
           }
         }
 
@@ -3356,10 +3364,6 @@ nsExternalHelperAppService::ValidateFileNameForSaving(
     }
   }
 
-  // Windows ignores terminating dots. So we have to as well, so
-  // that our security checks do "the right thing"
-  fileName.Trim(".", false);
-
   // If an empty filename is allowed, then return early. It will be saved
   // using the filename of the temporary file that was created for the download.
   if (aFlags & VALIDATE_ALLOW_EMPTY && fileName.IsEmpty()) {

uriloader/exthandler/tests/mochitest/save_filenames.html

@@ -60,10 +60,11 @@
 <img id="i14" src="http://localhost:8000/save_filename.sjs?type=jpeg&filename=anotherjpg.jpg.exe" data-filename="anotherjpg.jpg.jpg" data-filename-platformlinux="anotherjpg.jpg.jpeg">
 
 <!-- jpeg with no filename portion -->
-<img id="i15" src="http://localhost:8000/save_filename.sjs?type=jpeg&filename=.jpg" data-filename="index.jpg">
+<img id="i15" src="http://localhost:8000/save_filename.sjs?type=jpeg&filename=.jpg"
+     data-filename="jpg.jpg" data-filename-platformlinux="jpg.jpeg">
 
 <!-- png with no filename portion and invalid extension -->
-<img id="i16" src="http://localhost:8000/save_filename.sjs?type=png&filename=.exe" data-filename="index.png">
+<img id="i16" src="http://localhost:8000/save_filename.sjs?type=png&filename=.exe" data-filename="exe.png">
 
 <!-- png with escaped characters -->
 <img id="i17" src="http://localhost:8000/save_filename.sjs?type=png&filename=first%20file.png" data-filename="first file.png">
@@ -295,6 +296,17 @@
 <!-- embedded child html -->
 <object id="i77" data="http://localhost:8000/save_filename.sjs?type=html&filename=child.par"
         data-filename="child.par" data-unknown="true"></object>
+
+<!-- file starting with a dot with and unknown extension -->
+<img id="i78" src="http://localhost:8000/save_filename.sjs?type=png&filename=.extension" data-filename="extension.png">
+
+<!-- html file starting with a dot -->
+<object id="i79" data="http://localhost:8000/save_filename.sjs?type=html&filename=.alternate"
+        data-filename="alternate.html" data-filename-platformwin="alternate.htm" data-unknown="true"></object>
+
+<!-- unrecognized file type starting with a dot -->
+<object id="i80" data="http://localhost:8000/save_filename.sjs?type=otherimage&filename=.alternate" data-filename="alternate"
+        data-unknown="true"></object>
 </span>
 
 <!-- This set is used to test the filename specified by the download attribute is validated correctly. -->

uriloader/exthandler/tests/unit/test_filename_sanitize.js

@@ -134,7 +134,7 @@ add_task(async function validate_filename_method() {
   );
 
   // no filename, so index is used by default.
-  Assert.equal(checkFilename(".png", 0), "index.png");
+  Assert.equal(checkFilename(".png", 0), "png.png");
 
   // sanitization only, so index is not added, but initial period is stripped.
   Assert.equal(