From dd4570e13977e6c23852978ea4d1a74090f10d3b Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Thu, 15 Jun 2023 07:22:54 -0700 Subject: [PATCH] Revert an upstream Go change that broke us. See https://github.com/golang/go/issues/60818 Updates tailscale/corp#12296 Signed-off-by: Brad Fitzpatrick --- http2/transport.go | 10 ++--- http2/transport_test.go | 84 ----------------------------------------- 2 files changed, 5 insertions(+), 89 deletions(-) diff --git a/http2/transport.go b/http2/transport.go index 4f08ccba9..8e5417849 100644 --- a/http2/transport.go +++ b/http2/transport.go @@ -1266,11 +1266,11 @@ func (cc *ClientConn) RoundTrip(req *http.Request) (*http.Response, error) { return res, nil } - cancelRequest := func(cs *clientStream, err error) error { + cancelRequest := func(cs *clientStream, markDoNotReuse bool, err error) error { cs.cc.mu.Lock() cs.abortStreamLocked(err) bodyClosed := cs.reqBodyClosed - if cs.ID != 0 { + if markDoNotReuse && cs.ID != 0 { // This request may have failed because of a problem with the connection, // or for some unrelated reason. (For example, the user might have canceled // the request without waiting for a response.) Mark the connection as @@ -1318,12 +1318,12 @@ func (cc *ClientConn) RoundTrip(req *http.Request) (*http.Response, error) { return handleResponseHeaders() default: waitDone() - return nil, cancelRequest(cs, cs.abortErr) + return nil, cancelRequest(cs, true, cs.abortErr) } case <-ctx.Done(): - return nil, cancelRequest(cs, ctx.Err()) + return nil, cancelRequest(cs, false, ctx.Err()) case <-cs.reqCancel: - return nil, cancelRequest(cs, errRequestCanceled) + return nil, cancelRequest(cs, false, errRequestCanceled) } } } diff --git a/http2/transport_test.go b/http2/transport_test.go index 53999f6a0..5c659e746 100644 --- a/http2/transport_test.go +++ b/http2/transport_test.go @@ -6374,87 +6374,3 @@ func (c *blockReadConn) Read(b []byte) (n int, err error) { <-c.blockc return c.Conn.Read(b) } - -func TestTransportReuseAfterError(t *testing.T) { - serverReqc := make(chan struct{}, 3) - st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) { - serverReqc <- struct{}{} - }, optOnlyServer) - defer st.Close() - - var ( - unblockOnce sync.Once - blockc = make(chan struct{}) - connCountMu sync.Mutex - connCount int - ) - tr := &Transport{ - TLSClientConfig: tlsConfigInsecure, - DialTLS: func(network, addr string, cfg *tls.Config) (net.Conn, error) { - // The first connection dialed will block on reads until blockc is closed. - connCountMu.Lock() - defer connCountMu.Unlock() - connCount++ - conn, err := tls.Dial(network, addr, cfg) - if err != nil { - return nil, err - } - if connCount == 1 { - return &blockReadConn{ - Conn: conn, - blockc: blockc, - }, nil - } - return conn, nil - }, - } - defer tr.CloseIdleConnections() - defer unblockOnce.Do(func() { - // Ensure that reads on blockc are unblocked if we return early. - close(blockc) - }) - - req, _ := http.NewRequest("GET", st.ts.URL, nil) - - // Request 1 is made on conn 1. - // Reading the response will block. - // Wait until the server receives the request, and continue. - req1c := make(chan struct{}) - go func() { - defer close(req1c) - res1, err := tr.RoundTrip(req.Clone(context.Background())) - if err != nil { - t.Errorf("request 1: %v", err) - } else { - res1.Body.Close() - } - }() - <-serverReqc - - // Request 2 is also made on conn 1. - // Reading the response will block. - // The request is canceled once the server receives it. - // Conn 1 should now be flagged as unfit for reuse. - req2Ctx, cancel := context.WithCancel(context.Background()) - go func() { - <-serverReqc - cancel() - }() - _, err := tr.RoundTrip(req.Clone(req2Ctx)) - if err == nil { - t.Errorf("request 2 unexpectedly succeeded (want cancel)") - } - - // Request 3 is made on a new conn, and succeeds. - res3, err := tr.RoundTrip(req.Clone(context.Background())) - if err != nil { - t.Fatalf("request 3: %v", err) - } - res3.Body.Close() - - // Unblock conn 1, and verify that request 1 completes. - unblockOnce.Do(func() { - close(blockc) - }) - <-req1c -}