]> Cypherpunks repositories - gostls13.git/commitdiff
net/http: add func NewRequestWithContext, Request.Clone
authorBrad Fitzpatrick <bradfitz@golang.org>
Mon, 29 Apr 2019 17:57:10 +0000 (17:57 +0000)
committerBrad Fitzpatrick <bradfitz@golang.org>
Fri, 3 May 2019 15:20:15 +0000 (15:20 +0000)
Fixes #23544

Change-Id: Iaa31d76c4cda8ce22412d73c9025fc57e4fb1967
Reviewed-on: https://go-review.googlesource.com/c/go/+/174324
Reviewed-by: Andrew Bonventre <andybons@golang.org>
src/net/http/client_test.go
src/net/http/clone.go [new file with mode: 0644]
src/net/http/httputil/reverseproxy.go
src/net/http/request.go

index 1c59ce74352167edd60ca2c73f0fc62da07b4334..cb3b86d97795ed7780001c5c1181505f52578891 100644 (file)
@@ -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 (file)
index 0000000..5f2784d
--- /dev/null
@@ -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
+}
index 3c522b2af4fde1c8754b071e30872cc85a5a2cdc..a9bfcae4877e6c7a8af00867d97bb83be9765b06 100644 (file)
@@ -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
 
index 8afe1a7c0c0d6c1e813adde0a76f2fff04009541..fa63175c20c7a91541395b1eacb3ea12ade3566e 100644 (file)
@@ -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",