-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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: No error with http2 client on empty reply body and content-length != 0 #46071
Comments
cc @bradfitz |
cc @fraenkel |
https://datatracker.ietf.org/doc/html/rfc7540#section-8.1.2.6
|
Looks like a bug. And I'm surprised, as I thought it handled this. @fraenkel, that's about HEAD responses primarily, no? Doesn't apply to this case where it's just a GET? |
Unfortunately it's under malformed requests and responses, not tied to a method. |
@fraenkel The quote is missing the reference to RFC7230:
In RFC 7230 section 3.3 the last paragraph seems to describe all cases when a response has a message body, see https://datatracker.ietf.org/doc/html/rfc7230#section-3.3 . And a |
Thanks for catching that. It's would seem it's not just method bit also status code. |
Yeah, when I said "HEAD primarily" I was being lazy and referring to: // bodyAllowedForStatus reports whether a given response status code
// permits a body. See RFC 7230, section 3.3.
func bodyAllowedForStatus(status int) bool {
switch {
case status >= 100 && status <= 199:
return false
case status == 204:
return false
case status == 304:
return false
}
return true
} Which we use in various places in both net/http and http2 already, e.g.: if clen == "" && rws.handlerDone && bodyAllowedForStatus(rws.status) && (len(p) > 0 || !isHeadResp) {
clen = strconv.Itoa(len(p))
} if w.req.Method != "HEAD" && w.contentLength != -1 && w.bodyAllowed() && w.contentLength != w.written {
// Did not write enough. Avoid getting out of sync.
return false
} I see net/http has checks for writing too much:
But I don't see anything in |
Change https://golang.org/cl/354141 mentions this issue: |
…content length Fixes golang/go#46071
…content length Fixes golang/go#46071
…length Fixes golang/go#46071 Change-Id: I8b8262ba8e28a129f6aacfca43b7b8d9e605a0ce GitHub-Last-Rev: 11b92cc8ba6e1d08716aac816d33b659563a893f GitHub-Pull-Request: golang/net#114 Reviewed-on: https://go-review.googlesource.com/c/net/+/354141 Reviewed-by: Damien Neil <[email protected]> Trust: Damien Neil <[email protected]> Trust: Alexander Rakoczy <[email protected]> Run-TryBot: Alexander Rakoczy <[email protected]> TryBot-Result: Go Bot <[email protected]>
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
)?go env
OutputWhat did you do?
Issue a GET request to a HTTP/2 server which then replies with the Content-Length header but afterwards fails to send data to the client for some reason. That is the reply has an empty body. See the Go playground link for a minimal example.
https://play.golang.org/p/jbhV10xhWKI
What did you expect to see?
Now, the client should receive an "Unexpected EOF" error when reading the reply:
What did you see instead?
Note that
res.ContentLength
is set to zero despite the Content-Length header in the reply. This lets clients erroneously assume that they successfully received a reply to their request.Executing the example on the Go playground currently deadlocks.
Go playground error
Disabling the use of HTTP2 (comment out the
ts.EnableHTTP2 = true
line) or sending at least one byte (seew.Write([]byte("a"))
) leads to the expected error.The text was updated successfully, but these errors were encountered: