From 606d4a38b9ae76df30cc1bcaeee79923a5792e59 Mon Sep 17 00:00:00 2001 From: Ross Light Date: Sat, 26 Sep 2020 08:49:56 -0700 Subject: 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 Reviewed-by: Brad Fitzpatrick Trust: Damien Neil Trust: Brad Fitzpatrick Run-TryBot: Damien Neil TryBot-Result: Go Bot --- src/net/http/transfer.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) (limited to 'src/net/http/transfer.go') 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 } -- cgit v1.2.1 From 1b09d430678d4a6f73b2443463d11f75851aba8a Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Fri, 16 Oct 2020 00:49:02 -0400 Subject: all: update references to symbols moved from io/ioutil to io The old ioutil references are still valid, but update our code to reflect best practices and get used to the new locations. Code compiled with the bootstrap toolchain (cmd/asm, cmd/dist, cmd/compile, debug/elf) must remain Go 1.4-compatible and is excluded. Also excluded vendored code. For #41190. Change-Id: I6d86f2bf7bc37a9d904b6cee3fe0c7af6d94d5b1 Reviewed-on: https://go-review.googlesource.com/c/go/+/263142 Trust: Russ Cox Run-TryBot: Russ Cox TryBot-Result: Go Bot Reviewed-by: Emmanuel Odeke --- src/net/http/transfer.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'src/net/http/transfer.go') diff --git a/src/net/http/transfer.go b/src/net/http/transfer.go index c3234f30cc..fbb0c39829 100644 --- a/src/net/http/transfer.go +++ b/src/net/http/transfer.go @@ -10,7 +10,6 @@ import ( "errors" "fmt" "io" - "io/ioutil" "net/http/httptrace" "net/http/internal" "net/textproto" @@ -156,7 +155,7 @@ func newTransferWriter(r interface{}) (t *transferWriter, err error) { // servers. See Issue 18257, as one example. // // The only reason we'd send such a request is if the user set the Body to a -// non-nil value (say, ioutil.NopCloser(bytes.NewReader(nil))) and didn't +// non-nil value (say, io.NopCloser(bytes.NewReader(nil))) and didn't // set ContentLength, or NewRequest set it to -1 (unknown), so then we assume // there's bytes to send. // @@ -370,7 +369,7 @@ func (t *transferWriter) writeBody(w io.Writer) (err error) { return err } var nextra int64 - nextra, err = t.doBodyCopy(ioutil.Discard, body) + nextra, err = t.doBodyCopy(io.Discard, body) ncopy += nextra } if err != nil { @@ -992,7 +991,7 @@ func (b *body) Close() error { var n int64 // Consume the body, or, which will also lead to us reading // the trailer headers after the body, if present. - n, err = io.CopyN(ioutil.Discard, bodyLocked{b}, maxPostHandlerReadBytes) + n, err = io.CopyN(io.Discard, bodyLocked{b}, maxPostHandlerReadBytes) if err == io.EOF { err = nil } @@ -1003,7 +1002,7 @@ func (b *body) Close() error { default: // Fully consume the body, which will also lead to us reading // the trailer headers after the body, if present. - _, err = io.Copy(ioutil.Discard, bodyLocked{b}) + _, err = io.Copy(io.Discard, bodyLocked{b}) } b.closed = true return err @@ -1075,7 +1074,7 @@ func (fr finishAsyncByteRead) Read(p []byte) (n int, err error) { return } -var nopCloserType = reflect.TypeOf(ioutil.NopCloser(nil)) +var nopCloserType = reflect.TypeOf(io.NopCloser(nil)) // isKnownInMemoryReader reports whether r is a type known to not // block on Read. Its caller uses this as an optional optimization to -- cgit v1.2.1