1777207 - Since I updated to Firefox 102 there is a problem with files in extension .js

Status: VERIFIED FIXED

(Firefox :: File Handling, defect, P2)

Description

[Mk51]

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:102.0) Gecko/20100101 Firefox/102.0

Steps to reproduce:

Since I upgraded to Firefox 102, so whenever I download a .js file (example: bouncer.js) from the web manager on Webmin it arrives in my "Downloads" folder but it's not "bouncer.js" but "bouncer.js.xml"
Try it you will see!

Comment 1

[Mk51]

Attachment: 38bce9578d257fb1a1fed2624be79811.png

each time it adds ".xml" at the end of the file

Comment 2

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

The Bugbug bot thinks this bug should belong to the 'Firefox::File Handling' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 3

[Mk51]

I created an "index.html" file and put this inside:

<a href="https://raw.githubusercontent.com/freenode/jbnc/master/bouncer.js">https://raw.githubusercontent.com/freenode/jbnc/master/bouncer.js</a>

then right click on the link to save as, and the file will save with "bouncer.js.txt", this time it's not .xml but .txt"
The problem really comes from Firefox 102

Comment 4

[Mk51]

In fact, no need to create an html file, because this page is already in html
Just right click here and save as: https://raw.githubusercontent.com/freenode/jbnc/master/bouncer.js
You will see the "txt" extension at the end

Comment 5

[Mk51]

When will it be fixed on firefox? this week ?
Because it will be quite boring to rename files to ".js" 30 times a day

Comment 6

[Mk51]

I fixed the issue by downgrading to Firefox 101.0.1 using "--allow-downgrade" on first launch to bypass downgrade blocker
".js" files are in ".js" now

Comment 7

[Mk51]

I tested on Windows 11 with latest version of Firefox, it saves the javascript file as "bouncer.js.txt" instead of "bouncer.js"
You have to rename the file.js.txt file to file.js each time to open it with VSCode.

Before updating firefox on Windows 11, I had firefox 101.0.1 and it saved the bouncer.js file directly without putting ".txt" at the end.

On Windows 7 the extension at the end is not ".js.txt" but ".js.xml", that's a problem, it should be ".js" only

Comment 8

[Neil Deakin]

I don't see this issue myself.

Can you post your handlers.json?

Does this happen in a new profile?

Comment 9

[Mk51]

"Does this happen in a new profile?"
Yes

handlers.json (new profile) :
{"defaultHandlersVersion":{},"mimeTypes":{},"schemes":{},"isDownloadsImprovementsAlreadyMigrated":true,"isSVGXMLAlreadyMigrated":true}

Comment 10

[Mk51]

I tested downloading these following links with Firefox 101.0.1 and 102, I right clicked and saved as directly on the url:

• https://raw.githubusercontent.com/freenode/jbnc/master/bouncer.js (Firefox 101.0.1 it downloads the file in extension ".js" but on Firefox 102 it downloads in .js.txt)
• https://code.jquery.com/jquery-3.6.0.min.js (Firefox 101.0.1 and 102 download the file in ".js" correctly)

Firefox seems to give importance to the header of a file rather than to the extension, and this from the Firefox 102 version

Comment 11

[Negritas Sergiu, Desktop QA]

Hello I have tried to reproduce the issue with firefox 104.0a1(2022-07-07), 103.0b5 on Windows 10 but unfortunately I wasn't able to reproduce the issue as described in the first comment, in my case clicking on the "Save Page As" it saves the file as bouncer.js.

Could you please answer the following questions in order to further investigate this issue

1. Does this issue happen with a new profile? Here is a link on how to create one: https://support.mozilla.org/en-US/kb/profile-manager-create-remove-switch-firefox-profiles
2. Does this issue happen in the latest nightly? Here is a link from where you can download it: https://www.mozilla.org/en-US/firefox/channel/desktop/
3. Do you have any addons installed if so can you list them?

Comment 12

[Mk51]

Does this issue happen with a new profile? Here is a link on how to create one: https://support.mozilla.org/en-US/kb/profile-manager-create-remove-switch-firefox-profiles

Yes, and also under a new formatting of Windows 11 (VM) without any migration from an old OS

--

Do you have any addons installed if so can you list them?
Adblock plus, (but on Windows 11 I installed none, everything is default and the problem still occurs with the extension of downloaded js files)

--

Does this issue happen in the latest nightly? Here is a link from where you can download it: https://www.mozilla.org/en-US/firefox/channel/desktop/

I will test

Comment 13

[Mk51]

I tested with Firefox Nightly 104.0a1 and check out this gif:
https://www.zupimages.net/up/22/27/rp4d.gif (5MB)
It's a Firefox without any plugins added
Has been tested on Windows 11 (in VM) and it does exactly the same thing on my Windows 7 (but I keep my version 101.0.1 I won't test because it's a hassle then to go back to version 101.0.1)

Comment 14

[Mk51]

On your computer, in the options of Windows Explorer, uncheck "Hide extensions whose type is known" you will see better, you will see that the esntesions are in bouncer.js.txt

Comment 15

[Marco Bonardo [:mak]]

comment 10 is easily reproducible, just right click on the link, Save As, will save as bouncer.js.txt.
The header says content-type: text/plain; charset=utf-8

I suspect this may be related to bug 1772758? .js is in the "executable" list, it is served as text/plain and thus we append .txt.
See https://bugzilla.mozilla.org/show_bug.cgi?id=1772758#c3 and https://bugzilla.mozilla.org/show_bug.cgi?id=1772758#c4

Comment 16

[Ardelean Oana, Desktop QA [:oardelean]]

This issue is still reproducible on Firefox 103.0b8(build ID: 20220712185923) and Nightly 104.0a1(build ID: 20220712093327) using the first link from Comment 10 and the steps from Comment 15. Tested on Windows 7, Windows 10 and Windows 11(all 64-bits). macOS and Ubuntu are unaffected.

Comment 17

[Monica Chiorean]

Marking as New based on comment16.

Comment 18

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

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

For more information, please visit auto_nag documentation.

Comment 19

[:Gijs (he/him)]

I suspect that both here and in bug 1772758 we're seeing increased reports because this only used to be an issue when setting files to open automatically and now we validate "even" for files "just" saved to disk. The automatic opening, as well as bug 1568003, was why we had/have this validation in the first place (.js files will normally automatically open with the windows script runner, and that's basically the same as automatically running a .exe file). I'm not entirely convinced these are really the same issues.

Either way, maybe we can consider not overwriting the filename in these cases, but disabling auto-open if it's set (instead saving to disk) ? Neil, does that sound reasonable, or do the abstractions in the code make that very tricky? Are there other solutions that don't reintroduce security problems that I'm missing?

Comment 20

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

Set release status flags based on info from the regressing bug 1746052

Comment 21

[Neil Deakin]

Either way, maybe we can consider not overwriting the filename in these cases, but disabling auto-open if it's set (instead saving to disk) ? Neil, does that sound reasonable, or do the abstractions in the code make that very tricky? Are there other solutions that don't reintroduce security problems that I'm missing?

I'm not clear what's being asked here. Which are the 'cases' that you are asking to distinguish?

Comment 22

[:Gijs (he/him)]

(In reply to Neil Deakin from comment #21)

Either way, maybe we can consider not overwriting the filename in these cases, but disabling auto-open if it's set (instead saving to disk) ? Neil, does that sound reasonable, or do the abstractions in the code make that very tricky? Are there other solutions that don't reintroduce security problems that I'm missing?

I'm not clear what's being asked here. Which are the 'cases' that you are asking to distinguish?

Cases where we decide the filename is for an executable file and as a result we overwrite it with .txt or another extension based on the mimetype (ie bug 1772758 where we overwrite with zip). By overwriting we're mitigating against auto-executing, but the filename ends up being wrong (when compared to content-disposition header or download link attribute). I'm asking if we can keep the filename as-is, but change the action we take for that specific file away from "always execute", even if that is what is set for that mimetype. Does that make more sense?

Comment 23

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

Set release status flags based on info from the regressing bug 1746052

Comment 24

[Neil Deakin]

Cases where we decide the filename is for an executable file and as a result we overwrite it with .txt or another extension based on the mimetype (ie bug 1772758 where we overwrite with zip). By overwriting we're mitigating against auto-executing, but the filename ends up being wrong (when compared to content-disposition header or download link attribute). I'm asking if we can keep the filename as-is, but change the action we take for that specific file away from "always execute", even if that is what is set for that mimetype. Does that make more sense?

I don't believe the code that changes the extension (in HelperAppDlg.jsm) relevant to this bug gets run when a helper application is assigned to open for that type. I assume this is what you mean by auto-execute. This code (and bug 1772758) only apply when the user chooses Save. Two cases could happen:

• the file chooser appears defaulting to 'bouncer.js' but it is then saved as 'bouncer.js.txt', which is different that what the user asked for. This also happens if the user entered some other filename that is identified as an executable.
• the file chooser is skipped, and the file is automatically saved with the wrong extension.

Comment 25

[Ryan VanderMeulen [:RyanVM]]

Is there anything we can do to help move this bug forward? Seems like a pretty high-visibility S2 regression to be carrying over from release to release (and soon be introducing to the enterprise crowd when they're migrated from 91 to 102).

Comment 26

[:Gijs (he/him)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #25)

Is there anything we can do to help move this bug forward?

I'm pretty swamped right now and about to go on PTO. Perhaps Neil has cycles to investigate further exactly what is happening here and if we could fix this somehow (I'm being a bit handwavy because I don't think I fully understand the root cause given recent comments) without running the risk of files being opened with the wrong helper app as in bug 1568003.

Seems like a pretty high-visibility S2 regression to be carrying over from release to release (and soon be introducing to the enterprise crowd when they're migrated from 91 to 102).

Honestly I'm not sure how high-visibility and/or S2 it should be considered. Like, I don't like this bug because I'm an engineer and I want this code to not have bugs, and I understand it's super annoying for the reporter. But realistically, AFAICT this happens for some file extensions (ones in the executable list), being served with the "wrong" mimetype (text/plain for JS; zip file mimetype in bug 1772758).

It's not 100% clear to me if they have the same root cause, given Neil's recent comments. The zip case honestly seems more common than this one. The jar/zip thing has duplicates, but here it's been over 2 months and there are no dupes or "me too" comments or CCs. So I'm surfacing this for Molly for (re-)consideration for S2 triage. (To be clear: keeping this as S2 is very much a legitimate outcome. I just think we should take another look at this point.)

Comment 27

[Molly Howell (she/her) (no longer active)]

(In reply to :Gijs (he/him) from comment #26)

So I'm surfacing this for Molly for (re-)consideration for S2 triage.

The definition of S2 requires that no reasonable workaround for the problem exist. Given that, I think it's quite clear that this bug does not qualify, because changing the extension manually is a pretty reasonable workaround.

Comment 28

[Neil Deakin]

I think the code in question that is causing this bug at https://searchfox.org/mozilla-central/rev/3f9dcc016dd96a0336d46f4a19aeabdd796ab9e9/toolkit/mozapps/downloads/HelperAppDlg.jsm#464 may just not be needed any more. The filename and extension is now already being validated by validateFileNameForSaving().

I verified that this code does not run when launching an application and only runs when saving a file.

Comment 29

[Neil Deakin]

Attachment: Bug 1777207, don't replace extensions of executables when saving on Windows any more; the filename has already been validated and it can cause the filename a user chose to be incorrect, r=mhowell

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9fd7ef3c9d43
don't replace extensions of executables when saving on Windows any more; the filename has already been validated and it can cause the filename a user chose to be incorrect, r=mhowell

Comment 31

[Atila Butkovits]

Backed out for causing failures at browser_save_filenames.js.

Backout link: https://hg.mozilla.org/integration/autoland/rev/c4b67ce8ba06c38251a213e5bb45410662c2931d

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=AqsyHnNsTGmLoiEcScIGnA.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=9fd7ef3c9d4366985953b05511f81f4e9a77b742

Failure log: https://treeherder.mozilla.org/logviewer?job_id=389893501&repo=autoland&lineNumber=91439

Comment 32

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8833ae91b6fb
don't replace extensions of executables when saving on Windows any more; the filename has already been validated and it can cause the filename a user chose to be incorrect, r=mhowell

Comment 33

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/8833ae91b6fb

Comment 34

[Ardelean Oana, Desktop QA [:oardelean]]

I managed to reproduce this issue on Firefox 105.0(build ID: 20220915150737) on Windows 10 using the first link from Comment 10 and the steps from Comment 15. Verified as fixed on Firefox 106.0b4(build ID: 20220925185751) and Nightly 107.0a1(build ID: 20220926213808) on Windows 10, Windows 11.

Comment 35

[Ryan VanderMeulen [:RyanVM]]

Hi Neil, this seems like something we'll want to take on ESR as well. Please nominate when you get a chance.

Comment 36

[Donal Meehan [:dmeehan]]

This is needed in esr102 as it is a dependant to uplift Bug 1778597

Comment 37

[Neil Deakin]

Comment on attachment 9293558 [details]
Bug 1777207, don't replace extensions of executables when saving on Windows any more; the filename has already been validated and it can cause the filename a user chose to be incorrect, r=mhowell

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: The filename when saving on Windows can sometimes be given the wrong extension, different from the one selected in the save dialog.
• User impact if declined: User could be confused and believe that a file was not saved.
• Fix Landed on Version: 106
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Removes a windows-specific block of code that is now redundant.

Comment 38

[Donal Meehan [:dmeehan]]

Comment on attachment 9293558 [details]
Bug 1777207, don't replace extensions of executables when saving on Windows any more; the filename has already been validated and it can cause the filename a user chose to be incorrect, r=mhowell

Approved for 102.5esr.

Comment 39

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-esr102/rev/8843b2a3a37a

Comment 40

[Ardelean Oana, Desktop QA [:oardelean]]

Hello,
I confirm this fix is verified on Firefox 102.5.0esr(build ID: 20221107191025) on Windows 10 and Windows 11.