Skip to content

Commit

Permalink
net/http: introduce DialerAndTLSConfigSupportsHTTP2 in Transport
Browse files Browse the repository at this point in the history
Even when a custom TLS config or custom dialer is specified,
enables HTTP/2 if DialerAndTLSConfigSupportsHTTP2 is true.
By this change, avoid automatically enabling HTTP/2 if DialContext is set.

This change also ensures that DefaultTransport still automatically
enable HTTP/2 as discussed in #14391.

Updates #14391
Fixes #27011

Change-Id: Icc46416810bee61dbd65ebc96468335030b80573
Reviewed-on: https://go-review.googlesource.com/c/go/+/130256
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Kunpei Sakai <namusyaka@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
  • Loading branch information
namusyaka authored and bradfitz committed Apr 16, 2019
1 parent 1eed2a5 commit 94e7200
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 5 deletions.
18 changes: 13 additions & 5 deletions src/net/http/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,11 @@ var DefaultTransport RoundTripper = &Transport{
KeepAlive: 30 * time.Second,
DualStack: true,
}).DialContext,
MaxIdleConns: 100,
IdleConnTimeout: 90 * time.Second,
TLSHandshakeTimeout: 10 * time.Second,
ExpectContinueTimeout: 1 * time.Second,
DialerAndTLSConfigSupportsHTTP2: true,
MaxIdleConns: 100,
IdleConnTimeout: 90 * time.Second,
TLSHandshakeTimeout: 10 * time.Second,
ExpectContinueTimeout: 1 * time.Second,
}

// DefaultMaxIdleConnsPerHost is the default value of Transport's
Expand Down Expand Up @@ -257,6 +258,12 @@ type Transport struct {
// h2transport (via onceSetNextProtoDefaults)
nextProtoOnce sync.Once
h2transport h2Transport // non-nil if http2 wired up

// DialerAndTLSConfigSupportsHTTP2 controls whether HTTP/2 is enabled when a non-zero
// TLSClientConfig or Dial, DialTLS or DialContext func is provided. By default, use of any those fields conservatively
// disables HTTP/2. To use a customer dialer or TLS config and still attempt HTTP/2
// upgrades, set this to true.
DialerAndTLSConfigSupportsHTTP2 bool
}

// h2Transport is the interface we expect to be able to call from
Expand Down Expand Up @@ -296,12 +303,13 @@ func (t *Transport) onceSetNextProtoDefaults() {
// Transport.
return
}
if t.TLSClientConfig != nil || t.Dial != nil || t.DialTLS != nil {
if !t.DialerAndTLSConfigSupportsHTTP2 && (t.TLSClientConfig != nil || t.Dial != nil || t.DialTLS != nil || t.DialContext != nil) {
// Be conservative and don't automatically enable
// http2 if they've specified a custom TLS config or
// custom dialers. Let them opt-in themselves via
// http2.ConfigureTransport so we don't surprise them
// by modifying their tls.Config. Issue 14275.
// However, if DialerAndTLSConfigSupportsHTTP2 is true, it overrides the above checks.
return
}
t2, err := http2configureTransport(t)
Expand Down
14 changes: 14 additions & 0 deletions src/net/http/transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3593,6 +3593,13 @@ func TestTransportAutomaticHTTP2(t *testing.T) {
testTransportAutoHTTP(t, &Transport{}, true)
}

func TestTransportAutomaticHTTP2_DialerAndTLSConfigSupportsHTTP2AndTLSConfig(t *testing.T) {
testTransportAutoHTTP(t, &Transport{
DialerAndTLSConfigSupportsHTTP2: true,
TLSClientConfig: new(tls.Config),
}, true)
}

// golang.org/issue/14391: also check DefaultTransport
func TestTransportAutomaticHTTP2_DefaultTransport(t *testing.T) {
testTransportAutoHTTP(t, DefaultTransport.(*Transport), true)
Expand Down Expand Up @@ -3623,6 +3630,13 @@ func TestTransportAutomaticHTTP2_Dial(t *testing.T) {
}, false)
}

func TestTransportAutomaticHTTP2_DialContext(t *testing.T) {
var d net.Dialer
testTransportAutoHTTP(t, &Transport{
DialContext: d.DialContext,
}, false)
}

func TestTransportAutomaticHTTP2_DialTLS(t *testing.T) {
testTransportAutoHTTP(t, &Transport{
DialTLS: func(network, addr string) (net.Conn, error) {
Expand Down

0 comments on commit 94e7200

Please sign in to comment.