Skip to content

Commit

Permalink
http2: in Server, disarm connection's ReadTimeout after first request
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
bradfitz committed Sep 30, 2016
1 parent a333c53 commit 6972930
Showing 1 changed file with 13 additions and 0 deletions.
13 changes: 13 additions & 0 deletions http2/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -1475,6 +1475,19 @@ func (sc *serverConn) processHeaders(f *MetaHeadersFrame) error {
handler = new400Handler(err)
}

// 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.
// Unlike http1, though, we never re-arm it yet, though.
// TODO(bradfitz): figure out golang.org/issue/14204
// (IdleTimeout) and how this relates. Maybe the default
// IdleTimeout is ReadTimeout.
if sc.hs.ReadTimeout != 0 {
sc.conn.SetReadDeadline(time.Time{})
}

go sc.runHandler(rw, req, handler)
return nil
}
Expand Down

0 comments on commit 6972930

Please sign in to comment.