-
Notifications
You must be signed in to change notification settings - Fork 18k
net/http: include Content-Length header for 400 responses #61348
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
Comments
Caddy also has this bug, but I'm assuming that's because they use net/http. |
Is a RFC 9110 says:
(Note that that is “SHOULD”, not “MUST”.) And RFC 9112 says:
As far as I can tell, per RFC 9112 this is a valid HTTP/1.1 response. (CC @neild) |
I concur. In addition, RFC 9110 says:
This response is small enough that measuring progress is unnecessary (the response should fit in a single packet), and the connection is closed immediately after the response so connection reuse is impossible. It would not be incorrect for us to send a Content-Length here, but I believe this is a valid HTTP/1.1 response. |
Thank you for the quick responses. I agree that the spec only suggests the inclusion of a TE or CL header; I was mistaken. That said, net/http is the only HTTP library of which I am aware that does not send a TE or CL header in its responses. Among those that do send TE or CL headers in 400 responses are
Given that the spec says that you EDIT: Typo |
What's the benefit in sending Content-Length here? If there are existing broken client implementations in common use that can't cope with this response, then that might be an argument for change, but I'm dubious that this particular form of buggy client is common. |
The benefit of sending CL is for clarity and ease of parsing. This is the top answer to a StackOverflow question asking what happens when an HTTP response has no CL header:
Believing this answer would lead to exactly the sort of bug you describe. That said, I think I'm convinced that the best course of action is not to add a CL header, but to remove the message bodies from the default 400 responses. They're redundant because of the reason string, and removing them should improve performance marginally, and improve compatibility with other servers. |
The Stack Overflow response is wrong. In the absence of evidence that the current behavior is causing real world problems, I believe that not only is there no good reason to change it, but there is a small but real value in preserving it as a minor discouragement to the development of new, buggy HTTP/1.1 client implementations that cannot handle this valid response. |
I am aware that the SO response is wrong; I sent it to demonstrate that my misunderstanding of the standard is a widespread one. If you'd like net/http to be the lone upholder of a little-known part of the HTTP standard, then that is your choice, and I will close the issue. |
Here's another portion of the standard that encourages being explicit about lengths, from RFC 9112 Section 6.3:
Thus, a client will have no way of determining with reasonable certainty that it received the entirety of |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes.
What operating system and processor architecture are you using (
go env
)?What did you do?
What did you expect to see?
A valid 400 response informing me that my request needs a
Host
header.What did you see instead?
The following 400 response that is invalid because it is missing its
Content-Length
header:The text was updated successfully, but these errors were encountered: