fix(reverseproxy): preserve and rewind request body on dial timeout and error delegation

go.mod

@@ -0,0 +1,3 @@
+module github.com/condoraltidoi32/caddy
+
+go 1.22.2

modules/caddyhttp/reverseproxy/reverseproxy.go

@@ -2,8 +2,6 @@ package reverseproxy
 
 import (
 	"bytes"
-	"context"
-	"errors"
 	"io"
 	"net/http"
 	"sync"
@@ -20,9 +18,8 @@ type dialProtectedBody struct {
 }
 
 func newDialProtectedBody(orig io.ReadCloser) *dialProtectedBody {
-	if orig == nil {
+	if orig == nil || orig == http.NoBody {
 		return nil
- junta
 	}
 	return &dialProtectedBody{orig: orig}
 }
@@ -41,7 +38,7 @@ func (b *dialProtectedBody) Close() error {
 		return nil
 	}
 	// If the transport closes the body before any read happened (e.g. dial failure),
-	// avoid closing the underlying body so downstream error handlers can read it.
+	// avoid closing the underlying body so downstream error handlers or retries can read it.
 	if !b.readStarted {
 		return nil
 	}
@@ -55,11 +52,23 @@ func (b *dialProtectedBody) HasReadStarted() bool {
 	return b.readStarted
 }
 
+// ForceClose closes the underlying reader when request processing is completely finished.
+func (b *dialProtectedBody) ForceClose() error {
+	b.mu.Lock()
+	defer b.mu.Unlock()
+	if b.closed {
+		return nil
+	}
+	b.closed = true
+	return b.orig.Close()
+}
+
 // Handler represents a reverse proxy handler.
 type Handler struct {
 	Transport      http.RoundTripper
 	BufferRequests bool
 	UpstreamAddr   string
+	Upstreams      []string
 	DialTimeout    time.Duration
 }
 
@@ -79,6 +88,12 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request, next func(ht
 		origBody = r.Body
 	}
 
+	transport := h.Transport
+	if transport == nil {
+		transport = http.DefaultTransport
+	}
+
+	// Prepare outbound request
 	outReq := r.Clone(r.Context())
 	var protected *dialProtectedBody
 
@@ -89,31 +104,37 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request, next func(ht
 		outReq.Body = protected
 	}
 
-	transport := h.Transport
-	if transport == nil {
-		transport = http.DefaultTransport
-	}
-
 	resp, err := transport.RoundTrip(outReq)
 	if err != nil {
-		// Restore request body for downstream error handlers
+		// Restore request body for downstream error handlers (e.g. handle_errors)
 		if bufferedBody != nil {
 			r.Body = io.NopCloser(bytes.NewReader(bufferedBody))
-		} else if protected != nil && !protected.HasReadStarted() {
-			r.Body = origBody
+		} else if protected != nil {
+			if !protected.HasReadStarted() {
+				r.Body = origBody
+			} else {
+				// Non-buffered streaming request partially or fully consumed before error:
+				// provide safe fallback empty reader to avoid http.ErrBodyReadAfterClose
+				r.Body = io.NopCloser(bytes.NewReader(nil))
+			}
+		} else {
+			r.Body = http.NoBody
 		}
 		return err
 	}
 	defer resp.Body.Close()
 
-	for k, vv := range resp.Header {
-		for _, v := range vv {
-			w.Header().Add(k, v)
+	if w != nil {
+		for k, vv := range resp.Header {
+			for _, v := range vv {
+				w.Header().Add(k, v)
+			}
 		}
+		w.WriteHeader(resp.StatusCode)
+		_, err = io.Copy(w, resp.Body)
+		return err
 	}
-	w.WriteHeader(resp.StatusCode)
-	_, err = io.Copy(w, resp.Body)
-	return err
+	return nil
 }
 
 // ErrorHandler handles errors by reading request body if present.

modules/caddyhttp/reverseproxy/reverseproxy_test.go

@@ -2,23 +2,21 @@ package reverseproxy
 
 import (
 	"bytes"
-	"context"
 	"errors"
 	"io"
 	"net"
 	"net/http"
 	"net/http/httptest"
 	"strings"
 	"testing"
-	"time"
 )
 
-// failingDialTransport simulates a dial timeout / failure.
+// failingDialTransport simulates a dial timeout / connection failure where transport closes body before read.
 type failingDialTransport struct{}
 
 func (f *failingDialTransport) RoundTrip(req *http.Request) (*http.Response, error) {
 	if req.Body != nil {
-		// Go's http.Transport closes req.Body on dial failure
+		// Go's net/http Transport closes req.Body on dial failures
 		_ = req.Body.Close()
 	}
 	return nil, &net.OpError{
@@ -28,6 +26,20 @@ func (f *failingDialTransport) RoundTrip(req *http.Request) (*http.Response, err
 	}
 }
 
+// partiallyReadingFailingTransport reads partial bytes from request body and then errors.
+type partiallyReadingFailingTransport struct {
+	bytesToRead int
+}
+
+func (p *partiallyReadingFailingTransport) RoundTrip(req *http.Request) (*http.Response, error) {
+	if req.Body != nil {
+		buf := make([]byte, p.bytesToRead)
+		_, _ = req.Body.Read(buf)
+		_ = req.Body.Close()
+	}
+	return nil, errors.New("connection reset by peer during payload transmission")
+}
+
 func TestReverseProxyDialTimeoutPreservesRequestBody(t *testing.T) {
 	payload := "important_payload_data_to_preserve"
 	handler := &Handler{
@@ -42,7 +54,7 @@ func TestReverseProxyDialTimeoutPreservesRequestBody(t *testing.T) {
 		t.Fatal("expected error from dial timeout, got nil")
 	}
 
-	// Emulate downstream error handler (handle_errors)
+	// Downstream error handler (handle_errors) reading body
 	bodyBytes, readErr := io.ReadAll(req.Body)
 	if readErr != nil {
 		t.Fatalf("failed to read req.Body in error handler: %v", readErr)
@@ -68,6 +80,7 @@ func TestReverseProxyBufferedRequestBodyPreservedOnDialFailure(t *testing.T) {
 		t.Fatal("expected error from dial timeout, got nil")
 	}
 
+	// Downstream error handler (handle_errors) reading buffered body
 	bodyBytes, readErr := io.ReadAll(req.Body)
 	if readErr != nil {
 		t.Fatalf("failed to read buffered req.Body in error handler: %v", readErr)
@@ -78,6 +91,54 @@ func TestReverseProxyBufferedRequestBodyPreservedOnDialFailure(t *testing.T) {
 	}
 }
 
+func TestReverseProxyPartiallyReadStreamingBodySafeFallback(t *testing.T) {
+	payload := "streaming_data_that_fails_midway"
+	handler := &Handler{
+		Transport: &partiallyReadingFailingTransport{bytesToRead: 10},
+	}
+
+	req := httptest.NewRequest(http.MethodPost, "http://localhost:8080/test", strings.NewReader(payload))
+	rec := httptest.NewRecorder()
+
+	err := handler.ServeHTTP(rec, req, nil)
+	if err == nil {
+		t.Fatal("expected error from transport, got nil")
+	}
+
+	// Error handlers or logging middleware attempting to read body must get EOF / empty reader, not panic
+	bodyBytes, readErr := io.ReadAll(req.Body)
+	if readErr != nil {
+		t.Fatalf("unexpected error reading fallback body: %v", readErr)
+	}
+	if len(bodyBytes) != 0 {
+		t.Fatalf("expected empty fallback body for consumed stream, got %d bytes", len(bodyBytes))
+	}
+}
+
+func TestReverseProxyNoBodySafeOnDialFailure(t *testing.T) {
+	handler := &Handler{
+		Transport: &failingDialTransport{},
+	}
+
+	req := httptest.NewRequest(http.MethodGet, "http://localhost:8080/test", nil)
+	rec := httptest.NewRecorder()
+
+	err := handler.ServeHTTP(rec, req, nil)
+	if err == nil {
+		t.Fatal("expected error from dial timeout, got nil")
+	}
+
+	if req.Body != nil && req.Body != http.NoBody {
+		bodyBytes, readErr := io.ReadAll(req.Body)
+		if readErr != nil {
+			t.Fatalf("unexpected error reading empty req.Body: %v", readErr)
+		}
+		if len(bodyBytes) != 0 {
+			t.Fatalf("expected 0 bytes, got %d", len(bodyBytes))
+		}
+	}
+}
+
 func TestReverseProxySuccessStreamingBody(t *testing.T) {
 	payload := "streamed_payload"
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -103,3 +164,32 @@ func TestReverseProxySuccessStreamingBody(t *testing.T) {
 		t.Fatalf("expected response body %q, got %q", payload, rec.Body.String())
 	}
 }
+
+func TestDialProtectedBodyLifecycle(t *testing.T) {
+	// Case 1: Close without read does not close underlying reader
+	r1 := io.NopCloser(strings.NewReader("hello"))
+	p1 := newDialProtectedBody(r1)
+	if p1.HasReadStarted() {
+		t.Fatal("expected readStarted to be false")
+	}
+	if err := p1.Close(); err != nil {
+		t.Fatalf("unexpected close error: %v", err)
+	}
+	buf := make([]byte, 5)
+	n, err := p1.Read(buf)
+	if err != nil || n != 5 || string(buf) != "hello" {
+		t.Fatalf("expected reading after unread Close to succeed, got n=%d, err=%v", n, err)
+	}
+
+	// Case 2: Read then Close closes underlying reader
+	r2 := io.NopCloser(bytes.NewBufferString("world"))
+	p2 := newDialProtectedBody(r2)
+	buf2 := make([]byte, 5)
+	_, _ = p2.Read(buf2)
+	if !p2.HasReadStarted() {
+		t.Fatal("expected readStarted to be true")
+	}
+	if err := p2.Close(); err != nil {
+		t.Fatalf("unexpected close error: %v", err)
+	}
+}