Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

mime/multipart: encode non-US-ASCII characters in Content-Disposition #33801

Open
odeke-em opened this issue Aug 23, 2019 · 11 comments
Open

mime/multipart: encode non-US-ASCII characters in Content-Disposition #33801

odeke-em opened this issue Aug 23, 2019 · 11 comments
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone
Backlog

Comments

@odeke-em
Copy link
Member

I am coming here from CL https://go-review.googlesource.com/c/go/+/190217 where @pascaldekloe has submitted a change that makes *mime/multipart.Writer generate a "Content-Disposition" value after invoking mime.FormatMediaType instead of using a quote escaper.

In a comment https://go-review.googlesource.com/c/go/+/190217/1#message-b06f1016c7862f1d35ef7a12dc95fc4426a48178
@mattn asks

I don't have strong opinion. Just for the note. This make forcibly the filename encode
with rfc 6266. So older browser will not work after this change. Do we need to leave
a way to output raw-filenames?

This issue is to have a discussion, but I'll also loop in some security folks @mikesamuel @empijei, to help evaluate or just beware of the update.
@odeke-em odeke-em added this to the Go1.14 milestone Aug 23, 2019
@pascaldekloe
Copy link
Contributor

http://test.greenbytes.de/tech/tc2231/

@bcmills bcmills added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Aug 26, 2019
@bcmills
Copy link
Contributor

bcmills commented Aug 26, 2019

CC @bradfitz @minux (although both of those owners seem to be away from GitHub at the moment)

@bcmills
Copy link
Contributor

bcmills commented Aug 26, 2019

@pascaldekloe, what is the rationale for / impact of the proposed change?

Is this a feature request to support non-ASCII value for those names, or a bugfix for names that RFC requires the library is required to support?

Either way, why is there no corresponding documentation change in the CL? (Presumably the change widens the set of acceptable names, so the doc comment should probably mention which names are acceptable either way.)

@bcmills bcmills added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Aug 26, 2019
@pascaldekloe
Copy link
Contributor

Not sure what to say here. I was surprised by the sloppy escape and fixed it. No more HTTP violation with UTF-8 or control characters in the header.

@gopherbot
Copy link

Change https://golang.org/cl/189858 mentions this issue: mime/multipart: fix CreateFormFile when filename is UTF8

@mengzhuo
Copy link
Contributor

Hmm... It seems to be a duplicate CL from me

@gopherbot
Copy link

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)

@odeke-em
Copy link
Member Author

Gopherbot this hasn't timed out.

@odeke-em odeke-em reopened this Sep 26, 2019
@odeke-em
Copy link
Member Author

Gopherbot, chill!

@odeke-em odeke-em reopened this Sep 26, 2019
@pascaldekloe
Copy link
Contributor

The problem is that label @bcmills.

@bcmills bcmills removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Sep 26, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@gopherbot
Copy link

Change https://golang.org/cl/190217 mentions this issue: multipart: encode non-US-ASCII characters in Content-Disposition

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
6 participants
@rsc @mengzhuo @pascaldekloe @odeke-em @bcmills @gopherbot