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 <[email protected]>
Run-TryBot: Damien Neil <[email protected]>
Reviewed-by: Cherry Mui <[email protected]>
Reviewed-by: Brad Fitzpatrick <[email protected]>

Author: neild

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()

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.