Commit 23690138 authored by Brad Fitzpatrick's avatar Brad Fitzpatrick

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/27450Reviewed-by: default avatarAndrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
parent 82c1e22e
......@@ -585,6 +585,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
......@@ -628,6 +629,9 @@ func (t *Transport) tryPutIdleConn(pconn *persistConn) error {
if pconn.isBroken() {
return errConnBroken
}
if pconn.alt != nil {
return errNotCachingH2Conn
}
pconn.markReused()
key := pconn.cacheKey
......
......@@ -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)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment