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 GET-with-body in Transport/Server #13722

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

Comments

Projects
None yet
6 participants
@bradfitz
Member

bradfitz commented Dec 23, 2015

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

This comment has been minimized.

Member

odeke-em commented Dec 23, 2015

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

This comment has been minimized.

Member

bradfitz commented Dec 23, 2015

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

@kostya-sh

This comment has been minimized.

Contributor

kostya-sh commented Dec 24, 2015

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

This comment has been minimized.

bits01 commented Dec 25, 2015

@bradfitz

This comment has been minimized.

Member

bradfitz commented Jul 29, 2016

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.

@bradfitz bradfitz added NeedsFix and removed NeedsDecision labels Oct 22, 2016

@bradfitz bradfitz changed the title from net/http: decide on GET-with-body policy to 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

This comment has been minimized.

Member

bradfitz commented Oct 22, 2016

Low priority. Punting to Go 1.9.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment