go
go copied to clipboard
golang
x/net/http2: Server.ReadTimeout does not fire for HTTP/2 requests
What version of Go are you using (go version)?
$ go version 1.17.3
Does this issue reproduce with the latest release?
yes
What did you do?
package main
import (
"io"
"log"
"net/http"
"net/http/httptest"
"strings"
"time"
)
func handler(w http.ResponseWriter, r *http.Request) {
log.Println("handler started")
start := time.Now()
buf, err := io.ReadAll(r.Body)
log.Printf("read %d in %v, err: %v", len(buf), time.Since(start), err)
}
type slowReader struct {
delay time.Duration
io.Reader
}
func (sr *slowReader) Read(buf []byte) (int, error) {
time.Sleep(sr.delay)
return sr.Reader.Read(buf)
}
func main() {
sv := httptest.NewUnstartedServer(http.HandlerFunc(handler))
sv.EnableHTTP2 = true
sv.Config.ReadTimeout = 1 * time.Second
sv.StartTLS()
resp, err := sv.Client().Post(sv.URL+"/", "text/plain", &slowReader{
delay: 5 * time.Second,
Reader: strings.NewReader("hello, HTTP/2"),
})
if err != nil {
log.Fatal(err)
}
defer resp.Body.Close()
}
What did you expect to see?
After 1 second, some error from io.ReadAll in handler indicating the request had timed out during reading.
What did you see instead?
(~/devel/http2bug) % go run main.go
2021/11/29 14:57:45 handler started
2021/11/29 14:57:55 read 13 in 10.00982654s, err: <nil>
The call to io.ReadAll completed in 10 seconds with no error.
This seems to be the expected behavior, I can see that for ReadTimeout is the same as IdleTimeout for http2,
https://github.com/golang/net/blob/d455829e376dcf0ca6aca82a86d093afbcc6a8ee/http2/server.go#L229-L234
and doesn't take the body into consideration
// ReadTimeout is the maximum duration for reading the entire
// request, including the body. A zero or negative value means
// there will be no timeout.
//
// Because ReadTimeout does not let Handlers make per-request
// decisions on each request body's acceptable deadline or
// upload rate, most users will prefer to use
// ReadHeaderTimeout. It is valid to use them both.
ReadTimeout time.Duration
there is also a test that checks that the handler is not affected by the timeout https://github.com/golang/net/blob/d455829e376dcf0ca6aca82a86d093afbcc6a8ee/http2/server_test.go#L3891-L3917
It seems this comment from @bradfitz explains the decision https://github.com/golang/go/issues/14204#issuecomment-245039239
... anyway, if my understanding is wrong I'm happy to help if I have some guidance :sweat_smile:
Thanks for finding the link to the previous discussion. IMO if this is the intended behaviour then the comment on ReadTimeout probably needs to be updated.
// ReadTimeout is the maximum duration for reading the entire
// request, including the body. A zero or negative value means
// there will be no timeout.
This isn't correct in the HTTP/2 context. At best, if the user has set WriteTimeout then it will fire at some point and tear down the stream, but ReadTimeout is ineffective in HTTP/2 contexts.
Sorry for missing this when it was originally filed.
So far as I can tell, Server.ReadTimeout is simply not implemented for HTTP/2 at the moment. It should be. Working on a fix now.
Change https://go.dev/cl/446255 mentions this issue: net/http: add tests for Server.ReadTimeout and server.WriteTimeout
Change https://go.dev/cl/446256 mentions this issue: http2: support Server.ReadTimeout
Change https://go.dev/cl/449935 mentions this issue: all: update vendored golang.org/x/net to v0.2.0