Bug 1667787 - fix saving webp images served with jpeg extensions without content-disposition information, r=mak

This changes two bits of Firefox that, together with the mime service, end up
very confused over webp + jpeg.

1) it changes contentAreaUtils.js' getDefaultExtension that if it gets an image
mimetype as the content type, it should ignore the URL. It doesn't have full channel
info so it can't really do better anyway. This fixes the context menu's "save image as..."
case.
2) it changes the external helper app service to do a few things slightly differently:
a. If we're told not to get an extension out of a URL, really don't. Don't just get the
   filename and then get it from there anyway...
b. If we've got a suggested filename, and a primary extension for the mimetype,
   and the extension on the file is not one of the known extensions for the mimetype,
   replace it with the primary extension.
This fixes the link case.

It also adds tests for both of these mechanisms as well as "save image as."

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

browser/components/downloads/test/browser/browser.ini

@@ -10,6 +10,10 @@ support-files =
   !/toolkit/content/tests/browser/common/mockTransfer.js
 [browser_first_download_panel.js]
 skip-if = os == "linux" # Bug 949434
+[browser_image_mimetype_issues.js]
+support-files =
+  not-really-a-jpeg.jpeg
+  not-really-a-jpeg.jpeg^headers^
 [browser_overflow_anchor.js]
 skip-if = os == "linux" # Bug 952422
 [browser_confirm_unblock_download.js]

browser/components/downloads/test/browser/browser_image_mimetype_issues.js

@@ -0,0 +1,113 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+const TEST_ROOT = getRootDirectory(gTestPath).replace(
+  "chrome://mochitests/content",
+  "http://example.com"
+);
+
+var MockFilePicker = SpecialPowers.MockFilePicker;
+MockFilePicker.init(window);
+
+/*
+ * Popular websites implement image optimization as serving files with
+ * extension ".jpg" but content type "image/webp". If we save such images,
+ * we should actually save them with a .webp extension as that is what
+ * they are.
+ */
+
+/**
+ * Test the above with the "save image as" context menu.
+ */
+add_task(async function test_save_image_webp_with_jpeg_extension() {
+  await BrowserTestUtils.withNewTab(
+    `data:text/html,<img src="${TEST_ROOT}/not-really-a-jpeg.jpeg?convert=webp">`,
+    async browser => {
+      let menu = document.getElementById("contentAreaContextMenu");
+      let popupShown = BrowserTestUtils.waitForEvent(menu, "popupshown");
+      BrowserTestUtils.synthesizeMouse(
+        "img",
+        5,
+        5,
+        { type: "contextmenu", button: 2 },
+        browser
+      );
+      await popupShown;
+
+      await new Promise(resolve => {
+        MockFilePicker.showCallback = function(fp) {
+          ok(
+            fp.defaultString.endsWith("webp"),
+            `filepicker for image has "${fp.defaultString}", should end in webp`
+          );
+          setTimeout(resolve, 0);
+          return Ci.nsIFilePicker.returnCancel;
+        };
+        EventUtils.synthesizeMouseAtCenter(
+          menu.querySelector("#context-saveimage"),
+          {}
+        );
+      });
+    }
+  );
+});
+
+/**
+ * Test with the "save link as" context menu.
+ */
+add_task(async function test_save_link_webp_with_jpeg_extension() {
+  await BrowserTestUtils.withNewTab(
+    `data:text/html,<a href="${TEST_ROOT}/not-really-a-jpeg.jpeg?convert=webp">Nice image</a>`,
+    async browser => {
+      let menu = document.getElementById("contentAreaContextMenu");
+      let popupShown = BrowserTestUtils.waitForEvent(menu, "popupshown");
+      BrowserTestUtils.synthesizeMouse(
+        "a[href]",
+        5,
+        5,
+        { type: "contextmenu", button: 2 },
+        browser
+      );
+      await popupShown;
+
+      await new Promise(resolve => {
+        MockFilePicker.showCallback = function(fp) {
+          ok(
+            fp.defaultString.endsWith("webp"),
+            `filepicker for link has "${fp.defaultString}", should end in webp`
+          );
+          setTimeout(resolve, 0);
+          return Ci.nsIFilePicker.returnCancel;
+        };
+        EventUtils.synthesizeMouseAtCenter(
+          menu.querySelector("#context-savelink"),
+          {}
+        );
+      });
+    }
+  );
+});
+
+/**
+ * Test with the main "save page" command.
+ */
+add_task(async function test_save_page_on_image_document() {
+  await BrowserTestUtils.withNewTab(
+    `${TEST_ROOT}/not-really-a-jpeg.jpeg?convert=webp`,
+    async browser => {
+      await new Promise(resolve => {
+        MockFilePicker.showCallback = function(fp) {
+          ok(
+            fp.defaultString.endsWith("webp"),
+            `filepicker for "save page" has "${fp.defaultString}", should end in webp`
+          );
+          setTimeout(resolve, 0);
+          return Ci.nsIFilePicker.returnCancel;
+        };
+        document.getElementById("Browser:SavePage").doCommand();
+      });
+    }
+  );
+});

browser/components/downloads/test/browser/not-really-a-jpeg.jpeg^headers^

@@ -0,0 +1,2 @@
+Content-Type: image/webp
+

dom/security/test/general/browser_test_data_text_csv.js

@@ -41,7 +41,7 @@ function test() {
     function(win) {
       is(
         win.document.getElementById("location").value,
-        "text/csv;foo,bar,foobar",
+        ".csv",
         "file name of download should match"
       );
       win.close();

toolkit/content/contentAreaUtils.js

@@ -1149,6 +1149,16 @@ function getDefaultExtension(aFilename, aURI, aContentType) {
     return "";
   } // temporary fix for bug 120327
 
+  // For images, rely solely on the mime type if known.
+  // All the extension is going to do is lie to us.
+  if (aContentType?.startsWith("image/")) {
+    let mimeInfo = getMIMEInfoForType(aContentType, "");
+    let exts = Array.from(mimeInfo.getFileExtensions());
+    if (exts.length) {
+      return exts[0];
+    }
+  }
+
   // First try the extension from the filename
   var url = Cc["@mozilla.org/network/standard-url-mutator;1"]
     .createInstance(Ci.nsIURIMutator)

uriloader/exthandler/nsExternalHelperAppService.cpp

@@ -203,6 +203,7 @@ static bool GetFilenameAndExtensionFromChannel(nsIChannel* aChannel,
   bool handleExternally = false;
   uint32_t disp;
   nsresult rv = aChannel->GetContentDisposition(&disp);
+  bool gotFileNameFromURI = false;
   if (NS_SUCCEEDED(rv)) {
     aChannel->GetContentDispositionFilename(aFileName);
     if (disp == nsIChannel::DISPOSITION_ATTACHMENT) handleExternally = true;
@@ -229,6 +230,7 @@ static bool GetFilenameAndExtensionFromChannel(nsIChannel* aChannel,
     nsAutoCString leafName;
     url->GetFileName(leafName);
     if (!leafName.IsEmpty()) {
+      gotFileNameFromURI = true;
       rv = UnescapeFragment(leafName, url, aFileName);
       if (NS_FAILED(rv)) {
         CopyUTF8toUTF16(leafName, aFileName);  // use escaped name
@@ -236,14 +238,14 @@ static bool GetFilenameAndExtensionFromChannel(nsIChannel* aChannel,
     }
   }
 
-  // Extract Extension, if we have a filename; otherwise,
-  // truncate the string
-  if (aExtension.IsEmpty()) {
-    if (!aFileName.IsEmpty()) {
-      // Windows ignores terminating dots. So we have to as well, so
-      // that our security checks do "the right thing"
-      aFileName.Trim(".", false);
+  // If we have a filename and no extension, remove trailing dots from the
+  // filename and extract the extension if that is possible.
+  if (aExtension.IsEmpty() && !aFileName.IsEmpty()) {
+    // Windows ignores terminating dots. So we have to as well, so
+    // that our security checks do "the right thing"
+    aFileName.Trim(".", false);
 
+    if (!gotFileNameFromURI || aAllowURLExtension) {
       // XXX RFindCharInReadable!!
       nsAutoString fileNameStr(aFileName);
       int32_t idx = fileNameStr.RFindChar(char16_t('.'));
@@ -1291,12 +1293,32 @@ nsExternalAppHandler::nsExternalAppHandler(
   mSuggestedFileName.CompressWhitespace();
   mTempFileExtension.CompressWhitespace();
 
-  // Append after removing trailing whitespaces from the name.
-  if (originalFileExt.FindCharInSet(
-          KNOWN_PATH_SEPARATORS FILE_ILLEGAL_CHARACTERS) != kNotFound) {
-    // The file extension contains invalid characters and using it would
-    // generate an unusable file, thus use mTempFileExtension instead.
-    mSuggestedFileName.Append(mTempFileExtension);
+  // After removing trailing whitespaces from the name, if we have a
+  // temp file extension, replace the file extension with it if:
+  // - there is no extant file extension (or it only consists of ".")
+  // - the extant file extension contains invalid characters, or
+  // - the extant file extension is not known by the mimetype.
+  bool knownExtension = false;
+  // Note that originalFileExt is either empty or consists of an
+  // extension *including the dot* which we need to remove:
+  bool haveBogusExtension =
+      mMimeInfo && !originalFileExt.IsEmpty() &&
+      NS_SUCCEEDED(mMimeInfo->ExtensionExists(
+          Substring(NS_ConvertUTF16toUTF8(originalFileExt), 1),
+          &knownExtension)) &&
+      !knownExtension;
+  if (!mTempFileExtension.IsEmpty() &&
+      (originalFileExt.Length() == 0 || originalFileExt.EqualsLiteral(".") ||
+       originalFileExt.FindCharInSet(
+           KNOWN_PATH_SEPARATORS FILE_ILLEGAL_CHARACTERS) != kNotFound ||
+       haveBogusExtension)) {
+    int32_t pos = mSuggestedFileName.RFindChar('.');
+    if (pos != kNotFound) {
+      mSuggestedFileName =
+          Substring(mSuggestedFileName, 0, pos) + mTempFileExtension;
+    } else {
+      mSuggestedFileName.Append(mTempFileExtension);
+    }
     originalFileExt = mTempFileExtension;
   }