From a2f7d9d95a84dedb6909bf1907d6857c2c4a2ef5 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 20 Apr 2022 08:58:42 -0700 Subject: [PATCH] net/http: deflake TestTransportConnectionCloseOnRequest Fixes #52450 (hopefully) Change-Id: Ib723f8efb4a13af1b98c25cd02935425172d01e6 Reviewed-on: https://go-review.googlesource.com/c/go/+/401314 Run-TryBot: Brad Fitzpatrick TryBot-Result: Gopher Robot Reviewed-by: Bryan Mills Reviewed-by: Damien Neil --- src/net/http/export_test.go | 9 ++++++++ src/net/http/transport_test.go | 42 +++++++++++++++++++++++++--------- 2 files changed, 40 insertions(+), 11 deletions(-) diff --git a/src/net/http/export_test.go b/src/net/http/export_test.go index a849327f45..205ca83f40 100644 --- a/src/net/http/export_test.go +++ b/src/net/http/export_test.go @@ -306,3 +306,12 @@ func ExportCloseTransportConnsAbruptly(tr *Transport) { } tr.idleMu.Unlock() } + +// ResponseWriterConnForTesting returns w's underlying connection, if w +// is a regular *response ResponseWriter. +func ResponseWriterConnForTesting(w ResponseWriter) (c net.Conn, ok bool) { + if r, ok := w.(*response); ok { + return r.conn.rwc, true + } + return nil, false +} diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go index 6fcb458296..acdfae39ed 100644 --- a/src/net/http/transport_test.go +++ b/src/net/http/transport_test.go @@ -57,6 +57,12 @@ var hostPortHandler = HandlerFunc(func(w ResponseWriter, r *Request) { } w.Header().Set("X-Saw-Close", fmt.Sprint(r.Close)) w.Write([]byte(r.RemoteAddr)) + + // Include the address of the net.Conn in addition to the RemoteAddr, + // in case kernels reuse source ports quickly (see Issue 52450) + if c, ok := ResponseWriterConnForTesting(w); ok { + fmt.Fprintf(w, ", %T %p", c, c) + } }) // testCloseConn is a net.Conn tracked by a testConnSet. @@ -240,6 +246,12 @@ func TestTransportConnectionCloseOnResponse(t *testing.T) { connSet.check(t) } +// TestTransportConnectionCloseOnRequest tests that the Transport's doesn't reuse +// an underlying TCP connection after making an http.Request with Request.Close set. +// +// It tests the behavior by making an HTTP request to a server which +// describes the source source connection it got (remote port number + +// address of its net.Conn). func TestTransportConnectionCloseOnRequest(t *testing.T) { defer afterTest(t) ts := httptest.NewServer(hostPortHandler) @@ -250,7 +262,7 @@ func TestTransportConnectionCloseOnRequest(t *testing.T) { c := ts.Client() tr := c.Transport.(*Transport) tr.Dial = testDial - for _, connectionClose := range []bool{false, true} { + for _, reqClose := range []bool{false, true} { fetch := func(n int) string { req := new(Request) var err error @@ -262,29 +274,37 @@ func TestTransportConnectionCloseOnRequest(t *testing.T) { req.Proto = "HTTP/1.1" req.ProtoMajor = 1 req.ProtoMinor = 1 - req.Close = connectionClose + req.Close = reqClose res, err := c.Do(req) if err != nil { - t.Fatalf("error in connectionClose=%v, req #%d, Do: %v", connectionClose, n, err) + t.Fatalf("error in Request.Close=%v, req #%d, Do: %v", reqClose, n, err) } - if got, want := res.Header.Get("X-Saw-Close"), fmt.Sprint(connectionClose); got != want { - t.Errorf("For connectionClose = %v; handler's X-Saw-Close was %v; want %v", - connectionClose, got, !connectionClose) + if got, want := res.Header.Get("X-Saw-Close"), fmt.Sprint(reqClose); got != want { + t.Errorf("for Request.Close = %v; handler's X-Saw-Close was %v; want %v", + reqClose, got, !reqClose) } body, err := io.ReadAll(res.Body) if err != nil { - t.Fatalf("error in connectionClose=%v, req #%d, ReadAll: %v", connectionClose, n, err) + t.Fatalf("for Request.Close=%v, on request %v/2: ReadAll: %v", reqClose, n, err) } return string(body) } body1 := fetch(1) body2 := fetch(2) - bodiesDiffer := body1 != body2 - if bodiesDiffer != connectionClose { - t.Errorf("error in connectionClose=%v. unexpected bodiesDiffer=%v; body1=%q; body2=%q", - connectionClose, bodiesDiffer, body1, body2) + + got := 1 + if body1 != body2 { + got++ + } + want := 1 + if reqClose { + want = 2 + } + if got != want { + t.Errorf("for Request.Close=%v: server saw %v unique connections, wanted %v\n\nbodies were: %q and %q", + reqClose, got, want, body1, body2) } tr.CloseIdleConnections()