From dc3612e0d1a569e2ad1a3deea9ef6eab72616dc4 Mon Sep 17 00:00:00 2001

From: Brad Fitzpatrick <[email protected]>

Date: Fri, 19 Aug 2016 23:13:29 +0000

Subject: [PATCH 12/38] [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 <[email protected]>

Run-TryBot: Brad Fitzpatrick <[email protected]>

TryBot-Result: Gobot Gobot <[email protected]>

Reviewed-on: https://go-review.googlesource.com/28637

Reviewed-by: Brad Fitzpatrick <[email protected]>

---

 src/net/http/transport.go      |  4 +++

 src/net/http/transport_test.go | 55 ++++++++++++++++++++++++++++++++++++++++++

 2 files changed, 59 insertions(+)

diff --git a/src/net/http/transport.go b/src/net/http/transport.go

index 6672119..1f07634 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 749d453..298682d 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)

-- 

2.7.4