Draft: Support draft-18 - #176
Draft
englishm wants to merge 66 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
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.
Draft-17 §10.2 replaced the generic KVP value rule (even type = varint, odd type = length-prefixed bytes) with per-parameter value kinds for message parameters. Three parameters carry a single raw uint8 value instead of a varint: FORWARD (0x10), SUBSCRIBER_PRIORITY (0x20) and GROUP_ORDER (0x22). moq-rs still encoded/decoded these values as varints. SUBSCRIBER_PRIORITY defaults to 128, which is two bytes as a varint (0x80 0x80) but one byte as a uint8 (0x80). An inbound SUBSCRIBE (or REQUEST_UPDATE, PUBLISH_OK, REQUEST_OK, ...) from a conformant peer (imquic, moq-go) therefore desynchronised moq-rs's parameter decoder, and the request was dropped invisibly. FORWARD (0/1) and GROUP_ORDER (1/2) happened to coincide between the two encodings, so only SUBSCRIBER_PRIORITY actually broke. Fix the whole §10.2 message-parameter code space uniformly, matching the per-type registries in moq-go (paramKinds) and imquic (version-gated uint8 reads): - coding/kvp: thread an optional set of uint8-valued types through the per-pair codec; `decode_with_prev` / `encode_with_prev` delegate with an empty set so SETUP parameters, extension headers and track extensions (different code spaces) keep the generic even=varint rule. - message/params: define U8_VALUE_PARAMETER_TYPES and the decode_message_params / encode_message_params wrappers, and route every message that carries §10.2 parameters through them. - session: surface an inbound request decode failure at warn instead of debug, so this class of interop break is observable in production rather than silently dropped. Adds wire-vector tests pinning the byte layout (priority 128 -> single 0x80) against moq-go / imquic for SUBSCRIBE and REQUEST_UPDATE, plus coding-level tests that the generic KVP path still uses varints. Note: LARGEST_OBJECT (0x09, two bare varints) and TRACK_NAMESPACE_PREFIX (0x34, KindBytes) remain on the generic path; they are publisher-side / not yet exercised and are tracked as separate follow-ups.
draft-18 spec Table 5 assigns 0x50 to SUBSCRIBE_NAMESPACE. Our code had 0x11, which is not a message type in draft-18 at all (0x11 is an error code: INVALID_RANGE / CONTROL_MESSAGE_TIMEOUT). This caused peers sending SUBSCRIBE_NAMESPACE with the correct type byte to have their messages silently rejected as InvalidMessage(80). Also update the stale wire-format test name and expected byte.
The draft-18 integration resolved moq-relay-ietf and its siblings in favour of the draft-18 side, which dropped the relay work main had accumulated: track lookup through the coordinator, the PUBLISH_NAMESPACE fan-out to peer relays, the namespace and track change broadcasts, the pull-through cache and the upstream namespace manager. Without those a relay cannot route between hosts at track granularity, which the draft-16 line has done for a while. That merge recorded the port as deferred rather than unwanted. The crates come back from main unchanged, and the commit that follows adapts the session layer they call to draft-18 rather than reinventing the relay on top of it. Restoring wholesale would have reverted draft-18's ALPN version negotiation, so that is reapplied: the server offers every supported version and rejects a client that offers none, instead of accepting with no protocol and failing later at SETUP. This commit does not build alone.
…raft-18 The restored relay calls session APIs that landed on main after the draft-18 fork, so draft-18 never had them: SUBSCRIBE_NAMESPACE and direct PUBLISH in both directions, and SessionConfig with its connect and accept constructors. Adapted rather than copied. Each request owns a bidirectional stream and its responses omit the Request ID, so responses go through the bidi response paths. SUBSCRIBE_NAMESPACE sits outside the generic response map because NAMESPACE carries no Request ID to route on. The object-sending loop is shared between SUBSCRIBE and PUBLISH as ObjectForwarder. PUBLISH is accepted with REQUEST_OK, since draft-18 removed the dedicated PUBLISH_OK type; the old type is still accepted on receive. A protocol violation closes the session rather than being logged and discarded, and a track alias reused while live closes with DUPLICATE_TRACK_ALIAS. PUBLISH_DONE carries the real number of streams opened, which is what tells a subscriber whether it can discard state. pending_requests.rs is not ported. Its response routing is structural once each request owns a stream, and its outbound timeout would have covered PUBLISH alone while subscribe_open has never had one, so Published::publish documents that ok() is unbounded and leaves the limit to the caller. Interop cases in moq-test-client cover both new directions against a local relay.
State::lock and lock_mut unwrapped the inner mutex, so one panic while holding a state made every later access to it panic too. One State backs every subscription, reader and waiter built on it, so a single failed request could take the rest down with it. Both accessors now recover the guard, log once and clear the poison flag. The state can hold a half-finished update, which is what the log is for. try_lock and try_lock_mut still report poisoning, for callers that would rather decide. The drop path was worse: on a poisoned lock it returned before notify(), leaving every task awaiting that state waiting for a change that could never arrive. The unwraps are the same on main, so this applies there unchanged. Tests poison a state from a thread that panics under the lock, then assert access still works and that a later drop wakes a waiter. The waiter case panics holding a shared ref rather than a mutable one, because StateMut notifies as it unwinds and would make the test pass either way.
When a WebTransport client offered no MoQT version we understood, the relay
logged "no mutually supported MoQT version in WT-Available-Protocols" and
dropped the connection without saying what the peer had actually offered, or
answering the CONNECT at all. Diagnosing a rejected peer meant reading its
source: the log named the failure but none of the evidence.
Log the offered list next to our supported list, and answer the CONNECT with
406. draft-18 §3.1.3 has the client advertise MoQT identifiers in
WT-Available-Protocols, and draft-ietf-webtrans-http3 §3.3 permits the server
to "reject the request if the client did not include a suitable protocol", so
rejecting stays correct; only the diagnostics change.
Answering takes a little care: `reject` merely queues the response, and both
closing the connection and dropping it stop sending immediately, so a naive
reject-then-close truncates the very response it just wrote. The connection is
therefore held (a handle is cloned before `Request::accept` consumes it) for a
bounded 250ms while the peer reads the 406, and only then closed with a reason
phrase. Without that window the peer reports a truncated H3 exchange; with it,
it reports the status:
before: failed to exchange h3 connect: protocol error: unexpected end of input
after: failed to exchange h3 connect: protocol error: expected 200, got: Some(406)
The raw-QUIC branch's "unsupported ALPN" error now names the ALPNs we accept
for the same reason.
Verified against a relay from this branch: a client offering "moqt-17" now
produces
WARN rejecting WebTransport CONNECT: no mutually supported MoQT version in
WT-Available-Protocols cid=... ip=... offered=["moqt-17"]
supported=["moqt-18"]
and the WebTransport and raw-QUIC happy paths are unchanged.
Spec: draft-ietf-moq-transport-18 §3.1, §3.1.3;
draft-ietf-webtrans-http3 §3.3
https://www.ietf.org/archive/id/draft-ietf-moq-transport-18.html
**Orphaned doc comment.** `REJECT_FLUSH_TIMEOUT` had been inserted between `build_transport_config`'s doc comment and the function, so rustdoc attached that documentation to the const and left the function undocumented. The constants now sit above it with their own docs; verified in the generated HTML. **Peer-controlled logging is now actually bounded.** The offered protocol list is logged at WARN on a pre-authentication path. H3 caps the header block at 64 KiB, which still permits ~104 KiB of decoded text in a single entry, so both the entry count (16) and each entry's length (64 chars) are capped rather than just the count. **Fallback close uses H3_NO_ERROR (0x100).** Application code 0 is not a defined HTTP/3 error code. This is wire hygiene, not a behaviour fix — RFC 9114 §8 already requires an unknown code to be read as H3_NO_ERROR. H3_REQUEST_REJECTED would be wrong: §4.1.1 reserves it for requests the server did not process, and this one was answered with 406. The §15.10.1 MoQT session codes do not apply either, as there is no WebTransport or MoQT session to close at this point — that registry belongs to the CLOSE_WEBTRANSPORT_SESSION path in the relay. Behaviour is unchanged: a client offering "moqt-17" still gets 406 and the relay still logs `offered=["moqt-17"] supported=["moqt-18"]`; the WebTransport and raw-QUIC happy paths still pass. Spec: RFC 9114 §8.1, §4.1.1; draft-ietf-moq-transport-18 §3.5
PUBLISH_DONE carries a Stream Count so a subscriber knows how many data
streams may still be in flight (draft-18 §10.11). We decoded the field
and then dropped it: `recv_publish_done` removed the subscription and
`recv_done` set `writer = None` immediately, so any stream still arriving
was routed nowhere and its Objects were silently discarded. §10.11 instead
destroys subscription state "once all open streams for the subscription
have closed", and asks for a timer as a backstop.
The comment on `PublishedState::stream_count` already described the
requirement we were violating: "reporting 0 tells it to discard state
immediately and drop objects that were legitimately sent."
Add `StreamDrain`, shared by both subscription flavours since PUBLISH_DONE
terminates either one:
- Count a data stream when its SUBGROUP_HEADER is accepted, because
§10.11 counts "streams that contained no Objects (e.g., an empty
Subgroup)" and so cannot wait for a first Object. Datagrams are not
streams and do not count.
- Hold the subscription open until the announced streams have been
received *and* every open stream has been read.
- Fall back to a 5s timer, which §10.11 requires for a publisher that
over-counted, reset a stream before its SUBGROUP_HEADER, or sent the
2^62-1 "cannot count" sentinel. The timer runs as a session-owned task
so it is cancelled with the session, and concurrent drains are capped
so a peer cannot pin unbounded state.
PUBLISH_DONE is terminal on the request stream, so `abort_publish_received`
and `abort_subscribe` ran immediately afterwards and would have undone the
drain; both now leave a draining subscription for the drain to finish.
Two related defects found on the way:
- `remove_subscribe` released the alias and name reservations for a
request ID even when no SUBSCRIBE existed, so looking it up
speculatively stripped an inbound PUBLISH's Track Alias and orphaned
its streams. It now only releases what it actually removed.
- `ObjectForwarder::serve_subgroups` logged per-subgroup send failures
and returned `Ok`, so "serve succeeded" said nothing about whether any
byte reached the peer. Local faults now propagate, while a peer
cancelling one data stream (§11.4.1) still ends the subscription
normally rather than as INTERNAL_ERROR.
Verified end to end against a relay built from this branch: the relay now
logs "keeping state for in-flight streams", receives the subgroup, ends the
subscription once the announced stream closes, and forwards the Object. The
subscriber receives it, which it never did before.
Spec: draft-ietf-moq-transport-18 §10.11, §11.4.1
https://www.ietf.org/archive/id/draft-ietf-moq-transport-18.html
Closes the residual data-loss gap the review found, removes the
`.unwrap()` violations from `session/`, and reverts an out-of-scope behaviour
change.
**Residual drain gap (§10.11).** The drain only deferred teardown once
PUBLISH_DONE had arrived. If the request stream died *before* PUBLISH_DONE
while a data stream was mid-transfer, `abort_publish_received` /
`abort_subscribe` dropped the writer immediately and lost the Objects already
in flight — the same failure this MR exists to fix, reached by a different
path. `force_finish` is replaced by `abort`, which is `StreamDrain::arm(status,
0)`: nothing further can be announced on a dead request stream, but a stream
still being read holds the subscription open and completes it on close, with
the existing timer as backstop. A drain already in progress is left alone so
the publisher's own status code survives.
`MAX_CONCURRENT_DRAINS` applies to the new deferral too. The four deferral
sites now share one `begin_drain` helper rather than repeating the admission
check, so an abort cannot park unbounded state by never sending PUBLISH_DONE.
`DoneOutcome` is `#[must_use]`: dropping it silently leaks the subscription.
**`.unwrap()` calls in production.** Six sites in `session/`
converted to `ok_or(SessionError::Internal)?`: the two session-tuple unwraps in
`connect_with_config`, `Publisher::accept`, `Subscriber::accept`, and two on
peer-decoded `stream_header.subgroup_header`. Two more in `subscribed.rs` are
gone by binding `object_id` locally. `moq-transport/src/session/` now contains
no `unwrap`/`panic!`/`todo!`/`unimplemented!`/`dbg!` outside tests.
**UNSUBSCRIBE for an unknown ID no longer closes the session.** It had become
`ProtocolViolation`, which is session-fatal, but the peer's UNSUBSCRIBE can
legitimately cross with our own PUBLISH_DONE for the same subscription. Draft-18
does not define UNSUBSCRIBE at all — it ends requests by resetting the request
stream — so there is no MUST to enforce here. Back to log-and-continue, matching
how an unknown PUBLISH_DONE is already handled.
**Bidi response params use the typed codec.** `encode_bidi_response_frame` /
`decode_bidi_response` used the generic KVP codec while every message type uses
`encode_message_params` / `decode_message_params`. §10.2.7 says SUBSCRIBER_PRIORITY
defaults to 128, which is exactly where the two encodings diverge: a conformant
peer's `PUBLISH_OK{SUBSCRIBER_PRIORITY: 128}` had its `0x80` read as a two-byte
varint prefix, desynchronising the rest of the parameter block. All four
params-carrying response types (REQUEST_OK, SUBSCRIBE_OK, PUBLISH_OK, FETCH_OK)
are fixed on both sides. This codebase only ever emits FORWARD and
LARGEST_OBJECT on responses, both byte-identical under either codec, so
moq-rs↔moq-rs output does not change.
**Teardown is O(1) again.** `subscribed_names` / `published_names` were removed
by `retain` over the whole map. The subscriber side already had a two-way index;
it is now a shared `NameRegistry` used by both, so the duplicate check stays
keyed by name and removal stays keyed by request ID.
Also: README lists PUBLISH / PUBLISH_DONE and SUBSCRIBE_NAMESPACE as supported;
`MAX_CONCURRENT_BIDI_STREAMS` documents that a slot is held for the life of the
request, not just its setup; the relay's upstream-subscription comment cites
draft-18 §9.4 (verbatim) instead of draft-16 §8.4; "peer.s" typo fixed.
Spec: draft-ietf-moq-transport-18 §10.2.7, §10.11, §9.4, §5.1
https://www.ietf.org/archive/id/draft-ietf-moq-transport-18.html
… end `publish-track-subscribe` treated any error from `subscriber.subscribe()` as a failure, but a publisher that has sent everything ends the subscription with PUBLISH_DONE / TRACK_ENDED (draft-18 §10.11), which surfaces as `ServeError::Closed(0x2)`. Because the three arms run under `try_join!`, that "error" aborted the sibling future that was still reading the Object, so the test reported failure on the one outcome it should have accepted. Accept TRACK_ENDED (and Done/Cancel) as a normal end, and let the payload comparison decide the result. The 100ms delay before writing is kept deliberately, and now says why: it lets the subscriber attach first, so the Object is relayed to a live subscription instead of being served from whatever the relay buffered before the SUBSCRIBE arrived. Ending the track immediately after the write is kept for the same reason — that is what makes PUBLISH_DONE race the Object, which is the condition §10.11's Stream Count exists to handle. An earlier attempt to hold the track open until delivery was confirmed removed the race, and the test then passed against a transport that still had the bug. That race is exactly what it sounds like: without the §10.11 drain the outcome depends on whether the data or PUBLISH_DONE is processed first, and it was observed failing with the original "subscriber track mode failed: closed, code=2". With the drain in place the outcome is no longer timing-dependent and the full suite is 8/8. `publish-track-only` cannot observe receipt with no peer subscribed, so its doc now says what it does assert — the publisher's side — and points at `publish-track-subscribe` for end-to-end delivery. Depends on the §10.11 Stream Count drain in fix/publish-done-stream-count; without it the subscriber can tear down before the Object arrives. Spec: draft-ietf-moq-transport-18 §10.11 https://www.ietf.org/archive/id/draft-ietf-moq-transport-18.html
`PublishDoneCode` had TOO_FAR_BEHIND and EXPIRED transposed against the
§15.10.3 registry (Table 19), which assigns TOO_FAR_BEHIND 0x5 and
EXPIRED 0x6. A peer told a subscription had fallen too far behind would
have read it as expired, and vice versa. EXCESSIVE_LOAD (0x9) was absent.
`RequestErrorCode` was missing five codes from the §15.10.2 registry
(Table 18): GOING_AWAY 0x6, EXCESSIVE_LOAD 0x9, NAMESPACE_TOO_LARGE 0x31,
UNSUPPORTED_EXTENSION 0x33 and REDIRECT 0x34. UNSUPPORTED_EXTENSION is the
notable one: §2.5.1 requires it when a peer does not understand a Mandatory
Track Property (0x4000-0x7FFF) on PUBLISH, SUBSCRIBE_OK or FETCH_OK, and we
had no way to signal it.
Both enums now pin every registry entry with a test, so a future transposition
fails rather than reaching the wire.
Neither transposed variant had a production caller — every use is an `as u64`
cast and no code compares the raw values — so this changes no behaviour today.
It is a definitional fix that would otherwise have surfaced as a silent
interop failure the first time either code was sent.
Two adjacent problems are documented rather than changed, to keep this scoped:
- `ServeError::code` predates draft-18 and matches neither registry
(`NotFound` answers 0x4 where DOES_NOT_EXIST is 0x10; `Done` answers 0x0
where the PUBLISH_DONE path uses TRACK_ENDED). Its one remaining wire user
is PUBLISH_NAMESPACE_CANCEL. The comment now says so instead of claiming a
conformance the body does not have.
- `error::SubscribeDone` is an unused but public legacy table whose
`From<u64>` disagrees with §15.10.3 from 0x2 up. Deleting it is a breaking
API change, so it is documented as wrong and left for a follow-up.
REDIRECT is marked receive-only: §10.6.2 requires a trailing Redirect
structure that `RequestError` cannot encode yet.
Spec: draft-ietf-moq-transport-18 §15.10.2, §15.10.3, §2.5.1, §10.6.2
https://www.ietf.org/archive/id/draft-ietf-moq-transport-18.html
`ServeError::code`'s new comment claimed PUBLISH_NAMESPACE_CANCEL was the only remaining wire user. It is the only *direct* one: the public `SessionError::code` delegates here for its `Serve` variant, and its result is measured against the §15.10.1 session-termination registry — a third registry these values were never designed for. Nothing in this workspace calls it, but an embedder can, and anyone acting on the "retire this" TODO would have sized the work from the old sentence and missed the delegation. `RequestErrorCode::Redirect` cited §10.6.2, which imposes the requirement; the Redirect structure itself is defined in §10.6.1. Both are now cited. Doc-only; no behaviour change. Spec: draft-ietf-moq-transport-18 §10.6.1, §10.6.2, §15.10.1 https://www.ietf.org/archive/id/draft-ietf-moq-transport-18.html
Subscribers can attach draft-18 SUBSCRIBE parameters, including RENDEZVOUS_TIMEOUT. Sessions preserve TIMEOUT responses, reject supported parameters outside their permitted message scope, and can enforce a deadline before accepting a subscription without changing the existing no-parameter API.
A relay can hold a SUBSCRIBE when no publisher exists and resolve it from a publisher that appears locally or through the coordinator. The requested wait is capped at 30 seconds, with capacity limited to 128 holds per relay and 64 per session. Requests above either limit receive EXCESSIVE_LOAD with a spread retry interval.
Add rendezvous timeout support
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
4 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.