From 80db9c7776a62710c04a3363fdc1ab43e3528f22 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 6 Oct 2026 23:37:59 -0700 Subject: [PATCH 01/28] quest: claim ietf-fetch-location From c334121c34dee9605da2c9153a004e974e66abc7 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 6 Oct 2026 23:55:25 -0700 Subject: [PATCH 02/28] feat(ietf): serve draft-20 FETCH within one group Draft-20 moved FETCH's range into LOCATION_FILTER. The publisher now answers an absolute filter with an end as the standalone range it spells, reporting FETCH_OK's End Location inclusively as draft-20 does. A relay's upstream group fill sends the draft-20 form and reads the inclusive End Location back. Filters bounded by Largest Object stay refused NOT_SUPPORTED, as do several groups. Co-Authored-By: Claude Opus 5.5 --- doc/concept/standard.md | 21 +-- quest/m1/README.md | 1 - quest/m1/fetch-ok-properties.md | 10 +- quest/m1/ietf-fetch-location.md | 26 ---- quest/m1/subscribe-ranges/ietf.md | 9 +- rs/moq-net/src/ietf/fetch.rs | 2 +- rs/moq-net/src/ietf/publisher.rs | 218 ++++++++++++++++++++++-------- rs/moq-net/src/ietf/subscriber.rs | 158 ++++++++++++++-------- rs/moq-tokio/tests/broadcast.rs | 29 ++-- 9 files changed, 288 insertions(+), 186 deletions(-) delete mode 100644 quest/m1/ietf-fetch-location.md diff --git a/doc/concept/standard.md b/doc/concept/standard.md index 9b4730a795..110f6ee988 100644 --- a/doc/concept/standard.md +++ b/doc/concept/standard.md @@ -49,16 +49,17 @@ to model priority 127, where higher values are served first. A track that never sets a priority is 127 as well, so it goes out as 128 on IETF and 127 on moq-lite. -On drafts 14–19, the Rust publisher answers a standalone `FETCH` within one -group from the cache. A relay fetches a missing group upstream with a `FETCH` -of that one whole group, and an upstream refusal is the refusal the fetcher -sees. Once its last reader leaves, the relay cancels the upstream fetch, even -before `FETCH_OK`, and aborts an incomplete group instead of caching it as whole. -Drafts 14–16 use `FETCH_CANCEL`; drafts 17–19 stop and reset the request stream. -A range touching several groups is refused with `NOT_SUPPORTED`, as is -any `FETCH` on draft-20 and later, which moved the range into -`LOCATION_FILTER`. A standalone `FETCH` -carries no timestamps, since no `SUBSCRIBE_OK` declared a timescale for it. +The Rust publisher answers a standalone `FETCH` within one group from the +cache. On draft-20 and later, that is a `FETCH` whose `LOCATION_FILTER` names +an absolute start and end; one ending at Largest Object (no filter, a relative +start, or no end) is refused with `NOT_SUPPORTED`. A relay fetches a missing +group upstream with a `FETCH` of that one whole group, and an upstream refusal +is the refusal the fetcher sees. Once its last reader leaves, the relay cancels +the upstream fetch, even before `FETCH_OK`, and aborts an incomplete group +instead of caching it as whole. Drafts 14–16 use `FETCH_CANCEL`; later drafts +stop and reset the request stream. A range touching several groups is refused +with `NOT_SUPPORTED`. A standalone `FETCH` carries no timestamps, since no +`SUBSCRIBE_OK` declared a timescale for it. On drafts 14–19, the Rust publisher also serves relative and absolute joining `FETCH` requests for `NextObject` subscriptions, for the subscription group's diff --git a/quest/m1/README.md b/quest/m1/README.md index e6fca796ed..45bb87a4fd 100644 --- a/quest/m1/README.md +++ b/quest/m1/README.md @@ -82,7 +82,6 @@ blocks. The quests that gated m0 lines moved under them. - [Plan: untimed verbatim PES](/quest/m1/plan-ts-pes-untimed.md) - decide how a verbatim TS track carries a PES that has no PTS, then write the implementation quest - [Data consumer timestamps](/quest/m1/data-consumer-timestamps.md) - json and binary consumers return each value's timestamp, in Rust and every binding; snapshots add `latest()` beside an in-order `next()` - [JS data consumer timestamps](/quest/m1/js-data-consumer-timestamps.md) - @moq/json and @moq/flate consumers return each value's timestamp, with snapshot `next()` and `latest()` -- [Draft-20 FETCH](/quest/m1/ietf-fetch-location.md) - a draft-20+ FETCH within one group is served from its LOCATION_FILTER, as older drafts are - [FETCH_OK properties](/quest/m1/fetch-ok-properties.md) - our FETCH_OK carries the track properties SUBSCRIBE_OK does, as draft 16+ requires - [Fetch without SUBSCRIBE](/quest/m1/ietf-fetch-only.md) - a relay fetches from an IETF upstream without subscribing, finished tracks included, with End of Track always reported - [IETF object gaps](/quest/m1/ietf-object-gaps.md) - a gapped object ID is refused loudly like a subgroup in Rust and JS, and an overflowing one closes the session in Rust diff --git a/quest/m1/fetch-ok-properties.md b/quest/m1/fetch-ok-properties.md index c2b03a2f72..530b169cd4 100644 --- a/quest/m1/fetch-ok-properties.md +++ b/quest/m1/fetch-ok-properties.md @@ -33,10 +33,8 @@ sends an empty block. when the track is, as the [untimed model](/quest/m1/untimed-model.md) requires. Rejected: omitting timescale from FETCH_OK and leaving fetch-only readers untimed. -- Honour INCLUDE_PROPERTIES (0x35) on FETCH from draft 20, once - [Draft-20 FETCH](/quest/m1/ietf-fetch-location.md) serves draft-20 FETCH at - all (today our publisher refuses every one). It defaults to sending the - properties; at 0 the block is present but empty. `fetch.rs` decodes it on +- Honour INCLUDE_PROPERTIES (0x35) on FETCH from draft 20. It defaults to + sending the properties; at 0 the block is present but empty. `fetch.rs` decodes it on draft 20+ and drops it, so it is not honoured today. - Test per draft range: FETCH_OK round-trips the properties SUBSCRIBE_OK would carry, a standalone FETCH's objects arrive stamped, and @@ -45,10 +43,6 @@ sends an empty block. Public API: none. Wire: FETCH_OK gains its properties on drafts that define the block. Interop: run `just test interop --all`. -## Required - -- [Draft-20 FETCH](/quest/m1/ietf-fetch-location.md) - draft-20 FETCH is served at all, and edits the same `run_fetch_stream` - ## Related - [Fetch without SUBSCRIBE](/quest/m1/ietf-fetch-only.md) - asks TRACK_STATUS for the same properties when a publisher omits them diff --git a/quest/m1/ietf-fetch-location.md b/quest/m1/ietf-fetch-location.md deleted file mode 100644 index 944d3f479a..0000000000 --- a/quest/m1/ietf-fetch-location.md +++ /dev/null @@ -1,26 +0,0 @@ -# [S] Draft-20 FETCH - -## Goal - -On drafts 20 and later, a FETCH whose LOCATION_FILTER stays within one group -is answered the way drafts 14 to 19 answer a standalone FETCH: from cache, with -a miss fetched upstream, and an upstream refusal passed through. A range -touching several groups follows the same group-span rule as drafts 14 to 19, -through the shared path: refused `NOT_SUPPORTED` until -[moq-transport ranges](/quest/m1/subscribe-ranges/ietf.md) lifts that -refusal for every draft. - -## Plan - -Today both sides refuse FETCH on draft-20+ `NOT_SUPPORTED`: the publisher in -`run_fetch_stream`, and the relay's upstream group fill in `run_group_fetch`. -The codec already decodes and encodes the draft-20 layout as -`FetchType::Filtered`, so drop both refusals, and flip -`fetch_moq_transport_20` in `rs/moq-tokio/tests` back to a served fetch. A -FETCH carrying Range Filters stays refused. - -Update the draft-20 note in `doc/concept/standard.md`. - -## Related - -- [moq-transport ranges](/quest/m1/subscribe-ranges/ietf.md) - lifts the one-group refusal in `run_fetch_stream` for every draft, draft-20 included diff --git a/quest/m1/subscribe-ranges/ietf.md b/quest/m1/subscribe-ranges/ietf.md index 4c3d9eed16..e5babf2c47 100644 --- a/quest/m1/subscribe-ranges/ietf.md +++ b/quest/m1/subscribe-ranges/ietf.md @@ -19,13 +19,10 @@ the Largest Object, as the moq-transport drafts require. Serving downstream lifts the one-group refusal ("FETCH spanning several groups not supported") in `run_fetch_stream` (`rs/moq-net/src/ietf/publisher.rs`) for every draft, including draft-20's -`FetchType::Filtered` once [Draft-20 FETCH](/quest/m1/ietf-fetch-location.md) -serves it. +`FetchType::Filtered`. Capping at the Largest Object also lifts the draft-20 +refusal of a filter bounded by it ("FETCH relative to Largest Object not +supported"): no filter, a relative start, or an absolute start with no end. ## Required - [Model ranges](/quest/m1/subscribe-ranges/model.md) - the range requests this answers - -## Related - -- [Draft-20 FETCH](/quest/m1/ietf-fetch-location.md) - serves draft-20 FETCH through the same group-span path this widens diff --git a/rs/moq-net/src/ietf/fetch.rs b/rs/moq-net/src/ietf/fetch.rs index 0a50802c15..877b70c81c 100644 --- a/rs/moq-net/src/ietf/fetch.rs +++ b/rs/moq-net/src/ietf/fetch.rs @@ -191,7 +191,7 @@ impl Message for Fetch<'_> { } // The token is ignored: the session's grant is what authorizes the request, and - // INCLUDE_PROPERTIES only shapes a FETCH_OK we don't send on draft-20. + // INCLUDE_PROPERTIES only shapes FETCH_OK's Track Properties, which we leave empty. let (fetch_type, subscriber_priority, group_order, range_filters, fill_timeout) = match version { Version::Draft14 => { let subscriber_priority = buf.u8()?; diff --git a/rs/moq-net/src/ietf/publisher.rs b/rs/moq-net/src/ietf/publisher.rs index 54bf615531..581ef970f2 100644 --- a/rs/moq-net/src/ietf/publisher.rs +++ b/rs/moq-net/src/ietf/publisher.rs @@ -1283,8 +1283,8 @@ where } } - /// Answer a FETCH within one group: a standalone range of the named track, or a - /// joining FETCH's prefix of its subscription's group. + /// Answer a FETCH within one group: a standalone or filtered range of the named track, + /// or a joining FETCH's prefix of its subscription's group. /// /// The answer is buffered before replying: FETCH_OK names where the response ends, /// which a range running past the track only learns by reading it, and a refusal can @@ -1292,19 +1292,6 @@ where async fn run_fetch_stream(mut self, mut stream: Stream, msg: ietf::Fetch<'_>) -> Result<(), Error> { let priority = super::priority::from_wire(msg.subscriber_priority); - // Draft-20 moved the range into LOCATION_FILTER, which decodes to - // `FetchType::Filtered` but is not served yet. - if Filter::is_draft20(self.version) { - return self - .reject_fetch( - stream, - msg.request_id, - &Error::Unsupported, - "FETCH not supported on draft-20", - ) - .await; - } - // Serving a Range Filter unfiltered would deliver objects the subscriber excluded. if msg.range_filters { return self @@ -1330,7 +1317,46 @@ where .await; } - let (track, start, end, timescale, joined) = match msg.fetch_type { + // Draft-20's LOCATION_FILTER is answered as the standalone range it spells. + let fetch_type = match msg.fetch_type { + FetchType::Filtered { + namespace, + track, + filter: Filter::Absolute { start, end: Some(end) }, + } => { + // The filter's End Object is inclusive, where a standalone one is the last + // object plus one. Without one, both include the whole End Group. + let Some(object) = end.object.map_or(Some(0), |object| object.checked_add(1)) else { + return self + .reject_fetch(stream, msg.request_id, &Error::InvalidRange, "End Object overflows") + .await; + }; + FetchType::Standalone { + namespace, + track, + start, + end: Location { + group: end.group, + object, + }, + } + } + // Every other filter ends at Largest Object, so the one-group rule below cannot + // check it without resolving that first. + FetchType::Filtered { .. } => { + return self + .reject_fetch( + stream, + msg.request_id, + &Error::Unsupported, + "FETCH relative to Largest Object not supported", + ) + .await; + } + other => other, + }; + + let (track, start, end, timescale, joined) = match fetch_type { FetchType::Standalone { namespace, track, @@ -1373,7 +1399,7 @@ where Ok(joined) => joined, Err((err, reason)) => return self.reject_fetch(stream, msg.request_id, &err, reason).await, }; - let start = match msg.fetch_type { + let start = match fetch_type { FetchType::RelativeJoining { group_offset, .. } => end.group.saturating_sub(group_offset), FetchType::AbsoluteJoining { group_id, .. } if group_id <= end.group => group_id, _ => { @@ -1398,7 +1424,7 @@ where true, ) } - // Refused above: only draft-20 decodes this form. + // Rewritten as standalone or refused above. FetchType::Filtered { .. } => { return self .reject_fetch(stream, msg.request_id, &Error::Unsupported, "not supported") @@ -1468,6 +1494,21 @@ where false => (end, false), } }; + // Draft-20's End Location is inclusive, naming the last object. A whole group was read + // to its end, so that is known, and an empty answer still covers its start. + let end_location = match Filter::is_draft20(self.version) { + true => { + let object = match end_location.object { + 0 => group.end(), + object => object, + }; + Location { + group: group.sequence, + object: object.saturating_sub(1).max(start.object), + } + } + false => end_location, + }; // FETCH_OK on every draft, never REQUEST_OK: section 5.2 allows exactly one FETCH_OK or // REQUEST_ERROR in answer to a FETCH, and REQUEST_OK's own definition lists the other @@ -4464,33 +4505,29 @@ mod serve_tests { } } - #[moq_net_sim::test] - async fn a_joining_fetch_is_not_supported_on_draft20() { - let h = serve(Version::Draft20); - let mut buf = bytes::Bytes::from(joining_fetch(&h, 0).await.unwrap()); - assert_eq!( - crate::coding::decode_varint(&mut buf, Version::Draft20).unwrap(), - ietf::RequestError::ID - ); - assert_eq!( - crate::coding::decode_buf(&mut buf, Version::Draft20, ietf::RequestError::decode) - .unwrap() - .error_code, - 0x3 - ); - assert!(buf.is_empty()); - } - - /// Every draft that carries a standalone FETCH. - const FETCH_DRAFTS: [Version; 6] = [ + /// Every draft, each carrying a standalone FETCH or draft-20's filtered one. + const FETCH_DRAFTS: [Version; 9] = [ Version::Draft14, Version::Draft15, Version::Draft16, Version::Draft17, Version::Draft18, Version::Draft19, + Version::Draft20, + Version::Draft21, + Version::Draft22, ]; + /// FETCH_OK's End Location for a response whose last object is `last`: one past it + /// before draft 20, and the object itself from then on. + fn end_location(version: Version, group: u64, last: u64) -> Location { + let object = match Filter::is_draft20(version) { + true => last, + false => last + 1, + }; + Location { group, object } + } + /// Groups `0..count`, each holding `g-0` and `g-1`, skipping `hole`. fn publish_pairs(h: &mut Serve, count: u64, hole: Option) { for sequence in (0..count).filter(|sequence| Some(*sequence) != hole) { @@ -4505,7 +4542,36 @@ mod serve_tests { } /// Run a standalone FETCH of `room/video`, returning what the peer reads back. + /// + /// `end` is spelled as drafts 14 to 19 do: the last object plus one, or 0 for the whole + /// End Group. From draft 20 the same range goes out as an inclusive LOCATION_FILTER. async fn standalone_fetch(h: &Serve, start: Location, end: Location, group_order: GroupOrder) -> bytes::Bytes { + let namespace = crate::Path::new("room"); + let track = "video".into(); + let fetch_type = match Filter::is_draft20(h.publisher.version) { + true => FetchType::Filtered { + namespace, + track, + filter: Filter::Absolute { + start, + end: Some(EndLocation { + group: end.group, + object: end.object.checked_sub(1), + }), + }, + }, + false => FetchType::Standalone { + namespace, + track, + start, + end, + }, + }; + run_fetch(h, fetch_type, group_order).await + } + + /// Run a FETCH, returning what the peer reads back. + async fn run_fetch(h: &Serve, fetch_type: FetchType<'static>, group_order: GroupOrder) -> bytes::Bytes { let version = h.publisher.version; let mark = h.log.writes.lock().unwrap().len(); let stream = Stream::open(&mut h.session.clone(), version).await.unwrap(); @@ -4517,12 +4583,7 @@ mod serve_tests { request_id: FETCH_ID, subscriber_priority: 128, group_order, - fetch_type: FetchType::Standalone { - namespace: crate::Path::new("room"), - track: "video".into(), - start, - end, - }, + fetch_type, range_filters: false, fill_timeout: false, }, @@ -4602,15 +4663,42 @@ mod serve_tests { ) .await; let (ok, objects) = fetch_answer(buf, version); - assert_eq!(ok.end_location, Location { group: 3, object: 0 }, "{version}"); + let end = match Filter::is_draft20(version) { + true => Location { group: 2, object: 1 }, + false => Location { group: 3, object: 0 }, + }; + assert_eq!(ok.end_location, end, "{version}"); assert!(!ok.end_of_track, "{version}"); assert_eq!(objects, pairs([2]), "{version}"); assert!(h.log.resets().is_empty(), "{version}"); } } - /// The last group of a finished track ends the response one past its last object, - /// with End of Track set. + /// A range ending inside its group stops at its End Object, which draft 20 counts + /// inclusively and older drafts count as one past. + #[moq_net_sim::test] + async fn a_standalone_fetch_stops_at_its_end_object() { + for version in FETCH_DRAFTS { + let mut h = serve(version); + publish_pairs(&mut h, 5, None); + settle().await; + + let buf = standalone_fetch( + &h, + Location { group: 2, object: 0 }, + Location { group: 2, object: 1 }, + GroupOrder::Ascending, + ) + .await; + let (ok, objects) = fetch_answer(buf, version); + assert_eq!(ok.end_location, end_location(version, 2, 0), "{version}"); + assert!(!ok.end_of_track, "{version}"); + assert_eq!(objects, pairs([2])[..1].to_vec(), "{version}"); + } + } + + /// The last group of a finished track ends the response at its last object, with End + /// of Track set. #[moq_net_sim::test] async fn a_standalone_fetch_of_the_last_group_reports_the_end_of_track() { for version in FETCH_DRAFTS { @@ -4627,28 +4715,38 @@ mod serve_tests { ) .await; let (ok, objects) = fetch_answer(buf, version); - assert_eq!(ok.end_location, Location { group: 4, object: 2 }, "{version}"); + assert_eq!(ok.end_location, end_location(version, 4, 1), "{version}"); assert!(ok.end_of_track, "{version}"); assert_eq!(objects, pairs([4])[1..].to_vec(), "{version}"); } } - /// Draft-20 carries the range in LOCATION_FILTER, which is not read yet. + /// A draft-20 filter bounded by Largest Object is refused, even when that bound would + /// land in the start group: the one-group rule needs it resolved first. #[moq_net_sim::test] - async fn a_draft20_fetch_is_refused() { + async fn a_draft20_fetch_relative_to_largest_object_is_refused() { for version in [Version::Draft20, Version::Draft21, Version::Draft22] { - let mut h = serve(version); - publish_pairs(&mut h, 3, None); - settle().await; + for filter in [ + Filter::Unfiltered, + Filter::NextObject, + Filter::Relative(1), + Filter::Absolute { + start: Location { group: 2, object: 0 }, + end: None, + }, + ] { + let mut h = serve(version); + publish_pairs(&mut h, 3, None); + settle().await; - let buf = standalone_fetch( - &h, - Location { group: 1, object: 0 }, - Location { group: 1, object: 0 }, - GroupOrder::Ascending, - ) - .await; - assert_eq!(fetch_refusal(buf, version), 0x3, "{version}"); + let fetch_type = FetchType::Filtered { + namespace: crate::Path::new("room"), + track: "video".into(), + filter, + }; + let buf = run_fetch(&h, fetch_type, GroupOrder::Ascending).await; + assert_eq!(fetch_refusal(buf, version), 0x3, "{version}: {filter:?}"); + } } } diff --git a/rs/moq-net/src/ietf/subscriber.rs b/rs/moq-net/src/ietf/subscriber.rs index ceb5bd8f5c..c49d11baea 100644 --- a/rs/moq-net/src/ietf/subscriber.rs +++ b/rs/moq-net/src/ietf/subscriber.rs @@ -3369,8 +3369,8 @@ where Ok(true) } - /// Fetch a group from the publisher to fill a cache miss, with a standalone FETCH for - /// this subscription's track. It asks from the frame the reader wants through the end + /// Fetch a group from the publisher to fill a cache miss, with a standalone (or + /// draft-20 filtered) FETCH for this subscription's track. It asks from the frame the reader wants through the end /// of the group, so a publisher that evicted the prefix can still answer. /// /// The group is accepted only once FETCH_OK arrives, so a refusal reaches every @@ -3389,11 +3389,7 @@ where request.reject(Error::GoingAway); return; } - // Our FETCH still encodes the Fetch Type field that draft-20 removed. - if Filter::is_draft20(self.version) { - request.reject(Error::Unsupported); - return; - } + let draft20 = Filter::is_draft20(self.version); let fetch_id = match self.control.next_request_id(&self.runtime).await { Ok(id) => id, @@ -3409,6 +3405,36 @@ where Err(err) => return request.reject(err), }; + let namespace = broadcast.clone(); + let track = name.as_str().into(); + let from = ietf::Location { + group: sequence, + object: start, + }; + let fetch_type = match draft20 { + // No End Object includes the whole End Group. + true => FetchType::Filtered { + namespace, + track, + filter: Filter::Absolute { + start: from, + end: Some(ietf::EndLocation { + group: sequence, + object: None, + }), + }, + }, + // An End Object of 0 is the whole End Group. + false => FetchType::Standalone { + namespace, + track, + start: from, + end: ietf::Location { + group: sequence, + object: 0, + }, + }, + }; let res = async { stream.writer.varint(ietf::Fetch::ID).await?; stream @@ -3417,19 +3443,7 @@ where request_id: fetch_id, subscriber_priority: super::priority::to_wire(request.priority()), group_order: GroupOrder::Ascending, - // An End Object of 0 is the whole End Group. - fetch_type: FetchType::Standalone { - namespace: broadcast.clone(), - track: name.as_str().into(), - start: ietf::Location { - group: sequence, - object: start, - }, - end: ietf::Location { - group: sequence, - object: 0, - }, - }, + fetch_type, range_filters: false, fill_timeout: false, }) @@ -3468,9 +3482,19 @@ where } }; + // Draft-20's End Location is inclusive, so step one past it as older drafts spell it. + let mut end = ok.end_location; + if draft20 { + let Some(object) = end.object.checked_add(1) else { + request.reject(Error::ProtocolViolation); + let _ = stream.writer.close().await; + return; + }; + end.object = object; + } + // The publisher knows where the track ends, which a range FETCH downstream needs. if ok.end_of_track { - let end = ok.end_location; let Some(final_sequence) = end.group.checked_add(u64::from(end.object > 0)) else { request.reject(Error::ProtocolViolation); let _ = stream.writer.close().await; @@ -3480,7 +3504,6 @@ where } // An empty answer opens no fetch stream at all. - let end = ok.end_location; if (end.group, end.object) <= (sequence, start) { request.reject(Error::NotFound); let _ = stream.writer.close().await; @@ -8807,34 +8830,50 @@ mod joining_fetch_tests { /// numbers what arrives from there, so a publisher that evicted the prefix can answer. #[moq_net_sim::test] async fn a_group_fetch_asks_from_the_wanted_frame() { - const VERSION: Version = Version::Draft19; + for version in [Version::Draft19, Version::Draft20, Version::Draft22] { + group_fetch_from_frame(version).await; + } + } + + async fn group_fetch_from_frame(version: Version) { const GROUP: u64 = 4; const START: u64 = 2; + const LAST: u64 = START + 1; + let draft20 = Filter::is_draft20(version); + // The whole group, as each draft answers it: draft 20 names the last object, and + // older drafts one past the group. + let end_location = match draft20 { + true => ietf::Location { + group: GROUP, + object: LAST, + }, + false => ietf::Location { + group: GROUP + 1, + object: 0, + }, + }; let ok = message_bytes( ietf::FetchOk::ID, &ietf::FetchOk { request_id: None, group_order: GroupOrder::Ascending, end_of_track: false, - end_location: ietf::Location { - group: GROUP + 1, - object: 0, - }, + end_location, properties: Default::default(), }, - VERSION, + version, ); // The fetch stream answering our first request id, from object START on. let mut objects = Vec::new(); - crate::coding::Encoder::new(&mut objects, VERSION.into()) + crate::coding::Encoder::new(&mut objects, version.into()) .varint(ietf::FetchHeader::TYPE) .unwrap(); ietf::FetchHeader { request_id: RequestId(1), } - .encode(&mut crate::coding::Encoder::new(&mut objects, VERSION.into()), VERSION) + .encode(&mut crate::coding::Encoder::new(&mut objects, version.into()), version) .unwrap(); for (index, payload) in [b"c", b"d"].iter().enumerate() { let first = index == 0; @@ -8845,9 +8884,9 @@ mod joining_fetch_tests { priority: first.then_some(0), properties: None, } - .encode(&mut crate::coding::Encoder::new(&mut objects, VERSION.into()), VERSION) + .encode(&mut crate::coding::Encoder::new(&mut objects, version.into()), version) .unwrap(); - crate::coding::Encoder::new(&mut objects, VERSION.into()) + crate::coding::Encoder::new(&mut objects, version.into()) .varint(1u64) .unwrap(); objects.extend_from_slice(&payload[..]); @@ -8864,7 +8903,7 @@ mod joining_fetch_tests { peer::PeerSetup::default(), crate::Hop::new(1).unwrap(), None, - VERSION, + version, tasks, Default::default(), ); @@ -8890,7 +8929,7 @@ mod joining_fetch_tests { // The fetch stream, as the peer would open it. let (_, recv) = session.clone().open_bi().await.unwrap(); - let mut stream = Reader::new(recv, VERSION); + let mut stream = Reader::new(recv, version); subscriber.clone().recv_fill(&mut stream).await.expect("group fetch"); serving.await.expect("run_group_fetch"); @@ -8902,27 +8941,40 @@ mod joining_fetch_tests { } assert_eq!(payloads, [b"c".to_vec(), b"d".to_vec()]); - let messages = decode_messages(&session.log, VERSION); + let messages = decode_messages(&session.log, version); let fetch = messages.iter().find(|(id, _)| *id == ietf::Fetch::ID).expect("FETCH"); - let mut body = Decoder::new(&fetch.1, VERSION.into()); - let msg = ietf::Fetch::decode_msg(&mut body, VERSION).unwrap(); - let FetchType::Standalone { start, end, .. } = msg.fetch_type else { - panic!("a group fetch is standalone: {:?}", msg.fetch_type); + let mut body = Decoder::new(&fetch.1, version.into()); + let msg = ietf::Fetch::decode_msg(&mut body, version).unwrap(); + let from = ietf::Location { + group: GROUP, + object: START, }; - assert_eq!( - start, - ietf::Location { - group: GROUP, - object: START + match msg.fetch_type { + // No End Object is the whole End Group. + FetchType::Filtered { filter, .. } if draft20 => assert_eq!( + filter, + Filter::Absolute { + start: from, + end: Some(ietf::EndLocation { + group: GROUP, + object: None + }), + }, + "{version}: through the end of the group" + ), + // An End Object of 0 is the whole End Group. + FetchType::Standalone { start, end, .. } if !draft20 => { + assert_eq!(start, from, "{version}"); + assert_eq!( + end, + ietf::Location { + group: GROUP, + object: 0 + }, + "{version}: through the end of the group" + ); } - ); - assert_eq!( - end, - ietf::Location { - group: GROUP, - object: 0 - }, - "through the end of the group" - ); + other => panic!("{version}: unexpected group fetch: {other:?}"), + } } } diff --git a/rs/moq-tokio/tests/broadcast.rs b/rs/moq-tokio/tests/broadcast.rs index 9141fca8b1..6ed0529ab5 100644 --- a/rs/moq-tokio/tests/broadcast.rs +++ b/rs/moq-tokio/tests/broadcast.rs @@ -376,10 +376,10 @@ async fn broadcast_moq_lite_05_fetch_webtransport() { lite05_fetch_roundtrip("https").await; } -/// A cache miss over moq-transport is a standalone FETCH of the one group, served from -/// the publisher's cache, and a group the publisher lacks comes back as the publisher's -/// own refusal. Without `served`, the version has no FETCH and the miss is refused. -async fn transport_fetch_roundtrip(version: &str, served: bool) { +/// A cache miss over moq-transport is a FETCH of the one group, served from the +/// publisher's cache, and a group the publisher lacks comes back as the publisher's own +/// refusal. +async fn transport_fetch_roundtrip(version: &str) { let pub_origin = moq_tokio::origin::spawn(); let broadcast = pub_origin.create_broadcast("test").expect("failed to create broadcast"); broadcast @@ -444,19 +444,6 @@ async fn transport_fetch_roundtrip(version: &str, served: bool) { let fetched = tokio::time::timeout(TIMEOUT, async { bc.track("video").unwrap().fetch_group(1, None).await }) .await .expect("fetch timed out"); - if !served { - assert!( - matches!(fetched, Err(moq_net::Error::Unsupported)), - "expected an unsupported FETCH, got {:?}", - fetched.err() - ); - drop(connection); - server_handle - .await - .expect("server task panicked") - .expect("server task failed"); - return; - } let mut group = fetched.expect("fetch failed"); assert_eq!(group.sequence, 1); for frame in 0..2 { @@ -492,25 +479,25 @@ async fn transport_fetch_roundtrip(version: &str, served: bool) { #[tracing_test::traced_test] #[tokio::test] async fn fetch_moq_transport_14() { - transport_fetch_roundtrip("moq-transport-14", true).await; + transport_fetch_roundtrip("moq-transport-14").await; } #[tracing_test::traced_test] #[tokio::test] async fn fetch_moq_transport_16() { - transport_fetch_roundtrip("moq-transport-16", true).await; + transport_fetch_roundtrip("moq-transport-16").await; } #[tracing_test::traced_test] #[tokio::test] async fn fetch_moq_transport_18() { - transport_fetch_roundtrip("moq-transport-18", true).await; + transport_fetch_roundtrip("moq-transport-18").await; } #[tracing_test::traced_test] #[tokio::test] async fn fetch_moq_transport_20() { - transport_fetch_roundtrip("moq-transport-20", false).await; + transport_fetch_roundtrip("moq-transport-20").await; } /// A fetch must be served while a live subscription is active on the same track. From 09558a2f6857f2b7f554006ff16247d50d8e3923 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 6 Oct 2026 23:37:56 -0700 Subject: [PATCH 03/28] quest: claim ietf-fetch-only Co-Authored-By: Claude Opus 5.5 From c02ed6f6c4fa7491d673a0af0e76db1dba09e967 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 6 Oct 2026 23:52:50 -0700 Subject: [PATCH 04/28] feat(moq-net): serve IETF TRACK_STATUS and keep timestamps without properties Co-Authored-By: Claude Opus 5.5 --- rs/moq-net/src/ietf/adapter.rs | 8 + rs/moq-net/src/ietf/publisher.rs | 317 ++++++++++++++++++++++--------- rs/moq-net/src/ietf/track.rs | 239 ++++++++++++++++++++++- 3 files changed, 464 insertions(+), 100 deletions(-) diff --git a/rs/moq-net/src/ietf/adapter.rs b/rs/moq-net/src/ietf/adapter.rs index d72cc088e6..cbbb5f04fd 100644 --- a/rs/moq-net/src/ietf/adapter.rs +++ b/rs/moq-net/src/ietf/adapter.rs @@ -922,6 +922,14 @@ fn classify(type_id: u64, body: &Bytes, version: Version, namespaces: &Namespace } _ => Err(Error::UnexpectedMessage), }, + // TRACK_STATUS_OK and TRACK_STATUS_ERROR (v14 only): the one answer the request gets. + ietf::TrackStatusOk::ID_14 | ietf::TRACK_STATUS_ERROR_14 => match version { + Version::Draft14 => { + let id = decode_request_id(body, version)?; + Ok(Route::CloseStream(id)) + } + _ => Err(Error::UnexpectedMessage), + }, // PublishOk (0x1E) ietf::PublishOk::ID => { let id = decode_response_request_id(body, version)?; diff --git a/rs/moq-net/src/ietf/publisher.rs b/rs/moq-net/src/ietf/publisher.rs index 54bf615531..07142d10df 100644 --- a/rs/moq-net/src/ietf/publisher.rs +++ b/rs/moq-net/src/ietf/publisher.rs @@ -37,6 +37,25 @@ fn serving_subscription(subscriber_priority: u8) -> Subscription { } } +/// The Track Properties an answer describing the track carries: SUBSCRIBE_OK and +/// TRACK_STATUS_OK. Empty when the requester opted out with INCLUDE_PROPERTIES=0, which +/// keeps the block present but empty. +/// +/// Declaring the timescale is what opts the track into timestamps; every object Timestamp +/// is in these units. We serve the newest group first, matching moq-lite. A draft without +/// room for a property drops it on encode. +fn track_properties(info: &track::Info, wanted: bool) -> ietf::Properties { + if !wanted { + return ietf::Properties::default(); + } + ietf::Properties { + max_cache_duration: info.max_age, + timescale: Some(info.timescale), + priority: Some(super::priority::to_wire(info.priority)), + group_order: Some(GroupOrder::Descending), + } +} + enum FillStep { Batch, Partial(frame::Consumer), @@ -562,9 +581,10 @@ where if !data.is_empty() { return Err(Error::WrongSize); } + tracing::debug!(message = ?msg, "received track_status"); async move { - if let Err(err) = this.reject_track_status(stream, msg.request_id).await { - tracing::debug!(%err, "track status refusal failed"); + if let Err(err) = this.run_track_status_stream(stream, msg).await { + tracing::debug!(%err, "track_status stream error"); } } .maybe_boxed() @@ -671,12 +691,12 @@ where None => track::Position::after_group(end.group), }); let _ = track.update(subscription); - // A Timestamp goes out only when this SUBSCRIBE_OK actually carries TIMESCALE. - // Drafts 14-16 never write that property, so their objects stay unstamped. - let timescale = msg - .properties_wanted - .then(|| track.info().timescale) - .filter(|_| ietf::Properties::sends_timescale(self.version)); + // A Timestamp goes out wherever the draft can declare its units. A subscriber that + // opted out of this SUBSCRIBE_OK's properties learns them from TRACK_STATUS, so + // the opt-out strips nothing from the objects. Drafts 14-16 never write TIMESCALE, + // so their objects stay unstamped. + let timescale = + Some(track.info().timescale).filter(|_| ietf::Properties::sends_timescale(self.version)); // Draft-20 replaced joining FETCH with subscription fills. Older drafts save // the same boundary used by the subscription so the two streams never overlap. @@ -715,21 +735,7 @@ where track_alias: request_id.0, // The subscription floor and FETCH/FILL cap use this same snapshot. largest: edge.largest, - properties: match msg.properties_wanted { - // Declaring the timescale is what opts the track into timestamps; every - // object Timestamp below is in these units. `timescale` is None when this - // version cannot send TIMESCALE. We serve the newest group first, - // matching moq-lite. - true => ietf::Properties { - max_cache_duration: track.info().max_age, - timescale, - priority: Some(super::priority::to_wire(track.info().priority)), - group_order: Some(GroupOrder::Descending), - }, - // INCLUDE_PROPERTIES=0. The field stays present but empty, which also - // means the track opts out of timestamps for this subscriber. - false => ietf::Properties::default(), - }, + properties: track_properties(track.info(), msg.properties_wanted), }) .await?; @@ -1595,16 +1601,87 @@ where Ok(Err(refusal)) } - async fn reject_track_status(&self, mut stream: Stream, request_id: RequestId) -> Result<(), Error> { - let error_code = request::to_code(&Error::Unsupported, request::Kind::TrackStatus, self.version); + /// Answer a TRACK_STATUS with what a SUBSCRIBE_OK for the track would carry, without + /// subscribing. + /// + /// The track resolves as it would for a SUBSCRIBE, so a relay asks its upstream for a + /// track it does not hold yet and the answer waits on that. The Largest Location is + /// whatever the cache holds now: nothing waits for a fresher one. + async fn run_track_status_stream( + self, + mut stream: Stream, + msg: ietf::TrackStatus<'_>, + ) -> Result<(), Error> { + let request_id = msg.request_id; + let broadcast = match self + .serving_origin() + .await + .request_broadcast(&msg.track_namespace) + .await + { + Ok(broadcast) => broadcast, + Err(err) => return self.reject_track_status(stream, request_id, &err).await, + }; + let track = match broadcast.track(&msg.track_name) { + Ok(track) => track, + Err(err) => return self.reject_track_status(stream, request_id, &err).await, + }; + + // The requester abandoning the request is owed nothing. + let info = { + let query = track.query(); + let mut finished = false; + kio::wait(|waiter| { + let mut cx = waiter.context(); + if super::request_stream::poll_cancel(&mut stream, &mut finished, self.version, &mut cx).is_ready() { + return Poll::Ready(None); + } + query.poll_ok(waiter).map(Some) + }) + .await + }; + let info = match info { + Some(Ok(info)) => info, + Some(Err(err)) => return self.reject_track_status(stream, request_id, &err).await, + None => return Ok(()), + }; + + stream.writer.varint(ietf::TrackStatusOk::id(self.version)).await?; + stream + .writer + .encode(&ietf::TrackStatusOk { + request_id: match self.version { + Version::Draft14 | Version::Draft15 | Version::Draft16 => Some(request_id), + _ => None, + }, + largest: live_edge(&track).largest, + properties: track_properties(&info, msg.properties_wanted), + }) + .await?; + + // The answer is all this stream carries. See [`Self::reject_subscribe`] for why the + // close is not optional. + let _ = stream.writer.close().await; + Ok(()) + } + + async fn reject_track_status( + &self, + mut stream: Stream, + request_id: RequestId, + err: &Error, + ) -> Result<(), Error> { + let error_code = request::to_code(err, request::Kind::TrackStatus, self.version); + let reason_phrase = err.to_string().into(); if self.version == Version::Draft14 { - stream.writer.varint(0x0fu64).await?; // TRACK_STATUS_ERROR has the SUBSCRIBE_ERROR body. + // TRACK_STATUS_ERROR has the SUBSCRIBE_ERROR body. + stream.writer.varint(ietf::TRACK_STATUS_ERROR_14).await?; stream .writer .encode(&ietf::SubscribeError { request_id, error_code, - reason_phrase: "TRACK_STATUS is not supported".into(), + reason_phrase, }) .await?; } else { @@ -1614,7 +1691,7 @@ where .encode(&ietf::RequestError { request_id: matches!(self.version, Version::Draft15 | Version::Draft16).then_some(request_id), error_code, - reason_phrase: "TRACK_STATUS is not supported".into(), + reason_phrase, retry_interval: 0, }) .await?; @@ -3689,68 +3766,125 @@ mod serve_tests { } } - /// TRACK_STATUS is recognized but not implemented, and must get a complete refusal. + const ALL_VERSIONS: [Version; 9] = [ + Version::Draft14, + Version::Draft15, + Version::Draft16, + Version::Draft17, + Version::Draft18, + Version::Draft19, + Version::Draft20, + Version::Draft21, + Version::Draft22, + ]; + + fn track_status(namespace: &str, wanted: bool, version: Version) -> Vec { + let msg = ietf::TrackStatus { + request_id: RequestId(REQUEST_ID), + track_namespace: crate::Path::new(namespace), + track_name: "video".into(), + properties_wanted: wanted, + }; + let mut body = Vec::new(); + msg.encode_msg(&mut Encoder::new(&mut body, version.into()), version) + .unwrap(); + body + } + + /// TRACK_STATUS gets what a SUBSCRIBE_OK would carry, as far as each draft's answer has + /// room, without subscribing to the track. INCLUDE_PROPERTIES=0 empties the properties. #[moq_net_sim::test] - async fn track_status_is_refused_on_every_draft() { - for version in [ - Version::Draft14, - Version::Draft15, - Version::Draft16, - Version::Draft17, - Version::Draft18, - Version::Draft19, - Version::Draft20, - Version::Draft21, - Version::Draft22, - ] { - let h = serve(version); - let stream = Stream::open(&mut h.session.clone(), version).await.unwrap(); - let msg = ietf::TrackStatus { - request_id: RequestId(REQUEST_ID), - track_namespace: crate::Path::new("live"), - track_name: "video".into(), - }; - let mut body = Vec::new(); - msg.encode_msg(&mut Encoder::new(&mut body, version.into()), version) - .unwrap(); - let mark = h.log.writes.lock().unwrap().len(); - h.publisher - .clone() - .handle_stream(ietf::TrackStatus::ID, ietf::Body(bytes::Bytes::from(body)), stream) - .unwrap() - .await; - let actual = h.log.writes.lock().unwrap()[mark..].to_vec(); - let expected = { - let log = crate::lite::test_transport::Log::default(); - let mut writer = - crate::coding::Writer::new(crate::lite::test_transport::SinkSend::new(log.clone()), version); - if version == Version::Draft14 { - writer.varint(0x0fu64).await.unwrap(); - writer - .encode(&ietf::SubscribeError { - request_id: RequestId(REQUEST_ID), - error_code: 0x3, - reason_phrase: "TRACK_STATUS is not supported".into(), - }) - .await - .unwrap(); - } else { - writer.varint(ietf::RequestError::ID).await.unwrap(); - writer - .encode(&ietf::RequestError { - request_id: matches!(version, Version::Draft15 | Version::Draft16) - .then_some(RequestId(REQUEST_ID)), - error_code: 0x3, - reason_phrase: "TRACK_STATUS is not supported".into(), - retry_interval: 0, - }) - .await - .unwrap(); - } - log.writes.lock().unwrap().clone() + async fn track_status_is_answered_on_every_draft() { + for version in ALL_VERSIONS { + for wanted in [true, false] { + let h = serve(version); + let mut group = h.track.create_group(group::Info { sequence: 4 }).unwrap(); + group.write_frame(timestamp(), b"a".as_slice()).unwrap(); + group.write_frame(timestamp(), b"b".as_slice()).unwrap(); + moq_net_sim::sleep(std::time::Duration::from_millis(1)).await; + + let stream = Stream::open(&mut h.session.clone(), version).await.unwrap(); + let mark = h.log.writes.lock().unwrap().len(); + let body = track_status("room", wanted, version); + h.publisher + .clone() + .handle_stream(ietf::TrackStatus::ID, ietf::Body(bytes::Bytes::from(body)), stream) + .unwrap() + .await; + assert!(h.log.resets().is_empty(), "{version}: the answer was reset"); + assert!(h.track.subscription().is_none(), "{version}: TRACK_STATUS subscribed"); + + let wire = h.log.writes.lock().unwrap()[mark..].to_vec(); + let mut buf = Decoder::new(&wire, version.into()); + assert_eq!(buf.varint().unwrap(), ietf::TrackStatusOk::id(version), "{version}"); + let ok = ietf::TrackStatusOk::decode(&mut buf, version).unwrap(); + assert!(buf.is_empty(), "{version}: trailing bytes"); + + let expected = match version { + // GROUP_ORDER is a field on draft-14 and a parameter on draft-15. + Version::Draft14 | Version::Draft15 => ietf::Properties { + group_order: Some(GroupOrder::Descending), + ..Default::default() + }, + Version::Draft16 | Version::Draft17 => ietf::Properties::default(), + // Only draft-20 can carry the opt-out. + _ if !wanted && Filter::is_draft20(version) => ietf::Properties::default(), + _ => track_properties(&track::Info::default(), true), + }; + assert_eq!( + ok, + ietf::TrackStatusOk { + request_id: matches!(version, Version::Draft14 | Version::Draft15 | Version::Draft16) + .then_some(RequestId(REQUEST_ID)), + largest: Some(Location { group: 4, object: 1 }), + properties: expected, + }, + "{version} wanted={wanted}" + ); + } + } + } + + /// A TRACK_STATUS for a broadcast we do not serve is refused as a SUBSCRIBE would be. + #[moq_net_sim::test] + async fn track_status_for_a_missing_broadcast_is_refused() { + for version in ALL_VERSIONS { + let code = refusal(version, ietf::TrackStatus::ID, track_status("absent", true, version)).await; + let expected = match version { + Version::Draft14 => 0x4, + _ => 0x10, }; - assert_eq!(actual, expected, "{version}: wrong TRACK_STATUS refusal bytes"); - assert!(h.log.resets().is_empty(), "{version}: refusal was reset"); + assert_eq!(code, expected, "{version}"); + } + } + + /// Opting out of SUBSCRIBE_OK's properties does not strip the objects' timestamps: + /// their units come from TRACK_STATUS instead. + #[moq_net_sim::test] + async fn the_properties_opt_out_keeps_object_timestamps() { + let version = Version::Draft20; + let stamp = crate::Timestamp::from_millis(123_456).unwrap(); + let mut properties = Vec::new(); + ietf::encode_object_time( + &mut Encoder::new(&mut properties, version.into()), + stamp, + track::Info::default().timescale, + version, + ) + .unwrap(); + + for wanted in [true, false] { + let mut h = serve(version); + let mut group = h.track.create_group(group::Info { sequence: 0 }).unwrap(); + group.write_frame(stamp, b"stamped".as_slice()).unwrap(); + group.finish().unwrap(); + + let mut msg = subscribe(Filter::Unfiltered, None); + msg.properties_wanted = wanted; + run_live(&mut h, msg).await; + + assert_eq!(occurrences(&h.log, b"stamped"), 1, "wanted={wanted}"); + assert_eq!(occurrences(&h.log, &properties), 1, "wanted={wanted}: object unstamped"); } } @@ -3821,7 +3955,7 @@ mod serve_tests { ]; #[rustfmt::skip] - let cases: [(&str, u64, &[u8]); 3] = [ + let cases: [(&str, u64, &[u8]); 2] = [ ("SUBSCRIBE with a Range Filter", ietf::Subscribe::ID, &[ 0x02, // Number of Parameters 0x03, 0x03, 0x03, 0x00, 0xAA, // AUTHORIZATION TOKEN @@ -3833,11 +3967,6 @@ mod serve_tests { 0x17, 0x01, 0x01, // LOCATION_FILTER (0x21): from one group back 0x14, 0x01, // INCLUDE_PROPERTIES (0x35) = 1 ]), - ("TRACK_STATUS with parameters", ietf::TrackStatus::ID, &[ - 0x02, // Number of Parameters - 0x03, 0x03, 0x03, 0x00, 0xAA, // AUTHORIZATION TOKEN - 0x32, 0x00, // INCLUDE_PROPERTIES (0x35) = 0 - ]), ]; // A standalone FETCH we would otherwise serve, carrying FILL_TIMEOUT=0: cache only, diff --git a/rs/moq-net/src/ietf/track.rs b/rs/moq-net/src/ietf/track.rs index 616b5bf546..3d088e44a6 100644 --- a/rs/moq-net/src/ietf/track.rs +++ b/rs/moq-net/src/ietf/track.rs @@ -1,4 +1,4 @@ -//! IETF moq-transport track status messages (v14 + v15) +//! IETF moq-transport TRACK_STATUS and its answer. use std::borrow::Cow; @@ -7,7 +7,7 @@ use num_enum::{IntoPrimitive, TryFromPrimitive}; use crate::{ Path, coding::*, - ietf::{Filter, GroupOrder, RequestId, Subscribe}, + ietf::{Filter, GroupOrder, Location, Properties, RequestId, Subscribe, SubscribeOk}, }; use super::Message; @@ -23,6 +23,9 @@ pub struct TrackStatus<'a> { pub request_id: RequestId, pub track_namespace: Path<'a>, pub track_name: Cow<'a, str>, + /// Whether the requester wants Track Properties on the answer (INCLUDE_PROPERTIES, + /// draft-20). + pub properties_wanted: bool, } impl Message for TrackStatus<'_> { @@ -45,20 +48,147 @@ impl Message for TrackStatus<'_> { w.u8(0); // no parameters } _ => { - encode_params!(w, version,); + // Only the opt-out is worth bytes, and an older peer would read the + // parameter as unknown, which is a protocol violation. + let include_properties = (!self.properties_wanted && Filter::is_draft20(version)).then_some(false); + encode_params!(w, version, 0x35 => include_properties); } } Ok(()) } /// Every draft defines TRACK_STATUS as identical to SUBSCRIBE, so it decodes as one and - /// keeps only what names the track. We refuse the request, so the rest goes unread. + /// keeps what names the track and shapes the answer. The rest goes unread: there is no + /// subscription for it to configure. fn decode_msg(r: &mut Decoder<'_>, version: Version) -> Result { let subscribe = Subscribe::decode_msg(r, version)?; Ok(Self { request_id: subscribe.request_id, track_namespace: subscribe.track_namespace, track_name: subscribe.track_name, + properties_wanted: subscribe.properties_wanted, + }) + } +} + +/// TRACK_STATUS_OK: what a SUBSCRIBE_OK would say about the track, with no subscription. +/// +/// Draft-14 gives it its own type ([`Self::ID_14`]) and the SUBSCRIBE_OK body, with a +/// Track Alias of 0. Later drafts answer with a REQUEST_OK, which gained Track Properties +/// in draft-18; before that only the message parameters carry anything. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct TrackStatusOk { + /// Present on drafts 14-16 only. + pub request_id: Option, + + /// The largest Location in the track (LARGEST_OBJECT), absent while it has no content. + pub largest: Option, + + /// The Track Properties, on the drafts whose answer has room for them. + pub properties: Properties, +} + +/// TRACK_STATUS_ERROR on draft-14, with the SUBSCRIBE_ERROR body. Later drafts refuse +/// with REQUEST_ERROR. +pub const TRACK_STATUS_ERROR_14: u64 = 0x0F; + +impl TrackStatusOk { + /// The draft-14 message type. + pub const ID_14: u64 = 0x0E; + + /// The message type on `version`. + pub fn id(version: Version) -> u64 { + match version { + Version::Draft14 => Self::ID_14, + _ => Self::ID, + } + } + + /// Whether the answer carries a Track Properties block on `version`. + fn has_properties(version: Version) -> bool { + !matches!( + version, + Version::Draft14 | Version::Draft15 | Version::Draft16 | Version::Draft17 + ) + } +} + +impl Message for TrackStatusOk { + /// REQUEST_OK; see [`Self::id`]. + const ID: u64 = 0x07; + + fn encode_msg(&self, w: &mut Encoder<'_>, version: Version) -> Result<(), EncodeError> { + if version == Version::Draft14 { + return SubscribeOk { + request_id: self.request_id, + track_alias: 0, + largest: self.largest, + properties: self.properties, + } + .encode_msg(w, version); + } + + if matches!(version, Version::Draft15 | Version::Draft16) { + self.request_id + .expect("request_id required for draft14-16") + .encode(w, version)?; + } else { + assert!(self.request_id.is_none(), "request_id must be None for draft17+"); + } + + // The parameters SUBSCRIBE_OK carries: GROUP_ORDER is one only on draft-15, and a + // Track Property after. + let group_order = match version { + Version::Draft15 => self.properties.group_order, + _ => None, + }; + encode_params!(w, version, + 0x09 => self.largest, + 0x22 => group_order, + ); + + // Track Properties are the final field, so nothing may follow. + if Self::has_properties(version) { + self.properties.encode(w, version)?; + } + Ok(()) + } + + fn decode_msg(r: &mut Decoder<'_>, version: Version) -> Result { + if version == Version::Draft14 { + let ok = SubscribeOk::decode_msg(r, version)?; + return Ok(Self { + request_id: ok.request_id, + largest: ok.largest, + properties: ok.properties, + }); + } + + let request_id = match version { + Version::Draft15 | Version::Draft16 => Some(RequestId::decode(r, version)?), + _ => None, + }; + // Read as SUBSCRIBE_OK's are: EXPIRES is ignored, and MAX_CACHE_DURATION is a + // parameter only on draft-15. + decode_params!(r, version, + 0x04 => max_cache_duration: Option, + 0x08 => _expires: Option, + 0x09 => largest: Option, + 0x22 => group_order: Option, + ); + let mut properties = match Self::has_properties(version) { + true => Properties::decode(r, version)?, + false => Properties::default(), + }; + properties.group_order = properties.group_order.or(group_order); + if version == Version::Draft15 { + properties.max_cache_duration = max_cache_duration.map(std::time::Duration::from_millis); + } + + Ok(Self { + request_id, + largest, + properties, }) } } @@ -107,6 +237,7 @@ mod tests { request_id: RequestId(1), track_namespace: Path::new("test/ns"), track_name: "video".into(), + properties_wanted: true, }; let encoded = encode_message(&msg, Version::Draft14); @@ -123,6 +254,7 @@ mod tests { request_id: RequestId(1), track_namespace: Path::new("test/ns"), track_name: "video".into(), + properties_wanted: true, }; let encoded = encode_message(&msg, Version::Draft15); @@ -139,6 +271,7 @@ mod tests { request_id: RequestId(1), track_namespace: Path::new("test/ns"), track_name: "video".into(), + properties_wanted: true, }; let encoded = encode_message(&msg, Version::Draft17); @@ -155,6 +288,7 @@ mod tests { request_id: RequestId(1), track_namespace: Path::new("test/ns"), track_name: "video".into(), + properties_wanted: true, }; let encoded = encode_message(&msg, Version::Draft16); @@ -171,6 +305,7 @@ mod tests { request_id: RequestId(1), track_namespace: Path::new("test/ns"), track_name: "video".into(), + properties_wanted: true, }; let encoded = encode_message(&msg, Version::Draft18); @@ -181,9 +316,100 @@ mod tests { assert_eq!(decoded.track_name, "video"); } + const ALL: [Version; 9] = [ + Version::Draft14, + Version::Draft15, + Version::Draft16, + Version::Draft17, + Version::Draft18, + Version::Draft19, + Version::Draft20, + Version::Draft21, + Version::Draft22, + ]; + + /// INCLUDE_PROPERTIES only exists from draft-20, so the opt-out is dropped before it, + /// and the default (1) is never written. + #[test] + fn test_track_status_opts_out_of_properties_from_draft_20() { + for version in ALL { + for wanted in [true, false] { + let msg = TrackStatus { + request_id: RequestId(1), + track_namespace: Path::new("ns"), + track_name: "video".into(), + properties_wanted: wanted, + }; + let decoded: TrackStatus = decode_message(&encode_message(&msg, version), version) + .unwrap_or_else(|e| panic!("{version}: {e}")); + let expected = wanted || !Filter::is_draft20(version); + assert_eq!(decoded.properties_wanted, expected, "{version} wanted={wanted}"); + } + } + } + + /// TRACK_STATUS_OK carries what SUBSCRIBE_OK would, as far as each draft's answer + /// has room: draft-14 is a SUBSCRIBE_OK, drafts 15-17 a REQUEST_OK with only + /// parameters, and from draft-18 the REQUEST_OK ends with the Track Properties. + #[test] + fn test_track_status_ok_round_trip() { + let properties = Properties { + max_cache_duration: Some(std::time::Duration::from_secs(30)), + timescale: Some(crate::Timescale::MILLI), + priority: Some(7), + group_order: Some(GroupOrder::Descending), + }; + let largest = Some(Location { group: 5, object: 2 }); + + for version in ALL { + let msg = TrackStatusOk { + request_id: matches!(version, Version::Draft14 | Version::Draft15 | Version::Draft16) + .then_some(RequestId(3)), + largest, + properties, + }; + let decoded: TrackStatusOk = decode_message(&encode_message(&msg, version), version) + .unwrap_or_else(|e| panic!("{version}: {e}")); + + let expected = match version { + Version::Draft14 | Version::Draft15 => Properties { + group_order: Some(GroupOrder::Descending), + ..Default::default() + }, + Version::Draft16 | Version::Draft17 => Properties::default(), + _ => properties, + }; + assert_eq!(decoded.request_id, msg.request_id, "{version}"); + assert_eq!(decoded.largest, largest, "{version}"); + assert_eq!(decoded.properties, expected, "{version}"); + } + } + + /// Draft-14's TRACK_STATUS_OK is a SUBSCRIBE_OK with Track Alias 0, under its own type. + #[test] + fn test_track_status_ok_draft_14_is_a_subscribe_ok() { + let msg = TrackStatusOk { + request_id: Some(RequestId(3)), + largest: None, + properties: Properties::default(), + }; + assert_eq!(TrackStatusOk::id(Version::Draft14), 0x0E); + assert_eq!(TrackStatusOk::id(Version::Draft15), 0x07); + let subscribe_ok = SubscribeOk { + request_id: Some(RequestId(3)), + track_alias: 0, + largest: None, + properties: Properties::default(), + }; + assert_eq!( + encode_message(&msg, Version::Draft14), + encode_message(&subscribe_ok, Version::Draft14) + ); + } + /// TRACK_STATUS is identical to SUBSCRIBE on every draft, so it carries whatever - /// fields and parameters a SUBSCRIBE can. A peer that sends them must still reach - /// our refusal rather than having its session closed. + /// fields and parameters a SUBSCRIBE can. A peer that sends them must still be + /// answered rather than having its session closed. #[test] fn test_track_status_carries_subscribe_fields() { // Request ID 1, Track Namespace ("live"), Track Name ("video"). @@ -219,6 +445,7 @@ mod tests { let msg = TrackStatus::decode_msg(&mut buf, version).unwrap_or_else(|e| panic!("{version}: {e}")); assert!(buf.is_empty(), "{version}: trailing bytes"); assert_eq!(msg.track_name, "video", "{version}"); + assert_eq!(msg.properties_wanted, version != Version::Draft20, "{version}"); } } } From 4dcd0436b4fc22b9a3f60307be4e8e0bcb94b647 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 7 Oct 2026 00:01:33 -0700 Subject: [PATCH 05/28] feat(moq-net): learn fetch-only IETF tracks from TRACK_STATUS Co-Authored-By: Claude Opus 5.5 --- rs/moq-net/src/ietf/subscriber.rs | 236 +++++++++++++++++++++++++++++- rs/moq-net/tests/fetch_only.rs | 166 +++++++++++++++++++++ 2 files changed, 397 insertions(+), 5 deletions(-) create mode 100644 rs/moq-net/tests/fetch_only.rs diff --git a/rs/moq-net/src/ietf/subscriber.rs b/rs/moq-net/src/ietf/subscriber.rs index ceb5bd8f5c..211b3cac6c 100644 --- a/rs/moq-net/src/ietf/subscriber.rs +++ b/rs/moq-net/src/ietf/subscriber.rs @@ -444,6 +444,27 @@ impl Target { } } +/// The model's info for a track the publisher described (SUBSCRIBE_OK or TRACK_STATUS_OK). +/// +/// The copy keeps microsecond timestamps whatever units the publisher declared: each +/// object converts on receipt. +fn accepted_info(max_age: Option, priority: Option) -> track::Info { + track::Info::default() + .with_timescale(Timescale::MICRO) + .with_max_age(max_age) + .with_priority(super::priority::from_wire(priority.unwrap_or(128))) +} + +/// What a TRACK_STATUS for a track nobody subscribes to came to. +enum Status { + /// Accepted as a copy with no subscription. + Known(Idle), + /// The publisher does not implement TRACK_STATUS, so a SUBSCRIBE has to learn the track. + Unsupported(track::Request), + /// The request was answered: refused, or abandoned. + Ended, +} + /// A copy with no subscription upstream, kept for fetches and a returning subscriber. struct Idle { track: track::Producer, @@ -1735,7 +1756,21 @@ where let track_name = request.name().to_owned(); // Group FETCHes for cache misses: standalone, so they outlive each subscription. let mut group_fetches = TaskSet::owned(); - let mut target = Target::Request(request); + + // Demand with nobody subscribing (fetches, or a TRACK_STATUS downstream) learns the + // track from TRACK_STATUS rather than a SUBSCRIBE: a finished track refuses one, and + // one made only to learn the track races the fetches it was for. + let mut target = match request.subscription() { + Some(_) => Target::Request(request), + None => match self.track_status(&broadcast_path, &track_name, request).await { + Status::Known(idle) => match self.linger(&broadcast_path, &track_name, idle, &mut group_fetches).await { + Some(next) => Target::Resume(next), + None => return, + }, + Status::Unsupported(request) => Target::Request(request), + Status::Ended => return, + }, + }; loop { let Some(idle) = self .subscribe_once(&broadcast_path, &track_name, target, &mut group_fetches) @@ -1971,10 +2006,7 @@ where priority, largest, } = accepted; - let info = track::Info::default() - .with_timescale(Timescale::MICRO) - .with_max_age(max_age) - .with_priority(super::priority::from_wire(priority.unwrap_or(128))); + let info = accepted_info(max_age, priority); // The copy already holds the track: it is current again up to the answer's Largest // Location once the join's head lands or a newer group does, since leaving cut the // group it was in. @@ -2544,6 +2576,118 @@ where } } + /// Learn a track nobody subscribes to from TRACK_STATUS, and accept it as a copy with no + /// subscription, which [`Self::linger`] serves fetches from and subscribes once someone + /// does. + async fn track_status(&mut self, broadcast_path: &Path<'_>, track_name: &str, request: track::Request) -> Status { + let request_id = match self.control.next_request_id(&self.runtime).await { + Ok(id) => id, + Err(err) => { + request.reject(err); + return Status::Ended; + } + }; + let mut stream = match Stream::open(&mut self.session.clone(), self.version).await { + Ok(stream) => stream, + Err(err) => { + request.reject(err); + return Status::Ended; + } + }; + let written = async { + stream.writer.varint(ietf::TrackStatus::ID).await?; + stream + .writer + .encode(&ietf::TrackStatus { + request_id, + track_namespace: broadcast_path.to_owned(), + track_name: track_name.into(), + properties_wanted: true, + }) + .await + } + .await; + if let Err(err) = written { + request.reject(err); + return Status::Ended; + } + + // Nobody wanting the track any more ends the wait on a peer that may never answer. + let demand = request.demand(); + let response = { + let mut response = std::pin::pin!(self.read_track_status_response(&mut stream)); + loop { + let res = kio::wait(|waiter| { + // An answer that already arrived wins over abandonment. + if let Poll::Ready(res) = waiter.poll_future(response.as_mut()) { + return Poll::Ready(Some(res)); + } + demand.poll_unused(waiter).map(|_| None) + }) + .await; + match res { + Some(res) => break Some(res), + None if request.reject_unused(Error::Cancel) => break None, + // Demand returned in the gap. + None => continue, + } + } + }; + + let Some(response) = response else { + stream.reader.abort(&Error::Cancel); + stream.writer.abort(&Error::Cancel); + return Status::Ended; + }; + // The publisher has read the request, since it answered, and FINs after its answer: + // FIN our side too and let the stream drop. + let _ = stream.writer.finish(); + let ok = match response { + Ok(ok) => ok, + // A publisher that does not implement TRACK_STATUS still answers a SUBSCRIBE. + Err(Error::Unsupported) => return Status::Unsupported(request), + Err(err) => { + tracing::debug!(%err, broadcast = %self.origin.absolute(broadcast_path), track = %track_name, "track status refused"); + request.reject(err); + return Status::Ended; + } + }; + + tracing::debug!(broadcast = %self.origin.absolute(broadcast_path), track = %track_name, "track status known"); + let dynamic = request.dynamic(); + let mut track = request.accept(accepted_info(ok.properties.max_cache_duration, ok.properties.priority)); + // Nothing feeds this copy live: what it caches is fetched, never the live edge. + track.set_idle(); + Status::Known(Idle { + demand: track.demand(), + track, + dynamic, + timescale: ok.properties.timescale, + }) + } + + /// Read the answer to a TRACK_STATUS: TRACK_STATUS_OK, or the publisher's refusal as an + /// error. + async fn read_track_status_response(&self, stream: &mut Stream) -> Result { + let type_id = stream.reader.varint().await?; + let body: ietf::Body = stream.reader.decode().await?; + let mut data = body.decoder(self.version); + + let code = match type_id { + id if id == ietf::TrackStatusOk::id(self.version) => { + return Ok(ietf::TrackStatusOk::decode_msg(&mut data, self.version)?); + } + ietf::TRACK_STATUS_ERROR_14 if self.version == Version::Draft14 => { + ietf::SubscribeError::decode_msg(&mut data, self.version)?.error_code + } + ietf::RequestError::ID if self.version != Version::Draft14 => { + ietf::RequestError::decode_msg(&mut data, self.version)?.error_code + } + _ => return Err(Error::UnexpectedMessage), + }; + Err(request::from_code(code, request::Kind::TrackStatus, self.version)) + } + pub async fn recv_group(&mut self, stream: &mut Reader) -> Result<(), Error> { let mut group: ietf::GroupHeader = stream.decode().await?; @@ -5214,6 +5358,88 @@ mod tests { outstanding } + /// A publisher that does not implement TRACK_STATUS (an older release, or another + /// implementation) refuses it NOT_SUPPORTED. Fetch-only demand then learns the track + /// from a SUBSCRIBE, as it did before TRACK_STATUS was asked. + #[moq_net_sim::test] + async fn an_unsupported_track_status_falls_back_to_subscribe() { + for version in [ + Version::Draft14, + Version::Draft15, + Version::Draft17, + Version::Draft18, + Version::Draft22, + ] { + let log = crate::lite::test_transport::Log::default(); + let mut writer = + crate::coding::Writer::new(crate::lite::test_transport::SinkSend::new(log.clone()), version); + let reason = "TRACK_STATUS is not supported"; + match version { + Version::Draft14 => { + writer.varint(ietf::TRACK_STATUS_ERROR_14).await.unwrap(); + writer + .encode(&ietf::SubscribeError { + request_id: RequestId(0), + error_code: 0x3, + reason_phrase: reason.into(), + }) + .await + .unwrap(); + } + _ => { + writer.varint(ietf::RequestError::ID).await.unwrap(); + writer + .encode(&ietf::RequestError { + request_id: matches!(version, Version::Draft15 | Version::Draft16).then_some(RequestId(0)), + error_code: 0x3, + reason_phrase: reason.into(), + retry_interval: 0, + }) + .await + .unwrap(); + } + } + let refusal = log.writes.lock().unwrap().clone(); + + let session = crate::lite::test_transport::ScriptedSession::per_stream(vec![refusal]); + let log = session.log.clone(); + let (tasks, _task_set) = crate::util::TaskSet::new(); + let mut subscriber = Subscriber::new( + crate::time::Clock::sim(), + session, + crate::origin::Config::new(crate::Hop::new(1).unwrap()).produce(), + Control::new(None, false), + None, + peer::PeerSetup::default(), + crate::Hop::new(1).unwrap(), + None, + version, + tasks, + Default::default(), + ); + + let producer = crate::broadcast::Info::default().produce(); + let mut dynamic = producer.dynamic(); + let consumer = producer.consume(); + // Fetch-only demand: nobody subscribes. + let _fetching = consumer.track("video").unwrap().fetch_group(0, None); + let request = dynamic.requested_track().await.expect("no track requested"); + assert!(request.subscription().is_none()); + + let serving = moq_net_sim::spawn(async move { + subscriber.run_subscribe(Path::new("broadcast"), dynamic, request).await; + }); + settle().await; + + assert_eq!( + control_message_types(&log, version), + [ietf::TrackStatus::ID, ietf::Subscribe::ID], + "{version}: a refused TRACK_STATUS must fall back to SUBSCRIBE" + ); + serving.abort(); + } + } + /// The publisher opens no fetch stream for an empty range, so a fill against a track /// with no content is owed nothing. Leaving it outstanding would withhold every later /// group behind a head that is never coming. diff --git a/rs/moq-net/tests/fetch_only.rs b/rs/moq-net/tests/fetch_only.rs new file mode 100644 index 0000000000..a22f6cc96f --- /dev/null +++ b/rs/moq-net/tests/fetch_only.rs @@ -0,0 +1,166 @@ +//! Fetching a group from a moq-transport publisher never subscribes to its track. +//! +//! The subscriber learns the track from TRACK_STATUS instead, so a finished track is still +//! fetchable, and a relay's fetch-only demand puts no live subscription upstream to race +//! the fetches that report where the track ends. + +mod support; + +use std::sync::Arc; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::time::Duration; + +use moq_net::{Hop, Timestamp, Version}; +use support::harness::{MockConnectOptions, connect_mock}; + +const TIMEOUT: Duration = Duration::from_secs(5); + +fn produce_origin(hop: u64) -> moq_net::origin::Producer { + let (producer, driver) = moq_net::origin::Producer::new(moq_net::origin::Config::new(Hop::new(hop).unwrap())); + support::harness::spawn(driver); + producer +} + +async fn resolve(origin: &moq_net::origin::Producer) -> moq_net::broadcast::Consumer { + let consumer = origin.consume(); + moq_net_sim::timeout(TIMEOUT, consumer.routed("bcast")) + .await + .expect("announce timeout") + .expect("routed"); + moq_net_sim::timeout(TIMEOUT, consumer.request_broadcast("bcast")) + .await + .expect("resolve timeout") + .expect("broadcast resolves") +} + +/// Publish a finished track of three groups, and flag any subscription it ever gets. +fn publish_finished(origin: &moq_net::origin::Producer) -> (moq_net::broadcast::Producer, Arc) { + let broadcast = origin.create_broadcast("bcast").unwrap(); + let mut track = broadcast.create_track("video", None).unwrap(); + broadcast.announce(Default::default()).unwrap(); + for sequence in 0..3u64 { + let mut group = track.append_group().unwrap(); + for frame in 0..2 { + group + .write_frame(Timestamp::ZERO, format!("{sequence}-{frame}").into_bytes()) + .unwrap(); + } + group.finish().unwrap(); + } + track.finish().unwrap(); + + let subscribed = Arc::new(AtomicBool::new(false)); + let flag = subscribed.clone(); + drop(moq_net_sim::spawn(async move { + while let Ok(subscription) = track.subscription_changed().await { + if subscription.is_some() { + flag.store(true, Ordering::SeqCst); + } + } + })); + (broadcast, subscribed) +} + +async fn fetch(broadcast: &moq_net::broadcast::Consumer, sequence: u64) -> Result>, moq_net::Error> { + let mut group = moq_net_sim::timeout(TIMEOUT, async { + broadcast.track("video").unwrap().fetch_group(sequence, None).await + }) + .await + .expect("fetch timed out")?; + let mut frames = Vec::new(); + while let Some(frame) = moq_net_sim::timeout(TIMEOUT, group.read_frame()) + .await + .expect("read timed out")? + { + frames.push(frame.payload.to_vec()); + } + Ok(frames) +} + +fn frames(sequence: u64) -> Vec> { + (0..2).map(|frame| format!("{sequence}-{frame}").into_bytes()).collect() +} + +/// Drafts whose FETCH we serve, and the newest, whose FETCH we refuse until draft-20 +/// FETCH lands: it must still not subscribe. +const VERSIONS: &[(&str, bool)] = &[ + ("moq-transport-14", true), + ("moq-transport-15", true), + ("moq-transport-16", true), + ("moq-transport-17", true), + ("moq-transport-18", true), + ("moq-transport-19", true), + ("moq-transport-22", false), +]; + +async fn direct(version: &str, served: bool) { + let version: Version = version.parse().unwrap(); + let publisher = produce_origin(1); + let (broadcast, subscribed) = publish_finished(&publisher); + + let client = produce_origin(2); + let mut options = MockConnectOptions::new(version); + options.server_publish = Some(publisher.consume()); + options.client_subscribe = Some(client.clone()); + let pair = connect_mock(options).await; + + let remote = resolve(&client).await; + let fetched = fetch(&remote, 1).await; + match served { + true => assert_eq!(fetched.expect("fetch"), frames(1), "{version}"), + false => assert!(fetched.is_err(), "{version}: draft-20 FETCH is refused"), + } + assert!(!subscribed.load(Ordering::SeqCst), "{version}: the fetch subscribed"); + + drop((remote, pair, broadcast, client, publisher)); +} + +/// A finished track is fetched without a SUBSCRIBE, which it would refuse. +#[moq_net_sim::test] +async fn a_fetch_learns_the_track_without_subscribing() { + let mut failures = Vec::new(); + for (version, served) in VERSIONS { + if moq_net_sim::spawn(direct(version, *served)).await.is_err() { + failures.push(*version); + } + } + assert!(failures.is_empty(), "failed: {failures:?}"); +} + +async fn relayed(version: &str) { + let version: Version = version.parse().unwrap(); + let publisher = produce_origin(1); + let (broadcast, subscribed) = publish_finished(&publisher); + + let relay = produce_origin(2); + let mut options = MockConnectOptions::new(version); + options.server_publish = Some(publisher.consume()); + options.client_subscribe = Some(relay.clone()); + let upstream = connect_mock(options).await; + + let client = produce_origin(3); + let mut options = MockConnectOptions::new(version); + options.server_publish = Some(relay.consume()); + options.client_subscribe = Some(client.clone()); + let downstream = connect_mock(options).await; + + let remote = resolve(&client).await; + for sequence in [2, 0] { + assert_eq!(fetch(&remote, sequence).await.expect("fetch"), frames(sequence), "{version}"); + } + assert!(!subscribed.load(Ordering::SeqCst), "{version}: the relay subscribed upstream"); + + drop((remote, downstream, upstream, broadcast, client, relay, publisher)); +} + +/// A relay serves a downstream fetch with a fetch upstream, never a subscription. +#[moq_net_sim::test] +async fn a_relay_fetches_without_subscribing_upstream() { + let mut failures = Vec::new(); + for (version, served) in VERSIONS { + if *served && moq_net_sim::spawn(relayed(version)).await.is_err() { + failures.push(*version); + } + } + assert!(failures.is_empty(), "failed: {failures:?}"); +} From ee8656b37ba833eba301634f7b22214775c87540 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 7 Oct 2026 00:03:04 -0700 Subject: [PATCH 06/28] docs: TRACK_STATUS for fetch-only demand; carry the opt-out into its own quest Co-Authored-By: Claude Opus 5.5 --- doc/concept/standard.md | 13 +++++++- quest/m1/README.md | 2 +- quest/m1/fetch-ok-properties.md | 2 +- quest/m1/ietf-fetch-only.md | 49 ----------------------------- quest/m1/ietf-properties-opt-out.md | 39 +++++++++++++++++++++++ quest/m1/untimed-model.md | 7 ++--- 6 files changed, 56 insertions(+), 56 deletions(-) delete mode 100644 quest/m1/ietf-fetch-only.md create mode 100644 quest/m1/ietf-properties-opt-out.md diff --git a/doc/concept/standard.md b/doc/concept/standard.md index 9b4730a795..3919f38386 100644 --- a/doc/concept/standard.md +++ b/doc/concept/standard.md @@ -60,6 +60,17 @@ any `FETCH` on draft-20 and later, which moved the range into `LOCATION_FILTER`. A standalone `FETCH` carries no timestamps, since no `SUBSCRIBE_OK` declared a timescale for it. +A Rust subscriber learns a track that is only fetched, never subscribed, from +`TRACK_STATUS` instead of a `SUBSCRIBE`, so a finished track stays fetchable +and a relay puts no live subscription upstream to race its fetches. A +publisher that refuses `TRACK_STATUS` with `NOT_SUPPORTED` gets the +`SUBSCRIBE` instead. The Rust publisher answers `TRACK_STATUS` with what its +`SUBSCRIBE_OK` would carry, as far as the draft's answer has room: the Largest +Location on every draft, and the Track Properties from draft-18, when the +answer gained them. A `SUBSCRIBE` that sets `INCLUDE_PROPERTIES` to 0 gets an +empty properties block, but its objects keep their Timestamps, whose units +`TRACK_STATUS` reports. + On drafts 14–19, the Rust publisher also serves relative and absolute joining `FETCH` requests for `NextObject` subscriptions, for the subscription group's saved prefix only, while the subscription delivers later objects. One reaching @@ -91,7 +102,7 @@ A legal request that is not served is refused on its own with `NOT_SUPPORTED`, leaving the session open: a `SUBSCRIBE` with `FORWARD=0`, a `SUBSCRIBE` or `FETCH` carrying Range Filters (no `MAX_FILTER_RANGES` is advertised), a `FETCH` carrying `FILL_TIMEOUT` (Timed-Out gaps are not written), -`TRACK_STATUS`, `SUBSCRIBE_TRACKS` (draft-18 and later), and the `FETCH` +`TRACK_STATUS` to a JavaScript publisher, `SUBSCRIBE_TRACKS` (draft-18 and later), and the `FETCH` forms above. `NEW_GROUP_REQUEST` is ignored, as the draft allows a publisher without dynamic groups to do. A parameter the negotiated draft does not define still closes the session with diff --git a/quest/m1/README.md b/quest/m1/README.md index 21d2307fea..6ed8db97be 100644 --- a/quest/m1/README.md +++ b/quest/m1/README.md @@ -83,7 +83,7 @@ blocks. The quests that gated m0 lines moved under them. - [JS data consumer timestamps](/quest/m1/js-data-consumer-timestamps.md) - @moq/json and @moq/flate consumers return each value's timestamp, with snapshot `next()` and `latest()` - [Draft-20 FETCH](/quest/m1/ietf-fetch-location.md) - a draft-20+ FETCH within one group is served from its LOCATION_FILTER, as older drafts are - [FETCH_OK properties](/quest/m1/fetch-ok-properties.md) - our FETCH_OK carries the track properties SUBSCRIBE_OK does, as draft 16+ requires -- [Fetch without SUBSCRIBE](/quest/m1/ietf-fetch-only.md) - a relay fetches from an IETF upstream without subscribing, finished tracks included, with End of Track always reported +- [Properties opt-out](/quest/m1/ietf-properties-opt-out.md) - our IETF subscriber learns track properties once, from TRACK_STATUS, once the opt-out stops costing released publishers their properties and timestamps - [IETF object gaps](/quest/m1/ietf-object-gaps.md) - a gapped object ID is refused loudly like a subgroup in Rust and JS, and an overflowing one closes the session in Rust - [JS IETF datagrams](/quest/m1/js-ietf-datagram.md) - `@moq/net` sends and receives datagram groups over moq-transport, like Rust - [Datagrams are live-only](/quest/m1/datagram-unfetchable.md) - no FETCH, replay to a new subscriber, or cache fill ever returns a datagram group diff --git a/quest/m1/fetch-ok-properties.md b/quest/m1/fetch-ok-properties.md index c2b03a2f72..9eed24f5e5 100644 --- a/quest/m1/fetch-ok-properties.md +++ b/quest/m1/fetch-ok-properties.md @@ -51,4 +51,4 @@ the block. Interop: run `just test interop --all`. ## Related -- [Fetch without SUBSCRIBE](/quest/m1/ietf-fetch-only.md) - asks TRACK_STATUS for the same properties when a publisher omits them +- [Properties opt-out](/quest/m1/ietf-properties-opt-out.md) - our subscriber opting out of these properties, to learn them from TRACK_STATUS instead diff --git a/quest/m1/ietf-fetch-only.md b/quest/m1/ietf-fetch-only.md deleted file mode 100644 index c904d30e60..0000000000 --- a/quest/m1/ietf-fetch-only.md +++ /dev/null @@ -1,49 +0,0 @@ -# [M] Fetch without SUBSCRIBE - -## Goal - -A relay serves a FETCH-only demand for an IETF upstream track without -SUBSCRIBE upstream. A finished upstream track can still be fetched, and a -FETCH_OK that reaches the end of the track says so every time. - -## Plan - -The origin splices a route only once the track's info is known. moq-lite gets -it from TRACK_INFO, so its fetch-only demand never subscribes. IETF has no -TRACK_INFO, so today fetch-only demand still makes the relay SUBSCRIBE upstream. -Two symptoms follow: - -- A finished upstream track refuses the SUBSCRIBE, so its groups cannot be - fetched either. -- The live subscription races the group FETCHes. End of Track is known only - once an upstream FETCH_OK reports it, so a downstream FETCH that runs to the - end can answer before that and leave End of Track unset. moxygen's "FETCH - with large objects" case flakes on this. - -TRACK_STATUS is the likely source of the info. The publisher refuses it today, -so both sides are in scope. Keep the SUBSCRIBE path for real subscription -demand. - -Decided (2026-10-01): - -- Our subscriber learns a track's properties from TRACK_STATUS, sent in - parallel with every FETCH or SUBSCRIBE. From draft 20 it sets - INCLUDE_PROPERTIES to 0 on those requests, so the properties aren't sent - twice. Before draft 20 there's no opt-out, so it ignores the duplicate. -- First decouple object timestamps from INCLUDE_PROPERTIES: the publisher - derives its serving timescale from `properties_wanted` - (`publisher.rs:628`), so opting out today strips the Timestamp property - from every object and the subscriber falls back to arrival time. Test - TRACK_STATUS plus the opt-out with source timestamps that differ from - arrival. -- The publisher serves TRACK_STATUS. Its reply carries what it would set in a - SUBSCRIBE_OK (d22 §9.13), from the same code that fills SUBSCRIBE_OK and - FETCH_OK. -- The publisher still follows the spec for subscribers that don't opt out: - [FETCH_OK properties](/quest/m1/fetch-ok-properties.md). - -## Related - -- [FETCH_OK properties](/quest/m1/fetch-ok-properties.md) - the publisher side of the same properties - -- [JavaScript FETCH](/quest/m1/js-fetch.md) - the browser publisher answers these fetches diff --git a/quest/m1/ietf-properties-opt-out.md b/quest/m1/ietf-properties-opt-out.md new file mode 100644 index 0000000000..f8c8308a8c --- /dev/null +++ b/quest/m1/ietf-properties-opt-out.md @@ -0,0 +1,39 @@ +# [S] IETF subscriber opts out of duplicate properties + +## Goal + +Our IETF subscriber learns a track's properties once, from TRACK_STATUS, and +from draft 20 opts out of the copy in SUBSCRIBE_OK and FETCH_OK with +INCLUDE_PROPERTIES = 0, without losing properties or object timestamps +against a publisher that refuses TRACK_STATUS. + +## Plan + +Decided (2026-10-01), carried over from fetch-only demand (#PR_NUMBER): send +TRACK_STATUS in parallel with every SUBSCRIBE or FETCH, set +INCLUDE_PROPERTIES to 0 on those from draft 20, and ignore the duplicate +before draft 20. The publisher half already landed there: our publisher +serves TRACK_STATUS, and the opt-out no longer strips object timestamps. +Fetch-only demand already asks TRACK_STATUS instead of subscribing. + +Open, for the maintainer (found 2026-10-07): every released moq-rs +publisher, drafts 20-22 included, refuses TRACK_STATUS with NOT_SUPPORTED, +and on INCLUDE_PROPERTIES = 0 strips the Timestamp from every object. A +subscriber that opts out against one loses max cache duration, priority, +group order, and timestamps on a published version. Each SUBSCRIBE also +gains a request, which spends request-ID credit. Options: + +- Opt out only once this session's peer has answered a TRACK_STATUS, so the + first request on a session pays the duplicate. +- Opt out unconditionally, once releases that serve TRACK_STATUS are old + enough to drop. +- Drop the opt-out: SUBSCRIBE_OK and FETCH_OK keep carrying the properties, + and TRACK_STATUS stays for fetch-only demand. Then delete this quest. + +Test TRACK_STATUS plus the opt-out with source timestamps that differ from +arrival, and against a publisher that refuses TRACK_STATUS. + +## Related + +- [FETCH_OK properties](/quest/m1/fetch-ok-properties.md) - the FETCH_OK half of the same properties +- [Untimed model](/quest/m1/untimed-model.md) - decides what a track without declared units looks like to the subscriber diff --git a/quest/m1/untimed-model.md b/quest/m1/untimed-model.md index 93387fa29c..8b9ce3fee2 100644 --- a/quest/m1/untimed-model.md +++ b/quest/m1/untimed-model.md @@ -113,10 +113,10 @@ Things to look out for: Media consumers that need a time refuse an untimed frame, except for the end-marker rule above. -- A FETCH learns track units from SUBSCRIBE_OK (a joining or fill FETCH). - A standalone FETCH that learns none when it is accepted is untimed (decided +- A FETCH learns track units from SUBSCRIBE_OK (a joining or fill FETCH), and + fetch-only demand from the TRACK_STATUS_OK its track is accepted with. A + standalone FETCH that learns none when it is accepted is untimed (decided 2026-10-05), until [FETCH_OK properties](/quest/m1/fetch-ok-properties.md) - or TRACK_STATUS ([Fetch without SUBSCRIBE](/quest/m1/ietf-fetch-only.md)) carry them. Test that a track accepted with units still yields timestamps, alongside the untimed cases. - Which objects the malformed rule covers. Status-only objects (End of @@ -157,7 +157,6 @@ published drafts change as the timestamp draft says. ## Related -- [Fetch without SUBSCRIBE](/quest/m1/ietf-fetch-only.md) - opting out of properties today falls back to arrival time - [Publisher timeliness](/quest/m1/qos/publisher-timeliness.md) - relay ingest measures arrival minus timestamp, and must skip untimed frames - [One max_age meaning](/quest/m1/cache-max-age.md) - gives untimed groups a wall-clock staleness rule, fixes the failover stall this introduces, and replaces the latest-group start - [Translator](/quest/m1/rs2ts/translator.md) - flags the nested `Option` in first-start resolution this touches From 594e25e99ef73b093fdc625169ffe4dcb5e55d53 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 7 Oct 2026 00:05:22 -0700 Subject: [PATCH 07/28] style: rustfmt Co-Authored-By: Claude Opus 5.5 --- rs/moq-net/src/ietf/publisher.rs | 3 +-- rs/moq-net/src/ietf/subscriber.rs | 5 ++++- rs/moq-net/src/ietf/track.rs | 4 ++-- rs/moq-net/tests/fetch_only.rs | 11 +++++++++-- 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/rs/moq-net/src/ietf/publisher.rs b/rs/moq-net/src/ietf/publisher.rs index 07142d10df..83b3881890 100644 --- a/rs/moq-net/src/ietf/publisher.rs +++ b/rs/moq-net/src/ietf/publisher.rs @@ -695,8 +695,7 @@ where // opted out of this SUBSCRIBE_OK's properties learns them from TRACK_STATUS, so // the opt-out strips nothing from the objects. Drafts 14-16 never write TIMESCALE, // so their objects stay unstamped. - let timescale = - Some(track.info().timescale).filter(|_| ietf::Properties::sends_timescale(self.version)); + let timescale = Some(track.info().timescale).filter(|_| ietf::Properties::sends_timescale(self.version)); // Draft-20 replaced joining FETCH with subscription fills. Older drafts save // the same boundary used by the subscription so the two streams never overlap. diff --git a/rs/moq-net/src/ietf/subscriber.rs b/rs/moq-net/src/ietf/subscriber.rs index 211b3cac6c..498bd54ddb 100644 --- a/rs/moq-net/src/ietf/subscriber.rs +++ b/rs/moq-net/src/ietf/subscriber.rs @@ -1763,7 +1763,10 @@ where let mut target = match request.subscription() { Some(_) => Target::Request(request), None => match self.track_status(&broadcast_path, &track_name, request).await { - Status::Known(idle) => match self.linger(&broadcast_path, &track_name, idle, &mut group_fetches).await { + Status::Known(idle) => match self + .linger(&broadcast_path, &track_name, idle, &mut group_fetches) + .await + { Some(next) => Target::Resume(next), None => return, }, diff --git a/rs/moq-net/src/ietf/track.rs b/rs/moq-net/src/ietf/track.rs index 3d088e44a6..5d82d976d1 100644 --- a/rs/moq-net/src/ietf/track.rs +++ b/rs/moq-net/src/ietf/track.rs @@ -368,8 +368,8 @@ mod tests { largest, properties, }; - let decoded: TrackStatusOk = decode_message(&encode_message(&msg, version), version) - .unwrap_or_else(|e| panic!("{version}: {e}")); + let decoded: TrackStatusOk = + decode_message(&encode_message(&msg, version), version).unwrap_or_else(|e| panic!("{version}: {e}")); let expected = match version { Version::Draft14 | Version::Draft15 => Properties { diff --git a/rs/moq-net/tests/fetch_only.rs b/rs/moq-net/tests/fetch_only.rs index a22f6cc96f..dc531089d9 100644 --- a/rs/moq-net/tests/fetch_only.rs +++ b/rs/moq-net/tests/fetch_only.rs @@ -146,9 +146,16 @@ async fn relayed(version: &str) { let remote = resolve(&client).await; for sequence in [2, 0] { - assert_eq!(fetch(&remote, sequence).await.expect("fetch"), frames(sequence), "{version}"); + assert_eq!( + fetch(&remote, sequence).await.expect("fetch"), + frames(sequence), + "{version}" + ); } - assert!(!subscribed.load(Ordering::SeqCst), "{version}: the relay subscribed upstream"); + assert!( + !subscribed.load(Ordering::SeqCst), + "{version}: the relay subscribed upstream" + ); drop((remote, downstream, upstream, broadcast, client, relay, publisher)); } From 4ff8c9068207431c967c7cb2402c3f9730d4daf5 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 7 Oct 2026 00:07:37 -0700 Subject: [PATCH 08/28] fix(moq-net): fail a pending TRACK_STATUS with the session Co-Authored-By: Claude Opus 5.5 --- rs/moq-net/src/ietf/publisher.rs | 2 +- rs/moq-net/src/ietf/subscriber.rs | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/rs/moq-net/src/ietf/publisher.rs b/rs/moq-net/src/ietf/publisher.rs index 83b3881890..9eef587f68 100644 --- a/rs/moq-net/src/ietf/publisher.rs +++ b/rs/moq-net/src/ietf/publisher.rs @@ -695,7 +695,7 @@ where // opted out of this SUBSCRIBE_OK's properties learns them from TRACK_STATUS, so // the opt-out strips nothing from the objects. Drafts 14-16 never write TIMESCALE, // so their objects stay unstamped. - let timescale = Some(track.info().timescale).filter(|_| ietf::Properties::sends_timescale(self.version)); + let timescale = ietf::Properties::sends_timescale(self.version).then_some(track.info().timescale); // Draft-20 replaced joining FETCH with subscription fills. Older drafts save // the same boundary used by the subscription so the two streams never overlap. diff --git a/rs/moq-net/src/ietf/subscriber.rs b/rs/moq-net/src/ietf/subscriber.rs index 498bd54ddb..c7a21dc734 100644 --- a/rs/moq-net/src/ietf/subscriber.rs +++ b/rs/moq-net/src/ietf/subscriber.rs @@ -2615,8 +2615,10 @@ where return Status::Ended; } - // Nobody wanting the track any more ends the wait on a peer that may never answer. + // Nobody wanting the track any more ends the wait on a peer that may never answer, + // and the session ending fails it with the session's reason. let demand = request.demand(); + let mut closed = self.session.clone(); let response = { let mut response = std::pin::pin!(self.read_track_status_response(&mut stream)); loop { @@ -2625,6 +2627,9 @@ where if let Poll::Ready(res) = waiter.poll_future(response.as_mut()) { return Poll::Ready(Some(res)); } + if let Poll::Ready(err) = closed.poll_closed(&mut waiter.context()) { + return Poll::Ready(Some(Err(Error::from_transport(err)))); + } demand.poll_unused(waiter).map(|_| None) }) .await; From 047e1ae060809a045a2850fb09c5ee5da2986a80 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 7 Oct 2026 00:12:27 -0700 Subject: [PATCH 09/28] quest: link the opt-out quest to its PR Co-Authored-By: Claude Opus 5.5 --- quest/m1/ietf-properties-opt-out.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/quest/m1/ietf-properties-opt-out.md b/quest/m1/ietf-properties-opt-out.md index f8c8308a8c..b4a7dedd0e 100644 --- a/quest/m1/ietf-properties-opt-out.md +++ b/quest/m1/ietf-properties-opt-out.md @@ -9,7 +9,7 @@ against a publisher that refuses TRACK_STATUS. ## Plan -Decided (2026-10-01), carried over from fetch-only demand (#PR_NUMBER): send +Decided (2026-10-01), carried over from fetch-only demand (#4974): send TRACK_STATUS in parallel with every SUBSCRIBE or FETCH, set INCLUDE_PROPERTIES to 0 on those from draft 20, and ignore the duplicate before draft 20. The publisher half already landed there: our publisher From 24c38465ae724df9d86b4673606122a158466c51 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 7 Oct 2026 10:22:31 -0700 Subject: [PATCH 10/28] fix(ietf): refuse an empty draft-20 FETCH answer Draft-20's inclusive End Location always names an object, so a group holding nothing at or past the start is refused DOES_NOT_EXIST instead of answered with an End Location the subscriber reads as one object. Co-Authored-By: Claude Opus 5.5 --- rs/moq-net/src/ietf/publisher.rs | 37 +++++++++++++++++++++++++++++--- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/rs/moq-net/src/ietf/publisher.rs b/rs/moq-net/src/ietf/publisher.rs index 7d14d32726..d13c69a748 100644 --- a/rs/moq-net/src/ietf/publisher.rs +++ b/rs/moq-net/src/ietf/publisher.rs @@ -1469,6 +1469,14 @@ where Some(Err(err)) => return self.reject_fetch(stream, msg.request_id, &err, &err.to_string()).await, None => return Ok(()), }; + // Draft-20's inclusive End Location always names an object, so it cannot say the + // group holds none at or past the start. + let draft20 = Filter::is_draft20(self.version); + if draft20 && group.frames.is_empty() { + return self + .reject_fetch(stream, msg.request_id, &Error::NotFound, "no objects in range") + .await; + } let (end_location, end_of_track) = if joined { // The subscription starts at the saved Largest Object, so the prefix of that @@ -1495,8 +1503,8 @@ where } }; // Draft-20's End Location is inclusive, naming the last object. A whole group was read - // to its end, so that is known, and an empty answer still covers its start. - let end_location = match Filter::is_draft20(self.version) { + // to its end, so that is known. + let end_location = match draft20 { true => { let object = match end_location.object { 0 => group.end(), @@ -1504,7 +1512,7 @@ where }; Location { group: group.sequence, - object: object.saturating_sub(1).max(start.object), + object: object - 1, } } false => end_location, @@ -4697,6 +4705,29 @@ mod serve_tests { } } + /// A group holding no objects at or past the start is refused on draft 20, whose + /// inclusive End Location cannot describe an empty answer. Older drafts answer it empty. + #[moq_net_sim::test] + async fn a_standalone_fetch_past_the_last_object_is_empty() { + for version in FETCH_DRAFTS { + let mut h = serve(version); + publish_pairs(&mut h, 5, None); + settle().await; + + let buf = standalone_fetch( + &h, + Location { group: 2, object: 2 }, + Location { group: 2, object: 0 }, + GroupOrder::Ascending, + ) + .await; + match Filter::is_draft20(version) { + true => assert_eq!(fetch_refusal(buf, version), does_not_exist(version), "{version}"), + false => assert_eq!(fetch_answer(buf, version).1, Vec::new(), "{version}"), + } + } + } + /// The last group of a finished track ends the response at its last object, with End /// of Track set. #[moq_net_sim::test] From 39dfccf0ccf4cfe4f4991207c7cd42fda0315541 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 7 Oct 2026 10:39:14 -0700 Subject: [PATCH 11/28] feat(moq-net): ask for an IETF track's properties once per copy From draft-20, a SUBSCRIBE resuming a lingering copy and every group FETCH set INCLUDE_PROPERTIES=0: the copy already learned the track from SUBSCRIBE_OK or TRACK_STATUS_OK. A resumed copy keeps the timescale it learned. FETCH_OK's properties were already ignored once the track was accepted, so the code building info from them goes. Deletes quest/m1/ietf-properties-opt-out.md, which this covers. Co-Authored-By: Claude Opus 5.5 --- doc/concept/standard.md | 4 +- quest/m1/README.md | 1 - quest/m1/fetch-ok-properties.md | 17 +- quest/m1/ietf-properties-opt-out.md | 39 ----- rs/moq-net/src/fuzz.rs | 1 + rs/moq-net/src/ietf/fetch.rs | 209 ++++++++++++++---------- rs/moq-net/src/ietf/publisher.rs | 6 + rs/moq-net/src/ietf/subscriber.rs | 242 ++++++++++++++++------------ rs/moq-net/src/ietf/version.rs | 1 + rs/moq-net/tests/rejoin.rs | 55 +++++++ 10 files changed, 335 insertions(+), 240 deletions(-) delete mode 100644 quest/m1/ietf-properties-opt-out.md diff --git a/doc/concept/standard.md b/doc/concept/standard.md index 3919f38386..2a3d33dab7 100644 --- a/doc/concept/standard.md +++ b/doc/concept/standard.md @@ -69,7 +69,9 @@ publisher that refuses `TRACK_STATUS` with `NOT_SUPPORTED` gets the Location on every draft, and the Track Properties from draft-18, when the answer gained them. A `SUBSCRIBE` that sets `INCLUDE_PROPERTIES` to 0 gets an empty properties block, but its objects keep their Timestamps, whose units -`TRACK_STATUS` reports. +`TRACK_STATUS` reports. From draft-20, a Rust subscriber sets +`INCLUDE_PROPERTIES` to 0 on every later `SUBSCRIBE` or `FETCH` for a track +it already learned, instead of receiving the same properties again. On drafts 14–19, the Rust publisher also serves relative and absolute joining `FETCH` requests for `NextObject` subscriptions, for the subscription group's diff --git a/quest/m1/README.md b/quest/m1/README.md index a9d0e2bad7..f62eb398e9 100644 --- a/quest/m1/README.md +++ b/quest/m1/README.md @@ -81,7 +81,6 @@ blocks. The quests that gated m0 lines moved under them. - [JS data consumer timestamps](/quest/m1/js-data-consumer-timestamps.md) - @moq/json and @moq/flate consumers return each value's timestamp, with snapshot `next()` and `latest()` - [Draft-20 FETCH](/quest/m1/ietf-fetch-location.md) - a draft-20+ FETCH within one group is served from its LOCATION_FILTER, as older drafts are - [FETCH_OK properties](/quest/m1/fetch-ok-properties.md) - our FETCH_OK carries the track properties SUBSCRIBE_OK does, as draft 16+ requires -- [Properties opt-out](/quest/m1/ietf-properties-opt-out.md) - our IETF subscriber learns track properties once, from TRACK_STATUS, once the opt-out stops costing released publishers their properties and timestamps - [IETF object gaps](/quest/m1/ietf-object-gaps.md) - a gapped object ID is refused loudly like a subgroup in Rust and JS, and an overflowing one closes the session in Rust - [JS IETF datagrams](/quest/m1/js-ietf-datagram.md) - `@moq/net` sends and receives datagram groups over moq-transport, like Rust - [Datagrams are live-only](/quest/m1/datagram-unfetchable.md) - no FETCH, replay to a new subscriber, or cache fill ever returns a datagram group diff --git a/quest/m1/fetch-ok-properties.md b/quest/m1/fetch-ok-properties.md index 9eed24f5e5..6068736f2a 100644 --- a/quest/m1/fetch-ok-properties.md +++ b/quest/m1/fetch-ok-properties.md @@ -9,16 +9,14 @@ meets draft 16+'s rule to include all of a track's properties. ## Plan -Found (2026-10-01) in #4647: a group FETCH with no -prior SUBSCRIBE_OK now reads its max age from FETCH_OK, but our publisher -sends an empty block. +Found (2026-10-01) in #4647: our publisher's FETCH_OK sends an empty +properties block. Our own subscriber no longer reads it (#4974): it learns a +track from SUBSCRIBE_OK or TRACK_STATUS_OK before any FETCH, and from draft 20 +opts out with INCLUDE_PROPERTIES = 0. Other fetch-only readers still need it. - `rs/moq-net/src/ietf/publisher.rs` sends FETCH_OK with `properties: Default::default()`. SUBSCRIBE_OK fills them in the block near `publisher.rs:655`. Share that code so the two can't diverge. -- Our group-fetch accept path reads only the max age from FETCH_OK and - hardcodes a microsecond timescale. Apply the same properties SUBSCRIBE_OK - does (timescale, priority, group order) there too. - Drafts 14-15 allow the omission: MAX_CACHE_DURATION is a MAY there, and FETCH_OK has no properties block. From draft 16, a relay MUST include all Extension Headers / Properties associated with a track in FETCH_OK (d16 @@ -36,8 +34,8 @@ sends an empty block. - Honour INCLUDE_PROPERTIES (0x35) on FETCH from draft 20, once [Draft-20 FETCH](/quest/m1/ietf-fetch-location.md) serves draft-20 FETCH at all (today our publisher refuses every one). It defaults to sending the - properties; at 0 the block is present but empty. `fetch.rs` decodes it on - draft 20+ and drops it, so it is not honoured today. + properties; at 0 the block is present but empty. `fetch.rs` decodes it into + `Fetch::properties_wanted`, which the publisher does not honour yet. - Test per draft range: FETCH_OK round-trips the properties SUBSCRIBE_OK would carry, a standalone FETCH's objects arrive stamped, and INCLUDE_PROPERTIES = 0 empties the block. @@ -49,6 +47,3 @@ the block. Interop: run `just test interop --all`. - [Draft-20 FETCH](/quest/m1/ietf-fetch-location.md) - draft-20 FETCH is served at all, and edits the same `run_fetch_stream` -## Related - -- [Properties opt-out](/quest/m1/ietf-properties-opt-out.md) - our subscriber opting out of these properties, to learn them from TRACK_STATUS instead diff --git a/quest/m1/ietf-properties-opt-out.md b/quest/m1/ietf-properties-opt-out.md deleted file mode 100644 index b4a7dedd0e..0000000000 --- a/quest/m1/ietf-properties-opt-out.md +++ /dev/null @@ -1,39 +0,0 @@ -# [S] IETF subscriber opts out of duplicate properties - -## Goal - -Our IETF subscriber learns a track's properties once, from TRACK_STATUS, and -from draft 20 opts out of the copy in SUBSCRIBE_OK and FETCH_OK with -INCLUDE_PROPERTIES = 0, without losing properties or object timestamps -against a publisher that refuses TRACK_STATUS. - -## Plan - -Decided (2026-10-01), carried over from fetch-only demand (#4974): send -TRACK_STATUS in parallel with every SUBSCRIBE or FETCH, set -INCLUDE_PROPERTIES to 0 on those from draft 20, and ignore the duplicate -before draft 20. The publisher half already landed there: our publisher -serves TRACK_STATUS, and the opt-out no longer strips object timestamps. -Fetch-only demand already asks TRACK_STATUS instead of subscribing. - -Open, for the maintainer (found 2026-10-07): every released moq-rs -publisher, drafts 20-22 included, refuses TRACK_STATUS with NOT_SUPPORTED, -and on INCLUDE_PROPERTIES = 0 strips the Timestamp from every object. A -subscriber that opts out against one loses max cache duration, priority, -group order, and timestamps on a published version. Each SUBSCRIBE also -gains a request, which spends request-ID credit. Options: - -- Opt out only once this session's peer has answered a TRACK_STATUS, so the - first request on a session pays the duplicate. -- Opt out unconditionally, once releases that serve TRACK_STATUS are old - enough to drop. -- Drop the opt-out: SUBSCRIBE_OK and FETCH_OK keep carrying the properties, - and TRACK_STATUS stays for fetch-only demand. Then delete this quest. - -Test TRACK_STATUS plus the opt-out with source timestamps that differ from -arrival, and against a publisher that refuses TRACK_STATUS. - -## Related - -- [FETCH_OK properties](/quest/m1/fetch-ok-properties.md) - the FETCH_OK half of the same properties -- [Untimed model](/quest/m1/untimed-model.md) - decides what a track without declared units looks like to the subscriber diff --git a/rs/moq-net/src/fuzz.rs b/rs/moq-net/src/fuzz.rs index 03fb24810e..3e4b70eeb5 100644 --- a/rs/moq-net/src/fuzz.rs +++ b/rs/moq-net/src/fuzz.rs @@ -878,6 +878,7 @@ pub fn seeds() -> Vec { fetch_type, range_filters: false, fill_timeout: false, + properties_wanted: true, }; fetch.encode_bytes(*version).ok() }) else { diff --git a/rs/moq-net/src/ietf/fetch.rs b/rs/moq-net/src/ietf/fetch.rs index 0a50802c15..002c4e9405 100644 --- a/rs/moq-net/src/ietf/fetch.rs +++ b/rs/moq-net/src/ietf/fetch.rs @@ -133,6 +133,9 @@ pub struct Fetch<'a> { /// that ends in Timed-Out gaps we cannot write, so the request is refused rather than /// left to wait without it. Never encoded; we send no fill timeout. pub fill_timeout: bool, + /// Whether the requester wants Track Properties in FETCH_OK (INCLUDE_PROPERTIES, + /// draft-20+). Always `true` before draft-20, which has no way to opt out. + pub properties_wanted: bool, } impl Message for Fetch<'_> { @@ -174,10 +177,13 @@ impl Message for Fetch<'_> { }; encode_namespace(w, namespace)?; w.string(track)?; + // INCLUDE_PROPERTIES defaults to 1, so only the opt-out is worth bytes. + let include_properties = (!self.properties_wanted).then_some(false); encode_params!(w, version, 0x20 => self.subscriber_priority, 0x21 => *filter, 0x22 => group_order, + 0x35 => include_properties, ); } } @@ -190,92 +196,102 @@ impl Message for Fetch<'_> { let _required_request_id_delta = buf.varint()?; } - // The token is ignored: the session's grant is what authorizes the request, and - // INCLUDE_PROPERTIES only shapes a FETCH_OK we don't send on draft-20. - let (fetch_type, subscriber_priority, group_order, range_filters, fill_timeout) = match version { - Version::Draft14 => { - let subscriber_priority = buf.u8()?; - let group_order = GroupOrder::decode(buf, version)?; - let fetch_type = FetchType::decode(buf, version)?; - Parameters::skip(buf)?; - (fetch_type, Some(subscriber_priority), Some(group_order), false, false) - } - Version::Draft15 | Version::Draft16 | Version::Draft17 | Version::Draft18 | Version::Draft19 => { - let fetch_type = FetchType::decode(buf, version)?; - decode_params!(buf, version, - 0x03 => _authorization_token: Vec, - 0x0A => fill_timeout: Option, - 0x20 => subscriber_priority: Option, - 0x22 => group_order: Option, - 0x25 => subgroup_filter: Vec, - 0x26 => object_id_filter: Vec, - 0x27 => priority_filter: Vec, - 0x28 => object_property_filter: Vec, - ); - let range_filters = [ - subgroup_filter, - object_id_filter, - priority_filter, - object_property_filter, - ] - .iter() - .any(|filter| !filter.is_empty()); - - // An unknown message parameter is a protocol violation, so each stays rejected - // on the drafts that predate it: FILL_TIMEOUT arrived in draft-18. - let has_fill_timeout = !matches!(version, Version::Draft15 | Version::Draft16 | Version::Draft17); - if (fill_timeout.is_some() && !has_fill_timeout) || (range_filters && !has_range_filters(version)) { - return Err(DecodeError::InvalidValue); + // The token is ignored: the session's grant is what authorizes the request. + let (fetch_type, subscriber_priority, group_order, range_filters, fill_timeout, properties_wanted) = + match version { + Version::Draft14 => { + let subscriber_priority = buf.u8()?; + let group_order = GroupOrder::decode(buf, version)?; + let fetch_type = FetchType::decode(buf, version)?; + Parameters::skip(buf)?; + ( + fetch_type, + Some(subscriber_priority), + Some(group_order), + false, + false, + true, + ) } - - ( - fetch_type, - subscriber_priority, - group_order, - range_filters, - fill_timeout.is_some(), - ) - } - // Draft-20 names the track up front and moves the range into LOCATION_FILTER. - _ => { - let namespace = decode_namespace(buf)?; - let track = Cow::Owned(buf.string()?); - decode_params!(buf, version, - 0x03 => _authorization_token: Vec, - 0x0A => fill_timeout: Option, - 0x20 => subscriber_priority: Option, - 0x21 => filter: Option, - 0x22 => group_order: Option, - 0x25 => subgroup_filter: Vec, - 0x26 => object_id_filter: Vec, - 0x27 => priority_filter: Vec, - 0x28 => object_property_filter: Vec, - 0x35 => _include_properties: Option, - ); - let range_filters = [ - subgroup_filter, - object_id_filter, - priority_filter, - object_property_filter, - ] - .iter() - .any(|filter| !filter.is_empty()); - - let fetch_type = FetchType::Filtered { - namespace, - track, - // An absent LOCATION_FILTER fetches the whole track. - filter: filter.unwrap_or(Filter::Unfiltered), - }; - ( - fetch_type, - subscriber_priority, - group_order, - range_filters, - fill_timeout.is_some(), - ) - } - }; + Version::Draft15 | Version::Draft16 | Version::Draft17 | Version::Draft18 | Version::Draft19 => { + let fetch_type = FetchType::decode(buf, version)?; + decode_params!(buf, version, + 0x03 => _authorization_token: Vec, + 0x0A => fill_timeout: Option, + 0x20 => subscriber_priority: Option, + 0x22 => group_order: Option, + 0x25 => subgroup_filter: Vec, + 0x26 => object_id_filter: Vec, + 0x27 => priority_filter: Vec, + 0x28 => object_property_filter: Vec, + ); + let range_filters = [ + subgroup_filter, + object_id_filter, + priority_filter, + object_property_filter, + ] + .iter() + .any(|filter| !filter.is_empty()); + + // An unknown message parameter is a protocol violation, so each stays rejected + // on the drafts that predate it: FILL_TIMEOUT arrived in draft-18. + let has_fill_timeout = !matches!(version, Version::Draft15 | Version::Draft16 | Version::Draft17); + if (fill_timeout.is_some() && !has_fill_timeout) || (range_filters && !has_range_filters(version)) { + return Err(DecodeError::InvalidValue); + } + + ( + fetch_type, + subscriber_priority, + group_order, + range_filters, + fill_timeout.is_some(), + true, + ) + } + // Draft-20 names the track up front and moves the range into LOCATION_FILTER. + _ => { + let namespace = decode_namespace(buf)?; + let track = Cow::Owned(buf.string()?); + decode_params!(buf, version, + 0x03 => _authorization_token: Vec, + 0x0A => fill_timeout: Option, + 0x20 => subscriber_priority: Option, + 0x21 => filter: Option, + 0x22 => group_order: Option, + 0x25 => subgroup_filter: Vec, + 0x26 => object_id_filter: Vec, + 0x27 => priority_filter: Vec, + 0x28 => object_property_filter: Vec, + 0x35 => include_properties: Option, + ); + let range_filters = [ + subgroup_filter, + object_id_filter, + priority_filter, + object_property_filter, + ] + .iter() + .any(|filter| !filter.is_empty()); + + let fetch_type = FetchType::Filtered { + namespace, + track, + // An absent LOCATION_FILTER fetches the whole track. + filter: filter.unwrap_or(Filter::Unfiltered), + }; + ( + fetch_type, + subscriber_priority, + group_order, + range_filters, + fill_timeout.is_some(), + // Defaults to 1, so an absent parameter means the requester wants them. + include_properties.unwrap_or(true), + ) + } + }; Ok(Self { request_id, @@ -285,6 +301,7 @@ impl Message for Fetch<'_> { fetch_type, range_filters, fill_timeout, + properties_wanted, }) } } @@ -681,6 +698,7 @@ mod tests { }, range_filters: false, fill_timeout: false, + properties_wanted: true, }; let encoded = encode_message(&msg, Version::Draft14); @@ -704,6 +722,7 @@ mod tests { }, range_filters: false, fill_timeout: false, + properties_wanted: true, }; let encoded = encode_message(&msg, Version::Draft15); @@ -745,6 +764,7 @@ mod tests { }, range_filters: false, fill_timeout: false, + properties_wanted: true, }; let encoded = encode_message(&msg, Version::Draft16); @@ -768,6 +788,7 @@ mod tests { }, range_filters: false, fill_timeout: false, + properties_wanted: true, }; let encoded = encode_message(&msg, Version::Draft17); @@ -845,6 +866,7 @@ mod tests { }, range_filters: false, fill_timeout: false, + properties_wanted: true, }; let encoded = encode_message(&msg, Version::Draft18); @@ -924,6 +946,7 @@ mod tests { assert_eq!(fetch.group_order, GroupOrder::Ascending); assert!(fetch.range_filters, "{version}"); assert!(fetch.fill_timeout, "{version}"); + assert!(!fetch.properties_wanted, "{version}"); assert_eq!( fetch.fetch_type, FetchType::Filtered { @@ -954,6 +977,7 @@ mod tests { }, range_filters: false, fill_timeout: false, + properties_wanted: true, }; #[rustfmt::skip] @@ -965,6 +989,21 @@ mod tests { assert_eq!(encode_message(&fetch, Version::Draft20), expected); assert_eq!(decode_message::(&expected, Version::Draft20).unwrap(), fetch); + // Only the opt-out is written, as a uint8 with no Length. + let opted_out = Fetch { + properties_wanted: false, + ..fetch.clone() + }; + #[rustfmt::skip] + let expected = [FETCH_HEAD, &[ + 0x03, // Number of Parameters + 0x20, 0x80, // SUBSCRIBER_PRIORITY = 128 + 0x02, 0x02, // GROUP_ORDER (0x22) = Descending + 0x13, 0x00, // INCLUDE_PROPERTIES (0x35) = 0 + ]].concat(); + assert_eq!(encode_message(&opted_out, Version::Draft20), expected); + assert_eq!(decode_message::(&expected, Version::Draft20).unwrap(), opted_out); + // The tagged forms have no spelling from draft-20 on, and this one none before it. let mut buf = Vec::new(); assert!( diff --git a/rs/moq-net/src/ietf/publisher.rs b/rs/moq-net/src/ietf/publisher.rs index 9cd0ce5f92..cee9b9c1d9 100644 --- a/rs/moq-net/src/ietf/publisher.rs +++ b/rs/moq-net/src/ietf/publisher.rs @@ -3543,6 +3543,7 @@ mod serve_tests { }, range_filters: false, fill_timeout: false, + properties_wanted: true, } .encode_msg(&mut Encoder::new(&mut fetch_body, version.into()), version) .unwrap(); @@ -4175,6 +4176,7 @@ mod serve_tests { }, range_filters: false, fill_timeout: false, + properties_wanted: true, }, ) .await?; @@ -4653,6 +4655,7 @@ mod serve_tests { }, range_filters: false, fill_timeout: false, + properties_wanted: true, }, ) .await @@ -4893,6 +4896,7 @@ mod serve_tests { fetch_type, range_filters: false, fill_timeout: false, + properties_wanted: true, }, ) .await @@ -4936,6 +4940,7 @@ mod serve_tests { }, range_filters: false, fill_timeout: false, + properties_wanted: true, }, ) .await @@ -6729,6 +6734,7 @@ mod tests { fetch_type, range_filters: false, fill_timeout: false, + properties_wanted: true, }, ) .await diff --git a/rs/moq-net/src/ietf/subscriber.rs b/rs/moq-net/src/ietf/subscriber.rs index 512bc6c84c..63cffec8e5 100644 --- a/rs/moq-net/src/ietf/subscriber.rs +++ b/rs/moq-net/src/ietf/subscriber.rs @@ -1860,10 +1860,27 @@ where // Write Subscribe message. The aggregate is read now: a subscriber can join while // the request ID and stream were awaited, and nothing updates the priority after. let priority = target.subscription().map(|s| s.priority).unwrap_or(0); - if let Err(err) = self - .write_subscribe(&mut stream, request_id, broadcast_path, track_name, priority, join) - .await - { + let written = async { + stream.writer.varint(ietf::Subscribe::ID).await?; + stream + .writer + .encode(&ietf::Subscribe { + request_id, + track_namespace: broadcast_path.to_owned(), + track_name: track_name.into(), + subscriber_priority: super::priority::to_wire(priority), + group_order: GroupOrder::Descending, + filter: join.filter, + fill: join.fill, + // A resumed copy already knows the track, from the answer that accepted it. + properties_wanted: matches!(target, Target::Request(_)), + forward: true, + range_filters: false, + }) + .await + } + .await; + if let Err(err) = written { tracing::debug!(%err, "failed to write subscribe"); self.remove_subscribe(request_id); target.fail(err); @@ -2010,6 +2027,11 @@ where largest, } = accepted; let info = accepted_info(max_age, priority); + // A resumed copy opted out of the properties, so it keeps the units it learned. + let timescale = match &resumed { + Some(idle) => idle.timescale, + None => timescale, + }; // The copy already holds the track: it is current again up to the answer's Largest // Location once the join's head lands or a newer group does, since leaving cut the // group it was in. @@ -2381,34 +2403,6 @@ where Ok(()) } - async fn write_subscribe( - &self, - stream: &mut Stream, - request_id: RequestId, - broadcast: &Path<'_>, - track_name: &str, - priority: u8, - join: Join, - ) -> Result<(), Error> { - stream.writer.varint(ietf::Subscribe::ID).await?; - stream - .writer - .encode(&ietf::Subscribe { - request_id, - track_namespace: broadcast.to_owned(), - track_name: track_name.into(), - subscriber_priority: super::priority::to_wire(priority), - group_order: GroupOrder::Descending, - filter: join.filter, - fill: join.fill, - properties_wanted: true, - forward: true, - range_filters: false, - }) - .await?; - Ok(()) - } - /// Send the joining FETCH that follows a pre-draft-20 SUBSCRIBE, and keep reading its /// answer until the subscription ends. /// @@ -2480,6 +2474,7 @@ where fetch_type, range_filters: false, fill_timeout: false, + properties_wanted: true, }) .await?; Ok::<(), Error>(()) @@ -3584,6 +3579,9 @@ where }, range_filters: false, fill_timeout: false, + // The copy learned the track before any fetch, from SUBSCRIBE_OK or + // TRACK_STATUS_OK, so a FETCH_OK repeating it is waste. + properties_wanted: false, }) .await?; Ok::<_, Error>(()) @@ -3642,15 +3640,10 @@ where // the stream owes every object up to it. One past the group covers it whole. let end = (end.group == sequence).then_some(end.object); - // Only a track nothing has subscribed to yet takes this info, as SUBSCRIBE_OK would - // have set it. FETCH_OK carries the same Track Properties, so the retention window - // comes from it. - let info = track::Info::default() - .with_timescale(Timescale::MICRO) - .with_max_age(ok.properties.max_cache_duration); // Joined fetches still count until they pick the accepted group up from the cache. let joined = request.result.clone(); - let mut producer = match request.accept(info) { + // The track was accepted before any fetch was asked, so it keeps that info. + let mut producer = match request.accept(None) { Ok(producer) => producer, // Already served by a concurrent fetch, or the track closed. Err(err) => { @@ -5448,6 +5441,113 @@ mod tests { } } + /// A copy that already knows its track, here from TRACK_STATUS_OK, opts out of the + /// properties when a subscriber returns, and keeps the units it learned rather than + /// taking the empty block the opted-out SUBSCRIBE_OK carries. + #[moq_net_sim::test] + async fn a_resumed_subscribe_opts_out_of_known_properties() { + use crate::coding::Encode as _; + fn message(id: u64, msg: &M, version: Version) -> Vec { + let mut buf = Vec::new(); + crate::coding::Encoder::new(&mut buf, version.into()) + .varint(id) + .unwrap(); + msg.encode(&mut crate::coding::Encoder::new(&mut buf, version.into()), version) + .unwrap(); + buf + } + + let declared = Timescale::new(90_000).unwrap(); + for version in [Version::Draft20, Version::Draft22] { + let status_ok = message( + ietf::TrackStatusOk::ID, + &ietf::TrackStatusOk { + request_id: None, + largest: None, + properties: ietf::Properties { + timescale: Some(declared), + ..Default::default() + }, + }, + version, + ); + let session = crate::lite::test_transport::ScriptedSession::per_stream(vec![ + status_ok, + // Opted out, so its block is empty. + message( + ietf::SubscribeOk::ID, + &ietf::SubscribeOk { + request_id: None, + track_alias: 7, + largest: None, + properties: Default::default(), + }, + version, + ), + ]); + let log = session.log.clone(); + let (tasks, _task_set) = crate::util::TaskSet::new(); + let mut subscriber = Subscriber::new( + crate::time::Clock::sim(), + session, + crate::origin::Config::new(crate::Hop::new(1).unwrap()).produce(), + Control::new(None, false), + None, + peer::PeerSetup::default(), + crate::Hop::new(1).unwrap(), + None, + version, + tasks, + Default::default(), + ); + + let producer = crate::broadcast::Info::default().produce(); + let mut dynamic = producer.dynamic(); + let consumer = producer.consume(); + let track = consumer.track("video").unwrap(); + // Demand with nobody subscribing learns the track from TRACK_STATUS. + let mut query = std::pin::pin!(track.query()); + assert!(futures::poll!(query.as_mut()).is_pending()); + let request = dynamic.requested_track().await.expect("no track requested"); + + let probe = subscriber.clone(); + let serving = moq_net_sim::spawn(async move { + subscriber.run_subscribe(Path::new("broadcast"), dynamic, request).await; + }); + settle().await; + let _subscription = track.subscribe(None); + settle().await; + + let writes = log.writes.lock().unwrap().clone(); + let mut buf = Decoder::new(&writes, version.into()); + let mut subscribes = Vec::new(); + while !buf.is_empty() { + let type_id = buf.varint().unwrap(); + let size = buf.u16().unwrap(); + let mut body = Decoder::new(buf.slice(size as usize).unwrap(), version.into()); + if type_id == ietf::Subscribe::ID { + subscribes.push(ietf::Subscribe::decode_msg(&mut body, version).unwrap()); + } + } + assert_eq!(subscribes.len(), 1, "{version}: the returning subscriber subscribes"); + assert!( + !subscribes[0].properties_wanted, + "{version}: TRACK_STATUS_OK already gave the properties" + ); + + let timescale = probe + .state + .lock() + .subscribes + .values() + .next() + .expect("the subscription is registered") + .timescale; + assert_eq!(timescale, Some(declared), "{version}: the learned units are kept"); + serving.abort(); + } + } + /// The publisher opens no fetch stream for an empty range, so a fill against a track /// with no content is owed nothing. Leaving it outstanding would withhold every later /// group behind a head that is never coming. @@ -8973,70 +9073,6 @@ mod joining_fetch_tests { assert!(fetch.await.is_err(), "the accepted group was aborted"); } - /// A group FETCH for a track nothing subscribed to takes its retention window from - /// FETCH_OK's properties, as a subscription would from SUBSCRIBE_OK's. - #[moq_net_sim::test] - async fn a_group_fetch_takes_fetch_ok_max_age() { - const GROUP: u64 = 4; - const MAX_AGE: Duration = Duration::from_secs(12); - - for version in [Version::Draft17, Version::Draft19] { - let ok = message_bytes( - ietf::FetchOk::ID, - &ietf::FetchOk { - request_id: None, - group_order: GroupOrder::Ascending, - end_of_track: false, - end_location: ietf::Location { - group: GROUP + 1, - object: 0, - }, - properties: ietf::Properties { - max_cache_duration: Some(MAX_AGE), - ..Default::default() - }, - }, - version, - ); - - let session = ScriptedSession::per_stream_reset(vec![ok]); - let (tasks, _task_set) = crate::util::TaskSet::new(); - let subscriber = Subscriber::new( - crate::time::Clock::sim(), - session.clone(), - crate::origin::Config::new(crate::Hop::new(1).unwrap()).produce(), - Control::new(None, false), - None, - peer::PeerSetup::default(), - crate::Hop::new(1).unwrap(), - None, - version, - tasks, - Default::default(), - ); - - // A track the publisher has not accepted, so the fetch supplies its info. - let broadcast = crate::broadcast::Info::new().produce(); - let reserved = broadcast.reserve_track("video").unwrap(); - let dynamic = reserved.dynamic(); - let track = broadcast.consume().track("video").unwrap(); - let mut fetch = std::pin::pin!(track.fetch_group(GROUP, None)); - assert!(futures::poll!(fetch.as_mut()).is_pending()); - let request = dynamic.requested_group().await.expect("no group requested"); - - subscriber - .clone() - .run_group_fetch(Path::new("broadcast").to_owned(), "video".into(), request, None) - .await; - - let info = moq_net_sim::timeout(Duration::from_secs(1), track.query()) - .await - .expect("the fetch installed the track info") - .expect("info"); - assert_eq!(info.max_age, Some(MAX_AGE), "{version}"); - } - } - /// A cache miss for a group's tail asks upstream from the frame the reader wants and /// numbers what arrives from there, so a publisher that evicted the prefix can answer. #[moq_net_sim::test] diff --git a/rs/moq-net/src/ietf/version.rs b/rs/moq-net/src/ietf/version.rs index 7f0e1b44d9..4e2af0633a 100644 --- a/rs/moq-net/src/ietf/version.rs +++ b/rs/moq-net/src/ietf/version.rs @@ -137,6 +137,7 @@ mod tests { }, range_filters: false, fill_timeout: false, + properties_wanted: false, }; let group = GroupHeader { diff --git a/rs/moq-net/tests/rejoin.rs b/rs/moq-net/tests/rejoin.rs index f8621f9cd6..ec2221b689 100644 --- a/rs/moq-net/tests/rejoin.rs +++ b/rs/moq-net/tests/rejoin.rs @@ -462,3 +462,58 @@ async fn leaving_after_the_cache_window_keeps_the_latest_group() { .unwrap_or_else(|_| panic!("{version}: timed out")); } } + +/// A rejoin keeps the source's timestamps. +/// +/// The returning subscription opts out of the track's properties from draft-20, since the +/// copy already learned them, so its units must come from that first answer rather than +/// the empty block the second one carries. +#[moq_net_sim::test] +async fn rejoin_keeps_source_timestamps() { + for version in ["moq-transport-19", "moq-transport-22"] { + moq_net_sim::timeout(TEST_TIMEOUT, async { + let publisher = produce_origin(1); + let client = produce_origin(2); + + let broadcast = publisher.create_broadcast("bench").unwrap(); + let track = broadcast.create_track("video", None).unwrap(); + broadcast.announce(Default::default()).unwrap(); + + let mut options = MockConnectOptions::new(version.parse::().unwrap()); + options.server_publish = Some(publisher.consume()); + options.client_subscribe = Some(client.clone()); + let _pair = connect_mock(options).await; + + let consumer = client.consume(); + consumer.routed("bench").await.unwrap(); + let remote = consumer.request_broadcast("bench").await.unwrap(); + // Far from any arrival time, so a frame stamped on arrival cannot pass. + let ts = |ms| Timestamp::from_millis(ms).unwrap(); + + for (sequence, ms) in [(0, 123_456), (1, 654_321)] { + let mut sub = remote.track("video").unwrap().subscribe(None).await.unwrap(); + track.demand().used().await.unwrap(); + let mut group = track.append_group().unwrap(); + group.write_frame(ts(ms), b"frame".as_ref()).unwrap(); + group.finish().unwrap(); + + let mut group = loop { + let group = sub.recv_group().await.unwrap().unwrap(); + if group.sequence == sequence { + break group; + } + }; + let frame = group.read_frame().await.unwrap().unwrap(); + assert_eq!( + frame.timestamp.as_millis(), + u128::from(ms), + "{version} group {sequence}" + ); + drop((group, sub)); + track.demand().unused().await.unwrap(); + } + }) + .await + .unwrap_or_else(|_| panic!("{version}: timed out")); + } +} From b40a1b9c77652e3cca2e1c6ad04e2b6e188fd82c Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 7 Oct 2026 10:55:11 -0700 Subject: [PATCH 12/28] fix(ietf): report a draft-20 FETCH's last object read as its End Location An End Object past a finished group's end reported the requested end, claiming objects the response never holds. Co-Authored-By: Claude Opus 5.5 --- rs/moq-net/src/ietf/publisher.rs | 43 ++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/rs/moq-net/src/ietf/publisher.rs b/rs/moq-net/src/ietf/publisher.rs index d13c69a748..700f72956e 100644 --- a/rs/moq-net/src/ietf/publisher.rs +++ b/rs/moq-net/src/ietf/publisher.rs @@ -1502,19 +1502,13 @@ where false => (end, false), } }; - // Draft-20's End Location is inclusive, naming the last object. A whole group was read - // to its end, so that is known. + // Draft-20's End Location is inclusive, naming the last object. The read stopped at + // either the requested end or the group's, so that is the last object read. let end_location = match draft20 { - true => { - let object = match end_location.object { - 0 => group.end(), - object => object, - }; - Location { - group: group.sequence, - object: object - 1, - } - } + true => Location { + group: group.sequence, + object: group.end() - 1, + }, false => end_location, }; @@ -4708,7 +4702,7 @@ mod serve_tests { /// A group holding no objects at or past the start is refused on draft 20, whose /// inclusive End Location cannot describe an empty answer. Older drafts answer it empty. #[moq_net_sim::test] - async fn a_standalone_fetch_past_the_last_object_is_empty() { + async fn a_standalone_fetch_past_the_last_object_is_empty_or_refused() { for version in FETCH_DRAFTS { let mut h = serve(version); publish_pairs(&mut h, 5, None); @@ -4728,6 +4722,29 @@ mod serve_tests { } } + /// On draft 20, an End Object past a finished group's last object reports that last + /// object, which is all the response holds. + #[moq_net_sim::test] + async fn a_draft20_fetch_past_a_group_end_reports_its_last_object() { + for version in [Version::Draft20, Version::Draft21, Version::Draft22] { + let mut h = serve(version); + publish_pairs(&mut h, 5, None); + settle().await; + + let buf = standalone_fetch( + &h, + Location { group: 2, object: 0 }, + Location { group: 2, object: 8 }, + GroupOrder::Ascending, + ) + .await; + let (ok, objects) = fetch_answer(buf, version); + assert_eq!(ok.end_location, Location { group: 2, object: 1 }, "{version}"); + assert!(!ok.end_of_track, "{version}"); + assert_eq!(objects, pairs([2]), "{version}"); + } + } + /// The last group of a finished track ends the response at its last object, with End /// of Track set. #[moq_net_sim::test] From 793ae1b2be859e5e3313101006148a8e01214e2e Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 7 Oct 2026 11:28:28 -0700 Subject: [PATCH 13/28] feat(moq-net): a refused TRACK_STATUS refuses the fetch-only request Supported or refused: drop the NOT_SUPPORTED fallback to SUBSCRIBE. Co-Authored-By: Claude Opus 5.5 --- doc/concept/standard.md | 3 +- rs/moq-net/src/ietf/subscriber.rs | 135 ++++++------------------------ 2 files changed, 25 insertions(+), 113 deletions(-) diff --git a/doc/concept/standard.md b/doc/concept/standard.md index 2a3d33dab7..f0369d32b2 100644 --- a/doc/concept/standard.md +++ b/doc/concept/standard.md @@ -63,8 +63,7 @@ carries no timestamps, since no `SUBSCRIBE_OK` declared a timescale for it. A Rust subscriber learns a track that is only fetched, never subscribed, from `TRACK_STATUS` instead of a `SUBSCRIBE`, so a finished track stays fetchable and a relay puts no live subscription upstream to race its fetches. A -publisher that refuses `TRACK_STATUS` with `NOT_SUPPORTED` gets the -`SUBSCRIBE` instead. The Rust publisher answers `TRACK_STATUS` with what its +publisher that refuses `TRACK_STATUS` refuses the fetch. The Rust publisher answers `TRACK_STATUS` with what its `SUBSCRIBE_OK` would carry, as far as the draft's answer has room: the Largest Location on every draft, and the Track Properties from draft-18, when the answer gained them. A `SUBSCRIBE` that sets `INCLUDE_PROPERTIES` to 0 gets an diff --git a/rs/moq-net/src/ietf/subscriber.rs b/rs/moq-net/src/ietf/subscriber.rs index a2b371ef64..5bef935600 100644 --- a/rs/moq-net/src/ietf/subscriber.rs +++ b/rs/moq-net/src/ietf/subscriber.rs @@ -455,16 +455,6 @@ fn accepted_info(max_age: Option, priority: Option) -> .with_priority(super::priority::from_wire(priority.unwrap_or(128))) } -/// What a TRACK_STATUS for a track nobody subscribes to came to. -enum Status { - /// Accepted as a copy with no subscription. - Known(Idle), - /// The publisher does not implement TRACK_STATUS, so a SUBSCRIBE has to learn the track. - Unsupported(track::Request), - /// The request was answered: refused, or abandoned. - Ended, -} - /// A copy with no subscription upstream, kept for fetches and a returning subscriber. struct Idle { track: track::Producer, @@ -1762,17 +1752,18 @@ where // one made only to learn the track races the fetches it was for. let mut target = match request.subscription() { Some(_) => Target::Request(request), - None => match self.track_status(&broadcast_path, &track_name, request).await { - Status::Known(idle) => match self + None => { + let Some(idle) = self.track_status(&broadcast_path, &track_name, request).await else { + return; + }; + let Some(next) = self .linger(&broadcast_path, &track_name, idle, &mut group_fetches) .await - { - Some(next) => Target::Resume(next), - None => return, - }, - Status::Unsupported(request) => Target::Request(request), - Status::Ended => return, - }, + else { + return; + }; + Target::Resume(next) + } }; loop { let Some(idle) = self @@ -2576,20 +2567,26 @@ where /// Learn a track nobody subscribes to from TRACK_STATUS, and accept it as a copy with no /// subscription, which [`Self::linger`] serves fetches from and subscribes once someone - /// does. - async fn track_status(&mut self, broadcast_path: &Path<'_>, track_name: &str, request: track::Request) -> Status { + /// does. `None` once the request was refused or abandoned: a publisher that refuses + /// TRACK_STATUS refuses the fetch-only request too. + async fn track_status( + &mut self, + broadcast_path: &Path<'_>, + track_name: &str, + request: track::Request, + ) -> Option { let request_id = match self.control.next_request_id(&self.runtime).await { Ok(id) => id, Err(err) => { request.reject(err); - return Status::Ended; + return None; } }; let mut stream = match Stream::open(&mut self.session.clone(), self.version).await { Ok(stream) => stream, Err(err) => { request.reject(err); - return Status::Ended; + return None; } }; let written = async { @@ -2607,7 +2604,7 @@ where .await; if let Err(err) = written { request.reject(err); - return Status::Ended; + return None; } // Nobody wanting the track any more ends the wait on a peer that may never answer, @@ -2640,19 +2637,17 @@ where let Some(response) = response else { stream.reader.abort(&Error::Cancel); stream.writer.abort(&Error::Cancel); - return Status::Ended; + return None; }; // The publisher has read the request, since it answered, and FINs after its answer: // FIN our side too and let the stream drop. let _ = stream.writer.finish(); let ok = match response { Ok(ok) => ok, - // A publisher that does not implement TRACK_STATUS still answers a SUBSCRIBE. - Err(Error::Unsupported) => return Status::Unsupported(request), Err(err) => { tracing::debug!(%err, broadcast = %self.origin.absolute(broadcast_path), track = %track_name, "track status refused"); request.reject(err); - return Status::Ended; + return None; } }; @@ -2661,7 +2656,7 @@ where let mut track = request.accept(accepted_info(ok.properties.max_cache_duration, ok.properties.priority)); // Nothing feeds this copy live: what it caches is fetched, never the live edge. track.set_idle(); - Status::Known(Idle { + Some(Idle { demand: track.demand(), track, dynamic, @@ -5359,88 +5354,6 @@ mod tests { outstanding } - /// A publisher that does not implement TRACK_STATUS (an older release, or another - /// implementation) refuses it NOT_SUPPORTED. Fetch-only demand then learns the track - /// from a SUBSCRIBE, as it did before TRACK_STATUS was asked. - #[moq_net_sim::test] - async fn an_unsupported_track_status_falls_back_to_subscribe() { - for version in [ - Version::Draft14, - Version::Draft15, - Version::Draft17, - Version::Draft18, - Version::Draft22, - ] { - let log = crate::lite::test_transport::Log::default(); - let mut writer = - crate::coding::Writer::new(crate::lite::test_transport::SinkSend::new(log.clone()), version); - let reason = "TRACK_STATUS is not supported"; - match version { - Version::Draft14 => { - writer.varint(ietf::TRACK_STATUS_ERROR_14).await.unwrap(); - writer - .encode(&ietf::SubscribeError { - request_id: RequestId(0), - error_code: 0x3, - reason_phrase: reason.into(), - }) - .await - .unwrap(); - } - _ => { - writer.varint(ietf::RequestError::ID).await.unwrap(); - writer - .encode(&ietf::RequestError { - request_id: matches!(version, Version::Draft15 | Version::Draft16).then_some(RequestId(0)), - error_code: 0x3, - reason_phrase: reason.into(), - retry_interval: 0, - }) - .await - .unwrap(); - } - } - let refusal = log.writes.lock().unwrap().clone(); - - let session = crate::lite::test_transport::ScriptedSession::per_stream(vec![refusal]); - let log = session.log.clone(); - let (tasks, _task_set) = crate::util::TaskSet::new(); - let mut subscriber = Subscriber::new( - crate::time::Clock::sim(), - session, - crate::origin::Config::new(crate::Hop::new(1).unwrap()).produce(), - Control::new(None, false), - None, - peer::PeerSetup::default(), - crate::Hop::new(1).unwrap(), - None, - version, - tasks, - Default::default(), - ); - - let producer = crate::broadcast::Info::default().produce(); - let mut dynamic = producer.dynamic(); - let consumer = producer.consume(); - // Fetch-only demand: nobody subscribes. - let _fetching = consumer.track("video").unwrap().fetch_group(0, None); - let request = dynamic.requested_track().await.expect("no track requested"); - assert!(request.subscription().is_none()); - - let serving = moq_net_sim::spawn(async move { - subscriber.run_subscribe(Path::new("broadcast"), dynamic, request).await; - }); - settle().await; - - assert_eq!( - control_message_types(&log, version), - [ietf::TrackStatus::ID, ietf::Subscribe::ID], - "{version}: a refused TRACK_STATUS must fall back to SUBSCRIBE" - ); - serving.abort(); - } - } - /// A copy that already knows its track, here from TRACK_STATUS_OK, opts out of the /// properties when a subscriber returns, and keeps the units it learned rather than /// taking the empty block the opted-out SUBSCRIBE_OK carries. From ba732bbe4ad5c307efc93f12a9ecc9cf0dea8d0a Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 7 Oct 2026 12:08:19 -0700 Subject: [PATCH 14/28] fix(moq-net): a resumed draft-17 copy takes SUBSCRIBE_OK's timescale Draft-17's TRACK_STATUS_OK has no properties block and its SUBSCRIBE cannot opt out, so the answer to a resumed SUBSCRIBE is the first to declare the units. Keep the learned units only when the answer declares none. Co-Authored-By: Claude Opus 5.5 --- doc/concept/standard.md | 9 ++++---- rs/moq-net/src/ietf/subscriber.rs | 35 +++++++++++++++++-------------- 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/doc/concept/standard.md b/doc/concept/standard.md index f0369d32b2..b3863b9617 100644 --- a/doc/concept/standard.md +++ b/doc/concept/standard.md @@ -63,10 +63,11 @@ carries no timestamps, since no `SUBSCRIBE_OK` declared a timescale for it. A Rust subscriber learns a track that is only fetched, never subscribed, from `TRACK_STATUS` instead of a `SUBSCRIBE`, so a finished track stays fetchable and a relay puts no live subscription upstream to race its fetches. A -publisher that refuses `TRACK_STATUS` refuses the fetch. The Rust publisher answers `TRACK_STATUS` with what its -`SUBSCRIBE_OK` would carry, as far as the draft's answer has room: the Largest -Location on every draft, and the Track Properties from draft-18, when the -answer gained them. A `SUBSCRIBE` that sets `INCLUDE_PROPERTIES` to 0 gets an +publisher that refuses `TRACK_STATUS` refuses the fetch. The Rust publisher +answers `TRACK_STATUS` with what its `SUBSCRIBE_OK` would carry, as far as the +draft's answer has room: the Largest Location on every draft, and the Track +Properties from draft-18, when the answer gained them. A relay with no +subscription upstream reports the Largest Location it has cached. A `SUBSCRIBE` that sets `INCLUDE_PROPERTIES` to 0 gets an empty properties block, but its objects keep their Timestamps, whose units `TRACK_STATUS` reports. From draft-20, a Rust subscriber sets `INCLUDE_PROPERTIES` to 0 on every later `SUBSCRIBE` or `FETCH` for a track diff --git a/rs/moq-net/src/ietf/subscriber.rs b/rs/moq-net/src/ietf/subscriber.rs index 5bef935600..e3b359388b 100644 --- a/rs/moq-net/src/ietf/subscriber.rs +++ b/rs/moq-net/src/ietf/subscriber.rs @@ -2018,11 +2018,10 @@ where largest, } = accepted; let info = accepted_info(max_age, priority); - // A resumed copy opted out of the properties, so it keeps the units it learned. - let timescale = match &resumed { - Some(idle) => idle.timescale, - None => timescale, - }; + // A resumed copy opts out of the properties from draft-20, so it keeps the units it + // learned. Before that the answer still declares them, and may be the first to: draft-17's + // TRACK_STATUS_OK has no properties block. + let timescale = timescale.or(resumed.as_ref().and_then(|idle| idle.timescale)); // The copy already holds the track: it is current again up to the answer's Largest // Location once the join's head lands or a newer group does, since leaving cut the // group it was in. @@ -5356,7 +5355,9 @@ mod tests { /// A copy that already knows its track, here from TRACK_STATUS_OK, opts out of the /// properties when a subscriber returns, and keeps the units it learned rather than - /// taking the empty block the opted-out SUBSCRIBE_OK carries. + /// taking the empty block the opted-out SUBSCRIBE_OK carries. On draft-17, where + /// TRACK_STATUS_OK has no properties block and SUBSCRIBE cannot opt out, the copy takes + /// the units SUBSCRIBE_OK declares. #[moq_net_sim::test] async fn a_resumed_subscribe_opts_out_of_known_properties() { use crate::coding::Encode as _; @@ -5371,29 +5372,31 @@ mod tests { } let declared = Timescale::new(90_000).unwrap(); - for version in [Version::Draft20, Version::Draft22] { + for version in [Version::Draft17, Version::Draft20, Version::Draft22] { + let opts_out = ietf::Filter::is_draft20(version); + let declares = |yes: bool| ietf::Properties { + timescale: yes.then_some(declared), + ..Default::default() + }; let status_ok = message( ietf::TrackStatusOk::ID, &ietf::TrackStatusOk { request_id: None, largest: None, - properties: ietf::Properties { - timescale: Some(declared), - ..Default::default() - }, + properties: declares(opts_out), }, version, ); let session = crate::lite::test_transport::ScriptedSession::per_stream(vec![ status_ok, - // Opted out, so its block is empty. + // Empty when opted out. Draft-17 cannot opt out, so it declares the units. message( ietf::SubscribeOk::ID, &ietf::SubscribeOk { request_id: None, track_alias: 7, largest: None, - properties: Default::default(), + properties: declares(!opts_out), }, version, ), @@ -5443,8 +5446,8 @@ mod tests { } } assert_eq!(subscribes.len(), 1, "{version}: the returning subscriber subscribes"); - assert!( - !subscribes[0].properties_wanted, + assert_eq!( + subscribes[0].properties_wanted, !opts_out, "{version}: TRACK_STATUS_OK already gave the properties" ); @@ -5456,7 +5459,7 @@ mod tests { .next() .expect("the subscription is registered") .timescale; - assert_eq!(timescale, Some(declared), "{version}: the learned units are kept"); + assert_eq!(timescale, Some(declared), "{version}: the copy keeps the units it learned"); serving.abort(); } } From 3ad859e3b53eae3ed38f85f41044c6d425559502 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 7 Oct 2026 12:18:41 -0700 Subject: [PATCH 15/28] docs: rewrap the TRACK_STATUS paragraph Co-Authored-By: Claude Opus 5.5 --- doc/concept/standard.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/doc/concept/standard.md b/doc/concept/standard.md index b3863b9617..67fa849fd4 100644 --- a/doc/concept/standard.md +++ b/doc/concept/standard.md @@ -67,9 +67,10 @@ publisher that refuses `TRACK_STATUS` refuses the fetch. The Rust publisher answers `TRACK_STATUS` with what its `SUBSCRIBE_OK` would carry, as far as the draft's answer has room: the Largest Location on every draft, and the Track Properties from draft-18, when the answer gained them. A relay with no -subscription upstream reports the Largest Location it has cached. A `SUBSCRIBE` that sets `INCLUDE_PROPERTIES` to 0 gets an -empty properties block, but its objects keep their Timestamps, whose units -`TRACK_STATUS` reports. From draft-20, a Rust subscriber sets +subscription upstream reports the Largest Location it has cached. A +`SUBSCRIBE` that sets `INCLUDE_PROPERTIES` to 0 gets an empty properties +block, but its objects keep their Timestamps, whose units `TRACK_STATUS` +reports. From draft-20, a Rust subscriber sets `INCLUDE_PROPERTIES` to 0 on every later `SUBSCRIBE` or `FETCH` for a track it already learned, instead of receiving the same properties again. From ac1e723adf80a03326fc31208110f894b6c5495b Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 7 Oct 2026 16:18:36 -0700 Subject: [PATCH 16/28] style: cargo fmt the timescale assertion Co-Authored-By: Claude Opus 5.5 --- rs/moq-net/src/ietf/subscriber.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/rs/moq-net/src/ietf/subscriber.rs b/rs/moq-net/src/ietf/subscriber.rs index e3b359388b..c49d415dde 100644 --- a/rs/moq-net/src/ietf/subscriber.rs +++ b/rs/moq-net/src/ietf/subscriber.rs @@ -5459,7 +5459,11 @@ mod tests { .next() .expect("the subscription is registered") .timescale; - assert_eq!(timescale, Some(declared), "{version}: the copy keeps the units it learned"); + assert_eq!( + timescale, + Some(declared), + "{version}: the copy keeps the units it learned" + ); serving.abort(); } } From e0a40d2da9ae25e0f94a44855c962951f8412da4 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 7 Oct 2026 21:27:15 -0700 Subject: [PATCH 17/28] fix(moq-net): adapt TRACK_STATUS and tests to main's epoch and untimed frames Co-Authored-By: Claude Opus 5.5 --- rs/moq-net/src/ietf/publisher.rs | 2 +- rs/moq-net/tests/fetch_only.rs | 2 +- rs/moq-net/tests/rejoin.rs | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/rs/moq-net/src/ietf/publisher.rs b/rs/moq-net/src/ietf/publisher.rs index 7c154919c2..8249681589 100644 --- a/rs/moq-net/src/ietf/publisher.rs +++ b/rs/moq-net/src/ietf/publisher.rs @@ -1647,7 +1647,7 @@ where let broadcast = match self .serving_origin() .await - .request_broadcast(&msg.track_namespace) + .request_broadcast(&msg.track_namespace, None) .await { Ok(broadcast) => broadcast, diff --git a/rs/moq-net/tests/fetch_only.rs b/rs/moq-net/tests/fetch_only.rs index dc531089d9..db44f94c0d 100644 --- a/rs/moq-net/tests/fetch_only.rs +++ b/rs/moq-net/tests/fetch_only.rs @@ -27,7 +27,7 @@ async fn resolve(origin: &moq_net::origin::Producer) -> moq_net::broadcast::Cons .await .expect("announce timeout") .expect("routed"); - moq_net_sim::timeout(TIMEOUT, consumer.request_broadcast("bcast")) + moq_net_sim::timeout(TIMEOUT, consumer.request_broadcast("bcast", None)) .await .expect("resolve timeout") .expect("broadcast resolves") diff --git a/rs/moq-net/tests/rejoin.rs b/rs/moq-net/tests/rejoin.rs index 46afaaa78a..50b95b5522 100644 --- a/rs/moq-net/tests/rejoin.rs +++ b/rs/moq-net/tests/rejoin.rs @@ -496,7 +496,7 @@ async fn rejoin_keeps_source_timestamps() { let consumer = client.consume(); consumer.routed("bench").await.unwrap(); - let remote = consumer.request_broadcast("bench").await.unwrap(); + let remote = consumer.request_broadcast("bench", None).await.unwrap(); // Far from any arrival time, so a frame stamped on arrival cannot pass. let ts = |ms| Timestamp::from_millis(ms).unwrap(); @@ -515,8 +515,8 @@ async fn rejoin_keeps_source_timestamps() { }; let frame = group.read_frame().await.unwrap().unwrap(); assert_eq!( - frame.timestamp.as_millis(), - u128::from(ms), + frame.timestamp.map(Timestamp::as_millis), + Some(u128::from(ms)), "{version} group {sequence}" ); drop((group, sub)); From 0a5e3e19c46c60c2ff2192c5033bf7ddee2cffe3 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 7 Oct 2026 21:54:04 -0700 Subject: [PATCH 18/28] feat(moq-net): draft-17 fetch-only demand still subscribes Draft-17's REQUEST_OK answer to TRACK_STATUS has no properties block, so a copy learned from it cannot know whether the track is timed, and a later SUBSCRIBE_OK declaring TIMESCALE would feed stamped frames into an untimed track. Draft-17 is not an interop target, so it keeps SUBSCRIBE. Also tests that a SUBSCRIBE after TRACK_STATUS keeps timestamps on draft-18 and later. Co-Authored-By: Claude Opus 5.5 --- doc/concept/standard.md | 4 ++- rs/moq-net/src/ietf/subscriber.rs | 21 +++++++------- rs/moq-net/src/ietf/track.rs | 7 +++++ rs/moq-net/tests/fetch_only.rs | 48 +++++++++++++++++++++++++++++-- 4 files changed, 66 insertions(+), 14 deletions(-) diff --git a/doc/concept/standard.md b/doc/concept/standard.md index 94cfedc222..4be7308b7c 100644 --- a/doc/concept/standard.md +++ b/doc/concept/standard.md @@ -75,7 +75,9 @@ so every track there is untimed. A Rust subscriber learns a track that is only fetched, never subscribed, from `TRACK_STATUS` instead of a `SUBSCRIBE`, so a finished track stays fetchable and a relay puts no live subscription upstream to race its fetches. A -publisher that refuses `TRACK_STATUS` refuses the fetch. The Rust publisher +publisher that refuses `TRACK_STATUS` refuses the fetch. Draft-17 is the +exception and still subscribes: its `TRACK_STATUS` answer has no properties +block, so it can't say whether the track is timed. The Rust publisher answers `TRACK_STATUS` with what its `SUBSCRIBE_OK` would carry, as far as the draft's answer has room: the Largest Location on every draft, and the Track Properties from draft-18, when the answer gained them. A relay with no diff --git a/rs/moq-net/src/ietf/subscriber.rs b/rs/moq-net/src/ietf/subscriber.rs index 07ae61e4c2..160b873023 100644 --- a/rs/moq-net/src/ietf/subscriber.rs +++ b/rs/moq-net/src/ietf/subscriber.rs @@ -1759,10 +1759,10 @@ where // Demand with nobody subscribing (fetches, or a TRACK_STATUS downstream) learns the // track from TRACK_STATUS rather than a SUBSCRIBE: a finished track refuses one, and - // one made only to learn the track races the fetches it was for. + // one made only to learn the track races the fetches it was for. Draft-17 still + // SUBSCRIBEs: its TRACK_STATUS answer cannot say whether the track is timed. let mut target = match request.subscription() { - Some(_) => Target::Request(request), - None => { + None if ietf::TrackStatusOk::describes_track(self.version) => { let Some(idle) = self.track_status(&broadcast_path, &track_name, request).await else { return; }; @@ -1774,6 +1774,7 @@ where }; Target::Resume(next) } + _ => Target::Request(request), }; loop { let Some(idle) = self @@ -2028,8 +2029,7 @@ where largest, } = accepted; // A resumed copy opts out of the properties from draft-20, so it keeps the units it - // learned. Before that the answer still declares them, and may be the first to: draft-17's - // TRACK_STATUS_OK has no properties block. + // learned. Before that the answer declares them again. let timescale = timescale.or(resumed.as_ref().and_then(|idle| idle.timescale)); let info = accepted_info(timescale, max_age, priority); // The copy already holds the track: it is current again up to the answer's Largest @@ -5489,9 +5489,7 @@ mod tests { /// A copy that already knows its track, here from TRACK_STATUS_OK, opts out of the /// properties when a subscriber returns, and keeps the units it learned rather than - /// taking the empty block the opted-out SUBSCRIBE_OK carries. On draft-17, where - /// TRACK_STATUS_OK has no properties block and SUBSCRIBE cannot opt out, the copy takes - /// the units SUBSCRIBE_OK declares. + /// taking the empty block the opted-out SUBSCRIBE_OK carries. #[moq_net_sim::test] async fn a_resumed_subscribe_opts_out_of_known_properties() { use crate::coding::Encode as _; @@ -5506,7 +5504,7 @@ mod tests { } let declared = Timescale::new(90_000).unwrap(); - for version in [Version::Draft17, Version::Draft20, Version::Draft22] { + for version in [Version::Draft18, Version::Draft20, Version::Draft22] { let opts_out = ietf::Filter::is_draft20(version); let declares = |yes: bool| ietf::Properties { timescale: yes.then_some(declared), @@ -5517,13 +5515,14 @@ mod tests { &ietf::TrackStatusOk { request_id: None, largest: None, - properties: declares(opts_out), + properties: declares(true), }, version, ); let session = crate::lite::test_transport::ScriptedSession::per_stream(vec![ status_ok, - // Empty when opted out. Draft-17 cannot opt out, so it declares the units. + // Empty when opted out. Before draft-20 SUBSCRIBE cannot opt out, so it declares + // the units again. message( ietf::SubscribeOk::ID, &ietf::SubscribeOk { diff --git a/rs/moq-net/src/ietf/track.rs b/rs/moq-net/src/ietf/track.rs index 5d82d976d1..984b2488e1 100644 --- a/rs/moq-net/src/ietf/track.rs +++ b/rs/moq-net/src/ietf/track.rs @@ -104,6 +104,13 @@ impl TrackStatusOk { } } + /// Whether this answer describes the track as fully as SUBSCRIBE_OK on `version`. + /// Draft-17 is the gap: SUBSCRIBE_OK declares TIMESCALE, but REQUEST_OK has no + /// properties block to say whether the track is timed. + pub(crate) fn describes_track(version: Version) -> bool { + Self::has_properties(version) || !super::Properties::sends_timescale(version) + } + /// Whether the answer carries a Track Properties block on `version`. fn has_properties(version: Version) -> bool { !matches!( diff --git a/rs/moq-net/tests/fetch_only.rs b/rs/moq-net/tests/fetch_only.rs index db44f94c0d..60c991f483 100644 --- a/rs/moq-net/tests/fetch_only.rs +++ b/rs/moq-net/tests/fetch_only.rs @@ -82,12 +82,12 @@ fn frames(sequence: u64) -> Vec> { } /// Drafts whose FETCH we serve, and the newest, whose FETCH we refuse until draft-20 -/// FETCH lands: it must still not subscribe. +/// FETCH lands: it must still not subscribe. Draft-17 is absent: its TRACK_STATUS answer +/// cannot say whether the track is timed, so it still SUBSCRIBEs to learn the track. const VERSIONS: &[(&str, bool)] = &[ ("moq-transport-14", true), ("moq-transport-15", true), ("moq-transport-16", true), - ("moq-transport-17", true), ("moq-transport-18", true), ("moq-transport-19", true), ("moq-transport-22", false), @@ -160,6 +160,50 @@ async fn relayed(version: &str) { drop((remote, downstream, upstream, broadcast, client, relay, publisher)); } +/// A subscriber arriving at a copy learned from TRACK_STATUS still gets timestamped +/// frames: the copy is timed from TRACK_STATUS_OK, and the resumed SUBSCRIBE keeps it so. +#[moq_net_sim::test] +async fn a_subscribe_after_track_status_keeps_timestamps() { + for version in ["moq-transport-18", "moq-transport-19", "moq-transport-22"] { + moq_net_sim::timeout(TIMEOUT, async { + let publisher = produce_origin(1); + let broadcast = publisher.create_broadcast("bcast").unwrap(); + let track = broadcast.create_track("video", None).unwrap(); + broadcast.announce(Default::default()).unwrap(); + + let client = produce_origin(2); + let mut options = MockConnectOptions::new(version.parse::().unwrap()); + options.server_publish = Some(publisher.consume()); + options.client_subscribe = Some(client.clone()); + let _pair = connect_mock(options).await; + + let remote = resolve(&client).await; + let copy = remote.track("video").unwrap(); + let info = copy.query().await.expect("TRACK_STATUS resolves the track"); + assert!(info.timescale.is_some(), "{version}: the copy is untimed"); + assert!(track.subscription().is_none(), "{version}: the query subscribed"); + + let mut sub = copy.subscribe(None).await.unwrap(); + track.demand().used().await.unwrap(); + // Far from any arrival time, so a frame stamped on arrival cannot pass. + let stamp = Timestamp::from_millis(123_456).unwrap(); + let mut group = track.append_group().unwrap(); + group.write_frame(stamp, b"frame".as_ref()).unwrap(); + group.finish().unwrap(); + + let mut group = sub.recv_group().await.unwrap().unwrap(); + let frame = group.read_frame().await.unwrap().unwrap(); + assert_eq!( + frame.timestamp.map(Timestamp::as_millis), + Some(stamp.as_millis()), + "{version}" + ); + }) + .await + .unwrap_or_else(|_| panic!("{version}: timed out")); + } +} + /// A relay serves a downstream fetch with a fetch upstream, never a subscription. #[moq_net_sim::test] async fn a_relay_fetches_without_subscribing_upstream() { From 90cada6166831ccabda487badbde6e5a01c3a99d Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 7 Oct 2026 22:15:00 -0700 Subject: [PATCH 19/28] quest: drop the link to the finished ietf-fetch-only quest Co-Authored-By: Claude Opus 5.5 --- quest/m1/ietf-cold-largest.md | 1 - 1 file changed, 1 deletion(-) diff --git a/quest/m1/ietf-cold-largest.md b/quest/m1/ietf-cold-largest.md index 0b84d0bca4..e9ebe920cf 100644 --- a/quest/m1/ietf-cold-largest.md +++ b/quest/m1/ietf-cold-largest.md @@ -58,4 +58,3 @@ Public API: none expected. Wire: none new. ## Related - [moq-transport ranges](/quest/m1/subscribe-ranges/ietf.md) - the same upstream fill path, for multi-group ranges -- [Fetch without SUBSCRIBE](/quest/m1/ietf-fetch-only.md) - the other FETCH gap at a relay From 3f4520cff65dd82d57facd29b803ce1cef937b9a Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Thu, 8 Oct 2026 07:54:12 -0700 Subject: [PATCH 20/28] fix(ietf): answer an empty draft-20 FETCH and report the requested End Location Draft-20 section 10.13 answers a range with no objects with FETCH_HEADER and FIN, refusing only a start past Largest Object with INVALID_RANGE. Section 10.14 makes FETCH_OK's End Location the requested end, capped at Largest Object or the track's last object, rather than the last object sent. The relay's group fill no longer requires a draft-20 fetch stream to reach that End Location, since the objects missing before it do not exist. Co-Authored-By: Claude Opus 5.5 --- doc/concept/standard.md | 6 +- rs/moq-net/src/ietf/publisher.rs | 100 ++++++++++++++++++++---------- rs/moq-net/src/ietf/subscriber.rs | 73 ++++++++++++---------- 3 files changed, 112 insertions(+), 67 deletions(-) diff --git a/doc/concept/standard.md b/doc/concept/standard.md index 9820233203..53c41b5ea4 100644 --- a/doc/concept/standard.md +++ b/doc/concept/standard.md @@ -56,7 +56,11 @@ on moq-lite. The Rust publisher answers a standalone `FETCH` within one group from the cache. On draft-20 and later, that is a `FETCH` whose `LOCATION_FILTER` names an absolute start and end; one ending at Largest Object (no filter, a relative -start, or no end) is refused with `NOT_SUPPORTED`. A relay fetches a missing +start, or no end) is refused with `NOT_SUPPORTED`. There, `FETCH_OK`'s +inclusive End Location is the requested end, capped at Largest Object or the +track's last object, and objects missing before it do not exist: a range with +none is answered with an empty fetch stream, and only a start past Largest +Object is refused, with `INVALID_RANGE`. A relay fetches a missing group upstream with a `FETCH` of that one whole group, and an upstream refusal is the refusal the fetcher sees. Once its last reader leaves, the relay cancels the upstream fetch, even before `FETCH_OK`, and aborts an incomplete group diff --git a/rs/moq-net/src/ietf/publisher.rs b/rs/moq-net/src/ietf/publisher.rs index ac7e43914a..238abb778c 100644 --- a/rs/moq-net/src/ietf/publisher.rs +++ b/rs/moq-net/src/ietf/publisher.rs @@ -1497,12 +1497,16 @@ where Some(Err(err)) => return self.reject_fetch(stream, msg.request_id, &err, &err.to_string()).await, None => return Ok(()), }; - // Draft-20's inclusive End Location always names an object, so it cannot say the - // group holds none at or past the start. let draft20 = Filter::is_draft20(self.version); - if draft20 && group.frames.is_empty() { + let end_of_track = !joined && group.complete && track.final_sequence() == group.sequence.checked_add(1); + // Draft-20 caps the response at Largest Object, which for a finished group we know to + // be the latest is its last object. + let at_largest = end_of_track || (group.complete && track.latest() == Some(group.sequence)); + // A start past Largest Object is the one refusal draft-20 defines (section 10.13). + // Anything earlier with no objects is answered as an empty stream. + if draft20 && group.frames.is_empty() && at_largest { return self - .reject_fetch(stream, msg.request_id, &Error::NotFound, "no objects in range") + .reject_fetch(stream, msg.request_id, &Error::InvalidRange, "start past Largest Object") .await; } @@ -1525,7 +1529,6 @@ where } (end, false) } else { - let end_of_track = group.complete && track.final_sequence() == group.sequence.checked_add(1); // A group that ends the track ends the response at its last object; otherwise // the response ends where it was asked to. match end_of_track { @@ -1539,12 +1542,17 @@ where false => (end, false), } }; - // Draft-20's End Location is inclusive, naming the last object. The read stopped at - // either the requested end or the group's, so that is the last object read. + // Draft-20's End Location is inclusive: the requested end, capped at Largest Object + // (section 10.14). Objects missing before it do not exist. A whole-group request has + // no End Object to report, so it covers what the finished group holds, and at least + // the start an empty answer covered. let end_location = match draft20 { true => Location { group: group.sequence, - object: group.end() - 1, + object: match (until, at_largest) { + (Some(until), false) => until - 1, + _ => group.end().saturating_sub(1).max(start.object), + }, }, false => end_location, }; @@ -4980,33 +4988,37 @@ mod serve_tests { } } - /// A group holding no objects at or past the start is refused on draft 20, whose - /// inclusive End Location cannot describe an empty answer. Older drafts answer it empty. + /// A group holding no objects at or past the start is answered empty, on draft 20 with + /// an End Location covering the range asked for (section 10.13). The whole group covers + /// at least its start. #[moq_net_sim::test] - async fn a_standalone_fetch_past_the_last_object_is_empty_or_refused() { + async fn a_standalone_fetch_past_the_last_object_is_empty() { for version in FETCH_DRAFTS { - let mut h = serve(version); - publish_pairs(&mut h, 5, None); - settle().await; + for (end, covered) in [(0, 2), (6, 5)] { + let mut h = serve(version); + publish_pairs(&mut h, 5, None); + settle().await; - let buf = standalone_fetch( - &h, - Location { group: 2, object: 2 }, - Location { group: 2, object: 0 }, - GroupOrder::Ascending, - ) - .await; - match Filter::is_draft20(version) { - true => assert_eq!(fetch_refusal(buf, version), does_not_exist(version), "{version}"), - false => assert_eq!(fetch_answer(buf, version).1, Vec::new(), "{version}"), + let buf = standalone_fetch( + &h, + Location { group: 2, object: 2 }, + Location { group: 2, object: end }, + GroupOrder::Ascending, + ) + .await; + let (ok, objects) = fetch_answer(buf, version); + assert_eq!(objects, Vec::new(), "{version} end={end}"); + if Filter::is_draft20(version) { + assert_eq!(ok.end_location, Location { group: 2, object: covered }, "{version}"); + } } } } - /// On draft 20, an End Object past a finished group's last object reports that last - /// object, which is all the response holds. + /// On draft 20, a start past Largest Object is refused INVALID_RANGE (section 10.13): + /// here past the last object of the latest group. #[moq_net_sim::test] - async fn a_draft20_fetch_past_a_group_end_reports_its_last_object() { + async fn a_draft20_fetch_past_the_largest_object_is_refused() { for version in [Version::Draft20, Version::Draft21, Version::Draft22] { let mut h = serve(version); publish_pairs(&mut h, 5, None); @@ -5014,15 +5026,37 @@ mod serve_tests { let buf = standalone_fetch( &h, - Location { group: 2, object: 0 }, - Location { group: 2, object: 8 }, + Location { group: 4, object: 2 }, + Location { group: 4, object: 6 }, GroupOrder::Ascending, ) .await; - let (ok, objects) = fetch_answer(buf, version); - assert_eq!(ok.end_location, Location { group: 2, object: 1 }, "{version}"); - assert!(!ok.end_of_track, "{version}"); - assert_eq!(objects, pairs([2]), "{version}"); + assert_eq!(fetch_refusal(buf, version), invalid_range(version), "{version}"); + } + } + + /// On draft 20, an End Object past a finished group's last object is still the End + /// Location reported (section 10.14), unless the group holds Largest Object, which caps it. + #[moq_net_sim::test] + async fn a_draft20_fetch_past_a_group_end_reports_the_requested_end() { + for version in [Version::Draft20, Version::Draft21, Version::Draft22] { + for (group, reported) in [(2, 7), (4, 1)] { + let mut h = serve(version); + publish_pairs(&mut h, 5, None); + settle().await; + + let buf = standalone_fetch( + &h, + Location { group, object: 0 }, + Location { group, object: 8 }, + GroupOrder::Ascending, + ) + .await; + let (ok, objects) = fetch_answer(buf, version); + assert_eq!(ok.end_location, Location { group, object: reported }, "{version}"); + assert!(!ok.end_of_track, "{version}"); + assert_eq!(objects, pairs([group]), "{version}"); + } } } diff --git a/rs/moq-net/src/ietf/subscriber.rs b/rs/moq-net/src/ietf/subscriber.rs index 0bebe8e2d4..8d81a62e81 100644 --- a/rs/moq-net/src/ietf/subscriber.rs +++ b/rs/moq-net/src/ietf/subscriber.rs @@ -3703,8 +3703,8 @@ where let _ = stream.writer.close().await; return; } - // An exclusive End Location inside the group is its Largest Object plus one, so - // the stream owes every object up to it. One past the group covers it whole. + // An exclusive End Location inside the group bounds the stream; before draft-20 the + // stream also owes every object up to it. One past the group covers it whole. let end = (end.group == sequence).then_some(end.object); // Joined fetches still count until they pick the accepted group up from the cache. @@ -3892,7 +3892,8 @@ where } /// Decode one group's objects: all in the producer's group, numbered from `start` with - /// no gaps, and through `end` (exclusive) when FETCH_OK named one inside the group. + /// no gaps, and never past `end` (exclusive) when FETCH_OK named one inside the group. + /// Before draft-20 they must also reach it. async fn recv_group_fetch_objects( &self, stream: &mut Reader, @@ -3954,8 +3955,9 @@ where } // A clean FIN short of the promised end would otherwise cache a truncated group as - // a complete one. - if end.is_some_and(|end| next < end) { + // a complete one. Draft-20's End Location is the range covered, not the last object + // sent: objects missing before it do not exist (section 10.13). + if !Filter::is_draft20(self.version) && end.is_some_and(|end| next < end) { tracing::warn!( sequence, next, @@ -8627,37 +8629,42 @@ mod stitch_tests { assert!(matches!(res, Err(Error::MalformedTrack)), "{res:?}"); } - /// FETCH_OK's End Location inside the group promises every object before it. A stream - /// that FINs short of it, or runs past it, fails the group instead of caching it. + /// FETCH_OK's End Location inside the group bounds the stream: one that runs past it + /// fails the group instead of caching it. Before draft-20 the End Location also promises + /// every object before it, so a stream that FINs short fails too. From draft-20 it is the + /// range covered, and the objects missing before it do not exist (section 10.13). #[moq_net_sim::test] - async fn a_group_fetch_must_reach_its_end_location() { + async fn a_group_fetch_stays_within_its_end_location() { const END: u64 = 3; - for (count, complete) in [(2, false), (3, true), (4, false)] { - let payloads: Vec<&[u8]> = [b"a", b"b", b"c", b"d"][..count].iter().map(|p| &p[..]).collect(); - let mut run = GroupFetchRun::new(VERSION, group_fetch_objects(SEQUENCE, 0, &payloads)).await; - - let group = run.track.create_group(group::Info { sequence: SEQUENCE }).unwrap(); - let mut consumer = group.consume(); - let slot = kio::Producer::new(GroupFetch::Ready { - producer: group, - timescale: None, - start: 0, - end: Some(END), - }); - let res = run.subscriber.recv_group_fetch(&mut run.stream, slot).await; - assert_eq!(res.is_ok(), complete, "{count} objects: {res:?}"); - - let mut read = 0; - let end = loop { - match consumer.read_frame().await { - Ok(Some(_)) => read += 1, - Ok(None) => break Ok(read), - Err(err) => break Err(err), + for version in [Version::Draft19, VERSION] { + let short = Filter::is_draft20(version); + for (count, complete) in [(2, short), (3, true), (4, false)] { + let payloads: Vec<&[u8]> = [b"a", b"b", b"c", b"d"][..count].iter().map(|p| &p[..]).collect(); + let mut run = GroupFetchRun::new(version, group_fetch_objects(SEQUENCE, 0, &payloads)).await; + + let group = run.track.create_group(group::Info { sequence: SEQUENCE }).unwrap(); + let mut consumer = group.consume(); + let slot = kio::Producer::new(GroupFetch::Ready { + producer: group, + timescale: None, + start: 0, + end: Some(END), + }); + let res = run.subscriber.recv_group_fetch(&mut run.stream, slot).await; + assert_eq!(res.is_ok(), complete, "{version} {count} objects: {res:?}"); + + let mut read = 0; + let end = loop { + match consumer.read_frame().await { + Ok(Some(_)) => read += 1, + Ok(None) => break Ok(read), + Err(err) => break Err(err), + } + }; + match complete { + true => assert_eq!(end.expect("a complete group"), count as u64, "{version}"), + false => assert!(end.is_err(), "{version} {count} objects: the group must fail, not end"), } - }; - match complete { - true => assert_eq!(end.expect("a complete group"), END), - false => assert!(end.is_err(), "{count} objects: the group must fail, not end"), } } } From 4387ecffbf7808e3b1fbb15612483812310957f4 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Thu, 8 Oct 2026 08:40:41 -0700 Subject: [PATCH 21/28] fix(ietf): cap a draft-20 FETCH only at the track's end, and refuse an inverted End Location Capping at the cache's latest group misread Largest Object when a newer group held no objects yet, or when a relay without a live feed had just filled the group. Cap and refuse at the track's last object only, which even a relay knows. A draft-20 FETCH_OK whose End Location is before its start now closes the session with PROTOCOL_VIOLATION. Co-Authored-By: Claude Opus 5.5 --- doc/concept/standard.md | 2 +- rs/moq-net/src/ietf/publisher.rs | 74 ++++++++++++++++++----------- rs/moq-net/src/ietf/subscriber.rs | 77 ++++++++++++++++++++++++++++++- 3 files changed, 123 insertions(+), 30 deletions(-) diff --git a/doc/concept/standard.md b/doc/concept/standard.md index 2545f1b94b..ab93e124e3 100644 --- a/doc/concept/standard.md +++ b/doc/concept/standard.md @@ -26,7 +26,7 @@ maps everything else to "not supported" or a harmless equivalent. The subset drops. What a peer actually observes against this implementation: - **Pull.** Subscribers ask. Single-track `PUBLISH` offers are declined; announce a namespace and serve the resulting subscriptions. Announcements go out unsolicited, and we also ask for every prefix we may discover. The solicit `SETUP` option makes us wait to be asked. Without it, a draft 16+ peer also gets each match as a `NAMESPACE` on its `SUBSCRIBE_NAMESPACE` stream, so it hears the namespace twice. We never send `PUBLISH`: on drafts 16 and 17 a `SUBSCRIBE_NAMESPACE` asking only for `PUBLISH` is refused, one asking for both gets only `NAMESPACE`, and any other Subscribe Options value closes the session. -- **History is one group.** A `FETCH` returns one group from the cache, or on drafts 14 through 19 the saved prefix of the group a new subscription just joined. A range of groups is refused, as is a draft 20+ `LOCATION_FILTER` that ends at Largest Object rather than an absolute End Location. On draft 20 and later, `FETCH_OK`'s End Location is the requested end, capped at Largest Object or the track's end: a range holding no objects gets an empty fetch stream, and only a start past Largest Object is refused, with `INVALID_RANGE`. JavaScript publishing refuses every `FETCH`. Datagrams are never fetchable. A Rust reader that only fetches learns the track from `TRACK_STATUS` instead of subscribing, so a finished track stays fetchable; draft 17 still subscribes, since its `TRACK_STATUS` answer cannot say whether the track is timed. +- **History is one group.** A `FETCH` returns one group from the cache, or on drafts 14 through 19 the saved prefix of the group a new subscription just joined. A range of groups is refused, as is a draft 20+ `LOCATION_FILTER` that ends at Largest Object rather than an absolute End Location. On draft 20 and later, `FETCH_OK`'s End Location is the requested end, capped at the track's last object: a range holding no objects gets an empty fetch stream, and only a start past the track's last object is refused, with `INVALID_RANGE`. A relay without a live feed can't know Largest Object, so it is not used as the cap. JavaScript publishing refuses every `FETCH`. Datagrams are never fetchable. A Rust reader that only fetches learns the track from `TRACK_STATUS` instead of subscribing, so a finished track stays fetchable; draft 17 still subscribes, since its `TRACK_STATUS` answer cannot say whether the track is timed. - **Timing.** A track whose `SUBSCRIBE_OK` declares no `TIMESCALE` is untimed, as is every track on drafts 14 through 16. A Rust reader that never subscribed takes the units from `TRACK_STATUS` on draft 18 and later. - **Strict SETUP and parameters.** Repeated unknown `SETUP` options, GREASE included, are accepted if well-formed; a repeated known option is rejected. On draft 17 and later, a `GROUP_ORDER` other than Ascending (1) or Descending (2) closes the session. - **One credential per session**, carried in `SETUP` and forwarded to the [auth server](/bin/relay/auth#the-contract) unverified. A token attached to an individual request is ignored. An alias reference, a second token, or a parameter the negotiated draft does not define closes the session. diff --git a/rs/moq-net/src/ietf/publisher.rs b/rs/moq-net/src/ietf/publisher.rs index 757b93d9a2..941bdb2d26 100644 --- a/rs/moq-net/src/ietf/publisher.rs +++ b/rs/moq-net/src/ietf/publisher.rs @@ -1523,12 +1523,12 @@ where }; let draft20 = Filter::is_draft20(self.version); let end_of_track = !joined && group.complete && track.final_sequence() == group.sequence.checked_add(1); - // Draft-20 caps the response at Largest Object, which for a finished group we know to - // be the latest is its last object. - let at_largest = end_of_track || (group.complete && track.latest() == Some(group.sequence)); - // A start past Largest Object is the one refusal draft-20 defines (section 10.13). - // Anything earlier with no objects is answered as an empty stream. - if draft20 && group.frames.is_empty() && at_largest { + // Draft-20 caps the response at Largest Object, and refuses a start past it (section + // 10.13). We do both only at the track's last object: a relay without a live feed + // cannot know Largest Object, and a cap anywhere else would refuse a valid range. The + // read waits out an unfinished group, so echoing the requested end over a finished + // one only says objects it never held do not exist. + if draft20 && group.frames.is_empty() && end_of_track { return self .reject_fetch(stream, msg.request_id, &Error::InvalidRange, "start past Largest Object") .await; @@ -1566,14 +1566,14 @@ where false => (end, false), } }; - // Draft-20's End Location is inclusive: the requested end, capped at Largest Object - // (section 10.14). Objects missing before it do not exist. A whole-group request has - // no End Object to report, so it covers what the finished group holds, and at least + // Draft-20's End Location is inclusive: the requested end, capped at the track's last + // object (section 10.14). Objects missing before it do not exist. A whole-group request + // has no End Object to report, so it covers what the finished group holds, and at least // the start an empty answer covered. let end_location = match draft20 { true => Location { group: group.sequence, - object: match (until, at_largest) { + object: match (until, end_of_track) { (Some(until), false) => until - 1, _ => group.end().saturating_sub(1).max(start.object), }, @@ -5190,34 +5190,52 @@ mod serve_tests { } } - /// On draft 20, a start past Largest Object is refused INVALID_RANGE (section 10.13): - /// here past the last object of the latest group. + /// On draft 20, a start past the track's last object is refused INVALID_RANGE (section + /// 10.13). The same range on a group that does not end the track is answered empty: only + /// the track's end is a Largest Object a relay without a live feed can know. #[moq_net_sim::test] - async fn a_draft20_fetch_past_the_largest_object_is_refused() { + async fn a_draft20_fetch_past_the_track_end_is_refused() { for version in [Version::Draft20, Version::Draft21, Version::Draft22] { - let mut h = serve(version); - publish_pairs(&mut h, 5, None); - settle().await; + for finished in [true, false] { + let mut h = serve(version); + publish_pairs(&mut h, 5, None); + if finished { + h.track.finish().unwrap(); + } + settle().await; - let buf = standalone_fetch( - &h, - Location { group: 4, object: 2 }, - Location { group: 4, object: 6 }, - GroupOrder::Ascending, - ) - .await; - assert_eq!(fetch_refusal(buf, version), invalid_range(version), "{version}"); + let buf = standalone_fetch( + &h, + Location { group: 4, object: 2 }, + Location { group: 4, object: 6 }, + GroupOrder::Ascending, + ) + .await; + match finished { + true => assert_eq!(fetch_refusal(buf, version), invalid_range(version), "{version}"), + false => { + let (ok, objects) = fetch_answer(buf, version); + assert_eq!(ok.end_location, Location { group: 4, object: 5 }, "{version}"); + assert_eq!(objects, Vec::new(), "{version}"); + } + } + } } } /// On draft 20, an End Object past a finished group's last object is still the End - /// Location reported (section 10.14), unless the group holds Largest Object, which caps it. + /// Location reported (section 10.14), unless the group ends the track, which caps it. A + /// newer group with no objects yet does not make the older one hold Largest Object. #[moq_net_sim::test] async fn a_draft20_fetch_past_a_group_end_reports_the_requested_end() { for version in [Version::Draft20, Version::Draft21, Version::Draft22] { - for (group, reported) in [(2, 7), (4, 1)] { + for (group, finished, reported) in [(2, false, 7), (4, false, 7), (4, true, 1)] { let mut h = serve(version); publish_pairs(&mut h, 5, None); + match finished { + true => h.track.finish().unwrap(), + false => drop(h.track.create_group(group::Info { sequence: 5 }).unwrap()), + } settle().await; let buf = standalone_fetch( @@ -5228,8 +5246,8 @@ mod serve_tests { ) .await; let (ok, objects) = fetch_answer(buf, version); - assert_eq!(ok.end_location, Location { group, object: reported }, "{version}"); - assert!(!ok.end_of_track, "{version}"); + assert_eq!(ok.end_location, Location { group, object: reported }, "{version} group {group}"); + assert_eq!(ok.end_of_track, finished, "{version}"); assert_eq!(objects, pairs([group]), "{version}"); } } diff --git a/rs/moq-net/src/ietf/subscriber.rs b/rs/moq-net/src/ietf/subscriber.rs index bfada67a98..5a7e070fa2 100644 --- a/rs/moq-net/src/ietf/subscriber.rs +++ b/rs/moq-net/src/ietf/subscriber.rs @@ -3690,7 +3690,18 @@ where request.finish_track_at(final_sequence); } - // An empty answer opens no fetch stream at all. + // From draft-20 an empty answer still covers its start, so an End Location before it + // is malformed and closes the session (section 10.14). + if draft20 && (end.group, end.object) <= (sequence, start) { + let err = Error::ProtocolViolation; + tracing::warn!(group = sequence, ?end, "FETCH_OK's End Location is before its start"); + self.session + .clone() + .close(SessionError::from(&err).to_code(), err.to_string().as_ref()); + request.reject(err); + return; + } + // Before draft-20, an empty answer opens no fetch stream at all. if (end.group, end.object) <= (sequence, start) { request.reject(Error::NotFound); let _ = stream.writer.close().await; @@ -9621,6 +9632,70 @@ mod joining_fetch_tests { assert!(fetch.await.is_err(), "the accepted group was aborted"); } + /// From draft-20 an End Location before the FETCH's start is malformed (section 10.14): + /// the session closes with PROTOCOL_VIOLATION rather than reading it as an empty answer. + #[moq_net_sim::test] + async fn a_draft20_end_location_before_the_start_closes_the_session() { + const VERSION: Version = Version::Draft20; + const GROUP: u64 = 4; + const START: u64 = 3; + + let ok = message_bytes( + ietf::FetchOk::ID, + &ietf::FetchOk { + request_id: None, + group_order: GroupOrder::Ascending, + end_of_track: false, + end_location: ietf::Location { + group: GROUP, + object: START - 1, + }, + properties: Default::default(), + }, + VERSION, + ); + + let session = ScriptedSession::per_stream_eof(vec![ok]); + let (tasks, _task_set) = crate::util::TaskSet::new(); + let subscriber = Subscriber::new( + crate::time::Clock::sim(), + session.clone(), + crate::origin::Config::new(crate::Hop::new(1).unwrap()).produce(), + Control::new(None, false), + None, + peer::PeerSetup::default(), + crate::Hop::new(1).unwrap(), + None, + VERSION, + tasks, + Default::default(), + ); + + let track = track::Producer::new( + std::sync::Arc::new(crate::broadcast::Info::default()), + "video", + track::Info::default().with_timescale(None), + ); + let dynamic = track.dynamic(); + let consumer = track.consume(); + let mut fetch = std::pin::pin!(consumer.fetch_group(GROUP, group::Fetch::default().with_frame_start(START))); + assert!(futures::poll!(fetch.as_mut()).is_pending()); + let request = dynamic.requested_group().await.expect("no group requested"); + + subscriber + .clone() + .run_group_fetch(Path::new("broadcast").to_owned(), "video".into(), request, None) + .await; + + assert!(matches!(fetch.await, Err(Error::ProtocolViolation))); + let violation = SessionError::from(&Error::ProtocolViolation).to_code(); + assert!( + session.log.closes().iter().any(|(code, _)| *code == violation), + "{:?}", + session.log.closes() + ); + } + /// A cache miss for a group's tail asks upstream from the frame the reader wants and /// numbers what arrives from there, so a publisher that evicted the prefix can answer. #[moq_net_sim::test] From 5a47135004a2ba993d50d7e565af535c964d7fff Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Thu, 8 Oct 2026 08:54:05 -0700 Subject: [PATCH 22/28] fix(moq-net): gate TRACK_STATUS_OK's parameters per draft like SUBSCRIBE_OK Co-Authored-By: Claude Opus 5.5 --- rs/moq-net/src/ietf/track.rs | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/rs/moq-net/src/ietf/track.rs b/rs/moq-net/src/ietf/track.rs index 984b2488e1..243f06037b 100644 --- a/rs/moq-net/src/ietf/track.rs +++ b/rs/moq-net/src/ietf/track.rs @@ -178,10 +178,10 @@ impl Message for TrackStatusOk { // Read as SUBSCRIBE_OK's are: EXPIRES is ignored, and MAX_CACHE_DURATION is a // parameter only on draft-15. decode_params!(r, version, - 0x04 => max_cache_duration: Option, + 0x04 => max_cache_duration: Option where version == Version::Draft15, 0x08 => _expires: Option, 0x09 => largest: Option, - 0x22 => group_order: Option, + 0x22 => group_order: Option where version == Version::Draft15, ); let mut properties = match Self::has_properties(version) { true => Properties::decode(r, version)?, @@ -233,6 +233,22 @@ mod tests { buf.to_vec() } + /// GROUP_ORDER and MAX_CACHE_DURATION are TRACK_STATUS_OK parameters only on draft-15, + /// as on SUBSCRIBE_OK. Draft-16 ignores a GROUP_ORDER it defines elsewhere, even with a + /// value no field could hold, and rejects 0x04, which it does not define at all. Draft-18 + /// rejects a known parameter on a message that does not define it. + #[test] + fn track_status_ok_parameters_follow_the_draft() { + // Request 1, then GROUP_ORDER as the varint 256. + let ok = decode_message::(&[0x01, 0x01, 0x22, 0x41, 0x00], Version::Draft16) + .expect("draft-16 ignores GROUP_ORDER"); + assert_eq!(ok.properties.group_order, None); + // Request 1, then MAX_CACHE_DURATION of 5. + assert!(decode_message::(&[0x01, 0x01, 0x04, 0x05], Version::Draft16).is_err()); + // GROUP_ORDER = Ascending, then an empty properties block. + assert!(decode_message::(&[0x01, 0x22, 0x01], Version::Draft18).is_err()); + } + fn decode_message(bytes: &[u8], version: Version) -> Result { let mut buf = bytes::Bytes::from(bytes.to_vec()); crate::coding::decode_buf(&mut buf, version, M::decode_msg) From 190479ab2741b448561736c4343d894bcfb049a9 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Thu, 8 Oct 2026 09:03:19 -0700 Subject: [PATCH 23/28] fix(ietf): cap a draft-20 FETCH at Largest Object wherever the cache knows it The origin and a live relay copy know Largest Object, including behind a newer group with no objects yet, so they cap the End Location at it and refuse a start past it with INVALID_RANGE. Only a relay's copy with no upstream subscription, which cannot know it short of the track's end, echoes the requested end. A malformed draft-20 End Location is refused before it can mark the track's end. Co-Authored-By: Claude Opus 5.5 --- doc/concept/standard.md | 2 +- rs/moq-net/src/ietf/publisher.rs | 122 +++++++++++++++++++++--------- rs/moq-net/src/ietf/subscriber.rs | 23 +++--- rs/moq-net/src/model/track.rs | 9 +++ 4 files changed, 108 insertions(+), 48 deletions(-) diff --git a/doc/concept/standard.md b/doc/concept/standard.md index 8efef5bd05..1ecede7dda 100644 --- a/doc/concept/standard.md +++ b/doc/concept/standard.md @@ -26,7 +26,7 @@ maps everything else to "not supported" or a harmless equivalent. The subset drops. What a peer actually observes against this implementation: - **Pull.** Subscribers ask. Single-track `PUBLISH` offers are declined; announce a namespace and serve the resulting subscriptions. Announcements go out unsolicited, and we also ask for every prefix we may discover. The solicit `SETUP` option makes us wait to be asked. Without it, a draft 16+ peer also gets each match as a `NAMESPACE` on its `SUBSCRIBE_NAMESPACE` stream, so it hears the namespace twice. We never send `PUBLISH`: on drafts 16 and 17 a `SUBSCRIBE_NAMESPACE` asking only for `PUBLISH` is refused, one asking for both gets only `NAMESPACE`, and any other Subscribe Options value closes the session. -- **History is one group.** A `FETCH` returns one group from the cache, or on drafts 14 through 19 the saved prefix of the group a new subscription just joined. A range of groups is refused, as is a draft 20+ `LOCATION_FILTER` that ends at Largest Object rather than an absolute End Location. On draft 20 and later, `FETCH_OK`'s End Location is the requested end, capped at the track's last object: a range holding no objects gets an empty fetch stream, and only a start past the track's last object is refused, with `INVALID_RANGE`. A relay without a live feed can't know Largest Object, so it is not used as the cap. JavaScript publishing refuses every `FETCH`. Datagrams are never fetchable. A Rust reader that only fetches learns the track from `TRACK_STATUS` instead of subscribing, so a finished track stays fetchable; draft 17 still subscribes, since its `TRACK_STATUS` answer cannot say whether the track is timed. +- **History is one group.** A `FETCH` returns one group from the cache, or on drafts 14 through 19 the saved prefix of the group a new subscription just joined. A range of groups is refused, as is a draft 20+ `LOCATION_FILTER` that ends at Largest Object rather than an absolute End Location. On draft 20 and later, `FETCH_OK`'s End Location is the requested end, capped at Largest Object: a range holding no objects gets an empty fetch stream, and only a start past Largest Object is refused, with `INVALID_RANGE`. A relay's copy with no upstream subscription can't know Largest Object short of the track's end, so it echoes the requested end instead. JavaScript publishing refuses every `FETCH`. Datagrams are never fetchable. A Rust reader that only fetches learns the track from `TRACK_STATUS` instead of subscribing, so a finished track stays fetchable; draft 17 still subscribes, since its `TRACK_STATUS` answer cannot say whether the track is timed. - **Timing.** A track whose `SUBSCRIBE_OK` declares no `TIMESCALE` is untimed, as is every track on drafts 14 through 16. A Rust reader that never subscribed takes the units from `TRACK_STATUS` on draft 18 and later. - **Strict SETUP and parameters.** Repeated unknown `SETUP` options, GREASE included, are accepted if well-formed; a repeated known option is rejected. On draft 17 and later, a `GROUP_ORDER` other than Ascending (1) or Descending (2) closes the session. Message parameters follow the negotiated draft's lists: drafts 14 and 15 ignore an unknown or misplaced parameter, draft 16 ignores one defined only for another message but closes the session on an unknown one, and draft 17 and later close it on both. - **One credential per session**, carried in `SETUP` and forwarded to the [auth server](/bin/relay/auth#the-contract) unverified. A token attached to an individual request is ignored. An alias reference or a second token closes the session. diff --git a/rs/moq-net/src/ietf/publisher.rs b/rs/moq-net/src/ietf/publisher.rs index 941bdb2d26..bf8edbad82 100644 --- a/rs/moq-net/src/ietf/publisher.rs +++ b/rs/moq-net/src/ietf/publisher.rs @@ -1524,11 +1524,22 @@ where let draft20 = Filter::is_draft20(self.version); let end_of_track = !joined && group.complete && track.final_sequence() == group.sequence.checked_add(1); // Draft-20 caps the response at Largest Object, and refuses a start past it (section - // 10.13). We do both only at the track's last object: a relay without a live feed - // cannot know Largest Object, and a cap anywhere else would refuse a valid range. The - // read waits out an unfinished group, so echoing the requested end over a finished + // 10.13). The cache knows it at the track's end, or while it reflects a live feed. A + // relay's copy with no upstream subscription does not, so it neither caps nor refuses: + // the read waits out an unfinished group, so echoing the requested end over a finished // one only says objects it never held do not exist. - if draft20 && group.frames.is_empty() && end_of_track { + let largest = match end_of_track { + true => Some(group.end().checked_sub(1).map(|object| Location { + group: group.sequence, + object, + })), + false if !joined && track.is_live() => Some(live_edge(&track).largest), + false => None, + }; + let past_largest = largest.is_some_and(|largest| { + largest.is_none_or(|largest| (start.group, start.object) > (largest.group, largest.object)) + }); + if draft20 && past_largest { return self .reject_fetch(stream, msg.request_id, &Error::InvalidRange, "start past Largest Object") .await; @@ -1566,18 +1577,25 @@ where false => (end, false), } }; - // Draft-20's End Location is inclusive: the requested end, capped at the track's last - // object (section 10.14). Objects missing before it do not exist. A whole-group request - // has no End Object to report, so it covers what the finished group holds, and at least + // Draft-20's End Location is inclusive: the requested end, capped at Largest Object + // (section 10.14). Objects missing before it do not exist. A whole-group request has + // no End Object to report, so it covers what the finished group holds, and at least // the start an empty answer covered. let end_location = match draft20 { - true => Location { - group: group.sequence, - object: match (until, end_of_track) { - (Some(until), false) => until - 1, - _ => group.end().saturating_sub(1).max(start.object), - }, - }, + true => { + let requested = match until { + Some(until) => until - 1, + None => group.end().saturating_sub(1).max(start.object), + }; + let object = match largest.flatten() { + Some(largest) if largest.group == group.sequence => requested.min(largest.object), + _ => requested, + }; + Location { + group: group.sequence, + object, + } + } false => end_location, }; @@ -5190,18 +5208,41 @@ mod serve_tests { } } - /// On draft 20, a start past the track's last object is refused INVALID_RANGE (section - /// 10.13). The same range on a group that does not end the track is answered empty: only - /// the track's end is a Largest Object a relay without a live feed can know. + /// How much a draft-20 publisher knows of Largest Object. + #[derive(Clone, Copy, Debug)] + enum Largest { + /// The track is finished: its last object. + Finished, + /// A live feed: the newest object, here behind a newer group with no objects yet. + Live, + /// A relay's copy with no upstream subscription: unknown. + Idle, + } + + /// Groups 0 to 4 of two objects each, with Largest Object known as `largest` says. + fn publish_largest(h: &mut Serve, largest: Largest) -> Option { + publish_pairs(h, 5, None); + match largest { + Largest::Finished => { + h.track.finish().unwrap(); + None + } + Largest::Live => Some(h.track.create_group(group::Info { sequence: 5 }).unwrap()), + Largest::Idle => { + h.track.set_idle(); + None + } + } + } + + /// On draft 20, a start past Largest Object is refused INVALID_RANGE (section 10.13), + /// wherever the publisher knows it. A relay's idle copy does not, and answers empty. #[moq_net_sim::test] - async fn a_draft20_fetch_past_the_track_end_is_refused() { + async fn a_draft20_fetch_past_the_largest_object_is_refused() { for version in [Version::Draft20, Version::Draft21, Version::Draft22] { - for finished in [true, false] { + for largest in [Largest::Finished, Largest::Live, Largest::Idle] { let mut h = serve(version); - publish_pairs(&mut h, 5, None); - if finished { - h.track.finish().unwrap(); - } + let _newer = publish_largest(&mut h, largest); settle().await; let buf = standalone_fetch( @@ -5211,31 +5252,36 @@ mod serve_tests { GroupOrder::Ascending, ) .await; - match finished { - true => assert_eq!(fetch_refusal(buf, version), invalid_range(version), "{version}"), - false => { + match largest { + Largest::Idle => { let (ok, objects) = fetch_answer(buf, version); assert_eq!(ok.end_location, Location { group: 4, object: 5 }, "{version}"); assert_eq!(objects, Vec::new(), "{version}"); } + _ => assert_eq!( + fetch_refusal(buf, version), + invalid_range(version), + "{version} {largest:?}" + ), } } } } /// On draft 20, an End Object past a finished group's last object is still the End - /// Location reported (section 10.14), unless the group ends the track, which caps it. A - /// newer group with no objects yet does not make the older one hold Largest Object. + /// Location reported (section 10.14), unless the group holds Largest Object, which caps + /// it. A relay's idle copy does not know Largest Object, so it echoes the request. #[moq_net_sim::test] async fn a_draft20_fetch_past_a_group_end_reports_the_requested_end() { for version in [Version::Draft20, Version::Draft21, Version::Draft22] { - for (group, finished, reported) in [(2, false, 7), (4, false, 7), (4, true, 1)] { + for (group, largest, reported) in [ + (2, Largest::Live, 7), + (4, Largest::Live, 1), + (4, Largest::Finished, 1), + (4, Largest::Idle, 7), + ] { let mut h = serve(version); - publish_pairs(&mut h, 5, None); - match finished { - true => h.track.finish().unwrap(), - false => drop(h.track.create_group(group::Info { sequence: 5 }).unwrap()), - } + let _newer = publish_largest(&mut h, largest); settle().await; let buf = standalone_fetch( @@ -5246,8 +5292,12 @@ mod serve_tests { ) .await; let (ok, objects) = fetch_answer(buf, version); - assert_eq!(ok.end_location, Location { group, object: reported }, "{version} group {group}"); - assert_eq!(ok.end_of_track, finished, "{version}"); + assert_eq!( + ok.end_location, + Location { group, object: reported }, + "{version} group {group} {largest:?}" + ); + assert_eq!(ok.end_of_track, matches!(largest, Largest::Finished), "{version}"); assert_eq!(objects, pairs([group]), "{version}"); } } diff --git a/rs/moq-net/src/ietf/subscriber.rs b/rs/moq-net/src/ietf/subscriber.rs index 5a7e070fa2..f0fca8e2b4 100644 --- a/rs/moq-net/src/ietf/subscriber.rs +++ b/rs/moq-net/src/ietf/subscriber.rs @@ -3680,18 +3680,8 @@ where end.object = object; } - // The publisher knows where the track ends, which a range FETCH downstream needs. - if ok.end_of_track { - let Some(final_sequence) = end.group.checked_add(u64::from(end.object > 0)) else { - request.reject(Error::ProtocolViolation); - let _ = stream.writer.close().await; - return; - }; - request.finish_track_at(final_sequence); - } - // From draft-20 an empty answer still covers its start, so an End Location before it - // is malformed and closes the session (section 10.14). + // is malformed and closes the session (section 10.14), before it can mark the track's end. if draft20 && (end.group, end.object) <= (sequence, start) { let err = Error::ProtocolViolation; tracing::warn!(group = sequence, ?end, "FETCH_OK's End Location is before its start"); @@ -3701,6 +3691,17 @@ where request.reject(err); return; } + + // The publisher knows where the track ends, which a range FETCH downstream needs. + if ok.end_of_track { + let Some(final_sequence) = end.group.checked_add(u64::from(end.object > 0)) else { + request.reject(Error::ProtocolViolation); + let _ = stream.writer.close().await; + return; + }; + request.finish_track_at(final_sequence); + } + // Before draft-20, an empty answer opens no fetch stream at all. if (end.group, end.object) <= (sequence, start) { request.reject(Error::NotFound); diff --git a/rs/moq-net/src/model/track.rs b/rs/moq-net/src/model/track.rs index 782941d246..393a079885 100644 --- a/rs/moq-net/src/model/track.rs +++ b/rs/moq-net/src/model/track.rs @@ -2920,6 +2920,15 @@ impl Consumer { routes.serving() } + /// Whether the cache reflects the track's live feed (or its end), so the newest object + /// it holds is the track's largest. A relay's copy with no upstream subscription is not. + pub(crate) fn is_live(&self) -> bool { + if let Some(serving) = self.serving() { + return serving.is_live(); + } + self.state.read().readable() + } + /// Fetching a single past group, without holding a live subscription. /// /// Returns a [`kio::Pending`] that resolves to the [`group::Consumer`]: From 770fd1941177c6a1178c06a1c4149bc8cdb24777 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Thu, 8 Oct 2026 10:00:05 -0700 Subject: [PATCH 24/28] fix(ietf): bound a draft-20 FETCH by Largest Object only when the cache can name it in the group An idle relay copy that learned the track's end is still not fed live, and a live feed whose newest group the cache cannot read (a datagram, an aborted group) has an unknown edge rather than none. Cap and refuse only when Largest Object is known and in the fetched group. Co-Authored-By: Claude Opus 5.5 --- rs/moq-net/src/ietf/publisher.rs | 66 +++++++++++++++++++++----------- rs/moq-net/src/model/track.rs | 6 +-- 2 files changed, 47 insertions(+), 25 deletions(-) diff --git a/rs/moq-net/src/ietf/publisher.rs b/rs/moq-net/src/ietf/publisher.rs index bf8edbad82..477ed6ee90 100644 --- a/rs/moq-net/src/ietf/publisher.rs +++ b/rs/moq-net/src/ietf/publisher.rs @@ -1524,21 +1524,22 @@ where let draft20 = Filter::is_draft20(self.version); let end_of_track = !joined && group.complete && track.final_sequence() == group.sequence.checked_add(1); // Draft-20 caps the response at Largest Object, and refuses a start past it (section - // 10.13). The cache knows it at the track's end, or while it reflects a live feed. A - // relay's copy with no upstream subscription does not, so it neither caps nor refuses: - // the read waits out an unfinished group, so echoing the requested end over a finished - // one only says objects it never held do not exist. + // 10.13). Both only bite when Largest Object is in the fetched group, so this is its + // Object ID there, `Some(None)` for a final group with nothing from the start. The + // cache knows it at the track's end, or when a live feed's newest object it can name + // is in this group. Otherwise (a relay's copy with no upstream subscription, or a + // newest group it cannot read) it neither caps nor refuses: the read waits out an + // unfinished group, so echoing the requested end over a finished one only says + // objects it never held do not exist. let largest = match end_of_track { - true => Some(group.end().checked_sub(1).map(|object| Location { - group: group.sequence, - object, - })), - false if !joined && track.is_live() => Some(live_edge(&track).largest), + true => Some(group.end().checked_sub(1)), + false if !joined && track.is_live() => live_edge(&track) + .largest + .filter(|largest| largest.group == group.sequence) + .map(|largest| Some(largest.object)), false => None, }; - let past_largest = largest.is_some_and(|largest| { - largest.is_none_or(|largest| (start.group, start.object) > (largest.group, largest.object)) - }); + let past_largest = largest.is_some_and(|largest| largest.is_none_or(|object| start.object > object)); if draft20 && past_largest { return self .reject_fetch(stream, msg.request_id, &Error::InvalidRange, "start past Largest Object") @@ -1588,8 +1589,8 @@ where None => group.end().saturating_sub(1).max(start.object), }; let object = match largest.flatten() { - Some(largest) if largest.group == group.sequence => requested.min(largest.object), - _ => requested, + Some(largest) => requested.min(largest), + None => requested, }; Location { group: group.sequence, @@ -5217,6 +5218,10 @@ mod serve_tests { Live, /// A relay's copy with no upstream subscription: unknown. Idle, + /// An idle copy that learned a later end of track: still unknown. + IdleEnded, + /// A live feed whose newest group is a datagram the cache cannot read: unknown. + Datagram, } /// Groups 0 to 4 of two objects each, with Largest Object known as `largest` says. @@ -5232,15 +5237,30 @@ mod serve_tests { h.track.set_idle(); None } + Largest::IdleEnded => { + h.track.set_idle(); + h.track.finish_at(10).unwrap(); + None + } + Largest::Datagram => { + h.track.append_datagram(timestamp(), b"d".as_slice()).unwrap(); + None + } } } /// On draft 20, a start past Largest Object is refused INVALID_RANGE (section 10.13), - /// wherever the publisher knows it. A relay's idle copy does not, and answers empty. + /// wherever the publisher knows it. Where it does not, it answers empty. #[moq_net_sim::test] async fn a_draft20_fetch_past_the_largest_object_is_refused() { for version in [Version::Draft20, Version::Draft21, Version::Draft22] { - for largest in [Largest::Finished, Largest::Live, Largest::Idle] { + for largest in [ + Largest::Finished, + Largest::Live, + Largest::Idle, + Largest::IdleEnded, + Largest::Datagram, + ] { let mut h = serve(version); let _newer = publish_largest(&mut h, largest); settle().await; @@ -5253,16 +5273,16 @@ mod serve_tests { ) .await; match largest { - Largest::Idle => { - let (ok, objects) = fetch_answer(buf, version); - assert_eq!(ok.end_location, Location { group: 4, object: 5 }, "{version}"); - assert_eq!(objects, Vec::new(), "{version}"); - } - _ => assert_eq!( + Largest::Finished | Largest::Live => assert_eq!( fetch_refusal(buf, version), invalid_range(version), "{version} {largest:?}" ), + _ => { + let (ok, objects) = fetch_answer(buf, version); + assert_eq!(ok.end_location, Location { group: 4, object: 5 }, "{version} {largest:?}"); + assert_eq!(objects, Vec::new(), "{version} {largest:?}"); + } } } } @@ -5279,6 +5299,8 @@ mod serve_tests { (4, Largest::Live, 1), (4, Largest::Finished, 1), (4, Largest::Idle, 7), + (4, Largest::IdleEnded, 7), + (4, Largest::Datagram, 7), ] { let mut h = serve(version); let _newer = publish_largest(&mut h, largest); diff --git a/rs/moq-net/src/model/track.rs b/rs/moq-net/src/model/track.rs index 393a079885..cd87cd43cf 100644 --- a/rs/moq-net/src/model/track.rs +++ b/rs/moq-net/src/model/track.rs @@ -2920,13 +2920,13 @@ impl Consumer { routes.serving() } - /// Whether the cache reflects the track's live feed (or its end), so the newest object - /// it holds is the track's largest. A relay's copy with no upstream subscription is not. + /// Whether the cache is fed live, so the newest object it holds is the track's largest. + /// A relay's copy with no upstream subscription is not, even once it learns the end. pub(crate) fn is_live(&self) -> bool { if let Some(serving) = self.serving() { return serving.is_live(); } - self.state.read().readable() + self.state.read().feed == Feed::Live } /// Fetching a single past group, without holding a live subscription. From 05a047d9346697c7630f2c6bb5a51a0c06f86820 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Thu, 8 Oct 2026 10:48:08 -0700 Subject: [PATCH 25/28] fix(ietf): refuse a draft-20 FETCH of an empty group past the live Largest Object Co-Authored-By: Claude Opus 5.5 --- rs/moq-net/src/ietf/publisher.rs | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/rs/moq-net/src/ietf/publisher.rs b/rs/moq-net/src/ietf/publisher.rs index 477ed6ee90..e8ac8c13e4 100644 --- a/rs/moq-net/src/ietf/publisher.rs +++ b/rs/moq-net/src/ietf/publisher.rs @@ -1533,10 +1533,12 @@ where // objects it never held do not exist. let largest = match end_of_track { true => Some(group.end().checked_sub(1)), - false if !joined && track.is_live() => live_edge(&track) - .largest - .filter(|largest| largest.group == group.sequence) - .map(|largest| Some(largest.object)), + false if !joined && track.is_live() => match live_edge(&track).largest { + Some(largest) if largest.group == group.sequence => Some(Some(largest.object)), + // Behind a group that holds nothing: every start in it is past Largest Object. + Some(largest) if largest.group < group.sequence && group.frames.is_empty() => Some(None), + _ => None, + }, false => None, }; let past_largest = largest.is_some_and(|largest| largest.is_none_or(|object| start.object > object)); @@ -5288,6 +5290,28 @@ mod serve_tests { } } + /// On a live feed whose newest group finished with no objects, Largest Object sits in the + /// group before it, so a draft-20 FETCH of the empty group starts past it. + #[moq_net_sim::test] + async fn a_draft20_fetch_of_an_empty_newest_group_is_refused() { + for version in [Version::Draft20, Version::Draft21, Version::Draft22] { + let mut h = serve(version); + publish_pairs(&mut h, 5, None); + let empty = h.track.create_group(group::Info { sequence: 5 }).unwrap(); + empty.finish().unwrap(); + settle().await; + + let buf = standalone_fetch( + &h, + Location { group: 5, object: 0 }, + Location { group: 5, object: 4 }, + GroupOrder::Ascending, + ) + .await; + assert_eq!(fetch_refusal(buf, version), invalid_range(version), "{version}"); + } + } + /// On draft 20, an End Object past a finished group's last object is still the End /// Location reported (section 10.14), unless the group holds Largest Object, which caps /// it. A relay's idle copy does not know Largest Object, so it echoes the request. From a0b777bcb739908a708bc6752e455d3f4d2a863e Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Thu, 8 Oct 2026 11:24:43 -0700 Subject: [PATCH 26/28] style: cargo fmt the draft-20 FETCH bound Co-Authored-By: Claude Opus 5.5 --- rs/moq-net/src/ietf/publisher.rs | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/rs/moq-net/src/ietf/publisher.rs b/rs/moq-net/src/ietf/publisher.rs index e8ac8c13e4..9748f2a10e 100644 --- a/rs/moq-net/src/ietf/publisher.rs +++ b/rs/moq-net/src/ietf/publisher.rs @@ -1544,7 +1544,12 @@ where let past_largest = largest.is_some_and(|largest| largest.is_none_or(|object| start.object > object)); if draft20 && past_largest { return self - .reject_fetch(stream, msg.request_id, &Error::InvalidRange, "start past Largest Object") + .reject_fetch( + stream, + msg.request_id, + &Error::InvalidRange, + "start past Largest Object", + ) .await; } @@ -5205,7 +5210,14 @@ mod serve_tests { let (ok, objects) = fetch_answer(buf, version); assert_eq!(objects, Vec::new(), "{version} end={end}"); if Filter::is_draft20(version) { - assert_eq!(ok.end_location, Location { group: 2, object: covered }, "{version}"); + assert_eq!( + ok.end_location, + Location { + group: 2, + object: covered + }, + "{version}" + ); } } } @@ -5282,7 +5294,11 @@ mod serve_tests { ), _ => { let (ok, objects) = fetch_answer(buf, version); - assert_eq!(ok.end_location, Location { group: 4, object: 5 }, "{version} {largest:?}"); + assert_eq!( + ok.end_location, + Location { group: 4, object: 5 }, + "{version} {largest:?}" + ); assert_eq!(objects, Vec::new(), "{version} {largest:?}"); } } @@ -5340,7 +5356,10 @@ mod serve_tests { let (ok, objects) = fetch_answer(buf, version); assert_eq!( ok.end_location, - Location { group, object: reported }, + Location { + group, + object: reported + }, "{version} group {group} {largest:?}" ); assert_eq!(ok.end_of_track, matches!(largest, Largest::Finished), "{version}"); From a68224c3052f28215ef073dfdc7ceb4fcf0e12e5 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Thu, 8 Oct 2026 12:15:22 -0700 Subject: [PATCH 27/28] docs(ietf): say the draft-20 FETCH bound also refuses behind an empty group Co-Authored-By: Claude Opus 5.5 --- rs/moq-net/src/ietf/publisher.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rs/moq-net/src/ietf/publisher.rs b/rs/moq-net/src/ietf/publisher.rs index 9748f2a10e..2043477d1b 100644 --- a/rs/moq-net/src/ietf/publisher.rs +++ b/rs/moq-net/src/ietf/publisher.rs @@ -1524,10 +1524,10 @@ where let draft20 = Filter::is_draft20(self.version); let end_of_track = !joined && group.complete && track.final_sequence() == group.sequence.checked_add(1); // Draft-20 caps the response at Largest Object, and refuses a start past it (section - // 10.13). Both only bite when Largest Object is in the fetched group, so this is its - // Object ID there, `Some(None)` for a final group with nothing from the start. The - // cache knows it at the track's end, or when a live feed's newest object it can name - // is in this group. Otherwise (a relay's copy with no upstream subscription, or a + // 10.13). Both only bite when Largest Object is in the fetched group or behind it, so + // this is its Object ID there, or `Some(None)` when the group holds nothing at or + // past it. The cache knows it at the track's end, or when a live feed's newest object + // it can name is in or behind this group. Otherwise (a relay's copy with no upstream subscription, or a // newest group it cannot read) it neither caps nor refuses: the read waits out an // unfinished group, so echoing the requested end over a finished one only says // objects it never held do not exist. From 53629afcf08c54ced67cbade456982cb71b0e6d1 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Thu, 8 Oct 2026 23:27:36 -0700 Subject: [PATCH 28/28] test(ietf): pin a draft-20 FETCH behind an aborted newest group Co-Authored-By: Claude Opus 5.5 --- rs/moq-net/src/ietf/publisher.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/rs/moq-net/src/ietf/publisher.rs b/rs/moq-net/src/ietf/publisher.rs index 2043477d1b..fc841391b1 100644 --- a/rs/moq-net/src/ietf/publisher.rs +++ b/rs/moq-net/src/ietf/publisher.rs @@ -1527,10 +1527,10 @@ where // 10.13). Both only bite when Largest Object is in the fetched group or behind it, so // this is its Object ID there, or `Some(None)` when the group holds nothing at or // past it. The cache knows it at the track's end, or when a live feed's newest object - // it can name is in or behind this group. Otherwise (a relay's copy with no upstream subscription, or a - // newest group it cannot read) it neither caps nor refuses: the read waits out an - // unfinished group, so echoing the requested end over a finished one only says - // objects it never held do not exist. + // it can name is in or behind this group. Otherwise (a relay's copy with no upstream + // subscription, or a newest group it cannot read) it neither caps nor refuses: the + // read waits out an unfinished group, so echoing the requested end over a finished + // one only says objects it never held do not exist. let largest = match end_of_track { true => Some(group.end().checked_sub(1)), false if !joined && track.is_live() => match live_edge(&track).largest { @@ -5236,6 +5236,8 @@ mod serve_tests { IdleEnded, /// A live feed whose newest group is a datagram the cache cannot read: unknown. Datagram, + /// A live feed whose newest group was aborted mid-write: unknown. + Aborted, } /// Groups 0 to 4 of two objects each, with Largest Object known as `largest` says. @@ -5260,6 +5262,12 @@ mod serve_tests { h.track.append_datagram(timestamp(), b"d".as_slice()).unwrap(); None } + Largest::Aborted => { + let mut newer = h.track.create_group(group::Info { sequence: 5 }).unwrap(); + newer.write_frame(timestamp(), b"5-0".to_vec()).unwrap(); + newer.abort(Error::Cancel).unwrap(); + None + } } } @@ -5274,6 +5282,7 @@ mod serve_tests { Largest::Idle, Largest::IdleEnded, Largest::Datagram, + Largest::Aborted, ] { let mut h = serve(version); let _newer = publish_largest(&mut h, largest); @@ -5341,6 +5350,7 @@ mod serve_tests { (4, Largest::Idle, 7), (4, Largest::IdleEnded, 7), (4, Largest::Datagram, 7), + (4, Largest::Aborted, 7), ] { let mut h = serve(version); let _newer = publish_largest(&mut h, largest);