Repository navigation
fix(h2): close pending handshake on peer close_notify to prevent socket leak - #68
Conversation
…et leak Connection::finish_shutdown() only handled State::Connected, silently skipping State::Handshake. When a peer completed TLS with ALPN h2 but sent fewer than 24 bytes of the HTTP/2 client preface and then sent close_notify, the driver marked peer_closed and repeatedly called finish_shutdown() — a no-op while the handshake was pending. With Connection::eof() compiled out in production, the handshake future never resolved, is_closed() never became true, and the ABI should_close condition never fired. The accepted socket, its HttpSocket, TLS lane state, and a permanent retry-queue slot leaked indefinitely — an unauthenticated slow resource-exhaustion DoS. Handle State::Handshake in finish_shutdown() by transitioning directly to State::Closed, dropping the pending handshake future. The server preface already sits in Wire::output and stays flushable via take_output_limit, so the driver's should_close condition can fire once the wire drains.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #68 +/- ##
=========================================
Coverage 86.82% 86.83%
+ Complexity 616 615 -1
=========================================
Files 60 60
Lines 16979 17011 +32
Branches 490 490
=========================================
+ Hits 14742 14771 +29
- Misses 1913 1917 +4
+ Partials 324 323 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
Merging this PR will not alter performance
|
Detail bug report: View on Detail
Change
Bug:
Connection::finish_shutdown()incrates/bemo/src/http/h2.rsonly handledState::Connected, silently skippingState::Handshake. When a remote client completed the TLS handshake with ALPNh2but sent fewer than 24 bytes of the HTTP/2 client preface and then sent TLSclose_notify, the driver markedpeer_closedand repeatedly calledfinish_shutdown()— a no-op while the h2 handshake was pending. SinceConnection::eof()is compiled out in production (#[cfg(any(test, feature = "fuzzing"))]), the handshake future never resolved,is_closed()never became true, and the ABIshould_closecondition (is_closed() && wire.is_empty() && !http.sending) never fired. The accepted socket, itsHttpSocket, TLS lane state, and a permanent retry-queue slot leaked indefinitely — an unauthenticated slow resource-exhaustion DoS with no idle timeout to reclaim it.Fix:
finish_shutdown()now handles all threeStatevariants.State::Handshaketransitions directly toState::Closed, dropping the pending handshake future. The server preface already sits inWire::outputand stays flushable viatake_output_limit, so the driver'sshould_closecondition fires once the wire drains and the socket is reclaimed withEVENT_CLOSED.Affected layers: Rust only —
crates/bemo/src/http/h2.rs(theConnectionstate machine) andcrates/bemo/tests/http.rs(integration tests). No ABI, JVM, or Native Image changes.Closes #61
Validation
cargo nextest run -p bemo --locked -E "test(/http::h2::tests::/)"): 22/22 passed, including 3 new tests covering partial-preface +finish_shutdown, no-preface +finish_shutdown, and idempotency after close.cargo nextest run --workspace --locked --test http): 70/71 passed. The 1 failure (arm_receive_failure_leaves_socket_dormant) is pre-existing — confirmed by running it on the baseline commit without this change. Two new tests verify a realrustlsclient sending a partial (9-byte) and single-byte HTTP/2 preface followed byclose_notifyproducesEVENT_CLOSEDwithin a bounded number of driver polls, with noEVENT_REQUESTdispatched.cargo nextest run --workspace --locked --lib --tests): 387/389 passed; both failures (arm_receive_failure_leaves_socket_dormant,persistent_keepalive_reuses_receive_registration_and_fixed_send_state) are pre-existing on the baseline.cargo miri test -p bemo --locked --no-default-features --features rust-allocator --lib --test buffers --test abi --test pool --test workload): 187 passed, 0 failed (48 Miri-ignored socket/AWS-LC tests). All 3 new unit tests pass under Miri.cargo fuzz run h2 -- -runs=1000): 1000 runs, no crashes. Fuzz seed replay (cargo test --manifest-path fuzz/Cargo.toml --lib): 7/7 passed includingreplay::h2.cargo build --workspace --locked,cargo fmt -- --check,cargo clippy -p bemo --locked -- -D warnings, andgit diff --exit-code -- Cargo.lock fuzz/Cargo.lockall clean.epollbackend.Automatic Fixes PRs can be configured here.