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

Comments

Projects
None yet
3 participants
@OneOfOne
Copy link
Contributor

commented Feb 20, 2018

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

This comment has been minimized.

Copy link
Member

commented Feb 20, 2018

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.

@OneOfOne

This comment has been minimized.

Copy link
Contributor Author

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

This comment has been minimized.

Copy link
Member

commented Feb 21, 2018

Sure, we can accept multipart/mixed too.

@gopherbot

This comment has been minimized.

Copy link

commented Feb 21, 2018

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

@gopherbot gopherbot closed this in e02d6bb Feb 21, 2018

@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.
You can’t perform that action at this time.