Skip to content

fix: reuse the upstream connection after a response_filter error - #952

Open
stareezy-1 wants to merge 1 commit into
cloudflare:mainfrom
stareezy-1:fix/866-h1-keepalive-3xx
Open

fix: reuse the upstream connection after a response_filter error#952
stareezy-1 wants to merge 1 commit into
cloudflare:mainfrom
stareezy-1:fix/866-h1-keepalive-3xx

Conversation

@stareezy-1

Copy link
Copy Markdown

Summary

Fixes #866

Problem

When user code follows a 3xx redirect by returning Err from ProxyHttp::response_filter (with retry = true) and relies on the outer process_request retry loop, the upstream keep-alive connection is never returned to the pool — every retry and later request opens a new upstream connection.

Root cause: proxy_1to1 used tokio::try_join!, which cancels the upstream half the moment the downstream half errors. The remaining upstream response body (e.g. the 3xx body) was never consumed, so the connection was dropped instead of released.

Fix

  1. New PipeState::DownstreamFilterAborted: the downstream half signals it when h1_response_filter fails.
  2. proxy_1to1 now uses a select-based join instead of try_join!:
    • When the downstream half aborts in a response filter, the upstream half is polled to completion and drains the remaining response body (bounded by a 1 MiB cap plus the peer read timeout), then the connection is released to the pool for reuse.
    • All other error paths keep the previous fail-fast behavior (the sibling future is dropped immediately).
  3. The upstream half returns Ok(true) (reusable) only when the drain fully consumed the response; Ok(false) otherwise.

Tests

New self-contained tests/test_redirect_conn_reuse.rs (in-process proxy + two scripted TCP origins, no nginx):

Step Verifies
GET /a → 302 with a delayed split body (upstream half is still reading when the downstream half aborts) abort happens mid-response
response_filter errors with retry → outer retry serves /b from origin B retry still works
Later GET /c to origin A reuses the drained connection — origin A sees exactly 1 accept; without the fix it sees 2 (fails)

Lib tests pass (22), cargo fmt --check and clippy clean for the touched files.

When response_filter returns an error (e.g. a 3xx redirect followed by the
outer retry loop), proxy_1to1's try_join! cancelled the upstream half the
moment the downstream half errored, so the remaining response body was
never consumed and the connection was dropped instead of returned to the
pool — every retry and later request opened a new upstream connection.

Replace try_join! with a select-based join: when the downstream half
aborts in a response filter (signalled via a new
PipeState::DownstreamFilterAborted), the upstream half is polled to
completion and drains the remaining response body (bounded by a 1 MiB cap
plus the peer read timeout), after which the connection is released for
reuse. All other error paths keep the previous fail-fast behavior.

Closes cloudflare#866.
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.

HTTP/1.1 upstream keep-alive lost when following 3xx via response_filter error + outer retry loop

1 participant