|
1 From dc3612e0d1a569e2ad1a3deea9ef6eab72616dc4 Mon Sep 17 00:00:00 2001 |
|
2 From: Brad Fitzpatrick <[email protected]> |
|
3 Date: Fri, 19 Aug 2016 23:13:29 +0000 |
|
4 Subject: [PATCH 12/38] [release-branch.go1.7] net/http: fix unwanted HTTP/2 |
|
5 conn Transport crash after IdleConnTimeout |
|
6 |
|
7 Go 1.7 crashed after Transport.IdleConnTimeout if an HTTP/2 connection |
|
8 was established but but its caller no longer wanted it. (Assuming the |
|
9 connection cache was enabled, which it is by default) |
|
10 |
|
11 Fixes #16208 |
|
12 |
|
13 Change-Id: I9628757f7669e344f416927c77f00ed3864839e3 |
|
14 Reviewed-on: https://go-review.googlesource.com/27450 |
|
15 Reviewed-by: Andrew Gerrand <[email protected]> |
|
16 Run-TryBot: Brad Fitzpatrick <[email protected]> |
|
17 TryBot-Result: Gobot Gobot <[email protected]> |
|
18 Reviewed-on: https://go-review.googlesource.com/28637 |
|
19 Reviewed-by: Brad Fitzpatrick <[email protected]> |
|
20 --- |
|
21 src/net/http/transport.go | 4 +++ |
|
22 src/net/http/transport_test.go | 55 ++++++++++++++++++++++++++++++++++++++++++ |
|
23 2 files changed, 59 insertions(+) |
|
24 |
|
25 diff --git a/src/net/http/transport.go b/src/net/http/transport.go |
|
26 index 6672119..1f07634 100644 |
|
27 --- a/src/net/http/transport.go |
|
28 +++ b/src/net/http/transport.go |
|
29 @@ -590,6 +590,7 @@ var ( |
|
30 errReadLoopExiting = errors.New("http: persistConn.readLoop exiting") |
|
31 errServerClosedIdle = errors.New("http: server closed idle connection") |
|
32 errIdleConnTimeout = errors.New("http: idle connection timeout") |
|
33 + errNotCachingH2Conn = errors.New("http: not caching alternate protocol's connections") |
|
34 ) |
|
35 |
|
36 // transportReadFromServerError is used by Transport.readLoop when the |
|
37 @@ -633,6 +634,9 @@ func (t *Transport) tryPutIdleConn(pconn *persistConn) error { |
|
38 if pconn.isBroken() { |
|
39 return errConnBroken |
|
40 } |
|
41 + if pconn.alt != nil { |
|
42 + return errNotCachingH2Conn |
|
43 + } |
|
44 pconn.markReused() |
|
45 key := pconn.cacheKey |
|
46 |
|
47 diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go |
|
48 index 749d453..298682d 100644 |
|
49 --- a/src/net/http/transport_test.go |
|
50 +++ b/src/net/http/transport_test.go |
|
51 @@ -3511,6 +3511,61 @@ func TestTransportIdleConnTimeout(t *testing.T) { |
|
52 } |
|
53 } |
|
54 |
|
55 +// Issue 16208: Go 1.7 crashed after Transport.IdleConnTimeout if an |
|
56 +// HTTP/2 connection was established but but its caller no longer |
|
57 +// wanted it. (Assuming the connection cache was enabled, which it is |
|
58 +// by default) |
|
59 +// |
|
60 +// This test reproduced the crash by setting the IdleConnTimeout low |
|
61 +// (to make the test reasonable) and then making a request which is |
|
62 +// canceled by the DialTLS hook, which then also waits to return the |
|
63 +// real connection until after the RoundTrip saw the error. Then we |
|
64 +// know the successful tls.Dial from DialTLS will need to go into the |
|
65 +// idle pool. Then we give it a of time to explode. |
|
66 +func TestIdleConnH2Crash(t *testing.T) { |
|
67 + cst := newClientServerTest(t, h2Mode, HandlerFunc(func(w ResponseWriter, r *Request) { |
|
68 + // nothing |
|
69 + })) |
|
70 + defer cst.close() |
|
71 + |
|
72 + ctx, cancel := context.WithCancel(context.Background()) |
|
73 + defer cancel() |
|
74 + |
|
75 + gotErr := make(chan bool, 1) |
|
76 + |
|
77 + cst.tr.IdleConnTimeout = 5 * time.Millisecond |
|
78 + cst.tr.DialTLS = func(network, addr string) (net.Conn, error) { |
|
79 + cancel() |
|
80 + <-gotErr |
|
81 + c, err := tls.Dial(network, addr, &tls.Config{ |
|
82 + InsecureSkipVerify: true, |
|
83 + NextProtos: []string{"h2"}, |
|
84 + }) |
|
85 + if err != nil { |
|
86 + t.Error(err) |
|
87 + return nil, err |
|
88 + } |
|
89 + if cs := c.ConnectionState(); cs.NegotiatedProtocol != "h2" { |
|
90 + t.Errorf("protocol = %q; want %q", cs.NegotiatedProtocol, "h2") |
|
91 + c.Close() |
|
92 + return nil, errors.New("bogus") |
|
93 + } |
|
94 + return c, nil |
|
95 + } |
|
96 + |
|
97 + req, _ := NewRequest("GET", cst.ts.URL, nil) |
|
98 + req = req.WithContext(ctx) |
|
99 + res, err := cst.c.Do(req) |
|
100 + if err == nil { |
|
101 + res.Body.Close() |
|
102 + t.Fatal("unexpected success") |
|
103 + } |
|
104 + gotErr <- true |
|
105 + |
|
106 + // Wait for the explosion. |
|
107 + time.Sleep(cst.tr.IdleConnTimeout * 10) |
|
108 +} |
|
109 + |
|
110 type funcConn struct { |
|
111 net.Conn |
|
112 read func([]byte) (int, error) |
|
113 -- |
|
114 2.7.4 |
|
115 |