mirror of
https://github.com/golang/net.git
synced 2026-03-31 10:27:08 +09:00
http2: avoid Transport hang with Connection: close and AllowHTTP
CL 111835 changed Transport stream ID numbering to start at stream 3 when AllowHTTP is set. This was based on a misunderstanding: When a connection upgrades an HTTP/1.1 request to HTTP/2, the initial HTTP/1.1 request occupies stream 1. However, Transport does not perform HTTP protocol upgrades. When using a Transport to send unencrypted HTTP/2 requests, the entire connection uses HTTP/2, the first request is sent as HTTP/2, and there is no reason not to use stream 1 for this request. Starting from stream 3 is mostly harmless, but ClientConn.idleStateLocked assumes that client streams start from 1. This causes it to misidentify new single-use connections as having already sent a request (when AllowHTTP is set), and therefore not suitable for use. Revert to always starting stream IDs at 1. Fixes golang/go#67671 Change-Id: I97c89de4ae49623d916f9dbd200f8252d2fd4247 Reviewed-on: https://go-review.googlesource.com/c/net/+/591275 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Jonathan Amsterdam <jba@google.com>
This commit is contained in:
@@ -827,10 +827,6 @@ func (t *Transport) newClientConn(c net.Conn, singleUse bool) (*ClientConn, erro
|
||||
cc.henc.SetMaxDynamicTableSizeLimit(t.maxEncoderHeaderTableSize())
|
||||
cc.peerMaxHeaderTableSize = initialHeaderTableSize
|
||||
|
||||
if t.AllowHTTP {
|
||||
cc.nextStreamID = 3
|
||||
}
|
||||
|
||||
if cs, ok := c.(connectionStater); ok {
|
||||
state := cs.ConnectionState()
|
||||
cc.tlsState = &state
|
||||
|
||||
@@ -5401,3 +5401,23 @@ func TestIssue66763Race(t *testing.T) {
|
||||
|
||||
<-donec
|
||||
}
|
||||
|
||||
// Issue 67671: Sending a Connection: close request on a Transport with AllowHTTP
|
||||
// set caused a the transport to wedge.
|
||||
func TestIssue67671(t *testing.T) {
|
||||
ts := newTestServer(t, func(w http.ResponseWriter, r *http.Request) {})
|
||||
tr := &Transport{
|
||||
TLSClientConfig: tlsConfigInsecure,
|
||||
AllowHTTP: true,
|
||||
}
|
||||
defer tr.CloseIdleConnections()
|
||||
req, _ := http.NewRequest("GET", ts.URL, nil)
|
||||
req.Close = true
|
||||
for i := 0; i < 2; i++ {
|
||||
res, err := tr.RoundTrip(req)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
res.Body.Close()
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user