diff options
author | Brad Fitzpatrick <bradfitz@golang.org> | 2016-08-19 23:13:29 +0000 |
---|---|---|
committer | Chris Broadfoot <cbro@golang.org> | 2016-09-07 17:49:20 +0000 |
commit | dc3612e0d1a569e2ad1a3deea9ef6eab72616dc4 (patch) | |
tree | 3ece61331a2afce9df1a141219cca153c65fc4c3 | |
parent | 6f12826a869aa76e4abee4e1e1dfb27b19775782 (diff) | |
download | go-dc3612e0d1a569e2ad1a3deea9ef6eab72616dc4.tar.gz go-dc3612e0d1a569e2ad1a3deea9ef6eab72616dc4.zip |
[release-branch.go1.7] net/http: fix unwanted HTTP/2 conn Transport crash after IdleConnTimeout
Go 1.7 crashed after Transport.IdleConnTimeout if an HTTP/2 connection
was established but but its caller no longer wanted it. (Assuming the
connection cache was enabled, which it is by default)
Fixes #16208
Change-Id: I9628757f7669e344f416927c77f00ed3864839e3
Reviewed-on: https://go-review.googlesource.com/27450
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-on: https://go-review.googlesource.com/28637
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-rw-r--r-- | src/net/http/transport.go | 4 | ||||
-rw-r--r-- | src/net/http/transport_test.go | 55 |
2 files changed, 59 insertions, 0 deletions
diff --git a/src/net/http/transport.go b/src/net/http/transport.go index 66721190de..1f0763471b 100644 --- a/src/net/http/transport.go +++ b/src/net/http/transport.go @@ -590,6 +590,7 @@ var ( errReadLoopExiting = errors.New("http: persistConn.readLoop exiting") errServerClosedIdle = errors.New("http: server closed idle connection") errIdleConnTimeout = errors.New("http: idle connection timeout") + errNotCachingH2Conn = errors.New("http: not caching alternate protocol's connections") ) // transportReadFromServerError is used by Transport.readLoop when the @@ -633,6 +634,9 @@ func (t *Transport) tryPutIdleConn(pconn *persistConn) error { if pconn.isBroken() { return errConnBroken } + if pconn.alt != nil { + return errNotCachingH2Conn + } pconn.markReused() key := pconn.cacheKey diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go index 749d4530b8..298682d04d 100644 --- a/src/net/http/transport_test.go +++ b/src/net/http/transport_test.go @@ -3511,6 +3511,61 @@ func TestTransportIdleConnTimeout(t *testing.T) { } } +// Issue 16208: Go 1.7 crashed after Transport.IdleConnTimeout if an +// HTTP/2 connection was established but but its caller no longer +// wanted it. (Assuming the connection cache was enabled, which it is +// by default) +// +// This test reproduced the crash by setting the IdleConnTimeout low +// (to make the test reasonable) and then making a request which is +// canceled by the DialTLS hook, which then also waits to return the +// real connection until after the RoundTrip saw the error. Then we +// know the successful tls.Dial from DialTLS will need to go into the +// idle pool. Then we give it a of time to explode. +func TestIdleConnH2Crash(t *testing.T) { + cst := newClientServerTest(t, h2Mode, HandlerFunc(func(w ResponseWriter, r *Request) { + // nothing + })) + defer cst.close() + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + gotErr := make(chan bool, 1) + + cst.tr.IdleConnTimeout = 5 * time.Millisecond + cst.tr.DialTLS = func(network, addr string) (net.Conn, error) { + cancel() + <-gotErr + c, err := tls.Dial(network, addr, &tls.Config{ + InsecureSkipVerify: true, + NextProtos: []string{"h2"}, + }) + if err != nil { + t.Error(err) + return nil, err + } + if cs := c.ConnectionState(); cs.NegotiatedProtocol != "h2" { + t.Errorf("protocol = %q; want %q", cs.NegotiatedProtocol, "h2") + c.Close() + return nil, errors.New("bogus") + } + return c, nil + } + + req, _ := NewRequest("GET", cst.ts.URL, nil) + req = req.WithContext(ctx) + res, err := cst.c.Do(req) + if err == nil { + res.Body.Close() + t.Fatal("unexpected success") + } + gotErr <- true + + // Wait for the explosion. + time.Sleep(cst.tr.IdleConnTimeout * 10) +} + type funcConn struct { net.Conn read func([]byte) (int, error) |