reverseproxy: Prevent copying the response if a response handler ran (#…

…4388)

modules/caddyhttp/reverseproxy/reverseproxy.go

@@ -614,6 +614,11 @@ func (h *Handler) reverseProxy(rw http.ResponseWriter, req *http.Request, repl *
 		res.Body = h.bufferedBody(res.Body)
 	}
 
+	// the response body may get closed by a response handler,
+	// and we need to keep track to make sure we don't try to copy
+	// the response if it was already closed
+	bodyClosed := false
+
 	// see if any response handler is configured for this response from the backend
 	for i, rh := range h.HandleResponse {
 		if rh.Match != nil && !rh.Match.Match(res.StatusCode, res.Header) {
@@ -638,8 +643,6 @@ func (h *Handler) reverseProxy(rw http.ResponseWriter, req *http.Request, repl *
 			continue
 		}
 
-		res.Body.Close()
-
 		// set up the replacer so that parts of the original response can be
 		// used for routing decisions
 		for field, value := range res.Header {
@@ -649,7 +652,17 @@ func (h *Handler) reverseProxy(rw http.ResponseWriter, req *http.Request, repl *
 		repl.Set("http.reverse_proxy.status_text", res.Status)
 
 		h.logger.Debug("handling response", zap.Int("handler", i))
-		if routeErr := rh.Routes.Compile(next).ServeHTTP(rw, req); routeErr != nil {
+
+		// pass the request through the response handler routes
+		routeErr := rh.Routes.Compile(next).ServeHTTP(rw, req)
+
+		// always close the response body afterwards since it's expected
+		// that the response handler routes will have written to the
+		// response writer with a new body
+		res.Body.Close()
+		bodyClosed = true
+
+		if routeErr != nil {
 			// wrap error in roundtripSucceeded so caller knows that
 			// the roundtrip was successful and to not retry
 			return roundtripSucceeded{routeErr}
@@ -690,15 +703,17 @@ func (h *Handler) reverseProxy(rw http.ResponseWriter, req *http.Request, repl *
 	}
 
 	rw.WriteHeader(res.StatusCode)
-	err = h.copyResponse(rw, res.Body, h.flushInterval(req, res))
-	res.Body.Close() // close now, instead of defer, to populate res.Trailer
-	if err != nil {
-		// we're streaming the response and we've already written headers, so
-		// there's nothing an error handler can do to recover at this point;
-		// the standard lib's proxy panics at this point, but we'll just log
-		// the error and abort the stream here
-		h.logger.Error("aborting with incomplete response", zap.Error(err))
-		return nil
+	if !bodyClosed {
+		err = h.copyResponse(rw, res.Body, h.flushInterval(req, res))
+		res.Body.Close() // close now, instead of defer, to populate res.Trailer
+		if err != nil {
+			// we're streaming the response and we've already written headers, so
+			// there's nothing an error handler can do to recover at this point;
+			// the standard lib's proxy panics at this point, but we'll just log
+			// the error and abort the stream here
+			h.logger.Error("aborting with incomplete response", zap.Error(err))
+			return nil
+		}
 	}
 
 	if len(res.Trailer) > 0 {