From 0f53453b32054b681dac6e8b025c85a785396102 Mon Sep 17 00:00:00 2001 From: dqu123 Date: Sat, 10 Oct 2020 16:25:07 -0400 Subject: net/http: deep copy Request.TransferEncoding The existing implementation in Request.Clone() assigns the wrong pointer to r2.TransferEncoding. Fixes #41907 Change-Id: I7f220a41b1b46a55d1a1005e47c6dd69478cb025 Reviewed-on: https://go-review.googlesource.com/c/go/+/261258 Reviewed-by: Brad Fitzpatrick Reviewed-by: Emmanuel Odeke Trust: Emmanuel Odeke --- src/net/http/request.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/net/http/request.go') diff --git a/src/net/http/request.go b/src/net/http/request.go index fe6b60982c..54ec1c5593 100644 --- a/src/net/http/request.go +++ b/src/net/http/request.go @@ -382,7 +382,7 @@ func (r *Request) Clone(ctx context.Context) *Request { if s := r.TransferEncoding; s != nil { s2 := make([]string, len(s)) copy(s2, s) - r2.TransferEncoding = s + r2.TransferEncoding = s2 } r2.Form = cloneURLValues(r.Form) r2.PostForm = cloneURLValues(r.PostForm) -- cgit v1.2.1 From dfee3332e66bd3f3afd76615767d2cd9b1905b26 Mon Sep 17 00:00:00 2001 From: Ross Light Date: Thu, 27 Aug 2020 13:08:29 -0700 Subject: net/http: document concurrency expectations for Request.Body This is primarily aimed at client requests where the user can supply their own io.ReadCloser, but also clarifies server request behavior. A server request body can be one of: - *body - *http2RequestBody - *expectContinueReader - *maxBytesReader Of those, *expectContinueReader did not meet these expectations, so this change also removes the data race. Change-Id: Id4f1ae573d938347b1123a7b612b271aabb045a4 Reviewed-on: https://go-review.googlesource.com/c/go/+/251087 Reviewed-by: Damien Neil Reviewed-by: Russ Cox Trust: Damien Neil Run-TryBot: Damien Neil TryBot-Result: Go Bot --- src/net/http/request.go | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src/net/http/request.go') diff --git a/src/net/http/request.go b/src/net/http/request.go index 54ec1c5593..183606d0ff 100644 --- a/src/net/http/request.go +++ b/src/net/http/request.go @@ -175,6 +175,10 @@ type Request struct { // but will return EOF immediately when no body is present. // The Server will close the request body. The ServeHTTP // Handler does not need to. + // + // Body must allow Read to be called concurrently with Close. + // In particular, calling Close should unblock a Read waiting + // for input. Body io.ReadCloser // GetBody defines an optional func to return a new copy of -- cgit v1.2.1 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/request.go | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) (limited to 'src/net/http/request.go') diff --git a/src/net/http/request.go b/src/net/http/request.go index 183606d0ff..df73d5f62d 100644 --- a/src/net/http/request.go +++ b/src/net/http/request.go @@ -544,6 +544,7 @@ var errMissingHost = errors.New("http: Request.Write on Request with no Host or // extraHeaders may be nil // waitForContinue may be nil +// always closes body func (r *Request) write(w io.Writer, usingProxy bool, extraHeaders Header, waitForContinue func() bool) (err error) { trace := httptrace.ContextClientTrace(r.Context()) if trace != nil && trace.WroteRequest != nil { @@ -553,6 +554,15 @@ func (r *Request) write(w io.Writer, usingProxy bool, extraHeaders Header, waitF }) }() } + closed := false + defer func() { + if closed { + return + } + if closeErr := r.closeBody(); closeErr != nil && err == nil { + err = closeErr + } + }() // Find the target host. Prefer the Host: header, but if that // is not given, use the host from the request URL. @@ -671,6 +681,7 @@ func (r *Request) write(w io.Writer, usingProxy bool, extraHeaders Header, waitF trace.Wait100Continue() } if !waitForContinue() { + closed = true r.closeBody() return nil } @@ -683,6 +694,7 @@ func (r *Request) write(w io.Writer, usingProxy bool, extraHeaders Header, waitF } // Write body and trailer + closed = true err = tw.writeBody(w) if err != nil { if tw.bodyReadError == err { @@ -1387,10 +1399,11 @@ func (r *Request) wantsClose() bool { return hasToken(r.Header.get("Connection"), "close") } -func (r *Request) closeBody() { - if r.Body != nil { - r.Body.Close() +func (r *Request) closeBody() error { + if r.Body == nil { + return nil } + return r.Body.Close() } func (r *Request) isReplayable() bool { -- 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/request.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'src/net/http/request.go') diff --git a/src/net/http/request.go b/src/net/http/request.go index df73d5f62d..adba5406e9 100644 --- a/src/net/http/request.go +++ b/src/net/http/request.go @@ -15,7 +15,6 @@ import ( "errors" "fmt" "io" - "io/ioutil" "mime" "mime/multipart" "net" @@ -870,7 +869,7 @@ func NewRequestWithContext(ctx context.Context, method, url string, body io.Read } rc, ok := body.(io.ReadCloser) if !ok && body != nil { - rc = ioutil.NopCloser(body) + rc = io.NopCloser(body) } // The host's colon:port should be normalized. See Issue 14836. u.Host = removeEmptyPort(u.Host) @@ -892,21 +891,21 @@ func NewRequestWithContext(ctx context.Context, method, url string, body io.Read buf := v.Bytes() req.GetBody = func() (io.ReadCloser, error) { r := bytes.NewReader(buf) - return ioutil.NopCloser(r), nil + return io.NopCloser(r), nil } case *bytes.Reader: req.ContentLength = int64(v.Len()) snapshot := *v req.GetBody = func() (io.ReadCloser, error) { r := snapshot - return ioutil.NopCloser(&r), nil + return io.NopCloser(&r), nil } case *strings.Reader: req.ContentLength = int64(v.Len()) snapshot := *v req.GetBody = func() (io.ReadCloser, error) { r := snapshot - return ioutil.NopCloser(&r), nil + return io.NopCloser(&r), nil } default: // This is where we'd set it to -1 (at least @@ -1205,7 +1204,7 @@ func parsePostForm(r *Request) (vs url.Values, err error) { maxFormSize = int64(10 << 20) // 10 MB is a lot of text. reader = io.LimitReader(r.Body, maxFormSize+1) } - b, e := ioutil.ReadAll(reader) + b, e := io.ReadAll(reader) if e != nil { if err == nil { err = e -- cgit v1.2.1