herokuapp.com - Problems with spaces in filenames (content-disposition not quoted)
Categories
(Core :: Networking, defect, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox81 | --- | fixed |
People
(Reporter: hartmantam.enc, Assigned: Gijs)
References
(Blocks 1 open bug, )
Details
(Keywords: dev-doc-complete, Whiteboard: [necko-triaged])
Attachments
(1 file)
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:58.0) Gecko/20100101 Firefox/58.0 Build ID: 20180206200532 Steps to reproduce: Access https://fathomless-shelf-94559.herokuapp.com/ Actual results: Download the file with filename `Junk` Expected results: The file name should be `Junk Text.txt`
Reporter | ||
Comment 1•6 years ago
|
||
This error occur when the filename parameter contains space. Other browser such as Chrome, Edge, Vivaldi doesn't have this parsing problem.
Comment 2•6 years ago
|
||
NI :bz because bug 221028 is now in Core Graveyard, so it doesn't seem right to dupe this to it.
Comment 3•6 years ago
|
||
It's still the right place; I have no idea why someone stuck it in "graveyard".
Comment 4•6 years ago
|
||
Note that if there were a clear spec for how to support space here we could consider switching to it. But right now, the only spec for this stuff explicitly days spaces are only allowed inside a quoted string.
Reporter | ||
Comment 5•6 years ago
|
||
For connivance to other users, why stuck with spec while other vendors provide the functionality? I understand the importance of following spec. However, it will just drive end user crazy then switch to others. I think user experience take more place in this case than following spec.
Comment 6•6 years ago
|
||
Last I checked, there were cases in which the other browsers have incorrect behavior when the server is doing the right thing, as a result of them wanting to support this spec deviation... Again, if there is a behavior that does _not_ have that problem and is clearly described switching to it would not be a problem. But fixing up this sort of server error at the expense of breaking other things is not necessarily a great tradeoff.
Comment 7•6 years ago
|
||
(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #3) > It's still the right place; I have no idea why someone stuck it in > "graveyard". The entirety of Core :: File Handling was moved to Core Graveyard :: File Handling. So bug 221028 and its myriad of dupes ended up there. Your previous decision is at bug 221028, comment 7 and your explanation at bug 221028, comment 88.
Comment 9•6 years ago
|
||
No, I actually thought Julian had solved Content-Disposition by creating rather exhaustive tests for it, but I guess other browsers didn't bother to fix their bugs?
Comment 10•6 years ago
|
||
Looking at comment 7 I suspect Julian wouldn't disagree with Julian from 6 years ago, but if other browsers are forcing our hand here, it might be time to reconsider? It does seem like there's a huge number of duplicates for this bug...
Comment 11•6 years ago
|
||
My position on this hasn't changed. Get servers to send valid header fields instead of competing on lazy parsing. If a sender doesn't quote whitespace, it'll likely make other mistakes, such as handling non-ASCII incorrectly, or including other disallowed ASCII characters.
Reporter | ||
Comment 12•6 years ago
|
||
So how will Firefox dev team handle this? Fix it or following the standard?
Comment 13•6 years ago
|
||
I think that to "fix it" there needs to be a specific behavior proposed. Then we can at least evaluate whether that behavior is reasonable... Ideally all browsers would implement the same behavior here, obviously.
Reporter | ||
Comment 14•6 years ago
|
||
Then obviously Firefox behavior of handling spaces in Content-Disposition is not equal to other majority of browsers such as Chrome.
Comment 15•6 years ago
|
||
I have no idea what comment 14 is trying to say, for what that's worth.
Reporter | ||
Comment 16•6 years ago
|
||
I'm trying to respond comment 13, where you said (In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #13) > Ideally all browsers would implement the same behavior here, obviously. Firefox don't do the same as other browsers like Chrome if there were spaces in Content-Disposition header.
Reporter | ||
Comment 17•6 years ago
|
||
In addition to comment 16, should the dev team make Firefox behave like other browsers?
Comment 18•6 years ago
|
||
Probably yes, if the other browsers actually agree on all the edge cases (last I checked they didn't) and if there is a clear description of how they behave, instead of us implementing something that doesn't match them anyway. That's what I've said repeatedly in this issue.
Updated•6 years ago
|
Assignee | ||
Comment 21•5 years ago
|
||
This is networking code, not frontend code, so moving and needinfo'ing so this gets triaged despite being 2 years old.
Comment 22•5 years ago
|
||
(In reply to :Gijs (he/him) from comment #21)
This is networking code, not frontend code, so moving and needinfo'ing so this gets triaged despite being 2 years old.
Set this for P2.
Comment 25•5 years ago
|
||
https://bugs.chromium.org/p/chromium/issues/detail?id=1006345 has some discussion with Googlers on perhaps agreeing on a processing model. Nothing concrete thus far though.
Updated•4 years ago
|
Assignee | ||
Comment 28•4 years ago
|
||
Updated•4 years ago
|
Assignee | ||
Comment 29•4 years ago
|
||
So... I really don't know how to spec this. But as far as I can tell, this passes the mime header unit test so fallout should be OK, right? 😅
Less briefly, re comment #18, I looked at https://source.chromium.org/chromium/chromium/src/+/master:net/http/http_util.cc;l=1051;drc=aed22775d3d02301b9046b23a55dd84fef2b0caa and the associated code. As far as I can tell, chromium implements the header parsing by first stripping the inline/attachment value (which is the same thing we do) and then tokenizing on ;
in a way that ignores the ones that are in quotes (and ignores escaped quotes when looking at quotes). It then splits the resulting tokens by =
and then unquotes as necessary.
As a result, I think as long as we still split unquoted tokens by ;
, this should be a more-compatible change.
I don't think I have the expertise or the cycles to develop this into a more-in-tune-with-reality version of the venerable RFCs here. But this continues to be one of the most-reported bugs in Firefox's file handling code, and because I believe the code post-patch is more cross-browser-compatible than pre-patch, and because chromium have already indicated they are unlikely to change behaviour to move closer to the original RFC, I figured it was worth putting up as-is.
Comment 30•4 years ago
|
||
I filed https://github.com/httpwg/http-extensions/issues/1252 for the specification side of things. (It might have to move to Fetch or HTML if they are not willing.)
Updated•4 years ago
|
Assignee | ||
Comment 31•4 years ago
|
||
Comment 32•4 years ago
|
||
Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/autoland/rev/d1f68e042451 accept spaces and tabs in Content-Disposition header filename parameters because all the other browsers do, r=annevk,valentin,necko-reviewers
Updated•4 years ago
|
Comment 33•4 years ago
|
||
bugherder |
Comment 34•4 years ago
|
||
Description
•