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

net/http: support multipart/mixed in Request.MultipartReader() #23959

Closed
OneOfOne opened this issue Feb 20, 2018 · 4 comments
Closed

net/http: support multipart/mixed in Request.MultipartReader() #23959

OneOfOne opened this issue Feb 20, 2018 · 4 comments
Labels
FeatureRequest FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@OneOfOne
Copy link
Contributor

What did you do?

Ran https://play.golang.org/p/uW3bq_3Z6Ym then in the terminal:

➤ curl -v -i -X POST -H "Content-Type: multipart/mixed" -F "blob=@/tmp/blah.go" -F "metadata={\"something\":123456789};type=application/json" http://localhost:1222/bad -v

What did you expect to see?

No errors from req.MultipartReader()

What did you see instead?

request Content-Type isn't multipart/form-data

If this feature request is accepted, I'd gladly submit a PR to fix it, testing locally the change doesn't have any side effect on current code that expects multipart/form-data.

@OneOfOne OneOfOne changed the title net/http: support multipart/mixed in Request. net/http: support multipart/mixed in Request.MultipartReader() Feb 20, 2018
@bradfitz
Copy link
Contributor

Why?

The motivation for https://golang.org/pkg/net/http/#Request.MultipartReader was so you could get file uploads from browsers easily.

What uses multipart/mixed?

And what's wrong with your "good" handler? That seems fine to me. That is, our underlying https://golang.org/pkg/mime/multipart/#Reader already supports what you want. You just don't like the convenience wrapper, but it's not clear why it's convenient.

It's also not clear that this is safe to do. Isn't multipart/mixed used when people are sending "alternatives"? I'd be a little concerned that code that previously was unprepared for such forms would now blindly accept it, without considering which alternative they want.

I'd prefer people to "opt-in" to the expanded set of allowed inputs by calling the underlying Reader constructor themselves.

@bradfitz bradfitz added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Feb 20, 2018
@OneOfOne
Copy link
Contributor Author

OneOfOne commented Feb 20, 2018

@bradfitz my current use case is uploading a file + json metadata in one request rather than:

  1. POST req 1 (json), get an id, post req 2 (file) and use the id from req 1.
  2. Convert the file to base64 (33% size increase) and include it in the json request.

multipart/mixed is pretty much the same as multipart/form-data except each "part" can have different headers / content-type and it's generally used in SMTP, however few APIs use it here and there.

https://golang.org/pkg/net/http/#Request.MultipartReader already supports multipart/form-data, the underlying https://golang.org/pkg/mime/multipart/#Reader supports multipart/* in general.

edit you were thinking about multipart/alterntive not multipart/mixed.

@bradfitz
Copy link
Contributor

Sure, we can accept multipart/mixed too.

@bradfitz bradfitz added help wanted NeedsFix The path to resolution is known, but the work has not been done. FeatureRequest and removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Feb 21, 2018
@bradfitz bradfitz added this to the Go1.11 milestone Feb 21, 2018
@gopherbot
Copy link

Change https://golang.org/cl/95975 mentions this issue: net/http: support multipart/mixed in Request.MultipartReader()

@golang golang locked and limited conversation to collaborators Feb 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FeatureRequest FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

3 participants