1810197 - .exe extension becomes .com when downloading file

Status: RESOLVED FIXED

(Firefox for Android :: Downloads, defect, P2)

Description

[Christian Sadilek [:csadilek]]

From github: https://github.com/mozilla-mobile/fenix/issues/27621.

Steps to reproduce

1.visit website https://2.na.dl.wireshark.org/win64/Wireshark-win64-4.0.1.exe

Expected behaviour

Downloaded Wireshark-win64-4.0.1.exe

Actual behaviour

Downloaded Wireshark-win64-4.0.1.com

Device name

MIUI12.6

Android version

andeoid 11

Firefox release type

Firefox

Firefox version

107.1.0 armv8 From github

Device logs

No response

Additional information

No response

┆Issue is synchronized with this Jira Task

Change performed by the Move to Bugzilla add-on.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:cpeterson, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 2

[Mihai Adrian Carare [:mcarare]]

This might be a problem with the filename (contains dots) rather than the extension. Downloading .exe files with no dots in the filename does not reproduce this issue.

Comment 3

[Chris Peterson [:cpeterson]]

I can reproduce in both Fenix and Focus.

The problem is related to the .exe filename extension, not the multiple dots in the filename Wireshark-win64-4.0.1.exe: I can also reproduce the bug with the file Wireshark-win64-latest.exe, which has only one dot. Downloading Wireshark-win64-latest.msi, which doesn't have .exe, works correctly.

https://2.na.dl.wireshark.org/win64/

Comment 4

[happinessmncube]

(In reply to Chris Peterson [:cpeterson] from comment #3)

I can reproduce in both Fenix and Focus.

The problem is related to the .exe filename extension, not the multiple dots in the filename Wireshark-win64-4.0.1.exe: I can also reproduce the bug with the file Wireshark-win64-latest.exe, which has only one dot. Downloading Wireshark-win64-latest.msi, which doesn't have .exe, works correctly.

https://2.na.dl.wireshark.org/win64/

I have been investigating the issue and I have discovered that a simple solution will be to update the GENERIC_CONTENT_TYPES array DownloadUtils in by adding "application/x-msdos-program" but this will break this comment as it wont be aligned with the desktop values. So is it possible to update the fenix values first before updating the desktop values?

Comment 5

[Christian Sadilek [:csadilek]]

Thanks for investigating!

I would have expected getExtensionFromMimeType to return exe and match the current file extension, therefore leaving it untouched.

Does it return com for you?

Comment 6

[happinessmncube]

(In reply to Christian Sadilek [:csadilek] from comment #5)

Thanks for investigating!

I would have expected getExtensionFromMimeType to return exe and match the current file extension, therefore leaving it untouched.

Does it return com for you?

Yes it does, because application/x-msdos-program is not in GENERIC_CONTENT_TYPES line 179 in DownloadUtils will evaluate to false and changeExtension will be called and this will return com

Comment 7

[happinessmncube]

After looking at some issues in Bugzilla I discovered that this issue was similar https://bugzilla.mozilla.org/show_bug.cgi?id=1812785. The URL returns null as the content-type , see this comment and therefore firefox assigns .bin as the extension to a PDF file.

Comment 8

[Christian Sadilek [:csadilek]]

Yes it does, because application/x-msdos-program is not in GENERIC_CONTENT_TYPES line 179 in DownloadUtils

No, sorry, that wouldn't be (and fix) the root cause. In the method I shared above getExtensionFromMimeType we look up the extension for the mime type. What values are you getting there for mimeType and extension when debugging?

The URL returns null as the content-type , see this comment and therefore firefox assigns .bin as the extension to a PDF file.

If I test the links above, I get application/x-msdos-program as mimetype, and exe and extension returned from getExtensionFromMimeType. I don't think you're getting a null mime-type, because then the extension wouldn't change, see: https://github.com/mozilla-mobile/firefox-android/blob/5d90029285ebafc64ed20e146e2c3d63087e6efd/android-components/components/support/utils/src/main/java/mozilla/components/support/utils/DownloadUtils.kt#L342.

Feel free to put up a patch and we can discuss details there.

Comment 9

[jackyzy823]

Should if (typeFromExt?.equals(mimeType, ignoreCase = true) != false) { be if (typeFromExt?.equals(mimeType, ignoreCase = true) != true) { ?

Because we need to do getExtensionFromMimeType only when typeFromExt != mimeType.

Also a test shoud be added.
assertEquals("file.exe", DownloadUtils.guessFileName(null, null, "http://example.com/file.exe", "application/x-msdos-program"))

Reference: https://android.googlesource.com/platform/frameworks/base/+/master/core/java/android/webkit/URLUtil.java#374 which you said largely identical to
if (typeFromExt != null && !typeFromExt.equalsIgnoreCase(mimeType)) {

Comment 10

[Christian Sadilek [:csadilek]]

Not the author of this block, but this looks intentional to me. The first check is for the mimetype lookup to verify that the lookup was successful, before the file extension is compared.

But as I mentioned above, feel free to open a patch if you would like to work on a patch.

Comment 11

[BMO Github Automation]

Attachment: [mozilla-mobile/firefox-android] Bug 1810197 - Fix condition of changeExtension (#1572)

Comment 12

[BMO Github Automation]

Authored by https://github.com/jackyzy823
https://github.com/mozilla-mobile/firefox-android/commit/d7105bcdc0197b9edc84ccbfba2b9fd75c2299cd
[main] Bug 1810197 - Fix condition of changeExtension

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

Since nightly and release are affected, beta will likely be affected too.
For more information, please visit auto_nag documentation.

Comment 14

[Adina Petridean QA [:apetridean]]

Verified as fixed on Nightly 114.0a1 from 04/23 with Motorola Moto G9 plus (Android 11). "Wireshark-win64-4.0.1.exe" is downloaded with the extension ".exe".

Comment 15

[Adina Petridean QA [:apetridean]]

Attachment: exe.JPEG

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Arturo, did we want to nominate this for Beta uplift for v113? Please do if yes.

Comment 17

[Arturo Mejia [:amejia]]

Beta/Release Uplift Approval Request

User impact if declined: After downloading a .exe file users need to rename it after download as it get renamed to .com
Is this code covered by automated tests? Yes
Has the fix been verified in Nightly? Yes
Risk to taking this patch: Low
List of other uplifts needed: None
Why is the change risky/not risky? (and alternatives if risky) This patch is low risk as it's only changing the way we rename the extension part of the file when we don't have the content type available.
String changes made/needed No
Is Android affected? Yes

Comment 18

[:bclark]

I approve the uplift

Comment 19

[BMO Github Automation]

Attachment: [mozilla-mobile/firefox-android] Bug 1810197 - Revert "Fix condition of changeExtension". (#1809)

Comment 20

[BMO Github Automation]

Authored by https://github.com/Amejia481
https://github.com/mozilla-mobile/firefox-android/commit/e69dd91c6bf51612e7944f8614a73cae4a747cd3
[main] Bug 1810197 - Revert "Fix condition of changeExtension"

Comment 21

[Arturo Mejia [:amejia]]

@jackyzy823 I had to revert your patch as it was addressing this specific issue but introducing an unwanted code path, I added a longer explanation here, sorry about the misunderstanding. I'm drafting a possible fix locally that will also refactor the initial function which was bit tricky to parse.

Comment 22

[BMO Github Automation]

Attachment: [mozilla-mobile/firefox-android] Bug 1810197 - Improve DownloadUtils.changeExtension. (#1814)

Comment 23

[Arturo Mejia [:amejia]]

@jackyzy823 here you can see the patch, I refactor the function to make it bit clear and kept your original tests :)

Comment 24

[BMO Github Automation]

Authored by https://github.com/Amejia481
https://github.com/mozilla-mobile/firefox-android/commit/5d773ae7710265ab98e15efc43b1afb17ab2e404
[main] Bug 1810197 - Improve DownloadUtils.changeExtension.

Comment 25

[Adina Petridean QA [:apetridean]]

Verified as fixed on Nightly 115.0a1 from 05/25 with Motorola Moto G9 plus (Android 11). "Wireshark-win64-3.6.14.exe" is downloaded with the extension ".exe".

Comment 26

[Adina Petridean QA [:apetridean]]

Attachment: exe.JPEG

Comment 27

[Arturo Mejia [:amejia]]

Thanks Adina 🎉!