feat: plumb connection path through Session and Coordinator#145
feat: plumb connection path through Session and Coordinator#145englishm merged 6 commits intocloudflare:mainfrom
Conversation
c5c1b29 to
f9b83a1
Compare
c8d1a89 to
56c51c2
Compare
Add Transport enum (WebTransport | RawQuic) determined at the QUIC layer (ALPN negotiation) and threaded through Session, convenience wrappers, and all call sites. Extract and validate the connection path from WebTransport URL and/or CLIENT_SETUP PATH parameter (key 0x1) during Session::accept() and Session::connect(), storing it on the Session struct with public accessor connection_path(). Server-side (accept): - Extract path from WebTransport CONNECT URL (takes precedence) - Fall back to CLIENT_SETUP PATH parameter for raw QUIC connections - Validate: max 1024 bytes, must start with /, no dot segments, no percent-encoded characters, no empty segments Client-side (connect): - Auto-extract path from session URL - Send as PATH parameter in CLIENT_SETUP for raw QUIC connections only (WebTransport already carries the path in the HTTP/3 CONNECT URL) The connection path enables multi-tenant relay deployments where different paths map to different application scopes. Coordinator implementations can use it to scope registrations and lookups to the correct application context.
Add scope: Option<&str> parameter to Coordinator::register_namespace(), unregister_namespace(), and lookup() (breaking trait change). This enables namespace isolation — the same namespace in different scopes can route independently, supporting multi-tenant relay deployments. Thread the resolved scope through the relay plumbing: - Relay extracts scope from session.connection_path() after accept/connect - Producer and Consumer store scope and pass it to coordinator calls - RemotesConsumer::route() accepts scope for upstream lookups - Locals keyed by (scope, namespace) using a two-level HashMap for O(1) scope lookup without heap allocation on the hot path Forward connections derive scope from the announce URL path. The forward session intentionally skips scope resolution (operator-configured, not client-supplied) and always gets full read-write access. In-tree FileCoordinator updated with scoped file storage. ApiCoordinator accepts the scope parameter but ignores it (documented limitation — the external moq-api registry has no scope concept).
…sessions Add ScopeInfo and ScopePermissions types that model the resolved application scope for a connection. The new resolve_scope() method on the Coordinator trait translates a raw connection path into a scope identity (scope_id) and permissions (ReadWrite or ReadOnly). The relay now calls resolve_scope() on session accept and gates Producer/Consumer creation on the returned permissions: - Producer (serves SUBSCRIBEs) requires can_subscribe permission - Consumer (handles PUBLISH_NAMESPACEs) requires can_publish permission If scope resolution fails, the session is rejected with a QUIC APPLICATION_CLOSE (code 0x3 / PROTOCOL_VIOLATION) so the client gets a meaningful error rather than an abrupt reset. The default resolve_scope() implementation passes the connection path through as scope_id with ReadWrite permissions, preserving existing behavior. Coordinator implementations can override to implement custom path-to-scope mapping and permission resolution. This enables future support for multi-path scopes where different connection paths (e.g., publisher vs subscriber URLs) can map to the same scope with different permission levels.
…nd lingering subscriber support Add new types and trait methods to prepare the Coordinator API surface for upcoming protocol features. All new methods have default implementations that return no-op/empty results, so existing implementors compile without changes. New types: - ScopeConfig: Per-scope configuration (origin_fallback, lingering_subscribe) - NamespaceSubscription: RAII handle for SUBSCRIBE_NAMESPACE with existing matches - NamespaceInfo: Identity of a registered namespace - RelayInfo: Relay endpoint for forwarding notifications - TrackRegistration: RAII handle for track-level PUBLISH - TrackEntry: Identity of a registered track - TrackSubscription: RAII handle for lingering subscriber interest New Coordinator methods: - get_scope_config(scope): Get per-scope configuration - subscribe_namespace(scope, prefix): Register interest in namespace prefix - unsubscribe_namespace(scope, prefix): Remove namespace prefix interest - lookup_namespace_subscribers(scope, namespace): Find interested relays - register_track(scope, namespace, track): Register track availability - unregister_track(scope, namespace, track): Remove track registration - list_tracks(scope, namespace): List tracks under a namespace - subscribe_track(scope, namespace, track): Pre-register track interest - unsubscribe_track(scope, namespace, track): Remove track interest - lookup_track_subscribers(scope, namespace, track): Find waiting subscribers These stubs align with moq-worker RPCs (GetScopeConfig, RegisterSubscribeNamespace, ListNamespaces, LookupSubscribers, RegisterTrack, ListTracks, etc.) and enable the relay layer to be wired up once PUBLISH_NAMESPACE/SUBSCRIBE_NAMESPACE protocol support lands. Includes comprehensive test suite with MockCoordinator (full in-memory reference implementation) and MinimalCoordinator (verifies all default implementations). 38 test cases covering type construction, scope resolution, namespace operations, track registration, subscriber tracking, scope isolation, and multi-relay scenarios.
Replace the flat global namespace keying in ApiCoordinator with a
collision-free key format that properly isolates scopes and handles
arbitrary bytes in namespace tuple fields.
Registry keys hex-encode each namespace tuple field (preserving
arbitrary bytes without ambiguity) and join with "." as a field
separator. The scope is prepended with ":" as a delimiter:
Scoped: "{scope}:{hex_field0}.{hex_field1}..."
Unscoped: ":{hex_field0}.{hex_field1}..."
This is collision-free because:
- Hex output is [0-9a-f] only, so "." and ":" cannot appear in
encoded fields
- Different tuple field counts produce different keys
- Scoped and unscoped keys have distinct prefixes (scopes start
with "/", unscoped starts with ":")
- Arbitrary bytes in namespace fields round-trip correctly through
hex encoding (unlike to_utf8_path() which is lossy)
56c51c2 to
719e021
Compare
| /// Handle that unregisters a namespace when dropped and manages TTL refresh | ||
| struct NamespaceUnregisterHandle { | ||
| namespace: TrackNamespace, | ||
| namespace_key: String, |
There was a problem hiding this comment.
why are we switching to String instead of TrackNamespace?
There was a problem hiding this comment.
TrackNamespaces can collide between scopes, so the coordinator implementation needs its own identifier that won't collide across scopes. What that key looks like is up to the relay implementor.
There was a problem hiding this comment.
But strings can also collide across scopes, No?
There was a problem hiding this comment.
Jacob is right that the coordinator implementation needs a key that's collision-free across scopes. The String here is a composite key built from the scope + namespace, not a bare namespace. You can see the construction below in registry_key():
- Each namespace tuple field is hex-encoded, fields joined with
. - Scope is prepended with
:as separator - Unscoped keys get a leading
:prefix so they can't collide with scoped keys
We do lose some type safety here compared to a structured key type (like a RegistryKey { scope: Option<String>, namespace: TrackNamespace }). I went with the string approach because the ApiCoordinator is an example coordinator, and a more complex type felt like overkill for that, but it's a tradeoff worth acknowledging. If you think this warrants something more we could add it.
| #[derive(Clone)] | ||
| pub struct Locals { | ||
| lookup: Arc<Mutex<HashMap<TrackNamespace, TracksReader>>>, | ||
| lookup: Arc<Mutex<HashMap<ScopeKey, HashMap<TrackNamespace, TracksReader>>>>, |
There was a problem hiding this comment.
One doubt: since the lock will be held now on Scope, won't this cause lock contention and delays for tracknamespace under load?
There was a problem hiding this comment.
I don't think this is any worse than it was before, but I agree it could be an optimization to have another lock at the second layer for high relays with lots of traffic in a single scope
There was a problem hiding this comment.
Agreed with Jacob's analysis. The lock granularity is unchanged - it's the same single Mutex wrapping the whole structure. Before: one HashMap lookup under the lock. After: two HashMap lookups under the same lock (scope then namespace). The critical section duration is effectively the same.
A more granular locking strategy could be a nice optimization for relays with lots of concurrent scopes, but I think it's orthogonal to this change and probably not worth the complexity until we see contention in practice.
moq-relay-ietf/src/relay.rs
Outdated
| producer: if can_subscribe { | ||
| publisher.map(|publisher| Producer::new(publisher, locals.clone(), remotes, scope_id.clone())) | ||
| } else { | ||
| None | ||
| }, | ||
| consumer: if can_publish { | ||
| subscriber.map(|subscriber| Consumer::new(subscriber, locals, coordinator, forward, scope_id)) | ||
| } else { | ||
| None |
There was a problem hiding this comment.
If producer is None, does that mean all the publishing control message won't be handled and the session will look like in a hang state? Or are handling those control message explicitly?
There was a problem hiding this comment.
Good question, and it touches on one of the more confusing parts of our codebase. Let me walk through it.
Naming map:
- Relay
Producerwraps transportPublisher- handles incoming SUBSCRIBEs from the peer (the relay produces data for subscribers) - Relay
Consumerwraps transportSubscriber- handles incoming PUBLISH_NAMESPACEs from the peer (the relay consumes data from publishers)
What happens with ReadOnly permissions:
can_subscribe = true→ relayProducerIS created (handles SUBSCRIBEs - correct, ReadOnly clients can subscribe)can_publish = false→ relayConsumeris NOT created (won't process PUBLISH_NAMESPACEs - correct in a sense sinceReadOnlyclients shouldn't publish)
So Producer is always present for ReadOnly. The comment in the code about it being "forward-looking for a potential future WriteOnly variant" is referring to the symmetric case where Producer would be None.
But in looking at this more closely I did notice a real gap: if a ReadOnly peer sends an ANNOUNCE anyway, the transport layer accepts the message (both transport halves always exist - MoQT draft-14 doesn't have role negotiation, so every session is effectively PubSub at the transport level), but with no relay Consumer running, nobody drains the queue. The peer gets silence - no PUBLISH_NAMESPACE_OK, no error. Same thing would happen in the WriteOnly direction if we added that variant.
The right behavior would be to actively reject unauthorized messages. The transport types already have the right machinery for this: Announced sends PUBLISH_NAMESPACE_ERROR automatically on drop (announced.rs:89-108), and Subscribed sends SUBSCRIBE_ERROR on drop (subscribed.rs:162-188). So a fix could be something like a small drain-and-reject loop in session.rs or relay.rs:
// When consumer is None, reject any ANNOUNCEs
async fn reject_announces(mut subscriber: Subscriber) {
while let Some(announced) = subscriber.announced().await {
tracing::debug!(namespace = %announced.namespace, "rejecting ANNOUNCE: publish not permitted");
drop(announced); // Drop impl sends PUBLISH_NAMESPACE_ERROR
}
}Either way, this doesn't touch the Coordinator trait at all - the Coordinator's job ends at resolve_scope() returning the permissions, and the enforcement is purely relay-layer mechanics.
Should we include the proper reject behavior in this PR or handle it as a follow-up?
There was a problem hiding this comment.
i think it makes sense to treat it as a follow-up
…ed sessions When scope permissions disable a session half (e.g., ReadOnly disables publishing), the relay now actively drains and rejects unauthorized control messages instead of silently ignoring them. A ReadOnly peer that sends PUBLISH_NAMESPACE gets PUBLISH_NAMESPACE_ERROR back; a WriteOnly peer that sends SUBSCRIBE gets SUBSCRIBE_ERROR back. This leverages the existing Drop impls on Announced and Subscribed, which automatically send the appropriate error response when dropped without calling ok(). The unused transport halves are passed to new reject fields on the relay Session struct, where drain-and-reject loops pick up and drop each queued message. No Coordinator trait changes required - enforcement is purely relay-layer mechanics.
| pub producer: Option<Producer>, | ||
| pub consumer: Option<Consumer>, | ||
|
|
||
| /// When `consumer` is `None` (publish not permitted), the transport |
|
Lints clean, tests pass, internal bot review approved, @nnazo approved... merging! |
Summary
moq-rs currently parses but discards the WebTransport URL path and CLIENT_SETUP PATH parameter (key 0x1). This PR extracts and validates the connection path and makes it available to the
Coordinatortrait, which can use it (along with any other information available to the implementation) to resolve a scope identifier for the connection.The connection path is useful for multi-tenant relay deployments where different paths map to different application scopes. The new
resolve_scope()trait method is intentionally unopinionated about how the raw path maps to a scope identifier. It's up to each Coordinator implementation to decide what portion of the path to use, whether multiple paths should map to the same scope, and what permissions to assign. The resolved scope is then used to isolate namespace registrations and lookups, and to associate per-scope configuration (such as origin fallback URLs or lingering subscribe settings) with the connection. The default implementation passes the full path through as the scope_id with ReadWrite permissions.Since SUBSCRIBE_NAMESPACE, track-level PUBLISH, and rendezvous/lingering subscriber support will also require Coordinator trait changes, we batch those stubs into this PR to minimize churn for implementors. All new methods have default no-op implementations so they compile immediately.
Commits
1.
feat: add Transport enum and connection path extractionAdd
Transportenum (WebTransport|RawQuic) determined at the QUIC layer (ALPN negotiation) and threaded throughSession, convenience wrappers, and all call sites.Extract and validate the connection path during
Session::accept()andSession::connect()::path) takes precedenceconnection_path: Option<String>onSessionwith public accessor/required, no dot segments, no double slashes, no percent-encoded characters2.
feat: add scope parameter to Coordinator trait and thread through relayAdd
scope: Option<&str>toregister_namespace(),unregister_namespace(), andlookup()(breaking trait change). Thread the resolved scope through the relay:ProducerandConsumerstore scope and pass it to coordinator calls and localsRemotesConsumer::route()accepts scope for upstream lookupsLocalskeyed by(scope, namespace)using a two-level HashMap for O(1) scope lookupFileCoordinatorupdated with scoped file storageApiCoordinatoraccepts scope parameter3.
feat: add resolve_scope() to Coordinator trait with permission-gated sessionsAdd
ScopeInfoandScopePermissionstypes, and aresolve_scope()method that translates raw connection paths into scope identity + permissions. The relay calls this on each accepted session and gates Producer/Consumer creation on the result:ReadWrite: both publish and subscribeReadOnly: subscribe only (no Consumer created)Err: session rejected with QUIC APPLICATION_CLOSE (code 0x3)4.
feat: add Coordinator stubs for SUBSCRIBE_NAMESPACE, track PUBLISH, and lingering subscriber supportPurely additive. New types (
ScopeConfig,NamespaceSubscription,NamespaceInfo,RelayInfo,TrackRegistration,TrackEntry,TrackSubscription) and 10 new trait methods, all with default no-op implementations.Includes test suite with
MockCoordinator(full in-memory reference implementation) andMinimalCoordinator(verifies all defaults). 38 test cases.Design notes:
TrackEntryis intentionally named differently from PR feat: Replace ANNOUNCE -> (PUBLISH_NAMESPACE + PUBLISH) and SUBSCRIBE_NAMESPACE #134'sTrackInfo(identity-only vs data-plane state machine)NamespaceRegistrationconventionRENDEZVOUS_TIMEOUT, moq-transport PR #1447); code uses "lingering subscribe" (moq-transport issue #1402) for consistency with existing implementations5.
feat: add scope-aware namespace isolation to ApiCoordinatorReplace flat global namespace keying with collision-free hex-encoded registry keys. Each namespace tuple field is hex-encoded (handles arbitrary bytes) and joined with
.. Scope prepended with:separator. Scoped and unscoped keys have distinct prefixes.Files Changed
moq-transport/src/session/mod.rsTransportenum, path extraction/validation,connection_pathaccessor, unit testsmoq-transport/src/session/error.rsInvalidPatherror variantmoq-transport/src/session/publisher.rsTransportparametermoq-transport/src/session/subscriber.rsTransportparametermoq-relay-ietf/src/coordinator.rsScopeInfo,ScopePermissions,resolve_scope(), scope on core methods, stub types/methods, test suitemoq-relay-ietf/src/relay.rsresolve_scope()call, permission-gated sessions, forward scope extractionmoq-relay-ietf/src/producer.rsmoq-relay-ietf/src/consumer.rsmoq-relay-ietf/src/remote.rsroute()accepts scopemoq-relay-ietf/src/local.rs(scope, namespace)moq-relay-ietf/src/bin/.../file_coordinator.rsmoq-relay-ietf/src/bin/.../api_coordinator.rsmoq-relay-ietf/README.mdmoq-native-ietf/src/quic.rsTransportthrough accept/connectmoq-{clock,pub,sub,test-client}Transportthrough connect callsTesting
All 147 workspace tests pass.
cargo clippy --no-deps --all-targets -- -D warningsis clean.