Closed Bug 1684183 Opened 3 years ago Closed 3 years ago

When downloading a file that is internally a zip, but has a different extension (.rwp, .story, .t5script), the extension is overwritten to .zip

Categories

(Firefox :: File Handling, defect, P1)

Firefox 84
Desktop
All
defect

Tracking

()

VERIFIED FIXED
86 Branch
Tracking Status
firefox-esr78 --- unaffected
firefox84 --- wontfix
firefox85 + verified
firefox86 + verified

People

(Reporter: sr.meijer, Assigned: Gijs)

References

(Regression)

Details

(Keywords: regression)

Attachments

(3 files)

Attached image Download Screen

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

Steps to reproduce:

Downloading a .rwp (Train Simulator 2021 zipped folder) results in renaming the extension to .zip. An RWP is a ZIP folder with a specific header to tell an unpacker where to install the files.

Actual results:

When downloading .rwp the downloaded file is downloaded as a .zip, which makes opening those files without renaming the file extension to the original impossible.

When the download options window appears - where the user can chose to open the file with a program or just download it - it's said to be a .zip.

Expected results:

The file should be downloaded as the original, which is "filename.rwp".

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Product: Firefox → WebExtensions
Component: Untriaged → File Handling
Product: WebExtensions → Firefox

I think this is another regression from bug 1667787.

Regressed by: 1667787
Has Regression Range: --- → yes

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

This wouldn't happen if the webserver didn't send the mimetype application/zip. Why does it do that, if the file shouldn't be treated as a zipfile? :-\

We could fix this specific instance by removing application/zip from the list added in https://hg.mozilla.org/mozilla-central/rev/0a93a065053d#l4.17 . But all of this is so dumb - half the time, web servers send us the correct mimetype but the wrong file extension, and the other time they send the wrong mimetype and the correct file extension (like here). And whenever we pick the wrong thing, it's all our fault. It's infuriating.

I'll look at this more in January, but this isn't a severe enough issue to justify scrambling people to do a dot release to 84 over the Christmas break (like a security issue or similar).

Status: UNCONFIRMED → NEW
Ever confirmed: true
See Also: → 1684202

How does Chrome handle this (and bug 1667787)? If Chrome behaves differently, of course "It's all our fault(tm)".

(In reply to Masatoshi Kimura [:emk] from comment #6)

How does Chrome handle this (and bug 1667787)? If Chrome behaves differently, of course "It's all our fault(tm)".

I don't know about this specific case because there are no detailed steps to reproduce and I don't have time right now to set something up myself and hope that it behaves exactly as detailed here.

But broadly, last I checked, Chrome had a giant list of mimetypes of its own, and we mostly relied on the OS. I can't find a list as large as the one I recall anymore; possibly because my ability to search source.chromium.org is lacking, possibly because the implementation has changed - I don't know. Now, from a quick look I'm finding https://source.chromium.org/chromium/chromium/src/+/master:net/base/mime_util.cc;l=77?q=vnd.ms&ss=chromium%2Fchromium%2Fsrc (which looks pretty similar to what we do in general approach, if not in specifics) plus its own set of hacks (e.g. https://source.chromium.org/chromium/chromium/src/+/master:extensions/browser/api/file_handlers/mime_util.cc;l=105?q=application%2Fzip&sq=&ss=chromium ; I expect there's more but I don't know chromium's codebase so I don't know how to find them all).

There's no spec that I'm aware of, because once we stop talking about rendering and start talking about downloading the HTML spec stops caring. The fetch spec might not, but I'm not aware of it having any rules about filenames, extensions, how to determine them, and how to prevent doing so from causing security holes, either.

[Tracking Requested - why for this release]:
This is being reported repeatedly on SUMO (not for rwp, but for various other file types where webservers apparently also send application/zip). See also https://bugzilla.mozilla.org/show_bug.cgi?id=1652520#c28 for more context.

Aside: reports from SUMO seems to suggest (e.g. https://support.mozilla.org/en-US/questions/1319324 ) that we're also using guessed mimetypes for the channel content type, which probably shouldn't override the extension, either -- but that'll require a separate fix.

As a workaround, variations on this answer: https://support.mozilla.org/en-US/questions/1318447#answer-1379574 should help affected users in the meantime.

Assignee: nobody → gijskruitbosch+bugs
Severity: -- → S2
Status: NEW → ASSIGNED
OS: Unspecified → All
Priority: -- → P1
Hardware: Unspecified → Desktop
Summary: file extension changes when downloading → When downloading a file that is internally a zip, but has a different extension (.rwp, .story, .t5script), the extension is overwritten to .zip

Hm, odd, so I cannot reproduce on nightly with the testcase from https://support.mozilla.org/en-US/questions/1319659 . The other SUMO link requires a login. There's no testcase in comment #0. The test case from jscher posted at https://www.jeffersonscher.com/temp/story.php works for me in a vanilla nightly profile on Windows 10.

I have a patch ready, but because I cannot reproduce I cannot verify that it fixes the problem. Additionally, because I cannot reproduce the problem, I am now unclear on whether I understand the root cause correctly or not.

Bas / jscher, do you have a testcase that still reproduces this problem? If so, please can you link it and provide a copy of handlers.json from your Firefox profile directory with which you're seeing this issue?

Flags: needinfo?(sr.meijer)
Flags: needinfo?(jscher2000)

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

The test case from jscher posted at https://www.jeffersonscher.com/temp/story.php works for me in a vanilla nightly profile on Windows 10.

The story.php script sets

  • Content-Disposition: attachment; filename="story.story"
  • Content-Type: application/zip

Desired result: file is saved as story.story

Current Nightly: file is renamed to story.zip

It doesn't matter whether I have application/zip set to Always Ask or Save File in handlers.json.

More generally:

I agree it's impractical to keep lists of all the possible cases, if that is what Chrome is doing. Would it be an option to keep the file extension provided unless there is good reason to believe it would be unhelpful, like a server-side script extension such as php, jsp, asp, do, etc.? I guess with any change you can't be sure it's helping.

Flags: needinfo?(jscher2000)

(In reply to jscher2000 from comment #10)

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

The test case from jscher posted at https://www.jeffersonscher.com/temp/story.php works for me in a vanilla nightly profile on Windows 10.

The story.php script sets

  • Content-Disposition: attachment; filename="story.story"
  • Content-Type: application/zip

Desired result: file is saved as story.story

Current Nightly: file is renamed to story.zip

It's not for me, though. I get a story.story file, even before my patch, on both beta 85 and nightly 86. Can you reproduce on a clean Firefox profile? What OS are you testing on?

Flags: needinfo?(jscher2000)
See Also: → 1685102

Tom has kindly verified that the attached patch fixes the issue on Linux, and I've verified it fixes a similar issue reported in bug 1685102 for XML vs gpx.

I still don't understand why I can't reproduce the issue here (and/or on SUMO) on Windows (and neither could Tom), and I thus can't be sure the patch fixes the same issue there. jscher, if you could test the build from https://treeherder.mozilla.org/jobs?repo=try&revision=675c486ff886d984438b4c111b453b50a4e732f3 (64-bit build; 32-bit build) and let me know if it fixes the issue on your machine, that would be helpful.

It would be nice to understand why the issue appears on some Windows machines / Firefox profiles, and not others, but at the moment I don't see how to do so in a straightforward way, and time to fix this for 85 is somewhat tight, so I'm hoping the patch works anyway.

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

It's not for me, though. I get a story.story file, even before my patch, on both beta 85 and nightly 86. Can you reproduce on a clean Firefox profile? What OS are you testing on?

I've been testing on 64-bit Windows 10, in 64-bit Firefox and 64-bit Nightly. I've tested in a variety of profiles.

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

jscher, if you could test the build from https://treeherder.mozilla.org/jobs?repo=try&revision=675c486ff886d984438b4c111b453b50a4e732f3 (64-bit build; 32-bit build) and let me know if it fixes the issue on your machine, that would be helpful.

Yes, that works as desired.

It would be nice to understand why the issue appears on some Windows machines / Firefox profiles, and not others, but at the moment I don't see how to do so in a straightforward way, and time to fix this for 85 is somewhat tight, so I'm hoping the patch works anyway.

Hmm, I don't know.

Flags: needinfo?(jscher2000)
Attachment #9195438 - Attachment description: Bug 1684183 - stop replacing file extensions for zip, json and xml files, and add a pref in case further issues appear for the other document types, r?mak → Bug 1684183 - stop replacing file extensions for zip, json and xml files, and add a pref in case further issues appear for the other document types
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/181e16e12d9d
stop replacing file extensions for zip, json and xml files, and add a pref in case further issues appear for the other document types r=evilpie
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 86 Branch

Comment on attachment 9195438 [details]
Bug 1684183 - stop replacing file extensions for zip, json and xml files, and add a pref in case further issues appear for the other document types

Beta/Release Uplift Approval Request

  • User impact if declined: User/website download filenames have their extensions changed in incompatible ways
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: See the testcase at https://www.jeffersonscher.com/temp/story.php and/or https://jsfiddle.net/7qec3h10/ - the download should keep the story / gpx extension, instead of it being replaced by .zip / .xml . It's not clear right now what governs the pre-patch zip behaviour on Windows, so it's possible the zip case may not reliably reproduce on Windows - either way it should definitely not reproduce post-patch.
  • List of other uplifts needed: n/a
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): The patch adds a pref and removes specific mimetypes from a list governing the extension-changing behaviour. There are otherwise no logic changes.
  • String changes made/needed: nope
Attachment #9195438 - Flags: approval-mozilla-beta?
Flags: qe-verify+

Comment on attachment 9195438 [details]
Bug 1684183 - stop replacing file extensions for zip, json and xml files, and add a pref in case further issues appear for the other document types

approved for 85.0b6

Attachment #9195438 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
See Also: 1684202
QA Whiteboard: [qa-triaged]

Reproduced the initial issue using old Nightly from 2020-12-24 and https://www.jeffersonscher.com/temp/story.php sample (story.zip was downloaded instead of story.story). I did verify that using Firefox 85.0b6 this is no longer the case story.story was downloaded and which is: part of save file panel is story File. Also I did made an account on the website that the reporter downloaded from and I confirm that .rwp files are downloaded as .rwp (to be fair, on an affected build .rwp format was not changed to .zip for me).

A few things need clarification though:

  1. Firefox 86.0b6 - Using demo from https://jsfiddle.net/7qec3h10/ I do get example.gpx but the which is: section says Extensible Markup Language (XML) which I am not sure is correct. I did searched a bit and found out that .gpx "is an XML schema designed as a common GPS data format for software application" so I suspect this is correct.
  2. Latest Nightly from today (with the fix applied) - https://www.jeffersonscher.com/temp/story.php does download story.story but which is: section says ZIP Archive
  3. Latest Nightly from today (with the fix applied) - https://jsfiddle.net/7qec3h10/ does not trigger a download at all.
Flags: needinfo?(gijskruitbosch+bugs)

(In reply to Bogdan Maris [:bogdan_maris], Release Desktop QA from comment #22)

Created attachment 9196047 [details]
Screenshot showing potential issue on Nightly

Reproduced the initial issue using old Nightly from 2020-12-24 and https://www.jeffersonscher.com/temp/story.php sample (story.zip was downloaded instead of story.story). I did verify that using Firefox 85.0b6 this is no longer the case story.story was downloaded and which is: part of save file panel is story File. Also I did made an account on the website that the reporter downloaded from and I confirm that .rwp files are downloaded as .rwp (to be fair, on an affected build .rwp format was not changed to .zip for me).

Great!

A few things need clarification though:

  1. Firefox 86.0b6 - Using demo from https://jsfiddle.net/7qec3h10/ I do get example.gpx but the which is: section says Extensible Markup Language (XML) which I am not sure is correct. I did searched a bit and found out that .gpx "is an XML schema designed as a common GPS data format for software application" so I suspect this is correct.

Yes. The description is still looked up based on the mimetype, which is why we end up with that description.

  1. Latest Nightly from today (with the fix applied) - https://www.jeffersonscher.com/temp/story.php does download story.story but which is: section says ZIP Archive

Same thing here - you don't see this on beta because by default, Windows doesn't know the application/zip mimetype at all - but we added builtin support in bug 1681924.

  1. Latest Nightly from today (with the fix applied) - https://jsfiddle.net/7qec3h10/ does not trigger a download at all.

Interesting. This is a nightly-restricted regression from bug 1658878, which is happily already on file in bug 1667348.

So I think we can mark this bug as verified fixed. :-)
Thank you for your exhaustive testing!

Flags: needinfo?(sr.meijer)
Flags: needinfo?(gijskruitbosch+bugs)
Flags: needinfo?(bogdan.maris)

Thanks for the info Gijs, I'll mark this as verified.

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
Flags: qe-verify+
Flags: needinfo?(bogdan.maris)
Blocks: 1690051

This also happens with media files, https://media.discordapp.net/attachments/474510580191854593/929083614786895952/Snapchat-207719153_1_1.mp4 is saved as Snapchat-207719153_1_1.3gp

(In reply to Franpa_999 from comment #27)

Windows 10 21H2, Latest x64 Firefox (v93.0).
File extension should be .mp4 but is changed to .3gp when downloading.

(In reply to Franpa_999 from comment #28)

(In reply to Franpa_999 from comment #27)

Windows 10 21H2, Latest x64 Firefox (v93.0).

This is not latest - current release is 95, and the coming week 96 will be released.

File extension should be .mp4 but is changed to .3gp when downloading.

This is a separate bug, unrelated to this ticket, see e.g. bug 1734606 and various other issues filed as regressions from bug 1704115. I'd recommend filing a separate bug with more details (i.e. a specific file/website where this reproduces).

Flags: needinfo?(Franpa_999)

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

(In reply to Franpa_999 from comment #28)

(In reply to Franpa_999 from comment #27)

Windows 10 21H2, Latest x64 Firefox (v93.0).

This is not latest - current release is 95, and the coming week 96 will be released.

File extension should be .mp4 but is changed to .3gp when downloading.

This is a separate bug, unrelated to this ticket, see e.g. bug 1734606 and various other issues filed as regressions from bug 1704115. I'd recommend filing a separate bug with more details (i.e. a specific file/website where this reproduces).

Ah right, I had forgotten I had disabled updating Firefox. I've made a new report here: https://bugzilla.mozilla.org/show_bug.cgi?id=1749294

Flags: needinfo?(Franpa_999)
You need to log in before you can comment on or make changes to this bug.