Draft: Support draft-18#176
Draft
englishm wants to merge 39 commits into
Draft
Conversation
Replace Announce/Announced types with PublishNamespace/PublishedNamespace throughout moq-transport, moq-relay-ietf, moq-pub, moq-clock-ietf, and moq-test-client. The wire messages were already named correctly; this aligns the session-layer API and test case names with the spec terminology. Also replace .lock().unwrap() on mutex acquisitions with explicit poison error handling in publisher.rs and subscriber.rs.
Key-Value-Pairs now use delta-encoded types per §1.4.2: each Delta Type on the wire is the difference from the previous absolute type (or 0 for the first pair in a sequence). KeyValuePairs::encode sorts by ascending key before emitting deltas; decode accumulates the running type and rejects overflow. ExtensionHeaders shares the same delta scheme, threading prev through its own encode/decode loop. The Decode/Encode trait impls on KeyValuePair are removed to prevent context-free single-pair misuse. All callers must go through decode_with_prev/encode_with_prev, or the KeyValuePairs/ExtensionHeaders containers which manage the prev state correctly. TrackNamespace now enforces §2.4.1: 1-32 non-empty fields for full namespaces; zero-count or zero-length fields produce errors. Adds TrackNamespacePrefix (0-32 fields, empty allowed) for SUBSCRIBE_NAMESPACE use, and full_track_name_len() helper for the 4096-byte limit check. New error variants added to DecodeError and EncodeError for each new validation rule.
Version negotiation now uses ALPN only. The native QUIC ALPN identifier changes from 'moq-00' to 'moqt-16'. CLIENT_SETUP and SERVER_SETUP no longer carry a version list or selected version in their payloads; both messages carry setup parameters only. For native moqt:// connections the client now sends AUTHORITY (host:port) and PATH (path?query) setup parameters as required by draft-16 §9.3.1.1 and §9.3.1.2. The server-side version negotiation code and largest_common helper are removed since version is agreed at the ALPN layer. Adds Version::DRAFT_16 constant. Removes SessionError::Version as it is no longer reachable. Updates mlog events to match the new setup message shape.
…ge length Adds three new draft-16 message types: - REQUEST_OK (0x07): shared positive response for REQUEST_UPDATE, TRACK_STATUS, SUBSCRIBE_NAMESPACE, and PUBLISH_NAMESPACE - REQUEST_ERROR (0x05): shared negative response with Request ID, Error Code, Retry Interval, and reason phrase; replaces per-request error messages from earlier drafts - REQUEST_UPDATE (0x02): replaces SubscribeUpdate; carries Request ID, Existing Request ID, and parameters Wire IDs in the message table are corrected to match draft-16 Table 1. Legacy messages that had conflicting IDs (SubscribeError, PublishNamespaceOk, PublishNamespaceError, TrackStatusOk/Error, etc.) are reassigned to internal stub IDs (0x100+) so existing session dispatch compiles. The control message decode macro now enforces the 16-bit Length field: the payload is read into an exact-size slice and any remaining bytes after decoding produce a PROTOCOL_VIOLATION error.
Add a session-level RequestId manager with independent send and receive state under one shared handle. Publisher and Subscriber share outbound allocation so local requests use one monotonically increasing sequence, while inbound validation checks the peer sequence separately. Outbound allocation enforces the peer MAX_REQUEST_ID and emits REQUESTS_BLOCKED once per blocked limit. Incoming MAX_REQUEST_ID updates the outbound budget and must strictly increase. Incoming REQUESTS_BLOCKED increases the local advertised maximum and queues MAX_REQUEST_ID. Session receive handling now validates sequenced request IDs and handles GOAWAY, MAX_REQUEST_ID, and REQUESTS_BLOCKED directly. Add SessionError variants for INVALID_REQUEST_ID, TOO_MANY_REQUESTS, and PROTOCOL_VIOLATION.
PUBLISH_NAMESPACE_DONE now carries Request ID (not namespace) per §9.22. PUBLISH_NAMESPACE_CANCEL now carries Request ID (not namespace) per §9.24. Acceptance of PUBLISH_NAMESPACE is now sent as REQUEST_OK (§9.7) instead of the legacy PUBLISH_NAMESPACE_OK. Rejection is sent as REQUEST_ERROR (§9.8) instead of the legacy PUBLISH_NAMESPACE_ERROR. Session dispatch routes incoming REQUEST_OK and REQUEST_ERROR from the subscriber to the PUBLISH_NAMESPACE state by request ID. RequestOk and RequestError added to the subscriber role enum so published_namespace.rs can send them. PublishedNamespaceRecv now carries the request ID so PUBLISH_NAMESPACE_DONE and CANCEL can be looked up without a namespace key. drop_publish_namespace returns the removed recv value so callers can invoke recv_error or recv_done directly on it.
Subscription rejections (before SUBSCRIBE_OK) now use REQUEST_ERROR (draft-16 §9.8) instead of the legacy SUBSCRIBE_ERROR. The error code defaults to InternalError since ServeError does not map directly to a specific REQUEST_ERROR code at this layer. Duplicate SUBSCRIBE (same request ID) now sends REQUEST_ERROR with DUPLICATE_SUBSCRIPTION (0x19) per draft-16 §5.1 instead of closing the session. This is a per-request error, not a session-level violation. recv_subscribe_error is kept for exhaustive match but annotated as a legacy stub — stub ID 0x100 is never produced by the wire decoder so draft-16 subscription rejections only arrive as REQUEST_ERROR.
Add PUBLISH_DONE status code enum (§13.4.3) and map ServeError variants to the correct codes. TrackEnded is used for normal completion, InternalError for unexpected errors, and Closed passes through the application-specific code. Track the number of subgroup streams opened per subscription in SubscribedState so PUBLISH_DONE reports an accurate stream_count. Datagram-only subscriptions remain stream_count=0. UNSUBSCRIBE now removes publisher-side subscription state immediately and marks the subscription as unsubscribed. The Drop impl skips sending PUBLISH_DONE or REQUEST_ERROR when unsubscribed is set, since the subscriber already terminated and sending a terminal message would be spurious.
respond_ok now sends REQUEST_OK (§9.7) with a LARGEST_OBJECT parameter when objects have been published on the track. No Track Alias is included since draft-16 §9.19 does not use one for TRACK_STATUS responses. respond_error now sends REQUEST_ERROR (§9.8) instead of the legacy TRACK_STATUS_ERROR. Callers updated to use RequestErrorCode values: InternalError for the unknown-queue-full case, DoesNotExist for the relay track-not-found case.
Draft-16 §10.2.1.1 only allows Normal (0x0), EndOfGroup (0x3), and EndOfTrack (0x4). The value 0x1 existed in earlier drafts but was removed. Any received 0x1 or other unknown value now returns InvalidObjectStatus instead of silently succeeding.
Draft-16 §4: limited endpoints SHOULD respond with NOT_SUPPORTED rather than ignoring incoming request types they do not implement. Publisher-side: FETCH, REQUEST_UPDATE, and SUBSCRIBE_NAMESPACE now each send REQUEST_ERROR NOT_SUPPORTED back to the peer instead of returning a session-closing error. FETCH_CANCEL and UNSUBSCRIBE_NAMESPACE reference existing requests and are logged then ignored. Subscriber-side: PUBLISH (publisher-initiated subscription) now sends REQUEST_ERROR NOT_SUPPORTED. Legacy response stubs that can never arrive from a draft-16 peer are logged and ignored instead of closing the session.
Update metric description strings in moq-relay-ietf to reference PUBLISH_NAMESPACE and REQUEST_OK instead of ANNOUNCE/ANNOUNCE_OK. Metric names themselves are unchanged to avoid breaking existing dashboards and alerting rules. Update the RawQuic transport doc comment to reflect the correct ALPN string moqt-16 instead of the old moq-00.
- Move filter_type, subscriber_priority, group_order, and forward from message fields into SUBSCRIBE parameters per draft-16 - Add SubscriptionFilter parameter encoding/decoding - Introduce TrackName newtype to replace bare String track names - Simplify SubscribeOk and TrackStatus to draft-16 field sets - Add DeliveryFilter for applying subscription filters to objects and datagrams at serve time - Reject duplicate subscriptions by FullTrackName on the publisher - Fix subgroup object ID delta computation and defer subgroup writer creation until the first object is received - Add Namespace and NamespaceDone message variants
Conflict resolution during rebase incorrectly kept the old Subscriber::new(Queue, Arc<AtomicU64>, None) form instead of the new Subscriber::new(Queue, Option<mlog>, RequestId) signature. Fix the test helper and its import to match.
Replace the single hardcoded ALPN constant with a version registry (SUPPORTED_ALPNS) and a negotiate_version() function that selects the best mutually-supported MoQT version. Server-side (WebTransport): instead of silently accepting connections with no WT-Protocol header when the client's offered versions don't match, we now select the best mutual version or reject the connection with a clear error. This fixes interop with multi-version clients like moxygen that rely on WT-Protocol to know which draft was negotiated (facebookexperimental/moxygen#173). Server-side (raw QUIC): ALPN is now validated against the full SUPPORTED_ALPNS list instead of a single constant. Client-side (WebTransport): all supported versions are offered via WT-Available-Protocols instead of just one. The ALPN constant is preserved for backward compatibility but SUPPORTED_ALPNS is now the canonical source of truth.
Implement the new variable-length integer encoding defined in draft-ietf-moq-transport-17 §1.4.1, replacing the QUIC-style 2-bit tag encoding. The new encoding counts leading 1-bits in the first byte to determine length (1-9 bytes), supports the full u64 range (vs 2^62-1 before), and explicitly allows non-minimal encodings. The VarInt public API is preserved: from_u32(), into_inner(), all From/TryFrom impls. VarInt::MAX is now u64::MAX. TryFrom<u64> can no longer fail (all u64 values are representable). Test vectors from the draft spec are included. Existing tests across the codebase are updated for the new byte patterns — values 64-127 now encode as 1 byte instead of 2, which changes encoded sizes for extension headers, datagrams, and KVP delta keys in that range.
Replace separate CLIENT_SETUP (0x20) and SERVER_SETUP (0x21) messages with a single unified SETUP message (type 0x2F00) as defined in draft-ietf-moq-transport-18 §10.3. Both peers now send the same message format. Key changes: - New Setup struct replaces Client and Server in setup module - KVP decode_bounded()/encode_bounded(): length-bounded KVP encoding without count prefix, as required by draft-18 Setup Options - ALPN bumped from moqt-16 to moqt-18 - DRAFT_18 version constant added (0xff000012) - Message type 0x2F00 doubles as stream type identifier for the control stream (used in Commit 3 for uni stream dispatch) The control stream still uses bidirectional streams in this commit; the switch to unidirectional streams follows in the next commit.
…ft-18) Replace the single bidirectional control stream with a pair of unidirectional streams as defined in draft-ietf-moq-transport-18 §3.2. Each peer independently opens one uni stream (to send SETUP and subsequent control messages) and accepts one uni stream (to receive the peer's SETUP and control messages). The SETUP message type (0x2F00) serves as the stream type identifier on the wire. Connect flow: open_uni → write SETUP → accept_uni → read peer SETUP Accept flow: open_uni → accept_uni → read peer SETUP → write SETUP This is the final piece needed for a draft-18 SETUP exchange.
englishm
marked this pull request as draft
June 11, 2026 13:05
Draft-18 removed MAX_REQUEST_ID and REQUESTS_BLOCKED entirely (spec change #1471). This caused interop failures: third-party relays (imquic, moqt-nr) that don't send MAX_REQUEST_ID left peer_max=0, blocking all outbound request ID allocation with 'too many requests'. Changes: - session/request_id.rs: Remove peer_max tracking, Blocked variant, apply_max_request_id(), handle_requests_blocked(), and max_request_id_from_params(). RequestId::new() takes only local_first_id and peer_first_id. allocate() always succeeds (returns u64 directly, no Blocked variant). - session/mod.rs: Stop sending MAX_REQUEST_ID in SETUP params (both client and server). Remove Message::MaxRequestId and Message::RequestsBlocked handling from run_recv(). - session/publisher.rs, session/subscriber.rs: Simplify request ID allocation to use the now-infallible allocate() directly. - setup/param_types.rs: Remove MaxRequestId variant (type 0x2 is not in the draft-18 Setup Options registry). - message/mod.rs: Remove MaxRequestId (0x15) and RequestsBlocked (0x1a) from the message type enum and re-exports. - session/subscriber.rs: Map REQUEST_ERROR codes to semantic ServeError variants (DoesNotExist -> NotFound, etc.) instead of the opaque Closed(code). Fixes subscribe-error test producing 'closed, code=16' instead of a 'not found' error. Includes cargo fmt reformatting of touched and adjacent files.
…raft-18) Draft-18 moved all request messages (SUBSCRIBE, PUBLISH_NAMESPACE, etc.) from the control stream to individual bidirectional request streams (#1389). Response messages (REQUEST_OK, REQUEST_ERROR, SUBSCRIBE_OK) are sent back on the same bidi stream and omit the Request ID field since the stream identity already associates them with the request. Changes: - session/subscriber.rs: Add webtransport session field. subscribe_open() now opens a bidi stream via open_bi(), writes SUBSCRIBE, and spawns a reader task that decodes draft-18 responses (no Request ID) and routes them through the existing recv_message dispatch. - session/publisher.rs: publish_namespace() opens a bidi stream for PUBLISH_NAMESPACE and spawns a response reader with the same draft-18 decoding. - session/subscribe.rs: Add new_without_send() and wire_message() so the caller controls transport (bidi stream vs control stream). - session/publish_namespace.rs: Same new_without_send()/wire_message() pattern. - Both publisher.rs and subscriber.rs include decode_bidi_response() that handles the draft-18 response framing: reads type+length envelope, then decodes REQUEST_ERROR (0x5), REQUEST_OK (0x7), and SUBSCRIBE_OK (0x4) payloads without a Request ID field. The relay server-side still uses the control-stream path (Subscribe::new, PublishNamespace::new) for incoming requests from peers — the relay-side bidi request handling is a separate follow-up.
…draft-18 relay) Draft-18 requires peers to send request messages (SUBSCRIBE, PUBLISH_NAMESPACE, etc.) on individual bidirectional streams (#1389). The relay accepts these bidi streams, dispatches requests through existing handlers, and sends responses back on the same stream with the Request ID field omitted. Architecture: - BidiResponseMap: HashMap<u64, mpsc::UnboundedSender<Message>> shared between run_send and run_bidi_requests. Responses are forwarded via channel (not written directly) to ensure the bidi stream Writer stays in the same task context that accepted the stream — required for Quinn to properly transmit the QUIC STREAM frames. - handle_bidi_request: accepts bidi stream, reads request, registers channel, dispatches to handlers, then loops on rx.recv() writing responses via encode_bidi_response (no Request ID per draft-18). Terminal responses (REQUEST_ERROR, PUBLISH_DONE) include a zero- duration sleep before dropping the Writer to give Quinn's connection driver a scheduling opportunity to transmit the data before FIN. - encode_bidi_response: encodes all response types without Request ID. - RequestId::validate_incoming: relaxed to parity + duplicate check (no strict sequential order) for concurrent bidi stream arrival. - Message::response_target_id: identifies response messages for redirect from the outgoing queue to bidi channels. - Writer::finish: explicit FIN for clean stream closure. Test results: - Self-test (our client → our relay): 6/6 - imquic client → our relay: 6/6 - Our client → moqt-nr relay: 6/6
Address two AI code reviewer findings on the draft-18 bidi request
stream paths.
RFC-022 (WARNING): the publisher and subscriber bidi response reader
loops were launched with tokio::spawn and tracked only by sending the
JoinHandle to Session::run. Dropping a JoinHandle does not abort its
task, so those readers were effectively detached and could outlive the
session. Replace the spawns with boxed futures:
- BidiTaskSender now carries Pin<Box<dyn Future<Output = ()> + Send>>
(new BidiReaderFuture alias) instead of JoinHandle<()>.
- Publisher/Subscriber send Box::pin(async move { ... }) directly;
no task is spawned onto the runtime.
- Session::run collects them into FuturesUnordered<BidiReaderFuture>
and polls them cooperatively as a select arm. They progress only
while run() is polled and are dropped (cancelled) on session exit,
giving true structured concurrency.
SUGGESTION: Subscribe::new built the wire message via build_info and
then discarded it, forcing callers to rebuild the identical message
through wire_message(). Return the pre-built message::Subscribe from
Subscribe::new instead, drop the now-dead wire_message(), and have
subscribe_open() send the returned message directly — the SUBSCRIBE
frame is now constructed exactly once.
No wire-format or public-API behavior change. cargo test -p
moq-transport passes (207 tests); workspace builds clean.
Review follow-up: the bidi_task_tx field comments still described "spawned bidi reader task handles", which no longer matches the boxed- future design. Update them, and document that subscribe_open() / publish_namespace() require Session::run to be driven concurrently (run polls the bidi response reader). Doc-only; no behavior change.
Address two AI code reviewer findings on subscriber.rs.
WARNING — add unit tests for the subscribe cleanup paths. The prior
TODO noted these tests were dropped because constructing a Subscriber
needs a web_transport::Session and there is no mock constructor. Add a
small in-process QUIC/WebTransport loopback (rcgen self-signed cert,
no-verify client) in a #[cfg(test)] module that establishes a real
web_transport::Session, builds a Subscriber, and exercises the actual
cleanup logic:
- dropping_subscribe_removes_recv_state: Subscribe::drop removes the
subscribes-map entry (via Subscriber::remove_subscribe).
- remove_subscribe_clears_alias_map: remove_subscribe clears both the
subscribes map and the subscribe_alias_map.
Both run for real (not #[ignore]d) and fail if the cleanup regresses.
Removes the stale TODO. Adds rcgen + url as dev-dependencies.
SUGGESTION — finish the bidi send stream on the lock-poison path in
subscribe_open. open_request_stream now returns the request Writer
alongside the response Reader so subscribe_open holds the send side
(mirroring publisher.rs, which keeps its request writer in scope). If
the subscribes lock is poisoned after the stream is open, the writer is
explicitly finished (FIN) before returning instead of being dropped
implicitly.
cargo test -p moq-transport: 209 passed (loopback tests stable across
repeated runs); workspace builds clean; clippy has no new warnings.
Review follow-up: the server-accept task parked on std::future::pending(), so the trailing drop(session) was unreachable. Bind the session as _session and rely on the spawned task being cancelled at runtime shutdown. Test-only; no behavior change.
Review hardening: wrap the loopback client.connect in a 10s tokio::time::timeout so a future regression surfaces as a fast test failure instead of hanging CI. Adds the tokio "time" dev-dep feature. Test-only; no behavior change.
Address two AI code reviewer items on the draft-18 bidi request paths. SUGGESTION — subscribe_open did not finish() the request stream's send side on the success path; the writer fell out of scope and quinn emitted RESET_STREAM(0) instead of a clean FIN. Call request_writer.finish() after the reader future is handed off and before send.ok().await?, so a graceful FIN is sent on every exit path that holds the writer (happy path and a failed ack); the lock-poison path already finishes. INFO — run_bidi_requests had no per-task bound, so an alive-but- misbehaving peer could pin all MAX_CONCURRENT_BIDI_STREAMS slots (open a stream and never send a complete request, or stall a one-shot request after it is accepted) without tripping the QUIC idle timeout. Add BIDI_REQUEST_TIMEOUT (30s) applied to (a) the initial request read and (b) the response phase of one-shot request types (TRACK_STATUS, REQUEST_UPDATE). Long-lived request types (SUBSCRIBE, PUBLISH_NAMESPACE, PUBLISH, SUBSCRIBE_NAMESPACE, FETCH) are deliberately NOT bounded — bounding them would break the protocol — and rely on the QUIC connection idle timeout (configured by the transport layer) as the backstop for a dead peer. Decision documented at the constant. Also make the tokio "time" feature explicit on moq-transport (production code uses tokio::time::sleep/timeout; previously satisfied only via transitive feature unification). cargo test -p moq-transport: 209 passed; workspace builds clean; clippy has no new warnings.
Redo of the draft-18 integration using a robust 3-way MERGE of the full draft-18 work (fix/remove-max-request-id-draft-18 @ 2bf720c = original draft-18-dev a163ad5 + PR#178) onto current github/main (3ae6121). Why merge (not `git rebase`, not flat `git apply --3way`): - Flat `git diff f0a709a 2bf720c | git apply --3way` FAILS ATOMICALLY here: the branches restructured files differently (announce->publish_namespace, setup client/server -> unified setup.rs, message-file removals), so git apply rolls the whole patch back. `git merge` handles renames/deletes and surfaces every real difference as an explicit conflict — same cumulative-diff intent, nothing silently dropped. INTEGRATED (draft-18 bidi core; transport/src is verbatim 2bf720c + additive): - Bidi request streams (requests on individual bidi streams; responses omit the Request ID), MAX_REQUEST_ID/REQUESTS_BLOCKED removed (spec #1471), leading-1-bits varint, unified SETUP 0x2F00 + length-bounded KVP, ALPN moqt-18 + negotiation, unidirectional control stream. Kept main's non-conflicting improvements. DEFERRED (flagged, NOT silently dropped — see /workspace/REBASE_CONFLICTS.md): - main's inbound direct-PUBLISH (publish_received/published/pending_requests), full SUBSCRIBE_NAMESPACE + coordinator/perf relay work (control-stream model; need bidi port). Relay + moq-pub reverted to draft-18 (2bf720c) for a coherent build. moq-test-client publish-track tests (7,8) removed. VALIDATION: - cargo build --workspace: clean. cargo test --workspace: PASS (exit 0). (Fixed a real defect found in review: two loopback tests panicked under workspace feature unification because both `ring` and `aws-lc-rs` rustls providers link with no default; added a dev-dep + install_default() in the loopback helper. NOT flaky — was deterministic. Now green.) - Interop (scoped honestly): our draft-18 client passes 6/6 CONTROL-PLANE tests over WebTransport against ONE independent draft-18 relay (moq-go, Go/Yandex) via moq-interop-runner. Caveats: control-plane only (no data plane), 1 peer, WebTransport only, and the harness is lenient (only setup-only is a strict assertion). The public relay is NOT a valid target (runs our own code). Two reviewing sub-agents: merge review would stake $1000 on wire-semantics correctness with no silent drops beyond deferrals; interop review confirms the moq-go result is real but must be scoped as above.
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
2 participants
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.
Work in progress; may yet be rebased or rewritten.
Updates/bugfixes for draft-18 support should target this branch (
draft-18-dev), not main.Automatically deployed to
moqt://draft-18-interop.cloudflare.mediaoverquic.com:443.