WebSocket passthrough broken — proxy_to_socket buffers responses and strips Upgrade header #35

New issue

Closed

opened 2026-04-14 10:53:42 +00:00 by sameh-farouk · 1 comment

sameh-farouk commented

2026-04-14 10:53:42 +00:00

Member

Problem

proxy_to_socket() in server/routes.rs does not support WebSocket connections. When a browser sends a WebSocket upgrade request through hero_router to a backend service (e.g., hero_collab_ui, hero_whiteboard_ui), the handshake fails with:

WebSocket connection to 'ws://host/hero_collab/ui/ws/2' failed: Error during WebSocket handshake: 'Upgrade' header is missing

Root Cause

Two issues in proxy_to_socket() (routes.rs lines 537-565):

1. Response body is fully buffered (line 541):

let body_bytes = match resp.into_body().collect().await {
    Ok(b) => b.to_bytes(),
    Err(_) => Bytes::new(),
};

WebSocket requires a streaming bidirectional connection. Buffering the entire response body kills this.

2. Upgrade header is stripped from the response (line 556):

if matches!(
    k.as_str(),
    "transfer-encoding" | "content-length" | "connection"
    | "keep-alive" | "te" | "trailers"
    | "upgrade"          // ← This strips the WebSocket upgrade response
    | "x-frame-options" | "content-security-policy"
) {
    continue;
}

The request side is fine — incoming headers including Upgrade: websocket ARE forwarded to the backend (lines 509-523). But the response never makes it back.

Impact

This affects every Hero service that uses WebSocket on its ui.sock:

hero_collab_ui: /ws/{channel_id} — real-time chat sync (per TECH_SPEC Section 5)
hero_whiteboard_ui: /ws/{board_id} — real-time collaborative drawing
hero_proc_ui: /api/services/{name}/pty and /api/jobs/{id}/pty — terminal PTY

All three services implement WebSocket handlers with WebSocketUpgrade on their ui.sock, and use .with_upgrades() or axum::serve() to support the protocol. The WebSocket pattern is the standard Hero real-time architecture — hero_collab's TECH_SPEC explicitly calls it the "proven dual-channel pattern" learned from hero_whiteboard.

Proposed Fix

When an incoming request has Upgrade: websocket + Connection: Upgrade headers, use a bidirectional tunnel instead of the buffered request/response proxy:

Detect WebSocket upgrade request (check headers)
Connect to the backend Unix socket
Forward the raw HTTP upgrade request
Read the backend's 101 Switching Protocols response
Return the 101 response to the browser
Spawn two tasks: browser→backend and backend→browser, forwarding raw bytes

This is ~30-40 lines of code in a new branch alongside the existing proxy_to_socket. The non-WebSocket path stays unchanged.

Reference implementation

hero_proc_ui already implements this pattern internally (run_pty_proxy in routes.rs lines 517-577) — it accepts the browser WebSocket on ui.sock, then opens a second WebSocket to rpc.sock via tokio_tungstenite::client_async. hero_router needs the same capability at the proxy level so services don't each need their own WebSocket proxy.

hero_collab issue: lhumina_code/hero_collab#9 (blocked by this)

## Problem `proxy_to_socket()` in `server/routes.rs` does not support WebSocket connections. When a browser sends a WebSocket upgrade request through hero_router to a backend service (e.g., hero_collab_ui, hero_whiteboard_ui), the handshake fails with: ``` WebSocket connection to 'ws://host/hero_collab/ui/ws/2' failed: Error during WebSocket handshake: 'Upgrade' header is missing ``` ## Root Cause Two issues in `proxy_to_socket()` (routes.rs lines 537-565): **1. Response body is fully buffered (line 541):** ```rust let body_bytes = match resp.into_body().collect().await { Ok(b) => b.to_bytes(), Err(_) => Bytes::new(), }; ``` WebSocket requires a streaming bidirectional connection. Buffering the entire response body kills this. **2. `Upgrade` header is stripped from the response (line 556):** ```rust if matches!( k.as_str(), "transfer-encoding" | "content-length" | "connection" | "keep-alive" | "te" | "trailers" | "upgrade" // ← This strips the WebSocket upgrade response | "x-frame-options" | "content-security-policy" ) { continue; } ``` The request side is fine — incoming headers including `Upgrade: websocket` ARE forwarded to the backend (lines 509-523). But the response never makes it back. ## Impact This affects **every Hero service** that uses WebSocket on its ui.sock: - **hero_collab_ui:** `/ws/{channel_id}` — real-time chat sync (per TECH_SPEC Section 5) - **hero_whiteboard_ui:** `/ws/{board_id}` — real-time collaborative drawing - **hero_proc_ui:** `/api/services/{name}/pty` and `/api/jobs/{id}/pty` — terminal PTY All three services implement WebSocket handlers with `WebSocketUpgrade` on their ui.sock, and use `.with_upgrades()` or `axum::serve()` to support the protocol. The WebSocket pattern is the standard Hero real-time architecture — hero_collab's TECH_SPEC explicitly calls it the "proven dual-channel pattern" learned from hero_whiteboard. ## Proposed Fix When an incoming request has `Upgrade: websocket` + `Connection: Upgrade` headers, use a **bidirectional tunnel** instead of the buffered request/response proxy: 1. Detect WebSocket upgrade request (check headers) 2. Connect to the backend Unix socket 3. Forward the raw HTTP upgrade request 4. Read the backend's 101 Switching Protocols response 5. Return the 101 response to the browser 6. Spawn two tasks: browser→backend and backend→browser, forwarding raw bytes This is ~30-40 lines of code in a new branch alongside the existing `proxy_to_socket`. The non-WebSocket path stays unchanged. ### Reference implementation hero_proc_ui already implements this pattern internally (`run_pty_proxy` in routes.rs lines 517-577) — it accepts the browser WebSocket on ui.sock, then opens a second WebSocket to rpc.sock via `tokio_tungstenite::client_async`. hero_router needs the same capability at the proxy level so services don't each need their own WebSocket proxy. ## Related - hero_collab issue: https://forge.ourworld.tf/lhumina_code/hero_collab/issues/9 (blocked by this)

sameh-farouk

2026-04-14 10:53:42 +00:00

closed this issue
added the
type_bug

prio_critical
labels

sameh-farouk commented

2026-04-14 11:36:42 +00:00

Author

Member

Closing — WebSocket support already exists in hero_router

After deeper investigation, hero_router already has WebSocket passthrough implemented:

is_ws_upgrade() (line 409) detects Connection: upgrade + Upgrade: websocket
ws_proxy_inner() (line 424) resolves the backend socket
proxy_ws_tunnel() (line 466) does bidirectional raw tunnel via hyper::upgrade::on()
Line 847: WebSocket requests are caught BEFORE the buffered proxy_to_socket path

The Upgrade header stripping I reported (line 556) only applies to regular HTTP responses — WebSocket requests never reach that code path.

The actual issue is likely in hero_collab_ui's WebSocket handling or the version of hero_router deployed locally. Will investigate further on the hero_collab side.

Sorry for the false report!

## Closing — WebSocket support already exists in hero_router After deeper investigation, hero_router **already has WebSocket passthrough** implemented: - `is_ws_upgrade()` (line 409) detects `Connection: upgrade` + `Upgrade: websocket` - `ws_proxy_inner()` (line 424) resolves the backend socket - `proxy_ws_tunnel()` (line 466) does bidirectional raw tunnel via `hyper::upgrade::on()` - Line 847: WebSocket requests are caught BEFORE the buffered `proxy_to_socket` path The `Upgrade` header stripping I reported (line 556) only applies to regular HTTP responses — WebSocket requests never reach that code path. The actual issue is likely in hero_collab_ui's WebSocket handling or the version of hero_router deployed locally. Will investigate further on the hero_collab side. Sorry for the false report!