feat(moq-net): moq-lite-05 SETUP message + PATH parameter#1954
Merged
Conversation
Reintroduce the moq-lite Setup stream (removed in draft-03) for
moq-lite-05, carrying a SETUP message with an extensible parameter bag,
and wire the PATH parameter end to end so a client can advertise a
request path and the server can authorize on it before serving.
All of this is gated behind the opt-in `Lite05Wip` version (not in
`Versions::all()` or `ALPNS`), so the change is purely additive and does
not alter any shipping version's wire format or public API. Targets main
per request; kept leaner than the dev implementation (no ProbeLevel,
PeerSetup, or connecting.rs).
moq-net:
- lite/setup.rs: `Setup { path: Option<String> }` (size-prefixed Message
with a TLV parameter bag, PATH = 0x2; unknown params ignored),
`send_setup` (opens the uni `DataType::Setup` stream), and
`accept_setup` (server eagerly reads the client SETUP, resetting any
data stream that races ahead).
- `lite::Version::has_setup_stream()` gates the new behavior.
- `lite::start` gains an `our_setup` argument; the subscriber drains the
peer's Setup stream.
- Public, additive API: `Client::with_path()`, and
`Server::accept_request() -> Request` exposing `path()`,
`with_publish`/`with_consume`/`with_stats`, `ok()`, and `close()`
(modeled on the WebTransport Request). `accept()` is now a convenience
over `accept_request().ok()`.
The path is optional and wire-faithful at moq-net: it is omitted on
URL-carrying bindings, a server never sends one, and `path()` is `None`
when nothing was advertised in band. The `/` default lives in
moq-native, which owns the dial URL.
moq-native: classify each transport by scheme. WebTransport
(https/http), WebSocket (ws/wss), and iroh (h3 WebTransport) carry the
path in their request; the URL-less schemes (raw QUIC moqt/moql, qmux
over tcp/unix) advertise it in the SETUP via `Client::with_path`,
defaulting an empty path to `/`.
moq-relay: the unauthenticated internal listener (qmux over TCP/UDS, the
URL-less server path) now reads the client's in-band path via
`accept_request` and scopes its full internal access to that subtree.
No path means the empty root, as before.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @kixelated, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR adds moq-lite-05 Setup stream support and request-path propagation. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rs/moq-native/src/client.rs`:
- Around line 286-291: The MoQ SETUP path is incorrectly derived from url.path()
in the client setup flow, which leaks the Unix socket filesystem path and uses
the wrong request root for unix:// URLs. Update the path handling in the
relevant client methods that call with_path (including the setup/connect paths
referenced in this diff) so that Unix socket URLs do not advertise their
filesystem path as the MoQ request path; instead derive a proper request
path/root for SETUP while preserving the existing HTTP-style default behavior
for non-Unix URLs.
- Around line 269-291: The path handling in connect_client currently treats all
iroh URLs as already carrying the request path, which drops the SETUP path for
raw moq ALPN sessions. Update the transport check so only WebTransport-style
iroh sessions bypass with_path(), and keep applying the URL path for raw iroh
connections created via crate::iroh::connect and Session::raw; use the existing
connect_client and transport_carries_path logic to split the iroh case
appropriately.
In `@rs/moq-net/src/client.rs`:
- Around line 60-61: The public builder method client::Setup::with_path
currently stores whatever string it receives, but Setup::path is expected to be
non-empty and slash-prefixed. Update with_path to normalize the provided path
before assigning it to self.path, ensuring the stored value always matches the
documented SETUP path format. Keep the fix localized to with_path so all callers
get the normalized path consistently.
In `@rs/moq-net/src/lite/session.rs`:
- Around line 59-62: The SETUP send path in session::spawn currently only logs
and continues when send_setup fails, which leaves Lite05 peers waiting forever
in accept_setup. Update the async block around send_setup to treat this as a
fatal session setup failure: after the failed send, close/terminate the session
and avoid returning a usable session. Use the existing session, send_setup, and
tracing::warn call site to locate the change, and make sure the failure path
tears down the session consistently.
In `@rs/moq-net/src/lite/setup.rs`:
- Around line 31-33: The PATH handling in the setup request path validation only
checks for empty strings, so non-absolute values like "foo" can still pass
through encode/decode and reach auth scoping unchanged. Update the path
validation in setup.rs (the setup request encoding/decoding flow around the PATH
parameter, including accept_request() handling) to reject any value that does
not start with "/" on both sides of the wire boundary, and add a regression test
covering a non-slash-prefixed PATH value.
In `@rs/moq-net/src/server.rs`:
- Around line 73-75: Move the empty-origin warning out of `accept_request()` in
`Request::accept_request` and into `Request::ok()`, since `accept_request()` now
returns a builder-like `Request` that callers may still populate with
publish/consume origins after checking `path()`. Keep the warning check on
`publish`/`consume`, but run it only when finalizing the request in `ok()` so
scoped accepts don’t emit false “not publishing or consuming anything” logs.
In `@rs/moq-relay/src/internal.rs`:
- Around line 220-221: The unrestricted token is being minted from the raw
request path, which may not match the relay’s canonical root handling. In the
internal request flow around the `request.path()` to `AuthToken::unrestricted`
conversion, normalize the advertised URL path through the same URL-path-to-root
resolver used for authenticated relay requests, and reject invalid roots before
creating the token. Keep the fix local to the path-to-root logic in this code
path so the `root` passed into `AuthToken::unrestricted` is always canonical.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 29a630d3-1fe9-4c7c-9a16-1731f7462c02
📒 Files selected for processing (11)
rs/moq-native/src/client.rsrs/moq-net/src/client.rsrs/moq-net/src/lite/mod.rsrs/moq-net/src/lite/parameters.rsrs/moq-net/src/lite/session.rsrs/moq-net/src/lite/setup.rsrs/moq-net/src/lite/stream.rsrs/moq-net/src/lite/subscriber.rsrs/moq-net/src/lite/version.rsrs/moq-net/src/server.rsrs/moq-relay/src/internal.rs
Address review feedback on the lite-05 SETUP wire: - Reject a non-absolute PATH (not beginning with `/`) on both encode and decode, not just empty. URL-carrying transports can never produce a relative path, and the relay scopes auth from it, so it must not reach `accept_request()`. Adds a regression test. - Close the session when `send_setup` fails. The peer gates serving on receiving our SETUP, so logging and continuing would leave it waiting in `accept_setup` while this side held a usable session. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
PATH stays a moq-layer concern (the lite-05 SETUP), not a transport one: a raw iroh or raw QUIC session carries no request URI, so every client must be able to advertise PATH via MoQ regardless of transport. Drop the attempt to read it from the transport and fix the two URL-derivation bugs CodeRabbit flagged. moq-native: - Don't advertise a `unix://` URL's path: it's the filesystem socket path, not a request namespace. Default it to `/` and add `Client::connect_with_path(url, path)` to set the namespace explicitly. - Treat `iroh` as URL-less so it always advertises the path in band: its raw mode carries no request URI (only the HTTP/3 mode does), and sending it for an h3 session is harmless. moq-net: - `Client::with_path` normalizes to an absolute path (empty -> `/`, prepends a leading `/`). - Move the empty-origin warning from `accept_request` to `Request::ok`, so scoped accepts that attach origins after inspecting `path()` don't log a false positive. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ect_with_path Replace `Client::connect_with_path` with a `?path=` query parameter, so a single `connect(url)` / `reconnect(url)` covers every case and the path stays fully URL-driven. - `request_path()` derives the moq SETUP path from the dial URL: a `?path=` query wins (the only way to set one on `unix://`, whose URL path is the socket file), otherwise the URL path component (`tcp`, raw QUIC, `iroh`). A `unix://` URL with no `?path=` has no namespace. - Removes the awkward `connect_with_path` entry point. Also box `RequestKind::WebSocket` to match the other variants and silence the pre-existing large_enum_variant lint. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds moq-lite-05 SETUP support with a PATH parameter, kept entirely at the MoQ layer.
A raw iroh or raw QUIC session carries no request URI, so every moq-lite/moq-transport client must be able to advertise its path via MoQ regardless of transport — there is no transport-level dependency here. Everything is gated behind the opt-in
Lite05Wipversion (not inVersions::all()orALPNS), so this is purely additive: no shipping version's wire format or public API changes.What changed
moq-netlite/setup.rs(new):Setup { path: Option<String> }— a size-prefixedMessagewrapping a TLV parameter bag (PATH = 0x2; unknown IDs ignored). The PATH is rejected on both encode and decode unless it begins with/.send_setupopens the unidirectionalDataType::Setup(0x1) stream and sends one SETUP then FINs (closing the session if the send fails, so a peer doesn't hang inaccept_setup).accept_setuplets the server read the client's SETUP before serving, resetting any data stream that races ahead.lite::Version::has_setup_stream()gates the behavior;lite::startgainedour_setup; the subscriber drains the peer's Setup stream.Client::with_path(impl Into<String>)— normalized to an absolute path.Server::accept_request() -> Requestexposingpath() -> Option<&str>,with_publish/with_consume/with_stats,ok(),close()(WebTransport-Requeststyle).accept()is a convenience overaccept_request().ok().moq-native(client supplies the path, fully URL-driven)The request path is derived from the dial URL — no separate API:
tcp://h/anycast,moqt://h/anycast,iroh://node/anycast,https://h/anycast— the URL path component.unix:///run/moq.sock?path=anycast— a?path=query, because aunix://URL's path is the socket file, not a namespace.?path=works as an override on any scheme but is only required forunix://.The path is advertised in the moq SETUP for transports with no request URI of their own (raw QUIC, qmux over TCP/UDS, and raw iroh — its h3 mode carries one, but sending it there is harmless). WebTransport (
https/http) and WebSocket (ws/wss) convey the path in their own request, so it's omitted there.moq-relay(server consumes the path)The unauthenticated internal listener (qmux over TCP/UDS — the URL-less server path) reads
request.path()and scopes its full internal access to that subtree (no path → empty root, as before). WebTransport/WebSocket continue to derive the path from the transport request.Public API changes
moq_net::Client::with_path,moq_net::Server::accept_request,moq_net::Request(path/with_publish/with_consume/with_stats/ok/close).moq_net::Server::acceptunchanged for callers (delegates toaccept_request().ok()). No newmoq-nativepublic methods —connect(url)/reconnect(url)are unchanged.Test plan
moq-netunit tests: SETUP round-trip, unknown-param passthrough, non-absolute-path rejection,accept_requestpath surfacing, racing-Group skip — 363 lib tests pass.moq-nativetests: transport classification +request_pathderivation (incl.unix://?path=); lib tests pass.moq-relaybuilds.cargo fmt(via nix) clean on the three crates.Follow-ups (not in this PR)
js/net: moq-lite-05 SETUP-stream send/read only — no path (browsers use WebTransport, which carries it).doc/concept+doc/bin/relayfor the internal-listener path scoping.(Written by Claude Opus 4.8)