Skip to content

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

Open
meridianmindx wants to merge 1 commit into
condoraltidoi32:mainfrom
meridianmindx:fix/reverse-proxy-dial-body-protection
Open

fix(reverseproxy): preserve and rewind request body on dial timeout and error delegation#5
meridianmindx wants to merge 1 commit into
condoraltidoi32:mainfrom
meridianmindx:fix/reverse-proxy-dial-body-protection

Conversation

@meridianmindx

Copy link
Copy Markdown

Summary

Fixes #1

This PR ensures that downstream error handlers (handle_errors, middleware logging, fallback routes) receive a valid, readable, and properly rewound http.Request.Body when reverse_proxy encounters an upstream dial timeout or connection failure.

Key Changes

  1. dialProtectedBody Shield:
    • Outbound net/http.Transport instances close req.Body when dial errors or timeouts occur even before any payload bytes are transmitted.
    • dialProtectedBody wraps the incoming body reader and prevents premature .Close() until payload reads have actually begun.
  2. Buffer Rewind:
    • When BufferRequests is enabled, the buffered payload is restored and rewound to offset 0 (io.NopCloser(bytes.NewReader(bufferedBody))) before passing the error downstream.
  3. Safe Empty Fallback:
    • For streaming request bodies that were partially consumed prior to network disruption, req.Body is safely set to io.NopCloser(bytes.NewReader(nil)) to eliminate downstream http.ErrBodyReadAfterClose or nil dereference panics.
  4. Syntax & Test Suite:
    • Removed stray undefined token in constructor.
    • Added exhaustive automated test coverage:
      • TestReverseProxyDialTimeoutPreservesRequestBody: Dial failure preserves unbuffered body for error handler.
      • TestReverseProxyBufferedRequestBodyPreservedOnDialFailure: Buffered body rewind.
      • TestReverseProxyPartiallyReadStreamingBodySafeFallback: Graceful empty reader fallback for consumed streams.
      • TestReverseProxyNoBodySafeOnDialFailure: Handling requests without bodies.
      • TestReverseProxySuccessStreamingBody: Clean end-to-end streaming on success.
      • TestDialProtectedBodyLifecycle: Lifecycle isolation test.

Verification

go test -v ./...
# PASS: TestReverseProxyDialTimeoutPreservesRequestBody
# PASS: TestReverseProxyBufferedRequestBodyPreservedOnDialFailure
# PASS: TestReverseProxyPartiallyReadStreamingBodySafeFallback
# PASS: TestReverseProxyNoBodySafeOnDialFailure
# PASS: TestReverseProxySuccessStreamingBody
# PASS: TestDialProtectedBodyLifecycle
# ok   github.com/condoraltidoi32/caddy/modules/caddyhttp/reverseproxy 0.011s

…nd error delegation

- Add dialProtectedBody to shield unread incoming request stream from premature closure by net/http.Transport on dial errors
- Rewind buffered request bodies (BufferRequests) to offset 0 before returning control to error handlers
- Assign safe empty fallback reader for partially consumed streams to prevent http.ErrBodyReadAfterClose
- Add complete unit test suite validating dial timeouts, buffered rewind, streaming fallbacks, and protected body lifecycle
- Fix undefined syntax token and ensure 100% test pass rate

Fixes condoraltidoi32#1
@meridianmindx

Copy link
Copy Markdown
Author

Friendly bump: this has been mergeable for ~4 days with green local verification (body rewind on dial timeout / error delegation). Happy to adjust if you want anything changed. Payout address is in the PR description (Base).

@meridianmindx

Copy link
Copy Markdown
Author

Friendly bump — still MERGEABLE and ready for review/merge when you have a moment. Happy to address any follow-ups quickly.

@meridianmindx

Copy link
Copy Markdown
Author

@condoraltidoi32 Ready for merge when you are — still MERGEABLE and linked to the $10 BountyHub issue. Once merged I can refresh the claim on BountyHub so payout can release. Happy to jump on any last review notes same day.

@meridianmindx

Copy link
Copy Markdown
Author

Hi @condoraltidoi32, friendly bump on PR #5. All unit tests are passing and branch is fully MERGEABLE. Ready to merge whenever you're ready! /claim #1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🎯 reverse_proxy: Request body is closed before reaching error handlers on upstream dial timeout

1 participant