diff --git a/browser/base/content/test/general/browser_bug676619.js b/browser/base/content/test/general/browser_bug676619.js
index 47b4cb414a56..fd388199a292 100644
--- a/browser/base/content/test/general/browser_bug676619.js
+++ b/browser/base/content/test/general/browser_bug676619.js
@@ -48,7 +48,7 @@ async function testLink(link, name) {
is(
win.document.getElementById("location").value,
name,
- "file name should match"
+ `file name should match (${link})`
);
await BrowserTestUtils.closeWindow(win);
@@ -91,8 +91,6 @@ async function runTest(url) {
// htm vs html) on different OSes.
let oggExtension = getMIMEInfoForType("application/ogg").primaryExtension;
await testLink("link13", "no file extension." + oggExtension);
- let htmlExtension = getMIMEInfoForType("text/html").primaryExtension;
- await testLink("link14", "javascript." + htmlExtension);
BrowserTestUtils.removeTab(tab);
}
diff --git a/browser/base/content/test/general/download_page.html b/browser/base/content/test/general/download_page.html
index e6cba990cbd1..fe5006e5905c 100644
--- a/browser/base/content/test/general/download_page.html
+++ b/browser/base/content/test/general/download_page.html
@@ -35,8 +35,6 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=676619
download="example.com" id="link12" target="_blank">Download "example.com"
Download "force extension"
- Download "javascript.txt" (will correct extension due to mimetype)
Okay
diff --git a/dom/security/test/general/browser_test_data_text_csv.js b/dom/security/test/general/browser_test_data_text_csv.js
index 8aae2f67e751..6a116491e620 100644
--- a/dom/security/test/general/browser_test_data_text_csv.js
+++ b/dom/security/test/general/browser_test_data_text_csv.js
@@ -38,12 +38,6 @@ add_task(async function() {
let win = await windowPromise;
let expectedValue = "text/csv;foo,bar,foobar";
- let mimeInfo = getMIMEInfoForType("text/csv");
- try {
- expectedValue = "." + mimeInfo.primaryExtension;
- } catch (ex) {
- /* fails on Windows, bug 1671930 */
- }
is(
win.document.getElementById("location").value,
expectedValue,
diff --git a/uriloader/exthandler/nsExternalHelperAppService.cpp b/uriloader/exthandler/nsExternalHelperAppService.cpp
index 59ace07f55af..e736183cf64a 100644
--- a/uriloader/exthandler/nsExternalHelperAppService.cpp
+++ b/uriloader/exthandler/nsExternalHelperAppService.cpp
@@ -568,6 +568,38 @@ static const nsDefaultMimeTypeEntry nonDecodableExtensions[] = {
{APPLICATION_COMPRESS, "z"},
{APPLICATION_GZIP, "svgz"}};
+/**
+ * Mimetypes for which we enforce using a known extension.
+ *
+ * In addition to this list, we do this for all audio/, video/ and
+ * image/ mimetypes.
+ */
+static const char* forcedExtensionMimetypes[] = {
+ // OpenDocument formats
+ "application/vnd.oasis.opendocument.text",
+ "application/vnd.oasis.opendocument.presentation",
+ "application/vnd.oasis.opendocument.spreadsheet",
+ "application/vnd.oasis.opendocument.graphics",
+
+ // Legacy Microsoft Office
+ "application/msword", "application/vnd.ms-powerpoint",
+ "application/vnd.ms-excel",
+
+ // Office Open XML
+ "application/vnd.openxmlformats-officedocument.wordprocessingml.document",
+ "application/vnd.openxmlformats-officedocument.presentationml.presentation",
+ "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet",
+
+ APPLICATION_PDF,
+
+ APPLICATION_OGG,
+
+ APPLICATION_ZIP,
+
+ APPLICATION_JSON, APPLICATION_WASM,
+
+ TEXT_CALENDAR, TEXT_CSS, TEXT_VCARD, TEXT_XML};
+
/**
* Primary extensions of types whose descriptions should be overwritten.
* This extension is concatenated with "ExtHandlerDescription" to look up the
@@ -1275,25 +1307,68 @@ nsExternalAppHandler::nsExternalAppHandler(
mSuggestedFileName.CompressWhitespace();
mTempFileExtension.CompressWhitespace();
- // 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.
+ EnsureCorrectExtension(originalFileExt);
+
+ mBufferSize = Preferences::GetUint("network.buffer.cache.size", 4096);
+}
+
+nsExternalAppHandler::~nsExternalAppHandler() {
+ MOZ_ASSERT(!mSaver, "Saver should hold a reference to us until deleted");
+}
+
+bool nsExternalAppHandler::ShouldForceExtension(const nsString& aFileExt) {
+ nsAutoCString MIMEType;
+ if (!mMimeInfo || NS_FAILED(mMimeInfo->GetMIMEType(MIMEType))) {
+ return false;
+ }
+
+ bool canForce = StringBeginsWith(MIMEType, "image/"_ns) ||
+ StringBeginsWith(MIMEType, "audio/"_ns) ||
+ StringBeginsWith(MIMEType, "video/"_ns);
+
+ if (!canForce) {
+ for (const char* mime : forcedExtensionMimetypes) {
+ if (MIMEType.Equals(mime)) {
+ canForce = true;
+ break;
+ }
+ }
+ if (!canForce) {
+ return false;
+ }
+ }
+ // If we get here, we know for sure the mimetype allows us to overwrite the
+ // existing extension, if it's wrong. Return whether the extension is wrong:
+
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)) {
+ // Note that aFileExt is either empty or consists of an extension
+ // *including the dot* which we remove for ExtensionExists().
+ return (
+ aFileExt.IsEmpty() || aFileExt.EqualsLiteral(".") ||
+ (NS_SUCCEEDED(mMimeInfo->ExtensionExists(
+ Substring(NS_ConvertUTF16toUTF8(aFileExt), 1), &knownExtension)) &&
+ !knownExtension));
+}
+
+void nsExternalAppHandler::EnsureCorrectExtension(const nsString& aFileExt) {
+ // If we don't have an extension (which will include the .),
+ // just short-circuit.
+ if (mTempFileExtension.Length() <= 1) {
+ return;
+ }
+
+ // After removing trailing whitespaces from the name, if we have a
+ // temp file extension, there are broadly 2 cases where we want to
+ // replace the extension.
+ // First, if the file extension contains invalid characters.
+ // Second, for document type mimetypes, if the extension is either
+ // missing or not valid for this mimetype.
+ bool replaceExtension =
+ (aFileExt.FindCharInSet(KNOWN_PATH_SEPARATORS FILE_ILLEGAL_CHARACTERS) !=
+ kNotFound) ||
+ ShouldForceExtension(aFileExt);
+
+ if (replaceExtension) {
int32_t pos = mSuggestedFileName.RFindChar('.');
if (pos != kNotFound) {
mSuggestedFileName =
@@ -1301,18 +1376,16 @@ nsExternalAppHandler::nsExternalAppHandler(
} else {
mSuggestedFileName.Append(mTempFileExtension);
}
- originalFileExt = mTempFileExtension;
}
- // Make sure later we won't append mTempFileExtension if it's identical to
- // the currently used file extension.
- EnsureTempFileExtension(originalFileExt);
-
- mBufferSize = Preferences::GetUint("network.buffer.cache.size", 4096);
-}
-
-nsExternalAppHandler::~nsExternalAppHandler() {
- MOZ_ASSERT(!mSaver, "Saver should hold a reference to us until deleted");
+ /*
+ * Ensure we don't double-append the file extension if it matches:
+ */
+ if (replaceExtension ||
+ aFileExt.Equals(mTempFileExtension, nsCaseInsensitiveStringComparator)) {
+ // Matches -> mTempFileExtension can be empty
+ mTempFileExtension.Truncate();
+ }
}
void nsExternalAppHandler::DidDivertRequest(nsIRequest* request) {
@@ -1389,33 +1462,6 @@ void nsExternalAppHandler::RetargetLoadNotifications(nsIRequest* request) {
}
}
-/**
- * Make mTempFileExtension contain an extension exactly when its previous value
- * is different from mSuggestedFileName's extension, so that it can be appended
- * to mSuggestedFileName and form a valid, useful leaf name.
- * This is required so that the (renamed) temporary file has the correct
- * extension after downloading to make sure the OS will launch the application
- * corresponding to the MIME type (which was used to calculate
- * mTempFileExtension). This prevents a cgi-script named foobar.exe that
- * returns application/zip from being named foobar.exe and executed as an
- * executable file. It also blocks content that a web site might provide with a
- * content-disposition header indicating filename="foobar.exe" from being
- * downloaded to a file with extension .exe and executed.
- */
-void nsExternalAppHandler::EnsureTempFileExtension(const nsString& aFileExt) {
- // Make sure there is a mTempFileExtension (not "" or ".").
- // Remember that mTempFileExtension will always have the leading "."
- // (the check for empty is just to be safe).
- if (mTempFileExtension.Length() > 1) {
- // Now, compare fileExt to mTempFileExtension.
- if (aFileExt.Equals(mTempFileExtension,
- nsCaseInsensitiveStringComparator)) {
- // Matches -> mTempFileExtension can be empty
- mTempFileExtension.Truncate();
- }
- }
-}
-
nsresult nsExternalAppHandler::SetUpTempFile(nsIChannel* aChannel) {
// First we need to try to get the destination directory for the temporary
// file.
diff --git a/uriloader/exthandler/nsExternalHelperAppService.h b/uriloader/exthandler/nsExternalHelperAppService.h
index 1edde606dc65..b49686159aa7 100644
--- a/uriloader/exthandler/nsExternalHelperAppService.h
+++ b/uriloader/exthandler/nsExternalHelperAppService.h
@@ -461,10 +461,22 @@ class nsExternalAppHandler final : public nsIStreamListener,
bool GetNeverAskFlagFromPref(const char* prefName, const char* aContentType);
/**
- * Helper routine to ensure that mTempFileExtension only contains an extension
+ * Helper routine that checks whether we should enforce an extension
+ * for this file.
+ */
+ bool ShouldForceExtension(const nsString& aFileExt);
+
+ /**
+ * Helper routine to ensure that mSuggestedFileName ends in the correct
+ * extension, in case the original extension contains invalid characters
+ * or if this download is for a mimetype where we enforce using a specific
+ * extension (image/, video/, and audio/ based mimetypes, and a few specific
+ * document types).
+ *
+ * It also ensure that mTempFileExtension only contains an extension
* when it is different from mSuggestedFileName's extension.
*/
- void EnsureTempFileExtension(const nsString& aFileExt);
+ void EnsureCorrectExtension(const nsString& aFileExt);
typedef enum { kReadError, kWriteError, kLaunchError } ErrorType;
/**
diff --git a/uriloader/exthandler/tests/mochitest/browser.ini b/uriloader/exthandler/tests/mochitest/browser.ini
index 1ca94a2da3f7..b307fb467132 100644
--- a/uriloader/exthandler/tests/mochitest/browser.ini
+++ b/uriloader/exthandler/tests/mochitest/browser.ini
@@ -31,6 +31,7 @@ support-files =
file_with@@funny_name.png^headers^
file_with[funny_name.webm
file_with[funny_name.webm^headers^
+[browser_extension_correction.js]
[browser_open_internal_choice_persistence.js]
support-files =
file_pdf_application_pdf.pdf
diff --git a/uriloader/exthandler/tests/mochitest/browser_extension_correction.js b/uriloader/exthandler/tests/mochitest/browser_extension_correction.js
new file mode 100644
index 000000000000..0b5ac098bbac
--- /dev/null
+++ b/uriloader/exthandler/tests/mochitest/browser_extension_correction.js
@@ -0,0 +1,51 @@
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+async function testLinkWithoutExtension(type, expectExtension) {
+ info("Checking " + type);
+
+ let winPromise = BrowserTestUtils.domWindowOpenedAndLoaded();
+
+ await SpecialPowers.spawn(gBrowser.selectedBrowser, [type], mimetype => {
+ let link = content.document.createElement("a");
+ link.textContent = "Click me";
+ link.href = "data:" + mimetype + ",hello";
+ link.download = "somefile";
+ content.document.body.appendChild(link);
+ link.click();
+ });
+
+ let win = await winPromise;
+
+ let actualName = win.document.getElementById("location").value;
+ let expectedName = "somefile";
+ if (expectExtension) {
+ expectedName += "." + getMIMEInfoForType(type).primaryExtension;
+ is(actualName, expectedName, `${type} should get an extension`);
+ } else {
+ is(actualName, expectedName, `${type} should not get an extension`);
+ }
+
+ await BrowserTestUtils.closeWindow(win);
+}
+
+/**
+ * Check that for document types, images, videos and audio files,
+ * we enforce a useful extension.
+ */
+add_task(async function test_enforce_useful_extension() {
+ await BrowserTestUtils.withNewTab("data:text/html,", async browser => {
+ await testLinkWithoutExtension("image/png", true);
+ await testLinkWithoutExtension("audio/ogg", true);
+ await testLinkWithoutExtension("video/webm", true);
+ await testLinkWithoutExtension("application/msword", true);
+ await testLinkWithoutExtension("application/pdf", true);
+
+ await testLinkWithoutExtension("application/x-gobbledygook", false);
+ await testLinkWithoutExtension("application/octet-stream", false);
+ await testLinkWithoutExtension("binary/octet-stream", false);
+ await testLinkWithoutExtension("application/x-msdownload", false);
+ });
+});