From 045b33ecfe4db7af6344cb5227c3d31501eb8cba Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Wed, 11 Jan 2023 13:47:38 -0800 Subject: [PATCH] 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 Run-TryBot: Damien Neil Reviewed-by: Cherry Mui Reviewed-by: Brad Fitzpatrick --- src/net/http/transport.go | 6 ++++++ src/net/http/transport_test.go | 39 ++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) 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. -- 2.50.0