feat(uring): speak WebTransport for browser peers - #3081
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 937a2b6e71
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let n = std::future::poll_fn(|cx| web_transport_trait::poll::RecvStream::poll_read(recv, cx, &mut chunk)).await?; | ||
| match n { | ||
| Some(n) => { | ||
| buf.extend_from_slice(&chunk[..n]); |
There was a problem hiding this comment.
When an unauthenticated peer declares an arbitrarily large SETTINGS or CONNECT frame and continues supplying an incomplete body, both handshake loops repeatedly append here without enforcing a size limit; because reads keep the connection active and replenish flow-control credit, a client can grow this BytesMut until the process exhausts memory. Reject handshake frames once a reasonable configured limit is exceeded. (Written by GPT-5.6 Sol)
Useful? React with 👍 / 👎.
| // A capsule split across DATA frames is legal but nothing we talk to | ||
| // does it; treat a partial capsule as unparseable. | ||
| let mut payload: &[u8] = &peek[..len]; | ||
| let capsule = proto::Capsule::decode(&mut payload).map_err(|err| Error::Web(err.to_string()))?; |
There was a problem hiding this comment.
When a peer splits a CloseWebTransportSession capsule across HTTP/3 DATA frames, which Capsule Protocol permits, decoding only the first frame's payload returns an incomplete-capsule error. read_capsules then treats that as no capsule and closes with code 0, losing the peer's close code and reason; the parser should treat DATA payloads as one continuous capsule byte stream and also preserve multiple capsules carried in one DATA frame. (Written by GPT-5.6 Sol)
Useful? React with 👍 / 👎.
937a2b6 to
b093dcf
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b093dcf296
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| response.encode(&mut buf).map_err(|err| Error::Web(err.to_string()))?; | ||
| write_all(&mut self.send, &buf).await?; | ||
| web_transport_trait::poll::SendStream::finish(&mut self.send)?; | ||
| Ok(()) |
There was a problem hiding this comment.
When a server calls reject, returning here immediately drops the consumed Request; its unfinished local control stream and held peer control/QPACK receive streams then invoke the underlying stream Drop implementations, sending RESET_STREAM/STOP_SENDING. HTTP/3 treats attempts to close these critical streams as a connection error, so the client can receive an H3 connection failure instead of the intended rejected CONNECT response. Retain the critical streams until the response is delivered and shut the connection down deliberately.
Useful? React with 👍 / 👎.
| // The CONNECT stream ending closes the session with no error. | ||
| _ => conn.shared().close_code(proto::error_to_http3(0), ""), |
There was a problem hiding this comment.
When a peer sends a valid extension or greased capsule, any decoded non-close capsule reaches this fallback and closes the session with code 0; if the dependency rejects the unknown type, the parse-error path has the same result. Capsule Protocol requires unknown capsule types to be silently ignored, so an otherwise interoperable client can have its session terminated before a later close capsule is read. Continue scanning after unsupported capsule types rather than treating them as end-of-session.
Useful? React with 👍 / 👎.
b093dcf to
5b5362f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b5362fb07
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Poll::Ready(UniClass::Control | UniClass::Qpack) => { | ||
| web.state.borrow_mut().held_recv.push(stream.recv); |
There was a problem hiding this comment.
After establishment, a peer can repeatedly open finished unidirectional streams with control or QPACK type headers; every such stream is appended to held_recv, which is never drained or capped. Because completed QUIC streams restore stream credit, one active peer can repeat this indefinitely and grow the process heap despite the configured concurrent-stream limit. Track the single permitted stream of each critical type and terminate or otherwise bound the connection on duplicates instead of retaining every arrival. (Written by GPT-5.6 Sol)
Useful? React with 👍 / 👎.
| let mut framed = Vec::with_capacity(web.header_datagram.len() + payload.len()); | ||
| framed.extend_from_slice(&web.header_datagram); | ||
| framed.extend_from_slice(payload); | ||
| web_transport_trait::poll::Session::poll_send_datagram(&mut self.conn, cx, &framed) |
There was a problem hiding this comment.
When the peer closes a WebTransport session with a nonzero code, the capsule reader closes QUIC using error_to_http3(code), so a subsequent datagram send returns an Error::App containing that mapped HTTP/3 value. This branch returns the error unchanged, unlike receive/open/stream operations, and session_error() can consequently clamp the large mapped value to u32::MAX instead of reporting the peer's original close code. Apply unmap_err to the result before returning it. (Written by GPT-5.6 Sol)
Useful? React with 👍 / 👎.
5b5362f to
a3144f4
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a3144f4c1b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| inner: super::SendStream, | ||
| /// Header bytes still owed to the wire before any payload. | ||
| prefix: Bytes, | ||
| web: bool, |
There was a problem hiding this comment.
When a web-mode SendStream or RecvStream is dropped unfinished, the wrapper drops its inner stream without applying the WebTransport error mapping, so the inner Drop sends raw code 0 instead of error_to_http3(0). This makes routine application cancellation appear to browsers as an HTTP/3 stream error rather than a WebTransport cancellation; add wrapper drop handling that maps code 0 before the inner handle runs its fallback. (Written by GPT-5.6 Sol)
Useful? React with 👍 / 👎.
| let wrote = send.try_write(&frame); | ||
| if wrote == frame.len() { | ||
| let _ = web_transport_trait::poll::SendStream::finish(&mut send); | ||
| } |
There was a problem hiding this comment.
When connection or stream flow-control capacity is exhausted, or the close reason is larger than the available capacity, try_write can write only part of this frame. The unfinished send is then dropped and resets the CONNECT stream immediately, so the browser cannot decode the close capsule and loses the intended close code or reason; keep the stream alive and asynchronously finish the capsule until the grace deadline instead of abandoning every partial write. (Written by GPT-5.6 Sol)
Useful? React with 👍 / 👎.
a3144f4 to
a50ef4d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a50ef4d7cb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// Accept with a `200`, selecting `protocol` from the peer's | ||
| /// [`protocols`](Self::protocols) (or none, for peers that negotiate in | ||
| /// band instead). | ||
| pub async fn respond(mut self, protocol: Option<&str>) -> Result<Session, Error> { |
There was a problem hiding this comment.
The new public respond API exposes its sole optional setting as a positional parameter. When successful CONNECT responses need another header or policy knob, extending this signature will break consumers or produce an awkward parameter list. Introduce a future-proof response options type now while this API is new. (Written by GPT-5.6 Sol)
AGENTS.md reference: AGENTS.md:L167-L167
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| /// Refuse with `status`, ending the handshake. | ||
| pub async fn reject(mut self, status: http::StatusCode) -> Result<(), Error> { |
There was a problem hiding this comment.
External callers of reject must add their own matching http dependency to construct this argument, coupling the public API to a third-party crate and its selected major version. Use a project-owned rejection type, or re-export http and document that dependency as part of the compatibility contract. (Written by GPT-5.6 Sol)
AGENTS.md reference: AGENTS.md:L171-L171
Useful? React with 👍 / 👎.
| let recv = accept_uni(&mut conn).await?; | ||
| let mut pending = PendingUni::new(recv); | ||
| let class = std::future::poll_fn(|cx| pending.poll_classify(cx)).await; |
There was a problem hiding this comment.
If an early WebTransport unidirectional stream sends its type but stalls before completing the session-ID varint, awaiting its classification here prevents the loop from accepting a fully available control stream with SETTINGS. A peer can therefore keep the handshake stuck indefinitely while continuing to keep the QUIC connection active, and the stream-count bound never applies because only this one stream has been adopted. Track and poll partially classified streams concurrently, as the established-session path does. (Written by GPT-5.6 Sol)
Useful? React with 👍 / 👎.
| let n = self.inner.try_write(&self.prefix); | ||
| self.prefix.advance(n); | ||
| if !self.prefix.is_empty() { | ||
| return Err(Error::Web("no capacity to frame the stream before finishing".into())); |
There was a problem hiding this comment.
When connection-level flow-control credit is exhausted, finishing a newly opened empty WebTransport stream makes try_write return zero and reports a terminal Error::Web, even though capacity may become available normally after the peer advances flow control. Callers generally drop the stream after this unexpected failure, resetting a stream that should have finished cleanly. Ensure the prefix is queued before returning the opened stream, or retain enough state to complete it asynchronously instead of converting transient backpressure into an error. (Written by GPT-5.6 Sol)
Useful? React with 👍 / 👎.
quic::web::Request runs the server side of the HTTP/3 CONNECT handshake over the raw quiche adapter using web-transport-proto: SETTINGS exchange, CONNECT with URL and subprotocol selection, and the capsule close carrying codes and reasons to WebTransport.closed. quic::web::Session is the one transport type the runtime drives, wrapping raw QUIC (Session::raw) and WebTransport in the same machinery: web mode frames streams and datagrams with the session id, tolerates pipelined peers whose streams outrun the CONNECT response, holds the peer's control and QPACK streams open, and maps stream and close codes through the HTTP/3 error space both ways. Validated against web-transport-quinn (the stack browsers interop with): stream and datagram echo through the framing, close codes through the capsule, and a full moq-lite session over WebTransport. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XLJU7rEfPe7hn7K5jX77mb
Two problems on the CONNECT path, both reachable before a peer has authenticated anything. The handshake buffers grew without limit. A peer declares an HTTP/3 frame's length before sending its body, and reading what does arrive replenishes its flow-control credit, so announcing an enormous SETTINGS, CONNECT, or capsule frame and then dribbling it grew a `BytesMut` until the process ran out of memory. `read_some` now refuses past 64 KiB, which is far more than anything parsed here needs. The stream count before the control stream arrives is bounded the same way: a peer that never sends one could otherwise open unidirectional streams without end, since each is held or parked as an early WebTransport stream. Capsules were read one DATA frame at a time. HTTP/3 framing and the Capsule Protocol are independent layers: a capsule may span DATA frames and one frame may carry several, so a peer that split its `CloseWebTransportSession` across frames lost its close code and reason to a "close with 0", and a second capsule in one frame was dropped with the frame. `Capsules` splits the framing off and parses the concatenated payloads as the one continuous byte stream they are, skipping non-DATA frames and now also skipping GREASE and unknown capsule types rather than stopping at them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
a50ef4d to
82e4d52
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82e4d52037
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let mut early = Vec::new(); | ||
| loop { | ||
| if held.len() + early.len() >= HANDSHAKE_STREAMS { | ||
| return Err(Error::Web("too many streams before the control stream".into())); |
There was a problem hiding this comment.
When a peer reaches this stream cap, or triggers any later SETTINGS/CONNECT validation error, Request::accept returns without closing conn. In the inspected Endpoint::launch lifecycle, dropping the public Connection does not close QUIC: the endpoint retains its shared state until the driver observes a terminal connection state, and the backlog no longer counts the connection after it has been accepted. A peer can therefore keep each rejected handshake alive with valid traffic to prevent the idle timeout, causing endpoint connections and driver tasks to accumulate without bound; close the underlying connection on every handshake failure path. (Written by GPT-5.6 Sol)
AGENTS.md reference: AGENTS.md:L110-L110
Useful? React with 👍 / 👎.
1 participant
Summary
M4 PR 3 on #2875, stacked on #3078: the WebTransport-over-quiche adapter the epic's M4 requires ("implement the WebTransport-over-quiche adapter from M3c as
web_transport_trait::pollsession and stream types"). A browser cannot speak raw QUIC, so without this the relay integration could only serve native peers; with it,h3connections get the HTTP/3 CONNECT handshake on the same adapter #3033 built.quic::web::Requestruns the server handshake usingweb-transport-proto's sans-IO state machines over the existing poll streams (its async helpers want tokio I/O, so decoding is incremental over buffered bytes): SETTINGS exchange, the CONNECT request with its URL (what the relay routes and authenticates on), and subprotocol selection, the WebTransport equivalent of ALPN that carries the moq version, mirroring the moq-tokio dispatch.quic::web::Sessionis now the runtime's one transport type (Handle::Transportchanged fromquic::Connection), following web-transport-quinn'sSession::rawpattern rather than a delegation enum: raw QUIC and WebTransport run the same machinery, with the layering compiled down to anOption. Web mode frames opened streams (0x54/0x41+ session id, written lazily ahead of the first payload byte), classifies accepted streams by reading header varints exactly (never past them), prefixes and filters datagrams, holds the peer's control and QPACK streams open for the session's life, and maps stream/close codes through the HTTP/3 error space in both directions.close(code, reason)writes aCloseWebTransportSessioncapsule in a DATA frame on the CONNECT stream (whatWebTransport.closedreads), then closes the connection after a one-second grace or the peer acting, whichever first. Inbound, a capsule reader task surfaces the peer's code and reason as the session error, so moq's close codes survive the H3 mapping round trip; the echo test asserts code 42 and its reason arrive intact.Validation
tests/web.rsruns interop against web-transport-quinn, the reference stack browsers interoperate with, on a tokio thread dialing the uring server:All 27 moq-uring tests pass;
just checkandjust testare green. Existing raw-QUIC call sites wrap withSession::raw(deliberate API change to the unreleased runtime surface).Notes
moq-tokioandweb-transport-quinnjoin as Linux dev-dependencies only (the client side of the interop tests); the library gainsweb-transport-proto,http, andurl.(Written by Claude Fable 5)
🤖 Generated with Claude Code
https://claude.ai/code/session_01XLJU7rEfPe7hn7K5jX77mb