From f5c43b919499899fe006896643bbfebbea9d1995 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 29 Apr 2019 17:57:10 +0000 Subject: [PATCH] net/http: add func NewRequestWithContext, Request.Clone Fixes #23544 Change-Id: Iaa31d76c4cda8ce22412d73c9025fc57e4fb1967 Reviewed-on: https://go-review.googlesource.com/c/go/+/174324 Reviewed-by: Andrew Bonventre --- src/net/http/client_test.go | 3 +- src/net/http/clone.go | 64 ++++++++++++++++++++++++ src/net/http/httputil/reverseproxy.go | 4 +- src/net/http/request.go | 72 +++++++++++++++++++++------ 4 files changed, 122 insertions(+), 21 deletions(-) create mode 100644 src/net/http/clone.go diff --git a/src/net/http/client_test.go b/src/net/http/client_test.go index 1c59ce7435..cb3b86d977 100644 --- a/src/net/http/client_test.go +++ b/src/net/http/client_test.go @@ -317,8 +317,7 @@ func TestClientRedirectContext(t *testing.T) { return errors.New("redirected request's context never expired after root request canceled") } } - req, _ := NewRequest("GET", ts.URL, nil) - req = req.WithContext(ctx) + req, _ := NewRequestWithContext(ctx, "GET", ts.URL, nil) _, err := c.Do(req) ue, ok := err.(*url.Error) if !ok { diff --git a/src/net/http/clone.go b/src/net/http/clone.go new file mode 100644 index 0000000000..5f2784d280 --- /dev/null +++ b/src/net/http/clone.go @@ -0,0 +1,64 @@ +// Copyright 2019 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package http + +import ( + "mime/multipart" + "net/textproto" + "net/url" +) + +func cloneURLValues(v url.Values) url.Values { + if v == nil { + return nil + } + // http.Header and url.Values have the same representation, so temporarily + // treat it like http.Header, which does have a clone: + return url.Values(Header(v).Clone()) +} + +func cloneURL(u *url.URL) *url.URL { + if u == nil { + return nil + } + u2 := new(url.URL) + *u2 = *u + if u.User != nil { + u2.User = new(url.Userinfo) + *u2.User = *u.User + } + return u2 +} + +func cloneMultipartForm(f *multipart.Form) *multipart.Form { + if f == nil { + return nil + } + f2 := &multipart.Form{ + Value: (map[string][]string)(Header(f.Value).Clone()), + } + if f.File != nil { + m := make(map[string][]*multipart.FileHeader) + for k, vv := range f.File { + vv2 := make([]*multipart.FileHeader, len(vv)) + for i, v := range vv { + vv2[i] = cloneMultipartFileHeader(v) + } + m[k] = vv2 + } + f2.File = m + } + return f2 +} + +func cloneMultipartFileHeader(fh *multipart.FileHeader) *multipart.FileHeader { + if fh == nil { + return nil + } + fh2 := new(multipart.FileHeader) + *fh2 = *fh + fh2.Header = textproto.MIMEHeader(Header(fh.Header).Clone()) + return fh2 +} diff --git a/src/net/http/httputil/reverseproxy.go b/src/net/http/httputil/reverseproxy.go index 3c522b2af4..a9bfcae487 100644 --- a/src/net/http/httputil/reverseproxy.go +++ b/src/net/http/httputil/reverseproxy.go @@ -196,13 +196,11 @@ func (p *ReverseProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) { }() } - outreq := req.WithContext(ctx) // includes shallow copies of maps, but okay + outreq := req.Clone(ctx) if req.ContentLength == 0 { outreq.Body = nil // Issue 16036: nil Body for http.Transport retries } - outreq.Header = req.Header.Clone() - p.Director(outreq) outreq.Close = false diff --git a/src/net/http/request.go b/src/net/http/request.go index 8afe1a7c0c..fa63175c20 100644 --- a/src/net/http/request.go +++ b/src/net/http/request.go @@ -304,7 +304,7 @@ type Request struct { // // For server requests, this field is not applicable. // - // Deprecated: Use the Context and WithContext methods + // Deprecated: Set the Request's context with NewRequestWithContext // instead. If a Request's Cancel field and context are both // set, it is undefined whether Cancel is respected. Cancel <-chan struct{} @@ -345,6 +345,11 @@ func (r *Request) Context() context.Context { // For outgoing client request, the context controls the entire // lifetime of a request and its response: obtaining a connection, // sending the request, and reading the response headers and body. +// +// To create a new request with a context, use NewRequestWithContext. +// To change the context of a request (such as an incoming) you then +// also want to modify to send back out, use Request.Clone. Between +// those two uses, it's rare to need WithContext. func (r *Request) WithContext(ctx context.Context) *Request { if ctx == nil { panic("nil context") @@ -352,16 +357,38 @@ func (r *Request) WithContext(ctx context.Context) *Request { r2 := new(Request) *r2 = *r r2.ctx = ctx + r2.URL = cloneURL(r.URL) // legacy behavior; TODO: try to remove. Issue 23544 + return r2 +} - // Deep copy the URL because it isn't - // a map and the URL is mutable by users - // of WithContext. - if r.URL != nil { - r2URL := new(url.URL) - *r2URL = *r.URL - r2.URL = r2URL +// Clone returns a deep copy of r with its context changed to ctx. +// The provided ctx must be non-nil. +// +// For an outgoing client request, the context controls the entire +// lifetime of a request and its response: obtaining a connection, +// sending the request, and reading the response headers and body. +func (r *Request) Clone(ctx context.Context) *Request { + if ctx == nil { + panic("nil context") } - + r2 := new(Request) + *r2 = *r + r2.ctx = ctx + r2.URL = cloneURL(r.URL) + if r.Header != nil { + r2.Header = r.Header.Clone() + } + if r.Trailer != nil { + r2.Trailer = r.Trailer.Clone() + } + if s := r.TransferEncoding; s != nil { + s2 := make([]string, len(s)) + copy(s2, s) + r2.TransferEncoding = s + } + r2.Form = cloneURLValues(r.Form) + r2.PostForm = cloneURLValues(r.PostForm) + r2.MultipartForm = cloneMultipartForm(r.MultipartForm) return r2 } @@ -781,25 +808,34 @@ func validMethod(method string) bool { return len(method) > 0 && strings.IndexFunc(method, isNotToken) == -1 } -// NewRequest returns a new Request given a method, URL, and optional body. +// NewRequest wraps NewRequestWithContext using the background context. +func NewRequest(method, url string, body io.Reader) (*Request, error) { + return NewRequestWithContext(context.Background(), method, url, body) +} + +// NewRequestWithContext returns a new Request given a method, URL, and +// optional body. // // If the provided body is also an io.Closer, the returned // Request.Body is set to body and will be closed by the Client // methods Do, Post, and PostForm, and Transport.RoundTrip. // -// NewRequest returns a Request suitable for use with Client.Do or -// Transport.RoundTrip. To create a request for use with testing a -// Server Handler, either use the NewRequest function in the +// NewRequestWithContext returns a Request suitable for use with +// Client.Do or Transport.RoundTrip. To create a request for use with +// testing a Server Handler, either use the NewRequest function in the // net/http/httptest package, use ReadRequest, or manually update the -// Request fields. See the Request type's documentation for the -// difference between inbound and outbound request fields. +// Request fields. For an outgoing client request, the context +// controls the entire lifetime of a request and its response: +// obtaining a connection, sending the request, and reading the +// response headers and body. See the Request type's documentation for +// the difference between inbound and outbound request fields. // // If body is of type *bytes.Buffer, *bytes.Reader, or // *strings.Reader, the returned request's ContentLength is set to its // exact value (instead of -1), GetBody is populated (so 307 and 308 // redirects can replay the body), and Body is set to NoBody if the // ContentLength is 0. -func NewRequest(method, url string, body io.Reader) (*Request, error) { +func NewRequestWithContext(ctx context.Context, method, url string, body io.Reader) (*Request, error) { if method == "" { // We document that "" means "GET" for Request.Method, and people have // relied on that from NewRequest, so keep that working. @@ -809,6 +845,9 @@ func NewRequest(method, url string, body io.Reader) (*Request, error) { if !validMethod(method) { return nil, fmt.Errorf("net/http: invalid method %q", method) } + if ctx == nil { + return nil, errors.New("net/http: nil Context") + } u, err := parseURL(url) // Just url.Parse (url is shadowed for godoc). if err != nil { return nil, err @@ -820,6 +859,7 @@ func NewRequest(method, url string, body io.Reader) (*Request, error) { // The host's colon:port should be normalized. See Issue 14836. u.Host = removeEmptyPort(u.Host) req := &Request{ + ctx: ctx, Method: method, URL: u, Proto: "HTTP/1.1", -- 2.48.1