From 7cbc1058ea9240d9df92a339932d5c6dce694e7a Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Fri, 2 Sep 2016 05:00:05 +0000 Subject: net/http/httputil: make ReverseProxy send nil Body requests when possible The http.Transport's retry can't retry requests with non-nil bodies. When cloning an incoming server request into an outgoing client request, nil out the Body field if the ContentLength is 0. (For server requests, Body is always non-nil, even for GET, HEAD, etc.) Also, don't use the deprecated CancelRequest and use Context instead. And don't set Proto, ProtoMajor, ProtoMinor. Those are ignored in client requests, which was probably a later documentation clarification. Fixes #16036 Updates #16696 (remove useless Proto lines) Change-Id: I70a869e9bd4bf240c5838e82fb5aa695a539b343 Reviewed-on: https://go-review.googlesource.com/28412 Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot Reviewed-by: Joe Tsai --- src/net/http/httputil/reverseproxy.go | 70 ++++++++++------------------------- 1 file changed, 19 insertions(+), 51 deletions(-) (limited to 'src/net/http/httputil/reverseproxy.go') diff --git a/src/net/http/httputil/reverseproxy.go b/src/net/http/httputil/reverseproxy.go index 79831b3a97..47cd0ae97d 100644 --- a/src/net/http/httputil/reverseproxy.go +++ b/src/net/http/httputil/reverseproxy.go @@ -7,6 +7,7 @@ package httputil import ( + "context" "io" "log" "net" @@ -120,68 +121,35 @@ var hopHeaders = []string{ "Upgrade", } -type requestCanceler interface { - CancelRequest(*http.Request) -} - -type runOnFirstRead struct { - io.Reader // optional; nil means empty body - - fn func() // Run before first Read, then set to nil -} - -func (c *runOnFirstRead) Read(bs []byte) (int, error) { - if c.fn != nil { - c.fn() - c.fn = nil - } - if c.Reader == nil { - return 0, io.EOF - } - return c.Reader.Read(bs) -} - func (p *ReverseProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) { transport := p.Transport if transport == nil { transport = http.DefaultTransport } + ctx := req.Context() + if cn, ok := rw.(http.CloseNotifier); ok { + var cancel context.CancelFunc + ctx, cancel = context.WithCancel(ctx) + defer cancel() + notifyChan := cn.CloseNotify() + go func() { + select { + case <-notifyChan: + cancel() + case <-ctx.Done(): + } + }() + } + outreq := new(http.Request) *outreq = *req // includes shallow copies of maps, but okay - - if closeNotifier, ok := rw.(http.CloseNotifier); ok { - if requestCanceler, ok := transport.(requestCanceler); ok { - reqDone := make(chan struct{}) - defer close(reqDone) - - clientGone := closeNotifier.CloseNotify() - - outreq.Body = struct { - io.Reader - io.Closer - }{ - Reader: &runOnFirstRead{ - Reader: outreq.Body, - fn: func() { - go func() { - select { - case <-clientGone: - requestCanceler.CancelRequest(outreq) - case <-reqDone: - } - }() - }, - }, - Closer: outreq.Body, - } - } + if req.ContentLength == 0 { + outreq.Body = nil // Issue 16036: nil Body for http.Transport retries } + outreq = outreq.WithContext(ctx) p.Director(outreq) - outreq.Proto = "HTTP/1.1" - outreq.ProtoMajor = 1 - outreq.ProtoMinor = 1 outreq.Close = false // Remove headers with the same name as the connection-tokens. -- cgit v1.3-6-g1900