Skip to content

fix(mux): end a timed-out request on its backend before answering it - #1924

Open
FlorentinDUBOIS wants to merge 2 commits into
mainfrom
fix/mux-frontend-timeout-backend-detach
Open

FlorentinDUBOIS wants to merge 2 commits into
mainfrom
fix/mux-frontend-timeout-backend-detach

Conversation

@FlorentinDUBOIS

@FlorentinDUBOIS FlorentinDUBOIS commented Nov 12, 2026 •

Copy link
Copy Markdown
Collaborator

What was wrong

A request answered 504 by a timeout could leave its backend connection able to answer another request. An HTTP/2 frontend outlives the 504, so the client's next request on the same connection could receive the late response of the request that timed out. Two branches of Mux::timeout_inner (lib/src/protocol/mux/mod.rs) led there.

Backend timeout, the default timer order (back_timeout 30 s < front_timeout 60 s). The backend-token branch installed the 504 and only then called end_stream. ConnectionH1::end_stream pools the connection when the response is terminated: it read the 504 as the backend's own whole response and moved the connection to BackendStatus::KeepAlive with its request still unanswered. The session's next request for that backend was written behind it and read its answer. Only an HTTP/1.1 backend that had not started answering is affected; a cut response leaves back in error, and an HTTP/2 backend stream is reset.

Frontend timeout, when back_timeout is longer than front_timeout. The frontend-token branch answered a Linked stream (504, or a cut response) without ending it on the backend, and its second pass only ends streams still Linked. An H1 backend kept ConnectionH1::stream = Some(gid) and an H2 backend kept its wire mapping. Once the slot was recycled and reused, the backend's late answer was delivered on the new stream. Once Context::shrink_trailing_recycle had dropped the slot, a late read or close on it indexed past Context::streams and panicked.

Impact

  • A client can receive another request's response on the same HTTP/2 connection. It stays inside one client connection, so it crosses users only where an intermediary (a CDN, another proxy) multiplexes several of them onto it.
  • Triggering it needs no authentication: one URL slower than the timeout that fires first, then a second request on the connection.
  • The panic in the frontend-timeout case is proved at Mux level only; no end-to-end test drives it.
  • An HTTP/1.1 frontend closes after the default 504 (Connection: close) and is not affected with the default answers. A custom 504 template without Connection: close was not tested.

The fix

Both branches end the stream on the backend while Stream::back still holds what the backend sent, then install the answer. The H1 connection is closed instead of pooled or left attached, an H2 backend stream is reset, and a backend that answered whole is still pooled.

  • Frontend branch: end_stream before set_default_answer and before forcefully_terminate_answer.
  • Backend branch: one end_stream site, moved ahead of the arms. The deferral to a slow client (response_waits_on_client) is decided first and still skips it. A debug_assert! states the invariant: a backend whose response is neither terminated nor in error is never KeepAlive.

Tests

Each seen red with its hunk reverted, green with it, and red again with debug assertions compiled out.

lib/src/protocol/mux/mod.rs, over loopback sockets and real H2 frames:

  • a_frontend_timeout_answering_504_detaches_the_backend_it_was_waiting_on
  • a_frontend_timeout_cutting_a_response_detaches_the_backend_still_sending_it
  • a_late_backend_answer_never_reaches_the_request_that_reused_the_slot
  • a_late_h2_backend_answer_never_reaches_the_request_that_reused_the_slot
  • a_late_backend_answer_never_indexes_a_slot_the_session_dropped
  • a_backend_timeout_answering_504_closes_the_backend_that_still_owes_an_answer
  • a_backend_timeout_after_a_whole_response_keeps_the_backend_pooled (the pair)

e2e/src/tests/h2_timeout_backend_detach_tests.rs, real worker, TLS H2 client, scripted H1 backend:

  • test_h2_late_backend_answer_after_frontend_timeout_stays_off_the_next_request (front_timeout 2 s, back_timeout 20 s)
  • test_h2_late_backend_answer_after_backend_timeout_stays_off_the_next_request (back_timeout 2 s, front_timeout 60 s)

Before the fix, both e2e tests print the first request's answer on the second stream:

H2-BACK-TIMEOUT-DETACH: stream 1 status=Some(504), late=Some(Answered { conn: 0, path: "/slow" }), stream 3 status=Some(200) body="answer:/slow"

After:

H2-BACK-TIMEOUT-DETACH: stream 1 status=Some(504), late=Some(Closed { conn: 0 }), stream 3 status=Some(200) body="answer:/held"

Commands run

On this commit:

  • cargo +nightly fmt --all -- --check → exit 0
  • cargo clippy --all-targets --locked -- -D warnings → exit 0
  • cargo test -p sozu-lib --locked → 1722 passed; 0 failed
  • cargo test -p sozu-e2e --locked -- h2_timeout_backend_detach → 2 passed; 0 failed
  • python3 .github/scripts/check_doc_citations.py --base 072b610e7 → exit 0

On the same tree before the CHANGELOG.md entry was added:

  • cargo test -p sozu-e2e --locked -- --skip tests::fuzz_tests:: → 691 passed; 0 failed

Docs

  • lib/src/protocol/mux/LIFECYCLE.md invariant 22 and the comments that said end_stream runs "at the bottom of the loop" follow its new position.
  • CHANGELOG.md: a Security entry under Unreleased.

Relation to #1922

Independent of it: the same defects reproduced on its tip. #1922 merged first, and cd0f78e3d merges main (68f34840f) into this branch. Git merged every file on its own; CHANGELOG.md is rearranged by hand so the two Security entries under Unreleased share one heading. On the merged tree:

  • cargo +nightly fmt --all -- --check → exit 0
  • cargo clippy --all-targets --locked -- -D warnings → exit 0
  • cargo test -p sozu-lib --locked → 1729 passed; 0 failed
  • cargo test -p sozu-e2e --locked -- h2_timeout_backend_detach h2_stream_slot → 4 passed; 0 failed
  • python3 .github/scripts/check_doc_citations.py --base 68f34840f → exit 0

The full e2e suite was not re-run locally on the merged tree. It passed (693 passed) on the #1922 tip with this change applied, which differs from the merged tree by CHANGELOG.md and two workflow files.

A request answered 504 by a timeout could leave its backend connection
able to answer another request. An HTTP/2 frontend outlives the 504, so
the client's next request on the connection could read the late
response of the request that timed out.

Backend timeout, the default timer order: the backend-token branch of
`Mux::timeout_inner` installed the 504 and only then called
`end_stream`. `ConnectionH1::end_stream` pools the connection when the
response is terminated, read the 504 as the backend's own whole
response, and pooled a connection whose request the backend had not
answered. The session's next request for that backend was written
behind it and read its answer.

Frontend timeout, when `back_timeout` is longer than `front_timeout`:
the frontend-token branch answered a `Linked` stream (504, or a cut
response) without ending it on the backend, and its second pass only
ends streams still `Linked`. An H1 backend kept naming the stream slot
and an H2 backend kept its wire mapping. Once the slot was recycled and
reused, the backend's late answer was delivered on the new stream; once
`Context::shrink_trailing_recycle` had dropped the slot, a late read or
close indexed past `Context::streams` and panicked.

Both branches now end the stream on the backend while `Stream::back`
still holds what the backend sent, then install the answer. The H1
connection is closed, an H2 backend stream is reset, and a backend that
answered whole is still pooled. The backend branch keeps a single
`end_stream` site, ahead of its arms, and asserts that a backend still
owing an answer is never `KeepAlive`.

Tests, each seen red with its hunk reverted: seven `Mux`-level tests
over loopback sockets and real H2 frames, and two e2e tests with real
timers in `h2_timeout_backend_detach_tests.rs`. LIFECYCLE.md
invariant 22 follows the new position of `end_stream`.

Signed-off-by: Florentin Dubois <florentin.dubois@clever.cloud>
main gained #1922 and an actions bump. Git merged every file on its own; CHANGELOG.md is rearranged by hand so the two Security entries under Unreleased share one heading, main's first.

Signed-off-by: Florentin Dubois <florentin.dubois@clever.cloud>
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.

1 participant