Win10 .exe file download from WeTransfer picked up and saved with .jpg extension
Categories
(Toolkit :: Downloads API, defect, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr78 | --- | unaffected |
firefox82 | --- | unaffected |
firefox83 | --- | unaffected |
firefox84 | --- | verified |
firefox85 | --- | verified |
People
(Reporter: cfogel, Assigned: Gijs)
References
(Regression, )
Details
(Keywords: regression)
Attachments
(4 files)
Affected versions
- 84.0a1(2020-11-07)
Affected platforms
- Windows 10;
Steps to reproduce
- Launch Firefox;
- Access https://we.tl/t-tKmO6XTDcF
- Download file;
Expected result
- file is saved on device without issues;
Actual result
- file is saved with .jpg extension not .exe;
Regression range
- last good: 2020-10-20;
- first bad: 2020-10-21;
- Pushlog URL;
Additional notes
- attached screenshot to illustrate the issue;
- checked, but no weird extension/associations are made on the OS level;
- suggested severity is S4, since it appears to be 1off case;
- other browsers such as Chrome, handle the download of file as expected as well.
Reporter | ||
Comment 1•4 years ago
|
||
Attaching a .gif with associations already made with the browser, in case it might be of any help to debug this.
Reporter | ||
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Comment 2•4 years ago
|
||
Cannot reproduce this issue in order to get a regression range since this appears to be limited to 1 profile.
Assignee | ||
Comment 3•4 years ago
|
||
checked, but no weird extension/associations are made on the OS level;
It'll be about the handlers.json file in your profile.
(In reply to Raluca from comment #2)
Cannot reproduce this issue in order to get a regression range since this appears to be limited to 1 profile.
You can use --profile
to pass the profile into mozregression in order to reproduce.
I suspect this regressed with bug 1667787, and should have been fixed by bug 1652520, but it doesn't look like that's the case.
Can you confirm the regression window?
Assignee | ||
Comment 4•4 years ago
|
||
(In reply to :Gijs (he/him) from comment #3)
I suspect this regressed with bug 1667787, and should have been fixed by bug 1652520, but it doesn't look like that's the case.
In fact, after some testing, perhaps this has been broken longer. It'd be useful to know.
Reporter | ||
Comment 5•4 years ago
•
|
||
Thanks for the suggestion with the profile load.
Will update the main comment with the regression range complete data.
Indeed mozregression points towards bug 1667787. Prior to that, the downloader window was picking up the file as .exe and not prompting to open with other software.
Comment 6•4 years ago
|
||
Set release status flags based on info from the regressing bug 1667787
Assignee | ||
Comment 7•4 years ago
|
||
(In reply to Cristian Fogel, QA [:cfogel] from comment #5)
Thanks for the suggestion with the profile load.
Will update the main comment with the regression range complete data.Indeed mozregression points towards bug 1667787. Prior to that, the downloader window was picking up the file as .exe and not prompting to open with other software.
With what extension does the file get saved if you do choose to save it?
Assignee | ||
Comment 9•4 years ago
|
||
(In reply to Cristian Fogel, QA [:cfogel] from comment #8)
As .jpg
If this happens before bug 1667787 (which is what I think is the case), can you find a regression window for that?
Reporter | ||
Comment 10•4 years ago
•
|
||
Sorry for the late reply. As on comment 5, that was it.
Prior to the mozregression build that pointed towards that bug, the downloader did not pick up the file as .jpg or [Save As]/[Open With]; it got downloaded directly as .exe file.
Assignee | ||
Comment 11•4 years ago
•
|
||
(In reply to Cristian Fogel, QA [:cfogel] from comment #10)
Sorry for the late reply. As on comment 5, that was it.
Prior to the mozregression build that pointed towards that bug, the downloader did not pick up the file as .jpg or [Save As]/[Open With]; it got downloaded directly as .exe file.
But it's not fixed on current nightly? Can you attach the handlers.json
file from the profile with which you can reproduce this?
Reporter | ||
Comment 12•4 years ago
|
||
Attached file.
84.0a1(20201115093418) is still affected.
Updated•4 years ago
|
Assignee | ||
Comment 13•4 years ago
|
||
Setting this to affected again because this is a recent regression and it basically will break downloads for some users, so I think we need a fix in 84.
Assignee | ||
Comment 14•4 years ago
|
||
There's a whole sequence of stupid things combining to wreak havoc here...
- the website doesn't send a filename with
Content-Disposition: attachment
- the website sends a useless mimetype (
binary/octet-stream
) - the website does have the filename in the URL, but adds (tracking?) URL query parameters, which now properly disable getting the extension out of the URL (that changed with 1667787 which is how this went from "working" to "broken")
- the user profile has bogus information for the bogus mimetype (namely, use "jpg" for these files)
bug 1659008 should fix (4).
There are a number of tempting fixes here otherwise. The simplest that may still be upliftable is probably to partially revert (3) in that we should take the extension out of the URL (if there's no header info) for application/octet-stream, binary/octet-stream and application/x-msdownload or whatever that is.
Assignee | ||
Comment 15•4 years ago
|
||
This restores extension discovery logic to using the URL for requests where
there's no filename in the Content-Disposition header information, and where
the mimetype gives no indication of the 'correct' extension.
This is a short-term workaround for the larger issue that we should not have
bad local information for these mimetypes - fixing that is bug 1659008, but
requires using the extension to get a better mimetype, which this patch will
also help with.
Assignee | ||
Updated•4 years ago
|
Comment 16•4 years ago
|
||
The severity field is not set for this bug.
:mak, could you have a look please?
For more information, please visit auto_nag documentation.
Updated•3 years ago
|
Comment 17•3 years ago
|
||
Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/autoland/rev/8960c091c763 honour extensions from URLs with query parameters for useless mimetypes to avoid taking the extension from bogus local data, r=mak
Comment 18•3 years ago
|
||
Pushed by btara@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/cc51b2d4ae85 Fix clang-format issue CLOSED TREE
Comment 19•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/8960c091c763
https://hg.mozilla.org/mozilla-central/rev/cc51b2d4ae85
Assignee | ||
Comment 20•3 years ago
|
||
Comment on attachment 9189285 [details]
Bug 1676345 - honour extensions from URLs with query parameters for useless mimetypes to avoid taking the extension from bogus local data, r?mak
Beta/Release Uplift Approval Request
- User impact if declined: Files may download with wrong extensions
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: No
- Needs manual test from QE?: Yes
- If yes, steps to reproduce: 1. upload a .exe file to wetransfer
- create a new firefox profile. Open Firefox with it. Close Firefox.
- drop the handlers.json attachment from https://bugzilla.mozilla.org/show_bug.cgi?id=1676345#c12 into that profile
- open firefox again
- try to download the file
- List of other uplifts needed: n/a
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): There is decent test coverage and we're basically partially-restoring pre-regressing-patch behaviour, and it's a very very small change - the test was 90% of the effort in writing this...
If we take this, please also take and/or merge in the formatting patch; apparently my clang-format was outdated or something?
- String changes made/needed: none
Assignee | ||
Updated•3 years ago
|
Updated•3 years ago
|
Comment 21•3 years ago
|
||
Comment on attachment 9189285 [details]
Bug 1676345 - honour extensions from URLs with query parameters for useless mimetypes to avoid taking the extension from bogus local data, r?mak
Seems like this has been a game of whack-a-mole, but I agree that it'd be good to avoid shipping a new regression in 84. Thanks for including automated test coverage. Approved for 84.0b8.
Comment 22•3 years ago
|
||
bugherder uplift |
Reporter | ||
Comment 23•3 years ago
|
||
Verified with 85.0a1 (2020-12-03) and 84.0b4 on Windows 10.
Reporter | ||
Updated•3 years ago
|
Description
•