Skip to content

Commit 6972930

Browse files
committed
http2: in Server, disarm connection's ReadTimeout after first request
Fix a regression from Go 1.5 to Go 1.6: when we introduced automatic HTTP/2 support in Go 1.6, we never handled Server.ReadTimeout. If a user set ReadTimeout, the net/http package would set the read deadline on the connection during the TLS handshake, but then the http2 package would never disarm it, killing the likely-still-in-use connection after the timeout period. This CL changes it to disarm the timeout after the first request headers, similar to net/http.Server. Unlike net/http.Server, we don't re-arm it on each idle period, because the definition of idle is more complicated with HTTP/2. No tests here for now. Tests will be in the go repo once all the knobs are in place and this is re-bundled into std, testing both http1 and http2. Updates golang/go#16450 (minimal fix for now) Updates golang/go#14204 (considering a new http1+http2 IdleTimeout knob) Change-Id: Iaa1570c118efda7dc0a65ba84cd77885699ec8fc Reviewed-on: https://go-review.googlesource.com/30077 Reviewed-by: Ian Lance Taylor <[email protected]> Run-TryBot: Brad Fitzpatrick <[email protected]> TryBot-Result: Gobot Gobot <[email protected]>
1 parent a333c53 commit 6972930

File tree

1 file changed

+13
-0
lines changed

1 file changed

+13
-0
lines changed

http2/server.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1475,6 +1475,19 @@ func (sc *serverConn) processHeaders(f *MetaHeadersFrame) error {
14751475
handler = new400Handler(err)
14761476
}
14771477

1478+
// The net/http package sets the read deadline from the
1479+
// http.Server.ReadTimeout during the TLS handshake, but then
1480+
// passes the connection off to us with the deadline already
1481+
// set. Disarm it here after the request headers are read, similar
1482+
// to how the http1 server works.
1483+
// Unlike http1, though, we never re-arm it yet, though.
1484+
// TODO(bradfitz): figure out golang.org/issue/14204
1485+
// (IdleTimeout) and how this relates. Maybe the default
1486+
// IdleTimeout is ReadTimeout.
1487+
if sc.hs.ReadTimeout != 0 {
1488+
sc.conn.SetReadDeadline(time.Time{})
1489+
}
1490+
14781491
go sc.runHandler(rw, req, handler)
14791492
return nil
14801493
}

0 commit comments

Comments
 (0)