From 9617c6335bca5e4e80949a5b1dbe43273260e8a3 Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Thu, 6 Jun 2024 15:44:05 -0700 Subject: [PATCH 1/3] 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 Reviewed-by: Jonathan Amsterdam --- http2/transport.go | 4 ---- http2/transport_test.go | 20 ++++++++++++++++++++ 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/http2/transport.go b/http2/transport.go index 98a49c6b6..61f511f97 100644 --- a/http2/transport.go +++ b/http2/transport.go @@ -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 diff --git a/http2/transport_test.go b/http2/transport_test.go index ddeaf6137..498e27932 100644 --- a/http2/transport_test.go +++ b/http2/transport_test.go @@ -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() + } +} From 77708f716e46255944251965f8240a0eab01e099 Mon Sep 17 00:00:00 2001 From: Richard Miller Date: Thu, 4 Jul 2024 16:47:43 +0100 Subject: [PATCH 2/3] quic: skip tests which depend on unimplemented UDP functions on Plan 9 The ReadMsgUDP and WriteMsgUDP methods of UDPConn are not implemented (yet?) on Plan 9. Skip tests which require them. Fixes golang/go#68288 Change-Id: Ic6c81b19322d589c10b16da61e9b89284294be05 Reviewed-on: https://go-review.googlesource.com/c/net/+/596795 Reviewed-by: Dmitri Shuralyov LUCI-TryBot-Result: Go LUCI Reviewed-by: Than McIntosh --- quic/endpoint_test.go | 5 +++++ quic/udp_test.go | 2 ++ 2 files changed, 7 insertions(+) diff --git a/quic/endpoint_test.go b/quic/endpoint_test.go index 3cba1423e..dc1c51097 100644 --- a/quic/endpoint_test.go +++ b/quic/endpoint_test.go @@ -13,6 +13,7 @@ import ( "io" "log/slog" "net/netip" + "runtime" "testing" "time" @@ -69,6 +70,10 @@ func TestStreamTransfer(t *testing.T) { } func newLocalConnPair(t testing.TB, conf1, conf2 *Config) (clientConn, serverConn *Conn) { + switch runtime.GOOS { + case "plan9": + t.Skipf("ReadMsgUDP not supported on %s", runtime.GOOS) + } t.Helper() ctx := context.Background() e1 := newLocalEndpoint(t, serverSide, conf1) diff --git a/quic/udp_test.go b/quic/udp_test.go index d3732c140..5c4ba10fc 100644 --- a/quic/udp_test.go +++ b/quic/udp_test.go @@ -129,6 +129,8 @@ func runUDPTest(t *testing.T, f func(t *testing.T, u udpTest)) { if test.srcNet == "udp6" && test.dstNet == "udp" { t.Skipf("%v: no support for mapping IPv4 address to IPv6", runtime.GOOS) } + case "plan9": + t.Skipf("ReadMsgUDP not supported on %s", runtime.GOOS) } if runtime.GOARCH == "wasm" && test.srcNet == "udp6" { t.Skipf("%v: IPv6 tests fail when using wasm fake net", runtime.GOARCH) From e2310ae9eb6425ee6736cfc40f982f42e20f5850 Mon Sep 17 00:00:00 2001 From: Gopher Robot Date: Fri, 5 Jul 2024 11:12:42 +0000 Subject: [PATCH 3/3] go.mod: update golang.org/x dependencies Update golang.org/x dependencies to their latest tagged versions. Change-Id: Ibb0ab46488252d035430a654eed5dd4caab7509e Reviewed-on: https://go-review.googlesource.com/c/net/+/596895 LUCI-TryBot-Result: Go LUCI Auto-Submit: Gopher Robot Reviewed-by: Than McIntosh Reviewed-by: Dmitri Shuralyov --- go.mod | 6 +++--- go.sum | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/go.mod b/go.mod index 355673160..ea400e594 100644 --- a/go.mod +++ b/go.mod @@ -3,8 +3,8 @@ module golang.org/x/net go 1.18 require ( - golang.org/x/crypto v0.24.0 - golang.org/x/sys v0.21.0 - golang.org/x/term v0.21.0 + golang.org/x/crypto v0.25.0 + golang.org/x/sys v0.22.0 + golang.org/x/term v0.22.0 golang.org/x/text v0.16.0 ) diff --git a/go.sum b/go.sum index f7e8785a8..c3977102e 100644 --- a/go.sum +++ b/go.sum @@ -1,8 +1,8 @@ -golang.org/x/crypto v0.24.0 h1:mnl8DM0o513X8fdIkmyFE/5hTYxbwYOjDS/+rK6qpRI= -golang.org/x/crypto v0.24.0/go.mod h1:Z1PMYSOR5nyMcyAVAIQSKCDwalqy85Aqn1x3Ws4L5DM= -golang.org/x/sys v0.21.0 h1:rF+pYz3DAGSQAxAu1CbC7catZg4ebC4UIeIhKxBZvws= -golang.org/x/sys v0.21.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/term v0.21.0 h1:WVXCp+/EBEHOj53Rvu+7KiT/iElMrO8ACK16SMZ3jaA= -golang.org/x/term v0.21.0/go.mod h1:ooXLefLobQVslOqselCNF4SxFAaoS6KujMbsGzSDmX0= +golang.org/x/crypto v0.25.0 h1:ypSNr+bnYL2YhwoMt2zPxHFmbAN1KZs/njMG3hxUp30= +golang.org/x/crypto v0.25.0/go.mod h1:T+wALwcMOSE0kXgUAnPAHqTLW+XHgcELELW8VaDgm/M= +golang.org/x/sys v0.22.0 h1:RI27ohtqKCnwULzJLqkv897zojh5/DwS/ENaMzUOaWI= +golang.org/x/sys v0.22.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/term v0.22.0 h1:BbsgPEJULsl2fV/AT3v15Mjva5yXKQDyKf+TbDz7QJk= +golang.org/x/term v0.22.0/go.mod h1:F3qCibpT5AMpCRfhfT53vVJwhLtIVHhB9XDjfFvnMI4= golang.org/x/text v0.16.0 h1:a94ExnEXNtEwYLGJSIUxnWoxoRz/ZcCsV63ROupILh4= golang.org/x/text v0.16.0/go.mod h1:GhwF1Be+LQoKShO3cGOHzqOgRrGaYc9AvblQOmPVHnI=