diff --git a/tasks/websocket/review-001-ws-pump-consolidation.md b/tasks/websocket/review-001-ws-pump-consolidation.md index 2483478..4c30fdc 100644 --- a/tasks/websocket/review-001-ws-pump-consolidation.md +++ b/tasks/websocket/review-001-ws-pump-consolidation.md @@ -1,7 +1,7 @@ --- id: review-001-ws-pump-consolidation name: Dedupe pumps + byte-based caps + write-side validation (WS-11, WS-04, WS-05, WS-06, HY-09) -status: pending +status: completed depends_on: [] scope: moderate risk: medium @@ -36,12 +36,12 @@ instead of twice (axum path `:119-180`, tungstenite twin `:299-364`): ## Acceptance Criteria -- [ ] Pump bodies factored into one generic implementation (both paths share it) -- [ ] Write-side rejects `len > MAX_CHUNK_LEN` with a stream error (test); no `8 + len` overflow path -- [ ] Write `pending` byte-capped; inbound message/frame size explicitly configured (tests) -- [ ] Tungstenite path exercised by the shared test suite (COV-03 backfilled) -- [ ] 16 MiB round-trip and disconnect-cleanup tests still pass -- [ ] `cargo test` and `cargo clippy --all-targets -- -D warnings` pass +- [x] Pump bodies factored into one generic implementation (both paths share it) +- [x] Write-side rejects `len > MAX_CHUNK_LEN` with a stream error (test); no `8 + len` overflow path +- [x] Write `pending` byte-capped; inbound message/frame size explicitly configured (tests) +- [x] Tungstenite path exercised by the shared test suite (COV-03 backfilled) +- [x] 16 MiB round-trip and disconnect-cleanup tests still pass +- [x] `cargo test` and `cargo clippy --all-targets -- -D warnings` pass ## References @@ -55,4 +55,40 @@ instead of twice (axum path `:119-180`, tungstenite twin `:299-364`): ## Summary -> Filled on completion. \ No newline at end of file +Restructured `src/websocket/byte_adapter.rs` in three commits: + +- **WS-04/HY-09 (5024d99):** the write-side chunk parser validates the + 8-byte header's length field against alkcall's `MAX_CHUNK_LEN` + (re-exported as `alkhttp::websocket::MAX_CHUNK_LEN`) with saturating + `8 + len`. Oversized header → the pump closes the socket (1011) and + signals the `AsyncWrite` half through a one-shot error channel, so + `poll_write` fails loudly (`InvalidData`, "chunk length exceeds + MAX_CHUNK_LEN") instead of silently waiting toward ~4 GiB. Tested on + the tungstenite path over a raw duplex pair. +- **WS-05/WS-06 (92cc11a):** `pending` is byte-capped at + `PENDING_BUFFER_CAP` (= `MAX_CHUNK_LEN + 8`, checked pre-extend and + post-parse; a smaller "e.g. 1 MiB" cap false-positives because the + alkcall mux passes each payload up to 16 MiB as one `AsyncWrite` + call — the cap is the exact bound well-framed traffic can occupy). + Both pumps fail loudly on breach. Inbound caps + `INBOUND_WS_MESSAGE_CAP`/`INBOUND_WS_FRAME_CAP` (both 1 MiB) set on + the axum upgrade (`max_message_size`/`max_frame_size`) and the + tungstenite dial (`connect_async_with_config`+`WebSocketConfig`); + frame cap = message cap because the path does not use WS + fragmentation and tungstenite rejects a single oversize frame before + reassembly. A read-task inbound error surfaces a 1011 close to the + peer + stream error (tested). +- **WS-11/COV-03 (1db0ea8 + e9ddf94):** one generic implementation + ([`WsFraming`] trait + `run_read_pump`/`run_write_pump`) with two + flavor impls (axum/tungstenite); the EOF watch mechanism, `read_eof` + receiver, and from_wss monitor unchanged. Six byte-adapter unit + tests now exercise the tungstenite path directly (WS-04, WS-05, + well-framed passthrough, binary read, EOF retention, text→1002); + the shared ws_upgrade_session/ws_overlay_ops/from_wss suites cover + both paths end-to-end. + +Verification per commit: `cargo test`, `cargo clippy --all-targets -- +-D warnings`; final also `cargo test --all-features`, `cargo clippy +--all-features --all-targets -- -D warnings`, `cargo fmt --check` — +all green, including the 16 MiB round-trip and disconnect-cleanup +tests. \ No newline at end of file