net/http/httputil: make ReverseProxy panic on error while copying body

Fixes #23643.

Change-Id: I4f8195a36be817d79b9e7c61a5301f153b681493
Reviewed-on: https://go-review.googlesource.com/91675
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>

1 files changed, 44 insertions, 8 deletions

diff --git a/src/net/http/httputil/reverseproxy_test.go b/src/net/http/httputil/reverseproxy_test.go
index 2232042d3e..3dcc5c7287 100644
--- a/src/net/http/httputil/reverseproxy_test.go
+++ b/src/net/http/httputil/reverseproxy_test.go
@@ -649,18 +649,22 @@ func TestReverseProxy_CopyBuffer(t *testing.T) {
 	var proxyLog bytes.Buffer
 	rproxy := NewSingleHostReverseProxy(rpURL)
 	rproxy.ErrorLog = log.New(&proxyLog, "", log.Lshortfile)
-	frontendProxy := httptest.NewServer(rproxy)
+	donec := make(chan bool, 1)
+	frontendProxy := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		defer func() { donec <- true }()
+		rproxy.ServeHTTP(w, r)
+	}))
 	defer frontendProxy.Close()
 
-	resp, err := http.Get(frontendProxy.URL)
-	if err != nil {
-		t.Fatalf("failed to reach proxy: %v", err)
-	}
-	defer resp.Body.Close()
-
-	if _, err := ioutil.ReadAll(resp.Body); err == nil {
+	if _, err = frontendProxy.Client().Get(frontendProxy.URL); err == nil {
 		t.Fatalf("want non-nil error")
 	}
+	// The race detector complains about the proxyLog usage in logf in copyBuffer
+	// and our usage below with proxyLog.Bytes() so we're explicitly using a
+	// channel to ensure that the ReverseProxy's ServeHTTP is done before we
+	// continue after Get.
+	<-donec
+
 	expected := []string{
 		"EOF",
 		"read",
@@ -813,3 +817,35 @@ func (cc *checkCloser) Close() error {
 func (cc *checkCloser) Read(b []byte) (int, error) {
 	return len(b), nil
 }
+
+// Issue 23643: panic on body copy error
+func TestReverseProxy_PanicBodyError(t *testing.T) {
+	backendServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		out := "this call was relayed by the reverse proxy"
+		// Coerce a wrong content length to induce io.ErrUnexpectedEOF
+		w.Header().Set("Content-Length", fmt.Sprintf("%d", len(out)*2))
+		fmt.Fprintln(w, out)
+	}))
+	defer backendServer.Close()
+
+	rpURL, err := url.Parse(backendServer.URL)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	rproxy := NewSingleHostReverseProxy(rpURL)
+
+	// Ensure that the handler panics when the body read encounters an
+	// io.ErrUnexpectedEOF
+	defer func() {
+		err := recover()
+		if err == nil {
+			t.Fatal("handler should have panicked")
+		}
+		if err != http.ErrAbortHandler {
+			t.Fatal("expected ErrAbortHandler, got", err)
+		}
+	}()
+	req, _ := http.NewRequest("GET", "http://foo.tld/", nil)
+	rproxy.ServeHTTP(httptest.NewRecorder(), req)
+}