From 77e9ce7404c4a76853e101a9f5687a929ee56654 Mon Sep 17 00:00:00 2001
From: James Hartig <me@jameshartig.com>
Date: Tue, 12 May 2026 13:05:50 -0500
Subject: [PATCH] reverseproxy: further prevent body closes from dial errors
 (#7715)

---
 .../caddyhttp/reverseproxy/retries_test.go    | 55 +++++++++++++++++++
 .../caddyhttp/reverseproxy/reverseproxy.go    | 17 +++---
 2 files changed, 63 insertions(+), 9 deletions(-)

diff --git a/modules/caddyhttp/reverseproxy/retries_test.go b/modules/caddyhttp/reverseproxy/retries_test.go
index b0f78bac0..bfd70978c 100644
--- a/modules/caddyhttp/reverseproxy/retries_test.go
+++ b/modules/caddyhttp/reverseproxy/retries_test.go
@@ -730,3 +730,58 @@ func TestRetryMatchAllowsExpressionMixedWithOtherMatchers(t *testing.T) {
 		})
 	}
 }
+
+// TestSubrouteErrorFallbackWithBody is similar to TestDialErrorBodyRetry but
+// mimics Subroute's Error handler rather than testing retries specifically
+func TestSubrouteErrorFallbackWithBody(t *testing.T) {
+	// Good upstream: echoes the request body with 200 OK.
+	goodServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		body, err := io.ReadAll(r.Body)
+		if err != nil {
+			http.Error(w, "read body: "+err.Error(), http.StatusInternalServerError)
+			return
+		}
+		w.WriteHeader(http.StatusOK)
+		_, err = w.Write(body)
+		if err != nil {
+			t.Errorf("error writing in good server: %v", err)
+		}
+	}))
+	t.Cleanup(goodServer.Close)
+
+	// Handler which will dial error
+	badProxy := minimalHandler(0, &Upstream{Host: new(Host), Dial: deadUpstreamAddr(t)})
+
+	bodyReader := newCloseOnCloseReader("hello world")
+	req := httptest.NewRequest("POST", "http://localhost/", bodyReader)
+	// httptest.NewRequest wraps the reader in NopCloser; replace
+	// it with our close-aware reader so Close() is propagated.
+	req.Body = bodyReader
+
+	req = prepareTestRequest(req)
+	rec := httptest.NewRecorder()
+	err := badProxy.ServeHTTP(rec, req, caddyhttp.HandlerFunc(func(w http.ResponseWriter, r *http.Request) error {
+		return nil
+	}))
+	if err == nil {
+		t.Fatalf("Expected error from badProxy.ServeHTTP")
+	}
+
+	// Simulate the Subroute's Error handler by calling another handler with the
+	// same request and recorder
+	goodProxy := minimalHandler(0, &Upstream{Host: new(Host), Dial: goodServer.Listener.Addr().String()})
+	err = goodProxy.ServeHTTP(rec, req, caddyhttp.HandlerFunc(func(w http.ResponseWriter, r *http.Request) error {
+		return nil
+	}))
+
+	if err != nil {
+		t.Fatalf("Expected no error from goodProxy.ServeHTTP, got: %v", err)
+	}
+	if rec.Code != http.StatusOK {
+		t.Errorf("status: got %d, want %d", rec.Code, http.StatusOK)
+	}
+	expectedBody := "hello world"
+	if rec.Body.String() != expectedBody {
+		t.Errorf("body: got %q, want %q", rec.Body.String(), expectedBody)
+	}
+}
diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go
index cefe645ee..a11afcd79 100644
--- a/modules/caddyhttp/reverseproxy/reverseproxy.go
+++ b/modules/caddyhttp/reverseproxy/reverseproxy.go
@@ -488,20 +488,19 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddyht
 	reqHost := clonedReq.Host
 	reqHeader := clonedReq.Header
 
-	// When retries are configured and there is a body, wrap it in
-	// io.NopCloser to prevent Go's transport from closing it on dial
-	// errors. cloneRequest does a shallow copy, so clonedReq.Body and
+	// If the request contained a body, wrap it in io.NopCloser
+	// to prevent Go's transport from closing it on dial errors.
+	// cloneRequest does a shallow copy, so clonedReq.Body and
 	// r.Body share the same io.ReadCloser — a dial-failure Close()
-	// would kill the original body for all subsequent retry attempts.
-	// The real body is closed by the HTTP server when the handler
-	// returns.
+	// would kill the original body for all subsequent retry
+	// attempts or subsequent handlers. The real body is closed by
+	// the HTTP server when the handler returns.
 	//
 	// If the body was already fully buffered (via request_buffers),
 	// we also extract the buffer so the retry loop can replay it
-	// from the beginning on each attempt. (see #6259, #7546)
+	// from the beginning on each attempt. (see #6259, #7546, #7713)
 	var bufferedReqBody *bytes.Buffer
-	if clonedReq.Body != nil && h.LoadBalancing != nil &&
-		(h.LoadBalancing.Retries > 0 || h.LoadBalancing.TryDuration > 0) {
+	if clonedReq.Body != nil {
 		if reqBodyBuf, ok := clonedReq.Body.(bodyReadCloser); ok && reqBodyBuf.body == nil && reqBodyBuf.buf != nil {
 			bufferedReqBody = reqBodyBuf.buf
 			reqBodyBuf.buf = nil