Open Bug 1558251 Opened 5 years ago Updated 2 years ago

[Windows] Firefox is appending .xslm extension to an .xlsx filename when the server sends the wrong mimetype

Categories

(Firefox :: File Handling, defect, P5)

67 Branch
defect

Tracking

()

People

(Reporter: jholovacs, Unassigned)

Details

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/74.0.3729.169 Safari/537.36

Steps to reproduce:

Downloaded Firefox (latest version, 67.0.1, 64-bit)
downloaded xlsx file via POST
Selected to open the file with Excel

Actual results:

response header contains:

Content-Disposition: attachment; filename="MyFile.xlsx"
Content-Length: 32718
Content-Type: application/vnd.ms-excel.sheet.macroenabled.12

Excel reports: "Excel cannot open the file MyFile.xlsx-1.xlsm because the file format or extension is not valid. Verify that the file has not been corrupted and that the file extension matches the format of the file."

Expected results:

Granted, the Content-Type is wrong for a non-macro Excel file (we will be fixing that) but to forcibly append an extension other than the one provided by the response headers causes Excel to try and treat the file differently than it should.

Firefox should respect the explicit naming of the file when downloading over the MIME type supplied in the headers. All other modern browsers tested with this do so.

Hi,

Try to see if you can reproduce this issue on Nightly please, here is the link to download it https://www.mozilla.org/en-US/firefox/nightly/all/
Could you also provide an example file and add an screen recording of the issue please?
Thanks,

Flags: needinfo?(jholovacs)

Issue is reproducible with the current nightly build as of 18 Jun 2019.

I made a sample asp.net core project with an example file you can clone from here: https://github.com/jholovacs/MozillaBug1558251

From Visual Studio, run the project in the solution, navigate to http://localhost:5000, and it will download an .xlsx file. If you have Excel installed and try to open it you get a screen as found in the repo @ Screenshots/mozilla1558251_ss01.PNG.

If you click "OK" here, Excel opens up and presents you with the screen as found in the repo @ Screenshots/mozilla1558251_ss02.PNG.

I have no screen recording software installed, but this should be sufficient.

Flags: needinfo?(jholovacs)

Hi,
I tried to reproduce this issue but my Visual studio is not set up correctly and I'm getting a lot of errors when trying to open the project mentioned in comment 2 but this does seem like a File Handling issue, I will set a component and maybe one of our devs can take a look and reproduce this issue.

Thanks!

Component: Untriaged → File Handling
Flags: needinfo?(jholovacs)

Ah, you'll likely need to install the .net core sdk 2.2+ to run this.

Flags: needinfo?(jholovacs)

Fixing the content-type the server is sending is likely the expedient solution here.

AFAICT this behaviour is very, very old (bug 164816, bug 398551, etc.). In this case, it may be obvious to a human being that the extension should not be altered, but the "point" of this thing is to trust the mimetype provided by the server over the filename (a filename which is often auto-generated, and a mimetype which we hope is determined more accurately). When the two things don't match, we have to make a decision either way. It's possible that the decision should be different on today's web compared to the web of 15 years ago... but it's not clear to me how we'd figure out if that was the case. Marco, do you have ideas?

It'd be interesting to know if/when/how other browsers decide to add or not add file extensions on Windows (where you need the extension in order for apps/windows to not be upset).

Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(mak77)
Priority: -- → P5
Regressed by: 398551
Summary: Inverse of 1292507, Firefox is appending .xslm to an .xlsx file → [Windows] Firefox is appending .xslm extension to an .xlsx filename when the server sends the wrong mimetype

Oops, meant to remove that bug link, as I actually think this particular case is the "fault" of https://searchfox.org/mozilla-central/rev/40e889be8ff926e32f7567957f4c316f14f6fbef/uriloader/exthandler/nsExternalHelperAppService.cpp#2372-2376 .

No longer regressed by: 398551

I'll just comment that the above behavior is not observed in Chrome, IE, Edge, or Safari, so I would suggest that the decision has been made (passively) as to what is expected by the Internet-browsing community in general.

We did fix the content-type on the server (that was definitely a bug in our software) but it was difficult to track down until we realized it could only be produced on FireFox, which a fairly small percentage of our client base regularly uses.

(In reply to Jeremy Holovacs from comment #7)

I'll just comment that the above behavior is not observed in Chrome, IE, Edge, or Safari, so I would suggest that the decision has been made (passively) as to what is expected by the Internet-browsing community in general.

Sure, you mentioned that that's what's happening for the xlsx/xlsm case. The question I wonder about is whether this applies to all extensions (so if I send a .docx file as image/png, or a png image as application/x-msword or whatever it is, do other browsers always just keep the existing extension? That'd seem surprising to me, but who knows...), or if there's a windows trick we're missing to determine that the existing primary extension would prompt opening in the same app or something like that (which would cover the case in this bug but not the crazier examples I just gave).

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

When the two things don't match, we have to make a decision either way. It's possible that the decision should be different on today's web compared to the web of 15 years ago... but it's not clear to me how we'd figure out if that was the case. Marco, do you have ideas?

My understanding on this matter is same as yours, afair we decided time ago to always trust the mime type.
The webcompat argument is really strong here, if other browsers act differently, we should try to cope with that, provided it doesn't introduce new attack surface (sending a file as something else may become easier).

The best path forward would be primarily to figure out what other browsers do (at least Chrome and Safari, considered the marketshare) in their code, if they just trust Content-Disposition, and which kind of safety checks they added against abuse of that.

Flags: needinfo?(mak77)

I'm looking at this
https://cs.chromium.org/chromium/src/chrome/browser/download/download_target_determiner.cc?cl=GROK&g=0&l=255
and
https://cs.chromium.org/chromium/src/net/base/filename_util_internal.cc?cl=GROK&gsn=GenerateFileName&g=0&l=238

off-hand it seems to always trust content-disposition filename, then it runs a few additional checks against shell extensions that should not be used

(In reply to Marco Bonardo [::mak] from comment #10)

I'm looking at this
https://cs.chromium.org/chromium/src/chrome/browser/download/download_target_determiner.cc?cl=GROK&g=0&l=255
and
https://cs.chromium.org/chromium/src/net/base/filename_util_internal.cc?cl=GROK&gsn=GenerateFileName&g=0&l=238

off-hand it seems to always trust content-disposition filename, then it runs a few additional checks against shell extensions that should not be used

Hm, so they use it unless it's on the safebrowsing "full ping" list. Dimi, do you know off-hand what that means / what that list corresponds to in our safebrowsing implementation?

Flags: needinfo?(dlee)

(In reply to :Gijs (back Aug 12; he/him) from comment #11)

Hm, so they use it unless it's on the safebrowsing "full ping" list. Dimi, do you know off-hand what that means / what that list corresponds to in our safebrowsing implementation?

If I understand correctly, they only replace filename when it is considered safe by SafeBrowsing(Download Protection), otherwise, for example, an exe may be replaced by jpg, hence download protection check may be bypassed. But it is ok to replace jpg extension because it is safe so there is no harm to replace it from SafeBrowsing’s perspective.

Our corresponding list to their “full ping” list is kBinaryFileExtensions + sExecutableExts, see here[1] for a very detailed explanation :)

[1] https://searchfox.org/mozilla-central/rev/38c88cbf4be87dfa0636d15a5d3599a8ea0d1a72/toolkit/components/reputationservice/ApplicationReputation.cpp#108

Flags: needinfo?(dlee)
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.