net/http: Server doesn't use ReadHeaderTimeout for TLS handshake deadline

[bradfitz]

The net/http.Server does:

// Serve a new connection.
func (c *conn) serve(ctx context.Context) {
...
        if tlsConn, ok := c.rwc.(*tls.Conn); ok {
                if d := c.server.ReadTimeout; d > 0 {
                        c.rwc.SetReadDeadline(time.Now().Add(d))
                }
                if d := c.server.WriteTimeout; d > 0 {
                        c.rwc.SetWriteDeadline(time.Now().Add(d))
                }
                if err := tlsConn.HandshakeContext(ctx); err != nil {

But if only Server.ReadHeaderTimeout is set (and not ReadTimeout), it seems like ReadHeaderTimeout should be used for the TLS handshake deadline instead of infinite.

/cc @neild

[komuw]

related? #35626

[cherrymui]

Yeah, it seems similar to #35626. @bradfitz @neild feel free to close if you think this is a dup. Thanks.

[bradfitz]

Not a dup, but similar. Different bugs involving the same tunables.

[nulltrope]

Just would like to add that this has affected us, as we don't know the nature of the request upfront we can't safely enforce an overall ReadTimeout and instead rely on the ReadHeaderTimeout- but as a consequence have no easy way to set a timeout on the TLS handshake.

To get around this we've used the ConnState func of http.Server to set a connection deadline when state = new, then unset it when state = active, but this seems rather brittle and I feel like would be better served by the higher-level http.Server timeouts.

[gopherbot]

Change https://golang.org/cl/355870 mentions this issue: net/http: also use Server.ReadHeaderTimeout for TLS handshake deadline