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: bad handling of HEAD requests with a body #53960

Open
neild opened this issue Jul 19, 2022 · 4 comments
Open

net/http: bad handling of HEAD requests with a body #53960

neild opened this issue Jul 19, 2022 · 4 comments
Assignees
Labels
NeedsFix

Comments

@neild
Copy link
Contributor

neild commented Jul 19, 2022

HEAD requests may have a body, although RFC 7231 states that "some existing implementations" may reject a HEAD request which contains one.

The net/http package handles HEAD requests with a body in different ways:

  • HTTP/1, non-chunked: return a 400 error, close the connection.
  • HTTP/1, Content-Encoding: chunked: ignore the chunked body (trying to parse it as the next request on the connection). Clearly buggy. Not a request smuggling mechanism, since the chunked body data can never be a valid HTTP request.
  • HTTP/2: close the stream with an error.

We should either support HEAD requests with a body in all circumstances, or fix the HTTP/1 chunked case and add a test for the HTTP/1 identity case. I think support, but I could be argued into always-reject on the grounds that nobody ever actually sends a body in a HEAD request.

Thanks to Bahruz Jabiyev, Tommaso Innocenti, Anthony Gavazzi, Steven Sprecher, and Kaan Onarlioglu for reporting this issue.

@neild neild self-assigned this Jul 19, 2022
@gopherbot
Copy link

gopherbot commented Jul 20, 2022

Change https://go.dev/cl/418614 mentions this issue: net/http: accept HEAD requests with a body

@gopherbot
Copy link

gopherbot commented Jul 20, 2022

Change https://go.dev/cl/418634 mentions this issue: http2: accept HEAD requests with a body

@nightlyone
Copy link
Contributor

nightlyone commented Jul 20, 2022

@neild What can be gained in supporting such requests?

I would suggest to continue dropping the body and fix case 2 (trying to parse it as a next request) .

Supporting undefined behaviour seems like a road to incompatibilities.

Are you aware of a use case that relies on that is undefined behaviour or some IETF draft requiring this to work, so that the undefined behaviour becomes standardized soon?

@neild
Copy link
Contributor Author

neild commented Jul 20, 2022

There's no undefined behavior here.

The semantics of HEAD requests with a body are undefined--there's no RFC that I know of that defines what that body means--but the mechanics of sending such a request are well-defined. HEAD responses are defined as having no body, but HEAD requests may have one.

The argument I see against supporting HEAD requests with a body is that nobody sends these in practice, so receiving one probably indicates a mistake or malicious behavior of some kind.

The argument for supporting them is that they're valid requests, and that it's more work to reject them than it is to just handle them.

The actual state of support for HEAD-with-a-body in net/http is a bit of a mess as described above, so we need to do something. (The one case I forgot to mention is client behavior: net/http handles sending HEAD-with-a-body fine in all cases I've tested, including HTTP/1, HTTP/2, identity encoding, and chunked encoding.)

@seankhliao seankhliao added the NeedsFix label Aug 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix
Projects
None yet
Development

No branches or pull requests

4 participants