net/http: deflake TestTransportConnectionCloseOnRequest

Fixes #52450 (hopefully)

Change-Id: Ib723f8efb4a13af1b98c25cd02935425172d01e6
Reviewed-on: https://go-review.googlesource.com/c/go/+/401314
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Damien Neil <dneil@google.com>

diff --git a/src/net/http/export_test.go b/src/net/http/export_test.go
index a849327f4528b430cac53046168f4a4a9440aaf2..205ca83f40260931250004ef4c45a8f34f19382f 100644 (file)
--- 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 6fcb458296fdb039c5e3eb8c120a07a256564e01..acdfae39edc8fa199777389e43694eabdba649a1 100644 (file)
--- 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()