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

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
Milestone

Comments

@odeke-em
Copy link
Member

@odeke-em odeke-em commented Aug 23, 2019

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

@pascaldekloe pascaldekloe commented Aug 24, 2019

@bcmills bcmills added the NeedsDecision label Aug 26, 2019
@bcmills
Copy link
Member

@bcmills 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
Member

@bcmills 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 label Aug 26, 2019
@pascaldekloe
Copy link
Contributor

@pascaldekloe pascaldekloe commented Aug 26, 2019

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

@gopherbot gopherbot commented Sep 21, 2019

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

@mengzhuo
Copy link
Contributor

@mengzhuo mengzhuo commented Sep 21, 2019

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

@gopherbot
Copy link

@gopherbot gopherbot commented Sep 26, 2019

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

@odeke-em odeke-em commented Sep 26, 2019

Gopherbot this hasn't timed out.

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

@odeke-em odeke-em commented Sep 26, 2019

Gopherbot, chill!

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

@pascaldekloe pascaldekloe commented Sep 26, 2019

The problem is that label @bcmills.

@bcmills bcmills removed the WaitingForInfo label Sep 26, 2019
@rsc rsc removed this from the Go1.14 milestone Oct 9, 2019
@rsc rsc added this to the Backlog milestone Oct 9, 2019
@gopherbot
Copy link

@gopherbot gopherbot commented Jun 15, 2020

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
Labels
NeedsDecision
Projects
None yet
Development

No branches or pull requests

6 participants