net/http: ensure Request.Body.Close is called once and only once

Makes *Request.write always close the body, so that callers no longer
have to close the body on returned errors, which was the trigger for
double-close behavior.

Fixes #40382

Change-Id: I128f7ec70415f240d82154cfca134b3f692191e3
Reviewed-on: https://go-review.googlesource.com/c/go/+/257819
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Trust: Damien Neil <dneil@google.com>
Trust: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Go Bot <gobot@golang.org>

1 files changed, 12 insertions, 2 deletions

diff --git a/src/net/http/transfer.go b/src/net/http/transfer.go
index ab009177bc..c3234f30cc 100644
--- a/src/net/http/transfer.go
+++ b/src/net/http/transfer.go
@@ -330,9 +330,18 @@ func (t *transferWriter) writeHeader(w io.Writer, trace *httptrace.ClientTrace)
 	return nil
 }
 
-func (t *transferWriter) writeBody(w io.Writer) error {
-	var err error
+// always closes t.BodyCloser
+func (t *transferWriter) writeBody(w io.Writer) (err error) {
 	var ncopy int64
+	closed := false
+	defer func() {
+		if closed || t.BodyCloser == nil {
+			return
+		}
+		if closeErr := t.BodyCloser.Close(); closeErr != nil && err == nil {
+			err = closeErr
+		}
+	}()
 
 	// Write body. We "unwrap" the body first if it was wrapped in a
 	// nopCloser or readTrackingBody. This is to ensure that we can take advantage of
@@ -369,6 +378,7 @@ func (t *transferWriter) writeBody(w io.Writer) error {
 		}
 	}
 	if t.BodyCloser != nil {
+		closed = true
 		if err := t.BodyCloser.Close(); err != nil {
 			return err
 		}