content-disposition icon indicating copy to clipboard operation
content-disposition copied to clipboard

Support decode 'utf8' (no dash)

Open alexstrat opened this issue 8 years ago • 10 comments

I stumbled upon a field using utf8 (no utf-8) as encoding.

alexstrat avatar Mar 23 '17 14:03 alexstrat

Thanks, this is great! The RFC notes that any encoding can be specified in the header and only makes the two we support a MUST. But, ideally the RFC specifies that it SHOULD only be registered charsets, which utf8 is not a registered charset (registrations: https://www.iana.org/assignments/character-sets/character-sets.xhtml). That doesn't mean we can't add this, but before doing so would like to scrutinize it to make sure we are making the right decision here.

Can you provide any details for where you ran into this use?

dougwilson avatar Mar 23 '17 18:03 dougwilson

I ran into this when downloading files from files.slack.com (a file uploaded to Slack):

curl -I --cookie "xxxxxxxxxx" https://files.slack.com/files-pri/T029MPGH6-F4NPHFJDS/download/pasted_image_at_2017_03_23_16_59.png
HTTP/1.1 200 OK
Content-Type: image/png
Content-Length: 28292
Connection: keep-alive
Accept-Ranges: bytes
Cache-Control: max-age=315360000, public
Content-Disposition: attachment; filename="Pasted image at 2017_03_23 16_59.png"; filename*=utf8''Pasted%20image%20at%202017_03_23%2016_59.png
Date: Thu, 23 Mar 2017 19:11:03 GMT
Etag: "485e0240d05d82f4b1055dbba3b88f01"
X-Backend: imgproxy-0e7899cd2fc239c39
X-Robots-Tag: noindex
X-Cache: Miss from cloudfront
Via: 1.1 3943e81340bd903a74d536bc9599c3f3.cloudfront.net (CloudFront)
X-Amz-Cf-Id: AKTTZG6-RJ1RUhnNfIMJCn7YE2Swo-NzpnGvpOiSg03lI1bL7k83eQ==

I was surprised as well.

alexstrat avatar Mar 23 '17 19:03 alexstrat

Awesome information, @alexstrat ! I'll get a new version published tonight (US) 👍

dougwilson avatar Mar 23 '17 19:03 dougwilson

I've also been looking at other implementations and here is a running list of the results so far:

  • cURL: ignores filename* parameter
  • Firefox: in progress from what I can tell only supports UTF-8 (source code: https://hg.mozilla.org/mozilla-central/file/tip/netwerk/mime/nsMIMEHeaderParamImpl.cpp)
  • Chrome: seems to support any charset internal generic decoders in chrome support (source code: https://cs.chromium.org/chromium/src/net/http/http_content_disposition.cc?l=330; docs state "The name should be limited to the ASCII-7 alphanumerics range. The actual name will be resolved with the alias file using a case-insensitive string comparison that ignores leading zeroes and all non-alphanumeric characters. E.g., the names "UTF8", "utf-8", "u*T@f08" and "Utf 8" are all equivalent.")

dougwilson avatar Mar 23 '17 19:03 dougwilson

Another POV: should parse fail when content-disposition header contains a parameter which field is not decodable? Shouldn't it only ignore the parameter and move on? (in my case i was only interested in the type)

alexstrat avatar Mar 23 '17 20:03 alexstrat

Another POV: should parse fail when content-disposition header contains a parameter which field is not decodable? Shouldn't it only ignore the parameter and move on?

I think this is a great discussion and it would be a shame if it were to get either (a) lost as soon as this PR is merged & closed or (b) caused a derailment delaying the merge of the PR.

I would suggest opening a new issue raising this point. Some ideas on starting talking questions: (a) what specifically would be considered a recoverable parse failure vs an unrecoverable failure (b) when should be returned when the various different recoverable failures occur (c) how can we keep in tact a strict mode for any users who would want a complete failure (d) what, if any, guidance does the RFC have on this.

dougwilson avatar Mar 23 '17 20:03 dougwilson

Definitely! 👍 Opened https://github.com/jshttp/content-disposition/issues/14

alexstrat avatar Mar 24 '17 08:03 alexstrat

Sorry, forgot this PR; your post on the other issue reminded me I never merged it 😢 I'll get an update to match Chrome's behavior, which would work here as well.

dougwilson avatar Mar 28 '17 13:03 dougwilson

Hello, any update on this? I just came back to check the status of my issue (#20), and found here already a fix.

I actually wrote a patch that adds an option to ignore parsing errors on extended parameters, but after reading your comments in #14 I'm pretty sure you don't like the idea.

s25g5d4 avatar Jul 21 '18 12:07 s25g5d4

FYI: I reported the incorrect filename*=utf8 parameter problem to slack, and they fixed in 2 days. I'm surprised that it looks like no one reports the problem to slack before, and they fixed their server in such a short time.

s25g5d4 avatar Jul 25 '18 04:07 s25g5d4