From 44f0a5162d2d4dce165d48f326a6ce5b65cfaef0 Mon Sep 17 00:00:00 2001 From: Lann Martin Date: Tue, 4 Aug 2026 12:08:19 -0400 Subject: [PATCH] Pin the handshake edges: userinfo, redirects, transport size caps Three gaps from the spec sweep, each a place the hosts could diverge from the browser floor (or from each other) with nothing gating it. Userinfo in connect URLs was not rejected: the WHATWG constructor throws on credentials, so the jco host failed such URLs with the wrong taxonomy (connect-failed, from the constructor throw) while the native host would have connected. Both eager validators now fail invalid-url, the WIT invalid-url/connect docs name userinfo alongside fragments, and connect-invalid-url carries the case - it fails against both previous hosts (stash-verified). Handshake redirects were unpinned: browsers never follow them and tungstenite does not either, but no row asserted the shared behavior. A /redirect fault mode (302 toward a working /echo, so a client that followed would connect and expose itself) and a connect-redirect row pin connect-failed on every target. The native transport's size caps were tungstenite's fixed defaults (64 MiB message / 16 MiB frame), invisible today only because the 8 MiB buffer bound overflows first: an embedder raising the bound past the caps would get transport protocol errors (abnormal close) where a browser-backed host delivers the message into the budget's overflow-close path. The caps now scale with the configured bound, and a capacity error past them latches the same overflow taxonomy (backlog, then receive-buffer-overflow) instead of masquerading as an abnormal closure - the mid-frame read stream is torn down after the close frame is offered. That mapping is documented rather than conformance-gated: exercising it would need a >64 MiB flood. --- conformance/adapters/common/src/lib.rs | 1 + conformance/adapters/jco/driver.js | 1 + conformance/guest/src/lib.rs | 17 ++++++++++ conformance/server/PROTOCOL.md | 1 + conformance/server/src/lib.rs | 12 +++++++ conformance/tests.toml | 7 +++- js/jco/websocket.js | 5 +++ rust/wasmtime/src/websocket.rs | 45 ++++++++++++++++++++++++-- wit/websocket.wit | 15 +++++---- 9 files changed, 94 insertions(+), 10 deletions(-) diff --git a/conformance/adapters/common/src/lib.rs b/conformance/adapters/common/src/lib.rs index 82fc1fa..c2e741c 100644 --- a/conformance/adapters/common/src/lib.rs +++ b/conformance/adapters/common/src/lib.rs @@ -76,6 +76,7 @@ pub const TESTS: &[&str] = &[ "connect-invalid-protocols", "connect-refused", "connect-rejected", + "connect-redirect", "connect-timeout", "subprotocol-negotiated", "subprotocol-none-offered", diff --git a/conformance/adapters/jco/driver.js b/conformance/adapters/jco/driver.js index f64d6a3..185b66c 100644 --- a/conformance/adapters/jco/driver.js +++ b/conformance/adapters/jco/driver.js @@ -17,6 +17,7 @@ export const TESTS = [ "connect-invalid-protocols", "connect-refused", "connect-rejected", + "connect-redirect", "connect-timeout", "subprotocol-negotiated", "subprotocol-none-offered", diff --git a/conformance/guest/src/lib.rs b/conformance/guest/src/lib.rs index 587a6a7..47de53d 100644 --- a/conformance/guest/src/lib.rs +++ b/conformance/guest/src/lib.rs @@ -34,6 +34,7 @@ const CORPUS: &[(&str, &[&str])] = &[ ("connect-invalid-protocols", &["connect", "errors"]), ("connect-refused", &["connect", "errors"]), ("connect-rejected", &["connect", "errors"]), + ("connect-redirect", &["connect", "errors"]), ("connect-timeout", &["connect", "errors", "timeouts"]), ("subprotocol-negotiated", &["connect", "subprotocol"]), ("subprotocol-none-offered", &["connect", "subprotocol"]), @@ -260,9 +261,13 @@ async fn run(test_id: &str, config: &TestConfig) -> Result<(), String> { Ok(()) } "connect-invalid-url" => { + // server_url is `ws://host:port`; splice userinfo in after the + // scheme for the credentials case. + let with_userinfo = format!("ws://user:secret@{}/echo", &config.server_url[5..]); let cases: &[String] = &[ format!("http{}", &config.server_url[2..]), // http:// scheme format!("{}/echo#fragment", config.server_url), + with_userinfo, "not a url".to_string(), "/echo".to_string(), ]; @@ -318,6 +323,18 @@ async fn run(test_id: &str, config: &TestConfig) -> Result<(), String> { Err(other) => Err(format!("expected connect-failed, got {}", describe(&other))), } } + "connect-redirect" => { + // A redirect instead of the upgrade must fail the connect; + // clients never follow (the redirect target is a working echo + // endpoint, so a client that followed would connect and fail + // here). + let url = format!("{}/redirect", config.server_url); + match Websocket::connect(url.clone(), Vec::new()).await { + Err(Error::ConnectFailed(_)) => Ok(()), + Ok(_) => Err("connect followed a redirect".to_string()), + Err(other) => Err(format!("expected connect-failed, got {}", describe(&other))), + } + } "connect-timeout" => { // The adapter configures a short connect bound; /stall never // answers the handshake. diff --git a/conformance/server/PROTOCOL.md b/conformance/server/PROTOCOL.md index 4ae7ea8..b81d531 100644 --- a/conformance/server/PROTOCOL.md +++ b/conformance/server/PROTOCOL.md @@ -26,6 +26,7 @@ ignored; unknown paths answer HTTP 404. | `GET /healthz` | Plain HTTP 200. Readiness probe; not a WebSocket endpoint. | | `/echo` | Echo every text/binary message verbatim, preserving kind and boundaries. The closing handshake is echoed too: a client close frame is acknowledged with the same code and reason. | | `/reject` | Answer the upgrade with HTTP 403: the client observes a failed handshake. | +| `/redirect` | Answer the upgrade with HTTP 302 `Location: /echo`: clients must not follow (a client that did would reach a working echo endpoint and expose itself). | | `/stall` | Never answer the handshake (held up to 120 s): the client's connect bound must fire. **Holds a pending handshake**: browsers serialize in-flight WebSocket handshakes per endpoint, so concurrent connects to the same host:port queue behind it. | | `/close-after?count=N&code=C&reason=R` | Echo `N` messages (default 0), then the server initiates the close with code `C` and reason `R`; `code` omitted sends a code-less close frame (observed as 1005). Drains until the handshake completes. | | `/burst-then-close?count=N&size=S&code=C&reason=R` | Immediately send `N` binary messages (default 1) of `S` bytes (default 16), then a close frame as in `/close-after`. | diff --git a/conformance/server/src/lib.rs b/conformance/server/src/lib.rs index 2cf2ae0..159cf68 100644 --- a/conformance/server/src/lib.rs +++ b/conformance/server/src/lib.rs @@ -210,6 +210,18 @@ async fn handle_request(mut req: Request) -> anyhow::Result bool { self.overflowed.load(Ordering::SeqCst) } + + /// Latch the overflow directly (the transport rejected a message past + /// its cap before the budget could account it). + fn latch_overflow(&self) { + self.overflowed.store(true, Ordering::SeqCst); + } } /// A connection's inbound-message queue: the receiving half of the pump's @@ -285,7 +291,7 @@ pub(crate) struct ConnectConfig { } /// Validate a connect URL per the WIT contract: absolute `ws:`/`wss:`, no -/// fragment. +/// fragment, no userinfo. fn validate_url(url: &str) -> Result<(), String> { if url.contains('#') { return Err("URL must not have a fragment".to_string()); @@ -301,6 +307,14 @@ fn validate_url(url: &str) -> Result<(), String> { if uri.host().is_none_or(str::is_empty) { return Err("URL must have a host".to_string()); } + // The WHATWG WebSocket constructor rejects credentials in the URL; the + // eager taxonomy matches that floor uniformly. + if uri + .authority() + .is_some_and(|authority| authority.as_str().contains('@')) + { + return Err("URL must not have userinfo".to_string()); + } Ok(()) } @@ -392,9 +406,24 @@ impl Websocket { ); } + // The transport's own message/frame caps scale with the configured + // buffer bound instead of tungstenite's fixed defaults, so an + // embedder raising the bound cannot make the transport reject + // messages a browser-backed host would deliver (and the budget + // would overflow-close). Messages in (bound, cap] take the normal + // budget-overflow path; past the cap, the capacity error is mapped + // onto the same overflow taxonomy in the pump. + let transport_cap = config + .max_inbound_buffer_bytes + .saturating_mul(2) + .max(64 * 1024 * 1024); + let ws_config = tokio_tungstenite::tungstenite::protocol::WebSocketConfig::default() + .max_message_size(Some(transport_cap)) + .max_frame_size(Some(transport_cap)); + let (ws, response) = match tokio::time::timeout( config.connect_timeout, - tokio_tungstenite::connect_async(request), + tokio_tungstenite::connect_async_with_config(request, Some(ws_config), false), ) .await { @@ -712,6 +741,18 @@ impl Pump { let _ = self.bounded_write(ws.flush()).await; } Some(Ok(_)) => {} + Some(Err(tungstenite::Error::Capacity(_))) => { + // The transport rejected a message past its cap + // (which scales above the buffer bound), so the + // guest-observable outcome is the overflow + // contract, same as a message the budget rejected. + // The read stream is compromised mid-frame; close + // toward the peer and tear down rather than keep + // reading. + self.budget.latch_overflow(); + self.begin_close(&mut ws, None).await; + break; + } // A read error or EOF is the transport's verdict either // way; `peer_frame` already records whether a close // frame arrived first. diff --git a/wit/websocket.wit b/wit/websocket.wit index df34d1b..5161614 100644 --- a/wit/websocket.wit +++ b/wit/websocket.wit @@ -19,9 +19,9 @@ interface types { /// contents, and do not expect them to be non-empty — some /// implementations cannot observe failure details. variant error { - /// The supplied URL is not an absolute `ws:` or `wss:` URL without a - /// fragment. `connections.websocket.connect` fails with this eagerly, - /// before any network activity. + /// The supplied URL is not an absolute `ws:` or `wss:` URL, or has + /// a fragment or userinfo. `connections.websocket.connect` fails + /// with this eagerly, before any network activity. invalid-url(string), /// The connection attempt failed: name resolution, TCP, TLS, or the /// HTTP upgrade handshake (including a server whose subprotocol @@ -170,10 +170,11 @@ interface connections { resource websocket { /// Open a WebSocket connection. /// - /// `url` must be an absolute `ws:` or `wss:` URL without a fragment; - /// anything else fails `invalid-url` eagerly. `protocols` is the - /// subprotocol offer, possibly empty; a malformed offer (an invalid - /// token, or a duplicate entry) fails `invalid-argument` eagerly. + /// `url` must be an absolute `ws:` or `wss:` URL without a + /// fragment or userinfo; anything else fails `invalid-url` eagerly. + /// `protocols` is the subprotocol offer, possibly empty; a + /// malformed offer (an invalid token, or a duplicate entry) fails + /// `invalid-argument` eagerly. /// /// The returned future resolves once the WebSocket handshake has /// completed and the connection is open, or with `connect-failed`