src/net/http/clientserver_test.go | 15 +++++++++++++++ src/net/http/http2.go | 2 ++ src/net/http/internal/http2/server.go | 23 ----------------------- src/net/http/serve_test.go | 29 +++++++++++++++++++++++++++++ src/net/http/server.go | 2 ++ diff --git a/src/net/http/clientserver_test.go b/src/net/http/clientserver_test.go index 4216f87525b8e17fc6d59ced2ffbfe76b2564c41..ab47872fa832af8ae8c9d027388b888855f1021a 100644 --- a/src/net/http/clientserver_test.go +++ b/src/net/http/clientserver_test.go @@ -61,6 +61,11 @@ http2UnencryptedMode = testMode("h2unencrypted") // HTTP/2 http3Mode = testMode("h3") // HTTP/3 ) +type ( + testAddMode []testMode // default, plus these + testSkipMode []testMode // default, minus these +) + // http3SkippedMode is a convenient alias for []testMode{http1Mode, http2Mode}, // which was the default test mode used by run and runSynctest prior to HTTP/3 // development. @@ -104,6 +109,16 @@ modes := []testMode{http1Mode, http2Mode, http3Mode} parallel := true for _, opt := range opts { switch opt := opt.(type) { + case testAddMode: + for _, m := range opt { + if !slices.Contains(modes, m) { + modes = append(modes, m) + } + } + case testSkipMode: + modes = slices.DeleteFunc(modes, func(m testMode) bool { + return slices.Contains(opt, m) + }) case []testMode: modes = opt case testNotParallelOpt: diff --git a/src/net/http/http2.go b/src/net/http/http2.go index 3c886a838199848ec86db98b6a714684f28c3bd8..21468182ab021776d70546746bc0c9f836cfaca9 100644 --- a/src/net/http/http2.go +++ b/src/net/http/http2.go @@ -97,6 +97,8 @@ var serverUpgradeReq *http2.ServerRequest if upgradeReq != nil { serverUpgradeReq = http2ServerRequestFromRequest(upgradeReq) } + nc.SetReadDeadline(time.Time{}) + nc.SetWriteDeadline(time.Time{}) s.h2.ServeConn(nc, &http2.ServeConnOpts{ Context: ctx, Handler: http2Handler{h}, diff --git a/src/net/http/internal/http2/server.go b/src/net/http/internal/http2/server.go index aed43b255a8fc9bca19a895d0f5e6bbfdf56a341..f4fbbc07a0be0dc34d7428071f7506aba37e03e3 100644 --- a/src/net/http/internal/http2/server.go +++ b/src/net/http/internal/http2/server.go @@ -291,15 +291,6 @@ s.registerConn(sc) defer s.unregisterConn(sc) - // The net/http package sets the write deadline from the - // http.Server.WriteTimeout during the TLS handshake, but then - // passes the connection off to us with the deadline already set. - // Write deadlines are set per stream in serverConn.newStream. - // Disarm the net.Conn write deadline here. - if sc.hs.WriteTimeout() > 0 { - sc.conn.SetWriteDeadline(time.Time{}) - } - switch { case sc.hs.DisableClientPriority(): sc.writeSched = newRoundRobinWriteScheduler() @@ -1976,15 +1967,7 @@ } else if err := checkValidHTTP2RequestHeaders(req.Header); err != nil { handler = serve400Handler{err}.ServeHTTP } - // The net/http package sets the read deadline from the - // http.Server.ReadTimeout during the TLS handshake, but then - // passes the connection off to us with the deadline already - // set. Disarm it here after the request headers are read, - // similar to how the http1 server works. Here it's - // technically more like the http1 Server's ReadHeaderTimeout - // (in Go 1.8), though. That's a more sane option anyway. if sc.hs.ReadTimeout() > 0 { - sc.conn.SetReadDeadline(time.Time{}) st.readDeadline = time.AfterFunc(sc.hs.ReadTimeout(), st.onReadTimeout) } @@ -2003,12 +1986,6 @@ } rw := sc.newResponseWriter(st) rw.rws.req = *req req = &rw.rws.req - - // Disable any read deadline set by the net/http package - // prior to the upgrade. - if sc.hs.ReadTimeout() > 0 { - sc.conn.SetReadDeadline(time.Time{}) - } // This is the first request on the connection, // so start the handler directly rather than going diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go index 19aa63f8cc5beb088f89e377a14ea2b8ae085891..92c90d6ec0d6ba09b6b23a0b270eed90a391baaa 100644 --- a/src/net/http/serve_test.go +++ b/src/net/http/serve_test.go @@ -878,6 +878,35 @@ }) } } +func TestServerReadHeaderTimeoutIsCleared(t *testing.T) { + runSynctest(t, testServerReadHeaderTimeoutIsCleared, + testAddMode{http2UnencryptedMode}) +} +func testServerReadHeaderTimeoutIsCleared(t *testing.T, mode testMode) { + const timeout = time.Second + cst := newClientServerTest(t, mode, HandlerFunc(func(w ResponseWriter, r *Request) { + w.WriteHeader(200) + NewResponseController(w).Flush() + time.Sleep(2 * timeout) + io.WriteString(w, "ok") + }), func(s *Server) { + s.ReadHeaderTimeout = timeout + }, optFakeNet) + + res, err := cst.c.Get(cst.ts.URL) + if err != nil { + t.Fatal(err) + } + got, err := io.ReadAll(res.Body) + res.Body.Close() + if err != nil { + t.Fatalf("reading response body after ReadHeaderTimeout: %v", err) + } + if want := "ok"; string(got) != want { + t.Fatalf("response body = %q, want %q", got, want) + } +} + func TestServerReadTimeout(t *testing.T) { run(t, testServerReadTimeout, http3SkippedMode) } func testServerReadTimeout(t *testing.T, mode testMode) { respBody := "response body" diff --git a/src/net/http/server.go b/src/net/http/server.go index cdf5c43c2ab0404e30ceae69c5ceb901c7f03668..aff62f9a8dc595e7baaa266d3cbded0946ff8506 100644 --- a/src/net/http/server.go +++ b/src/net/http/server.go @@ -2249,6 +2249,8 @@ if c.server.h2 != nil { const sawClientPreface = true c.server.serveHTTP2Conn(ctx, c.rwc, serverHandler{c.server}, sawClientPreface, nil, nil) } else { + c.rwc.SetReadDeadline(time.Time{}) + c.rwc.SetWriteDeadline(time.Time{}) h := unencryptedHTTP2Request{ctx, c.rwc, serverHandler{c.server}} nextFunc(c.server, unencryptedTLSConn(c.rwc), h) }