net/http: close Request.Body when pconn write loop exits early

The pconn write loop closes a request's body after sending the
request, but in the case where the write loop exits with an
unsent request in writech the body is never closed.

Close the request body in this case.

Fixes #49621

Change-Id: Id94a92937bbfc0beb1396446f4dee32fd2059c7e
Reviewed-on: https://go-review.googlesource.com/c/go/+/461675
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

2 files changed, 45 insertions, 0 deletions

diff --git a/src/net/http/transport.go b/src/net/http/transport.go
index a90f36ff73..b8e4c4e97b 100644
--- a/src/net/http/transport.go
+++ b/src/net/http/transport.go
@@ -622,6 +622,12 @@ func (t *Transport) roundTrip(req *Request) (*Response, error) {
 			if e, ok := err.(transportReadFromServerError); ok {
 				err = e.err
 			}
+			if b, ok := req.Body.(*readTrackingBody); ok && !b.didClose {
+				// Issue 49621: Close the request body if pconn.roundTrip
+				// didn't do so already. This can happen if the pconn
+				// write loop exits without reading the write request.
+				req.closeBody()
+			}
 			return nil, err
 		}
 		testHookRoundTripRetried()
diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go
index 245f73bc9f..2879dee0fd 100644
--- a/src/net/http/transport_test.go
+++ b/src/net/http/transport_test.go
@@ -4092,6 +4092,45 @@ func testTransportDialCancelRace(t *testing.T, mode testMode) {
 	}
 }
 
+// https://go.dev/issue/49621
+func TestConnClosedBeforeRequestIsWritten(t *testing.T) {
+	run(t, testConnClosedBeforeRequestIsWritten, testNotParallel, []testMode{http1Mode})
+}
+func testConnClosedBeforeRequestIsWritten(t *testing.T, mode testMode) {
+	ts := newClientServerTest(t, mode, HandlerFunc(func(w ResponseWriter, r *Request) {}),
+		func(tr *Transport) {
+			tr.DialContext = func(_ context.Context, network, addr string) (net.Conn, error) {
+				// Connection immediately returns errors.
+				return &funcConn{
+					read: func([]byte) (int, error) {
+						return 0, errors.New("error")
+					},
+					write: func([]byte) (int, error) {
+						return 0, errors.New("error")
+					},
+				}, nil
+			}
+		},
+	).ts
+	// Set a short delay in RoundTrip to give the persistConn time to notice
+	// the connection is broken. We want to exercise the path where writeLoop exits
+	// before it reads the request to send. If this delay is too short, we may instead
+	// exercise the path where writeLoop accepts the request and then fails to write it.
+	// That's fine, so long as we get the desired path often enough.
+	SetEnterRoundTripHook(func() {
+		time.Sleep(1 * time.Millisecond)
+	})
+	defer SetEnterRoundTripHook(nil)
+	var closes int
+	_, err := ts.Client().Post(ts.URL, "text/plain", countCloseReader{&closes, strings.NewReader("hello")})
+	if err == nil {
+		t.Fatalf("expected request to fail, but it did not")
+	}
+	if closes != 1 {
+		t.Errorf("after RoundTrip, request body was closed %v times; want 1", closes)
+	}
+}
+
 // logWritesConn is a net.Conn that logs each Write call to writes
 // and then proxies to w.
 // It proxies Read calls to a reader it receives from rch.