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 <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>

1 files changed, 19 insertions, 51 deletions

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.