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

net/http: support GET-with-body in Transport/Server #13722

Open
bradfitz opened this issue Dec 23, 2015 · 6 comments
Open

net/http: support GET-with-body in Transport/Server #13722

bradfitz opened this issue Dec 23, 2015 · 6 comments
Labels
FeatureRequest NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unplanned

Comments

@bradfitz
Copy link
Contributor

RFC 2616 is super vague about whether a GET request with a body is valid, or what to do about it.

We should decide, document, enforce, and test.

I vote reject them.

RFC 2616 says:

The rules for when a message-body is allowed in a message differ for
requests and responses.

The presence of a message-body in a request is signaled by the
inclusion of a Content-Length or Transfer-Encoding header field in
the request's message-headers. A message-body MUST NOT be included in
a request if the specification of the request method (section 5.1.1)
does not allow sending an entity-body in requests. A server SHOULD
read and forward a message-body on any request; if the request method
does not include defined semantics for an entity-body, then the
message-body SHOULD be ignored when handling the request.

But the nothing (even in 5.1.1) about which methods have defined semantics for a body. At least GET and HEAD are not explicitly defined, so maybe that means reject them.

I recall somebody (from CloudFlare?) telling me they'd seen GET+body in the wild, though?

I still vote to reject them.
@bradfitz bradfitz self-assigned this Dec 23, 2015
@bradfitz bradfitz added this to the Go1.7 milestone Dec 23, 2015
@odeke-em
Copy link
Member

I think I've seen usages of GET+body with ElasticSearch before. In fact just opening the ElasticSearch docs https://www.elastic.co/guide/en/elasticsearch/reference/current/search-request-body.html shows
screen shot 2015-12-23 at 11 42 36 am

@bradfitz
Copy link
Contributor Author

Thanks for that reference. That's interesting. Then perhaps we should make sure it's permitted.

@kostya-sh
Copy link
Contributor

I don't know what is the current behaviour but if body is permitted for GET then it should be permitted for DELETE as well: https://www.elastic.co/guide/en/elasticsearch/plugins/2.0/delete-by-query-usage.html

@bits01
Copy link

bits01 commented Dec 25, 2015

Another point of reference:
https://groups.yahoo.com/neo/groups/rest-discuss/conversations/messages/9962

@bradfitz bradfitz modified the milestones: Go1.8, Go1.7 May 16, 2016
@bradfitz bradfitz mentioned this issue Jun 29, 2016
Closed
@bradfitz
Copy link
Contributor Author

Recent discussion on ietf-http-wg mailing list: https://lists.w3.org/Archives/Public/ietf-http-wg/2016JulSep/0243.html

I'm thinking we should probably make the Server reject GETs with bodies by default unless the server (or the handler) explicitly wants them, either via configuration on the server (some new bool knob?) or implicitly if the handler reads from the body, then we permit a response (similar to how we do automatic 100-continue responses on the first read of the request body).

On the client side, we can probably make it work on the transport by default if the user includes a body. But it would disable auto-retry, as if it were a POST.

@quentinmit quentinmit added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Oct 7, 2016
@bradfitz bradfitz added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Oct 22, 2016
@bradfitz bradfitz changed the title net/http: decide on GET-with-body policy net/http: support GET-with-body in Transport/Server Oct 22, 2016
@bradfitz bradfitz modified the milestones: Go1.9, Go1.8 Oct 22, 2016
@bradfitz
Copy link
Contributor Author

Low priority. Punting to Go 1.9.

@oschwald oschwald mentioned this issue Nov 11, 2016
Closed
@bradfitz bradfitz mentioned this issue Dec 8, 2016
Closed
@bradfitz bradfitz modified the milestones: Go1.10, Go1.9 May 24, 2017
@rsc rsc modified the milestones: Go1.10, Go1.11 Nov 22, 2017
@bradfitz bradfitz modified the milestones: Go1.11, Unplanned May 29, 2018
@odeke-em odeke-em mentioned this issue Oct 17, 2019
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
FeatureRequest NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
6 participants
@bradfitz @rsc @quentinmit @odeke-em @kostya-sh @bits01