feat(proxy): tunnel WebSocket upgrades through path-prefix forwarder #30

Merged

sameh-farouk merged 1 commit from feat/ws-upgrade-tunnel into development

2026-04-26 17:18:28 +00:00

sameh-farouk commented

2026-04-26 15:17:29 +00:00

Member

Closes #29.

Summary

Adds explicit WebSocket-upgrade tunnelling to proxy_handler's path-prefix forwarding path. Without this, browsers reconnect-loop on every WS connection through the proxy because the 101 reaches the client but no frames flow afterwards (proxy stays in HTTP/1.1 mode while the client flips to WS).

Mirrors hero_router::server::routes::proxy_ws_tunnel — same primitives, same patterns — but targets a TCP upstream (the router) instead of a Unix socket.

Changes

File	Change
`crates/hero_proxy_server/Cargo.toml`	+ `http-body-util = "0.1"`, `bytes = "1"`
`Cargo.lock`	regenerated for the two new direct deps
`crates/hero_proxy_server/src/proxy.rs`	+ `is_ws_upgrade(headers)` + `forward_ws_to_upstream(upstream_url, req)` (~200 LOC), and a 4-line branch in `proxy_handler` that short-circuits to the new helper before the existing `forward_to_upstream` call

Net: +210 lines, 0 removals. Existing non-WS code paths are untouched.

How it works

proxy_handler(req)
  ├─ existing: header strip, IP-identity injection, router-health check
  ├─ NEW: if is_ws_upgrade(req.headers()):
  │       ├─ grab hyper::upgrade::on(&mut req)  (client side, before consuming req)
  │       ├─ TCP-connect to router_url, http1::handshake() with_upgrades()
  │       ├─ forward upgrade request preserving all headers (Host overridden)
  │       ├─ on 101: forward to client + spawn copy_bidirectional task
  │       └─ on non-101: bail with the upstream's status
  └─ existing: forward_to_upstream(router_url, None, req)   ← non-WS path unchanged

Test plan

cargo build --release -p hero_proxy_server clean
Curl with Upgrade: websocket headers via :9997 (proxy) returns the same Sec-WebSocket-Accept as via :9988 (router direct), and presence-event payload streams back through the proxy path post-fix
Curl WITHOUT WS upgrade headers (e.g. GET /hero_collab/ui 200, POST /hero_collab/rpc/rpc JSON) still works — non-WS path unchanged
Browser smoke (collab UI through proxy) — single WS upgrade, no reconnect-storm, ping/pong + presence events flow normally
Reviewer to validate: domain-route WS path is intentionally NOT covered (no domain route fronts a WS service today; would need a UDS-targeted variant of the tunnel)

Out of scope (follow-up issues recommended)

dispatch_domain_route still uses plain forward_to_upstream; a UDS-targeted WS tunnel variant would be needed if any domain route ever fronts a WS service.
Cosmetic: the proxy response now contains duplicate vary / CORS headers (both router and proxy add them via their respective tower-http::cors::CorsLayer chains). Functionally harmless; can be cleaned up by stripping the headers the proxy is about to add. ~5 LOC.

Notes

The client_upgrade future is captured BEFORE req.into_parts() consumes the request — moving this call later breaks the upgrade.
The upstream connection's with_upgrades() is required — without it, hyper closes the connection after the response body (which doesn't exist for 101) drains.
All Sec-WebSocket-* headers (Key, Version, Extensions, Protocol, Accept) flow through transparently because we forward all incoming request headers (Host overridden) and all outgoing response headers.

Closes #29. ## Summary Adds explicit WebSocket-upgrade tunnelling to `proxy_handler`'s path-prefix forwarding path. Without this, browsers reconnect-loop on every WS connection through the proxy because the 101 reaches the client but no frames flow afterwards (proxy stays in HTTP/1.1 mode while the client flips to WS). Mirrors `hero_router::server::routes::proxy_ws_tunnel` — same primitives, same patterns — but targets a TCP upstream (the router) instead of a Unix socket. ## Changes | File | Change | |---|---| | `crates/hero_proxy_server/Cargo.toml` | + `http-body-util = "0.1"`, `bytes = "1"` | | `Cargo.lock` | regenerated for the two new direct deps | | `crates/hero_proxy_server/src/proxy.rs` | + `is_ws_upgrade(headers)` + `forward_ws_to_upstream(upstream_url, req)` (~200 LOC), and a 4-line branch in `proxy_handler` that short-circuits to the new helper before the existing `forward_to_upstream` call | Net: +210 lines, 0 removals. Existing non-WS code paths are untouched. ## How it works ``` proxy_handler(req) ├─ existing: header strip, IP-identity injection, router-health check ├─ NEW: if is_ws_upgrade(req.headers()): │ ├─ grab hyper::upgrade::on(&mut req) (client side, before consuming req) │ ├─ TCP-connect to router_url, http1::handshake() with_upgrades() │ ├─ forward upgrade request preserving all headers (Host overridden) │ ├─ on 101: forward to client + spawn copy_bidirectional task │ └─ on non-101: bail with the upstream's status └─ existing: forward_to_upstream(router_url, None, req) ← non-WS path unchanged ``` ## Test plan - [x] `cargo build --release -p hero_proxy_server` clean - [x] Curl with `Upgrade: websocket` headers via `:9997` (proxy) returns the same `Sec-WebSocket-Accept` as via `:9988` (router direct), and presence-event payload streams back through the proxy path post-fix - [x] Curl WITHOUT WS upgrade headers (e.g. `GET /hero_collab/ui` 200, `POST /hero_collab/rpc/rpc` JSON) still works — non-WS path unchanged - [x] Browser smoke (collab UI through proxy) — single WS upgrade, no reconnect-storm, ping/pong + presence events flow normally - [ ] Reviewer to validate: domain-route WS path is intentionally NOT covered (no domain route fronts a WS service today; would need a UDS-targeted variant of the tunnel) ## Out of scope (follow-up issues recommended) - `dispatch_domain_route` still uses plain `forward_to_upstream`; a UDS-targeted WS tunnel variant would be needed if any domain route ever fronts a WS service. - Cosmetic: the proxy response now contains duplicate `vary` / CORS headers (both router and proxy add them via their respective `tower-http::cors::CorsLayer` chains). Functionally harmless; can be cleaned up by stripping the headers the proxy is about to add. ~5 LOC. ## Notes - The `client_upgrade` future is captured BEFORE `req.into_parts()` consumes the request — moving this call later breaks the upgrade. - The upstream connection's `with_upgrades()` is required — without it, hyper closes the connection after the response body (which doesn't exist for 101) drains. - All Sec-WebSocket-* headers (Key, Version, Extensions, Protocol, Accept) flow through transparently because we forward all incoming request headers (Host overridden) and all outgoing response headers.

sameh-farouk added 1 commit

2026-04-26 15:17:29 +00:00

feat(proxy): tunnel WebSocket upgrades through path-prefix forwarder

Build & Test / check (pull_request) Successful in 1m34s

Details

42e1663741

The plain HTTP forwarder used by `proxy_handler` (`forward_to_upstream`)
is request/response only — it does not wire up the upgraded socket
after a `101 Switching Protocols`. Without this branch, browser WS
connections through the proxy reconnect-loop: the 101 reaches the
browser, no frames flow afterwards, the connection drops, the client
reconnects every ~1s.

Adds two helpers in `proxy.rs`:
  * `is_ws_upgrade(headers)` — detects the standard
    `Connection: upgrade` + `Upgrade: websocket` pair.
  * `forward_ws_to_upstream(upstream_url, req)` — TCP-targeted version
    of hero_router's `proxy_ws_tunnel`. Grabs the client-side
    `hyper::upgrade::on(&mut req)`, opens TCP to the router, does the
    HTTP/1.1 handshake `with_upgrades()`, sends the upgrade request
    with all original headers preserved (Host overridden), and once
    the upstream returns 101 splices the two upgrade IOs with
    `tokio::io::copy_bidirectional`.

`proxy_handler` short-circuits to the new helper before
`forward_to_upstream` whenever `is_ws_upgrade` matches.

Out of scope (separate follow-up): `dispatch_domain_route` (the
host-matched path with bearer/OAuth/signature/IP auth modes) still
uses regular HTTP forwarding. No domain route fronts a WebSocket
service today, so this isn't currently load-bearing.

Cosmetic: the proxy response now duplicates `vary` and a few CORS
headers (both router and proxy add them). Functionally harmless.

Adds two small deps used by the helper:
  * http-body-util = "0.1"  (Empty<Bytes> for the upgrade-request body)
  * bytes = "1"             (already a transitive dep of hyper)