From d724a9522bb8eaae3536810f761ddf4032347c60 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 10 Oct 2026 09:26:51 -0700 Subject: [PATCH 1/6] chore: claim request parameters backport Co-Authored-By: GPT-6 From 7d2385e6ffece9908fd4416913ae0f8de0185d3b Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 10 Oct 2026 10:06:46 -0700 Subject: [PATCH 2/6] fix(net): decode legacy request parameters by draft on release Co-Authored-By: GPT-6 --- js/net/src/ietf/ietf.test.ts | 9 +++++ rs/moq-net/src/ietf/parameters.rs | 35 +++++++++++++++-- rs/moq-net/src/ietf/subscribe_namespace.rs | 44 ++++++++++++++++++++-- 3 files changed, 80 insertions(+), 8 deletions(-) diff --git a/js/net/src/ietf/ietf.test.ts b/js/net/src/ietf/ietf.test.ts index fef65c8f9f..034435fc82 100644 --- a/js/net/src/ietf/ietf.test.ts +++ b/js/net/src/ietf/ietf.test.ts @@ -1945,3 +1945,12 @@ test("Subscribe v16: rejects INCLUDE_PROPERTIES", async () => { const body = framed([...TRACK_HEAD, 0x01, 0x35, 0x01, 0x00]); await expect(decodeVersioned(body, Subscribe.Subscribe.decode, Version.DRAFT_16)).rejects.toThrow(); }); + +for (const version of [Version.DRAFT_14, Version.DRAFT_15]) { + test(`Parameters ${version}: unknown key does not hide a later known value`, async () => { + for (const bytes of [new Uint8Array([2, 0x3e, 1, 0x20, 7]), new Uint8Array([2, 0x3d, 1, 42, 0x20, 7])]) { + const params = await Parameters.decode(new Reader(undefined, bytes, version), version); + expect(params.subscriberPriority).toBe(7); + } + }); +} diff --git a/rs/moq-net/src/ietf/parameters.rs b/rs/moq-net/src/ietf/parameters.rs index 2e7330a801..dfcb33409f 100644 --- a/rs/moq-net/src/ietf/parameters.rs +++ b/rs/moq-net/src/ietf/parameters.rs @@ -583,6 +583,16 @@ macro_rules! decode_params { continue; } )* + // Drafts 14 and 15 use Key-Value-Pairs and require unknown ids to be ignored. + if matches!(_version, $crate::ietf::Version::Draft14 | $crate::ietf::Version::Draft15) { + if _key % 2 == 0 { + >::decode($r, _version)?; + } else { + let _value = as $crate::coding::Decode<$crate::ietf::Version>>::decode($r, _version)?; + if _value.len() > (1 << 16) - 1 { return Err($crate::coding::DecodeError::BoundsExceeded); } + } + continue; + } return Err($crate::coding::DecodeError::InvalidValue); } } @@ -1015,10 +1025,27 @@ mod tests { let _ = val; Ok(()) })(); - assert!( - matches!(result, Err(DecodeError::InvalidValue)), - "expected InvalidValue for unknown param in {version}" - ); + if matches!(version, Version::Draft14 | Version::Draft15) { + result.unwrap(); + assert!(!bytes.has_remaining()); + } else { + assert!(matches!(result, Err(DecodeError::InvalidValue)), "{version}"); + } + } + } + + #[test] + fn legacy_unknown_parameter_is_ignored() { + for version in [Version::Draft14, Version::Draft15] { + for data in [&[2, 0x3e, 1, 0x20, 7][..], &[2, 0x3d, 1, 42, 0x20, 7][..]] { + let mut bytes = data; + let result: Result, DecodeError> = (|| { + decode_params!(&mut bytes, version, 0x20 => priority: Option); + Ok(priority) + })(); + assert_eq!(result.unwrap(), Some(7)); + assert!(bytes.is_empty()); + } } } diff --git a/rs/moq-net/src/ietf/subscribe_namespace.rs b/rs/moq-net/src/ietf/subscribe_namespace.rs index 252ba94b6b..f241a36608 100644 --- a/rs/moq-net/src/ietf/subscribe_namespace.rs +++ b/rs/moq-net/src/ietf/subscribe_namespace.rs @@ -138,10 +138,20 @@ impl Message for SubscribeNamespaceLegacy<'_> { }; // The token is ignored: the session's grant is what authorizes the request. - decode_params!(r, version, - 0x03 => _authorization_token: Vec, - HIDDEN_PARAM => hidden: Option, - ); + let hidden = if version == Version::Draft14 { + decode_params!(r, version, + 0x03 => _authorization_token: Vec, + HIDDEN_PARAM => hidden: Option, + ); + hidden + } else { + decode_params!(r, version, + 0x03 => _authorization_token: Vec, + 0x10 => _forward: Option, + HIDDEN_PARAM => hidden: Option, + ); + hidden + }; Ok(Self { request_id, @@ -433,6 +443,32 @@ mod tests { assert!(super::super::PublishNamespace::decode_body(&mut bytes, version, false).is_err()); } + #[test] + fn forward_follows_the_draft() { + for version in [Version::Draft14, Version::Draft15, Version::Draft16, Version::Draft17] { + for value in [0u64, 1, 2] { + let mut buf = BytesMut::new(); + RequestId(1).encode(&mut buf, version).unwrap(); + if version == Version::Draft17 { + 0u64.encode(&mut buf, version).unwrap(); + } + encode_namespace(&mut buf, &Path::default(), version).unwrap(); + if matches!(version, Version::Draft16 | Version::Draft17) { + 1u64.encode(&mut buf, version).unwrap(); + } + 1u64.encode(&mut buf, version).unwrap(); + 0x10u64.encode(&mut buf, version).unwrap(); + value.encode(&mut buf, version).unwrap(); + let mut buf = buf.freeze(); + let result = SubscribeNamespaceLegacy::decode_msg(&mut buf, version); + assert_eq!(result.is_ok(), version == Version::Draft14 || value <= 1, "{version} FORWARD={value}: {result:?}"); + if version == Version::Draft14 || value <= 1 { + assert!(buf.is_empty()); + } + } + } + } + #[test] fn message_ids() { // 0x11 through draft-17 (legacy), renumbered to 0x50 in draft-18 (#1542). From 17b756b657dfa2ade231c24f7937bc52f37b851a Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 10 Oct 2026 15:51:30 -0700 Subject: [PATCH 3/6] style(net): format legacy FORWARD regression Co-Authored-By: GPT-6 --- rs/moq-net/src/ietf/subscribe_namespace.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/rs/moq-net/src/ietf/subscribe_namespace.rs b/rs/moq-net/src/ietf/subscribe_namespace.rs index f241a36608..1084144d8f 100644 --- a/rs/moq-net/src/ietf/subscribe_namespace.rs +++ b/rs/moq-net/src/ietf/subscribe_namespace.rs @@ -461,7 +461,11 @@ mod tests { value.encode(&mut buf, version).unwrap(); let mut buf = buf.freeze(); let result = SubscribeNamespaceLegacy::decode_msg(&mut buf, version); - assert_eq!(result.is_ok(), version == Version::Draft14 || value <= 1, "{version} FORWARD={value}: {result:?}"); + assert_eq!( + result.is_ok(), + version == Version::Draft14 || value <= 1, + "{version} FORWARD={value}: {result:?}" + ); if version == Version::Draft14 || value <= 1 { assert!(buf.is_empty()); } From a59a4d1e029199719996bf8539e33f76afacf64b Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 10 Oct 2026 22:32:40 -0700 Subject: [PATCH 4/6] fix(net): recognize legacy request parameters before parsing Co-Authored-By: GPT-6 Astra --- js/net/src/ietf/ietf.test.ts | 51 +++++++++++++ js/net/src/ietf/parameters.ts | 38 +++++++++- js/net/src/ietf/subscribe_namespace.ts | 5 ++ rs/moq-net/src/ietf/fetch.rs | 27 +++---- rs/moq-net/src/ietf/parameters.rs | 7 +- rs/moq-net/src/ietf/publish.rs | 30 ++++---- rs/moq-net/src/ietf/subscribe.rs | 101 +++++++++++-------------- 7 files changed, 166 insertions(+), 93 deletions(-) diff --git a/js/net/src/ietf/ietf.test.ts b/js/net/src/ietf/ietf.test.ts index 034435fc82..1e8b28ddd1 100644 --- a/js/net/src/ietf/ietf.test.ts +++ b/js/net/src/ietf/ietf.test.ts @@ -1954,3 +1954,54 @@ for (const version of [Version.DRAFT_14, Version.DRAFT_15]) { } }); } + +test("Subscribe draft15 recognizes only its parameters", async () => { + for (const parameter of [ + [0x32, 7], + [0x04, 0], + [0x23, 1, 255], + [0x25, 1, 255], + [0x35, 1, 255], + ]) { + const body = framed([...TRACK_HEAD, 2, ...parameter, 0x20, 7]); + const decoded = await decodeVersioned(body, Subscribe.Subscribe.decode, Version.DRAFT_15); + expect(decoded.subscriberPriority).toBe(7); + expect(decoded.fill).toBeUndefined(); + expect(decoded.rangeFilters).toBe(false); + } + await expect( + decodeVersioned(framed([...TRACK_HEAD, 1, 0x21, 1, 255]), Subscribe.Subscribe.decode, Version.DRAFT_15), + ).rejects.toThrow(); +}); + +test("Publish draft15 ignores subscription-only parameters", async () => { + for (const parameter of [ + [0x02, 7], + [0x20, 0x41, 0], + [0x21, 1, 255], + ]) { + const body = framed([1, 1, 1, 116, 1, 118, 1, 2, ...parameter, 0x10, 0]); + const decoded = await decodeVersioned(body, Publish.decode, Version.DRAFT_15); + expect(decoded.forward).toBe(false); + } +}); + +for (const version of [Version.DRAFT_14, Version.DRAFT_15, Version.DRAFT_16, Version.DRAFT_17]) { + test(`SubscribeNamespace ${version}: FORWARD follows the draft`, async () => { + for (const value of [0, 1, 2]) { + const head = [ + 1, + ...(version === Version.DRAFT_17 ? [0] : []), + 0, + ...(version >= Version.DRAFT_16 ? [1] : []), + ]; + const decoded = decodeVersioned( + framed([...head, 1, 0x10, value]), + SubscribeNamespace.SubscribeNamespaceLegacy.decode, + version, + ); + if (version === Version.DRAFT_14 || value <= 1) await decoded; + else await expect(decoded).rejects.toThrow(); + } + }); +} diff --git a/js/net/src/ietf/parameters.ts b/js/net/src/ietf/parameters.ts index 1d316a048e..ec299e1079 100644 --- a/js/net/src/ietf/parameters.ts +++ b/js/net/src/ietf/parameters.ts @@ -237,6 +237,16 @@ const MSG_PARAM_REPEATABLE: readonly bigint[] = [ MSG_PARAM_TRACK_PROPERTY_FILTER, ]; +// These definitions are unknown KVPs in drafts 14 and 15. +const MSG_PARAM_AFTER_DRAFT15: readonly bigint[] = [ + MSG_PARAM_SUBGROUP_DELIVERY_TIMEOUT, + MSG_PARAM_FILL_TIMEOUT, + MSG_PARAM_FILL_PARAMETERS, + ...MSG_PARAM_RANGE_FILTERS, + MSG_PARAM_TRACK_PROPERTY_FILTER, + MSG_PARAM_INCLUDE_PROPERTIES, +]; + type MessageParamKind = "varint" | "uint8" | "bool" | "location" | "filter" | "bytes"; /** A `{Group, Object}` pair carried by a message parameter, such as LARGEST_OBJECT. */ export type MessageLocation = { groupId: bigint; objectId: bigint }; @@ -303,6 +313,7 @@ export class Parameters { #locations: Map; /** LOCATION_FILTER, kept decoded because its framing depends on the draft. */ #filter: Filter.Filter | undefined; + #filterBytes: { value: Uint8Array; version: IetfVersion } | undefined; /** Every instance of a parameter that may repeat, decoded only; we never send one. */ #repeated: Map; @@ -420,11 +431,15 @@ export class Parameters { /** LOCATION_FILTER: which Objects the subscription delivers. See `filter.ts`. */ get subscriptionFilter(): Filter.Filter | undefined { + if (this.#filterBytes && this.#filter === undefined) { + this.#filter = Filter.decode(this.#filterBytes.value, this.#filterBytes.version); + } return this.#filter; } set subscriptionFilter(v: Filter.Filter) { this.#filter = v; + this.#filterBytes = undefined; } /** FILL_PARAMETERS: the draft-20 backfill request, as its raw parameter value. */ @@ -471,7 +486,7 @@ export class Parameters { } async encode(w: Writer, version: IetfVersion) { - const filter = this.#filter; + const filter = this.subscriptionFilter; await w.u53(this.vars.size + this.bytes.size + this.#locations.size + (filter ? 1 : 0)); if (version === Version.DRAFT_14 || version === Version.DRAFT_15) { @@ -602,6 +617,17 @@ export class Parameters { prevType = id; } + // These definitions postdate the legacy drafts. Consume their KVP framing + // without interpreting a future value or exposing it to message validation. + if ( + (version === Version.DRAFT_14 || version === Version.DRAFT_15) && + MSG_PARAM_AFTER_DRAFT15.includes(id) + ) { + if (id % 2n === 0n) await r.u62(); + else await r.read(await r.u53()); + continue; + } + if (version === Version.DRAFT_14 || version === Version.DRAFT_15 || version === Version.DRAFT_16) { if (id % 2n === 0n) { if (params.vars.has(id)) { @@ -621,10 +647,16 @@ export class Parameters { params.#locations.set(id, decodeLocation(bytes)); } else if (id === MSG_PARAM_SUBSCRIPTION_FILTER && version !== Version.DRAFT_14) { // Draft-14 carries the filter in the SUBSCRIBE body, so 0x21 is undefined there. - if (params.#filter !== undefined) { + if (params.#filterBytes !== undefined || params.#filter !== undefined) { throw new Error(`duplicate message parameter id: ${id.toString()}`); } - params.#filter = Filter.decode(bytes, version); + // Legacy messages can carry a known parameter on the wrong message. + // Validate its semantics only when that message consumes the filter. + if (version === Version.DRAFT_15) { + params.#filterBytes = { value: bytes, version }; + } else { + params.#filter = Filter.decode(bytes, version); + } } else { if (params.bytes.has(id)) { throw new Error(`duplicate message parameter id: ${id.toString()}`); diff --git a/js/net/src/ietf/subscribe_namespace.ts b/js/net/src/ietf/subscribe_namespace.ts index a36090c910..f0b699c529 100644 --- a/js/net/src/ietf/subscribe_namespace.ts +++ b/js/net/src/ietf/subscribe_namespace.ts @@ -145,6 +145,11 @@ export class SubscribeNamespaceLegacy { subscribeOptions = await r.u53(); } const params = await Parameters.decode(r, version); + // FORWARD is defined here from draft-15. Its value is ignored, but must be a boolean. + const forward = params.vars.get(0x10n); + if (version !== Version.DRAFT_14 && forward !== undefined && forward > 1n) { + throw new Error(`invalid FORWARD parameter: ${forward}`); + } return new SubscribeNamespaceLegacy({ namespace, requestId, subscribeOptions, hidden: params.hidden }); } diff --git a/rs/moq-net/src/ietf/fetch.rs b/rs/moq-net/src/ietf/fetch.rs index 394857f4d8..af86f128bd 100644 --- a/rs/moq-net/src/ietf/fetch.rs +++ b/rs/moq-net/src/ietf/fetch.rs @@ -204,13 +204,13 @@ impl Message for Fetch<'_> { let fetch_type = FetchType::decode(buf, version)?; decode_params!(buf, version, 0x03 => _authorization_token: Vec, - 0x0A => fill_timeout: Option, + 0x0A => fill_timeout: Option where !matches!(version, Version::Draft15 | Version::Draft16 | Version::Draft17), 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, + 0x25 => subgroup_filter: Vec where has_range_filters(version), + 0x26 => object_id_filter: Vec where has_range_filters(version), + 0x27 => priority_filter: Vec where has_range_filters(version), + 0x28 => object_property_filter: Vec where has_range_filters(version), ); let range_filters = [ subgroup_filter, @@ -221,13 +221,6 @@ impl Message for Fetch<'_> { .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, @@ -242,14 +235,14 @@ impl Message for Fetch<'_> { let track = Cow::::decode(buf, version)?; decode_params!(buf, version, 0x03 => _authorization_token: Vec, - 0x0A => fill_timeout: Option, + 0x0A => fill_timeout: Option where !matches!(version, Version::Draft15 | Version::Draft16 | Version::Draft17), 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, + 0x25 => subgroup_filter: Vec where has_range_filters(version), + 0x26 => object_id_filter: Vec where has_range_filters(version), + 0x27 => priority_filter: Vec where has_range_filters(version), + 0x28 => object_property_filter: Vec where has_range_filters(version), 0x35 => _include_properties: Option, ); let range_filters = [ diff --git a/rs/moq-net/src/ietf/parameters.rs b/rs/moq-net/src/ietf/parameters.rs index dfcb33409f..faba9c9063 100644 --- a/rs/moq-net/src/ietf/parameters.rs +++ b/rs/moq-net/src/ietf/parameters.rs @@ -515,7 +515,8 @@ macro_rules! encode_params { /// optional parameters (defaults to `None` when absent) and bare types like `u8` /// for parameters where `T::default()` is an acceptable fallback. /// -/// Unknown parameters cause `DecodeError::InvalidValue`. +/// Unknown parameters are ignored in drafts 14/15 and otherwise cause `DecodeError::InvalidValue`. +/// A false `where` gate treats a declared key as unknown before decoding its value. /// Duplicate parameters cause `DecodeError::Duplicate`, unless the type allows a repeat /// (see [`Param::param_repeat`]), such as `Vec`. /// @@ -528,7 +529,7 @@ macro_rules! encode_params { /// let subscriber_priority = subscriber_priority.unwrap_or(128); /// ``` macro_rules! decode_params { - ($r:expr, $version:expr, $($key:expr => $name:ident: $ty:ty),* $(,)?) => { + ($r:expr, $version:expr, $($key:expr => $name:ident: $ty:ty $(where $gate:expr)?),* $(,)?) => { #[allow(unused)] const _: () = { let _keys: &[u64] = &[$($key),*]; @@ -574,7 +575,7 @@ macro_rules! decode_params { // An if-chain rather than a `match`, so a key can be a named constant: // the macro captures it as an expression, which is not a legal pattern. $( - if _key == $key { + if _key == $key $( && ($gate) )? { let _value = <$ty as $crate::ietf::Param>::param_decode($r, _version)?; $name = Some(match $name.take() { None => _value, diff --git a/rs/moq-net/src/ietf/publish.rs b/rs/moq-net/src/ietf/publish.rs index 379dbb7bd1..ec55793155 100644 --- a/rs/moq-net/src/ietf/publish.rs +++ b/rs/moq-net/src/ietf/publish.rs @@ -336,28 +336,17 @@ impl Message for Publish<'_> { // parameter fails the whole message, which would kill the session instead of // letting the request reach its NOT_SUPPORTED response. decode_params!(r, version, - 0x02 => object_delivery_timeout: Option, + 0x02 => _object_delivery_timeout: Option where version == Version::Draft15 || Filter::is_draft20(version), 0x03 => _authorization_token: Vec, - 0x06 => subgroup_delivery_timeout: Option, + 0x06 => _subgroup_delivery_timeout: Option where Filter::is_draft20(version), 0x08 => _expires: Option, 0x09 => largest_location: Option, 0x10 => forward: Option, - 0x20 => subscriber_priority: Option, - 0x21 => filter: Option, + 0x20 => _subscriber_priority: Option where Filter::is_draft20(version), + 0x21 => _filter: Option where Filter::is_draft20(version), 0x22 => group_order: Option, ); - // The values are dropped: we refuse the PUBLISH itself, so the subscription - // settings it proposes never take effect. They still have to be consumed. - let subscription_params = [ - object_delivery_timeout.is_some(), - subgroup_delivery_timeout.is_some(), - subscriber_priority.is_some(), - filter.is_some(), - ]; - if subscription_params.contains(&true) && !Filter::is_draft20(version) { - return Err(DecodeError::InvalidValue); - } let mut properties = Properties::decode(r, version)?; properties.group_order = properties.group_order.or(group_order); @@ -508,6 +497,17 @@ impl Message for PublishError<'_> { #[cfg(test)] mod tests { + #[test] + fn publish_draft15_ignores_subscription_only_parameters() { + for parameter in [&[0x02, 7][..], &[0x20, 0x41, 0], &[0x21, 1, 255]] { + let mut body = vec![1, 1, 1, b't', 1, b'v', 1, 2]; + body.extend_from_slice(parameter); + body.extend_from_slice(&[0x10, 0]); + let decoded: Publish = decode_message(&body, Version::Draft15).unwrap(); + assert!(!decoded.forward); + } + } + use super::*; /// Draft-20 moved the subscription parameters into PUBLISH. We refuse the request diff --git a/rs/moq-net/src/ietf/subscribe.rs b/rs/moq-net/src/ietf/subscribe.rs index f7c584cd90..7bf472ca96 100644 --- a/rs/moq-net/src/ietf/subscribe.rs +++ b/rs/moq-net/src/ietf/subscribe.rs @@ -37,11 +37,6 @@ pub struct Subscribe<'a> { pub range_filters: bool, } -/// NEW_GROUP_REQUEST (0x32) arrived in draft-16. -fn has_new_group_request(version: Version) -> bool { - !matches!(version, Version::Draft14 | Version::Draft15) -} - /// The Range Filters (0x25-0x29) arrived in draft-19. pub(super) fn has_range_filters(version: Version) -> bool { !matches!( @@ -91,19 +86,19 @@ impl Message for Subscribe<'_> { decode_params!(r, version, 0x02 => _object_delivery_timeout: Option, 0x03 => _authorization_token: Vec, - 0x04 => rendezvous_timeout: Option, - 0x06 => _subgroup_delivery_timeout: Option, + 0x04 => rendezvous_timeout: Option where !matches!(version, Version::Draft15 | Version::Draft16), + 0x06 => _subgroup_delivery_timeout: Option where !matches!(version, Version::Draft14 | Version::Draft15 | Version::Draft16 | Version::Draft17), 0x10 => forward: Option, 0x20 => subscriber_priority: Option, 0x21 => filter: Option, 0x22 => group_order: Option, - 0x23 => fill: Option, - 0x25 => subgroup_filter: Vec, - 0x26 => object_id_filter: Vec, - 0x27 => priority_filter: Vec, - 0x28 => object_property_filter: Vec, - 0x32 => new_group_request: Option, - 0x35 => include_properties: Option, + 0x23 => fill: Option where Filter::is_draft20(version), + 0x25 => subgroup_filter: Vec where has_range_filters(version), + 0x26 => object_id_filter: Vec where has_range_filters(version), + 0x27 => priority_filter: Vec where has_range_filters(version), + 0x28 => object_property_filter: Vec where has_range_filters(version), + 0x32 => _new_group_request: Option, + 0x35 => include_properties: Option where Filter::is_draft20(version), ); let range_filters = [ @@ -115,25 +110,9 @@ impl Message for Subscribe<'_> { .iter() .any(|filter| !filter.is_empty()); - // An unknown message parameter is a protocol violation, so each stays rejected - // on the drafts that predate it rather than being quietly tolerated. - if ((fill.is_some() || include_properties.is_some()) && !Filter::is_draft20(version)) - || (range_filters && !has_range_filters(version)) - || (new_group_request.is_some() && !has_new_group_request(version)) - { - return Err(DecodeError::InvalidValue); - } - // Defaults to 1, so an absent parameter means the subscriber wants them. let properties_wanted = include_properties.unwrap_or(true); - // RENDEZVOUS_TIMEOUT arrived in draft-17; 0x04 means MAX_CACHE_DURATION in - // draft-15, which is a publisher parameter with no business in a SUBSCRIBE. - // An unknown message parameter is a protocol violation, so reject it there. - if rendezvous_timeout.is_some() && matches!(version, Version::Draft15 | Version::Draft16) { - return Err(DecodeError::InvalidValue); - } - // The value is deliberately dropped: we always answer a SUBSCRIBE with what is // published right now, which is the shorter timeout the draft lets a relay pick. // We still have to parse it, or the parameter alone would kill the session. @@ -460,18 +439,13 @@ impl Message for SubscribeUpdate { decode_params!(r, version, 0x02 => _object_delivery_timeout: Option, 0x03 => _authorization_token: Vec, - 0x06 => _subgroup_delivery_timeout: Option, + 0x06 => _subgroup_delivery_timeout: Option where !matches!(version, Version::Draft14 | Version::Draft15 | Version::Draft16 | Version::Draft17), 0x10 => forward: Option, 0x20 => subscriber_priority: Option, 0x21 => _filter: Option, - 0x32 => new_group_request: Option, + 0x32 => _new_group_request: Option, ); - // NEW_GROUP_REQUEST arrived in draft-16. - if new_group_request.is_some() && !has_new_group_request(version) { - return Err(DecodeError::InvalidValue); - } - let subscriber_priority = subscriber_priority.unwrap_or(128); let forward = forward.unwrap_or(true); @@ -496,16 +470,16 @@ impl Message for SubscribeUpdate { decode_params!(r, version, 0x02 => _object_delivery_timeout: Option, 0x03 => _authorization_token: Vec, - 0x06 => _subgroup_delivery_timeout: Option, + 0x06 => _subgroup_delivery_timeout: Option where !matches!(version, Version::Draft14 | Version::Draft15 | Version::Draft16 | Version::Draft17), 0x10 => forward: Option, 0x20 => subscriber_priority: Option, 0x21 => _filter: Option, - 0x23 => fill: Option, - 0x25 => subgroup_filter: Vec, - 0x26 => object_id_filter: Vec, - 0x27 => priority_filter: Vec, - 0x28 => object_property_filter: Vec, - 0x29 => track_property_filter: Vec, + 0x23 => fill: Option where Filter::is_draft20(version), + 0x25 => subgroup_filter: Vec where has_range_filters(version), + 0x26 => object_id_filter: Vec where has_range_filters(version), + 0x27 => priority_filter: Vec where has_range_filters(version), + 0x28 => object_property_filter: Vec where has_range_filters(version), + 0x29 => track_property_filter: Vec where has_range_filters(version), 0x32 => _new_group_request: Option, ); @@ -640,14 +614,11 @@ mod tests { } } - /// 0x04 only means RENDEZVOUS_TIMEOUT from draft-17 on; in draft-15 it is - /// MAX_CACHE_DURATION, a publisher parameter that has no business in a SUBSCRIBE. + /// Draft-16 has no parameter 0x04; draft-15 ignores it on SUBSCRIBE. #[test] - fn rendezvous_timeout_is_rejected_before_draft17() { - for version in [Version::Draft15, Version::Draft16] { - let encoded = subscribe_with_rendezvous(0, version); - decode_message::(&encoded, version).expect_err(&format!("{version} must reject parameter 0x04")); - } + fn rendezvous_timeout_is_rejected_in_draft16() { + let encoded = subscribe_with_rendezvous(0, Version::Draft16); + decode_message::(&encoded, Version::Draft16).expect_err("draft-16 rejects parameter 0x04"); } /// The first message parameter key on an encoded SUBSCRIBE, or `None` when it carries no @@ -1441,13 +1412,33 @@ mod tests { } } - /// A parameter from a later draft is still an unknown parameter, which the draft makes - /// a protocol violation. + /// Draft-15 accepts NEW_GROUP_REQUEST and skips future definitions before parsing them. + #[test] + fn subscribe_draft15_recognizes_only_its_parameters() { + for parameter in [ + &[0x32, 7][..], + &[0x04, 0], + &[0x23, 1, 255], + &[0x25, 1, 255], + &[0x35, 1, 255], + ] { + let mut params = vec![2]; + params.extend_from_slice(parameter); + params.extend_from_slice(&[0x20, 7]); + let decoded: Subscribe = decode_message(&subscribe_body(¶ms), Version::Draft15).unwrap(); + assert_eq!(decoded.subscriber_priority, 7); + assert!(decoded.fill.is_none()); + assert!(!decoded.range_filters); + } + let body = [2, 1, 1, 0x32, 7]; + decode_message::(&body, Version::Draft15).unwrap(); + } + #[test] fn subscribe_refuses_parameters_before_their_draft() { for (version, params) in [ - // NEW_GROUP_REQUEST arrived in draft-16. - (Version::Draft15, &[0x01, 0x32, 0x00][..]), + // SUBGROUP_DELIVERY_TIMEOUT arrived in draft-18. + (Version::Draft16, &[0x01, 0x06, 0x00][..]), // The Range Filters arrived in draft-19. (Version::Draft18, &[0x01, 0x25, 0x00][..]), // INCLUDE_PROPERTIES arrived in draft-20. From e00346446026ba3e838b41dd29e29e7e06535fcf Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sun, 11 Oct 2026 09:44:30 -0700 Subject: [PATCH 5/6] fix(net): ignore repeated unrecognized legacy request parameters in JS Drafts 14 and 15 have a receiver ignore unrecognized request parameters, repeats included. Rust already skips them; JS still stored any unrecognized id and rejected a repeat as a duplicate. Skip ids JS does not define on the legacy drafts, and cover repeated unknown ids in both languages. Co-Authored-By: Claude Opus 5.5 --- js/net/src/ietf/ietf.test.ts | 12 ++++++++++++ js/net/src/ietf/parameters.ts | 19 +++++++++++++------ rs/moq-net/src/ietf/parameters.rs | 8 +++++++- 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/js/net/src/ietf/ietf.test.ts b/js/net/src/ietf/ietf.test.ts index 8324643007..18648eac66 100644 --- a/js/net/src/ietf/ietf.test.ts +++ b/js/net/src/ietf/ietf.test.ts @@ -1953,6 +1953,18 @@ for (const version of [Version.DRAFT_14, Version.DRAFT_15]) { expect(params.subscriberPriority).toBe(7); } }); + + test(`Parameters ${version}: only a recognized key must be unique`, async () => { + for (const bytes of [ + new Uint8Array([3, 0x3e, 1, 0x3e, 2, 0x20, 7]), + new Uint8Array([3, 0x3d, 1, 42, 0x3d, 1, 43, 0x20, 7]), + ]) { + const params = await Parameters.decode(new Reader(undefined, bytes, version), version); + expect(params.subscriberPriority).toBe(7); + } + const known = new Uint8Array([2, 0x20, 7, 0x20, 8]); + await expect(Parameters.decode(new Reader(undefined, known, version), version)).rejects.toThrow(/duplicate/); + }); } test("Subscribe draft15 recognizes only its parameters", async () => { diff --git a/js/net/src/ietf/parameters.ts b/js/net/src/ietf/parameters.ts index 47899568d4..b20a221b8d 100644 --- a/js/net/src/ietf/parameters.ts +++ b/js/net/src/ietf/parameters.ts @@ -269,7 +269,8 @@ type MessageParamKind = "varint" | "uint8" | "bool" | "location" | "filter" | "b /** A `{Group, Object}` pair carried by a message parameter, such as LARGEST_OBJECT. */ export type MessageLocation = { groupId: bigint; objectId: bigint }; -function getMessageParamKind(id: bigint): MessageParamKind { +/** The kind of a message parameter we define, or undefined for an unrecognized id. */ +function messageParamKind(id: bigint): MessageParamKind | undefined { switch (id) { case MSG_PARAM_DELIVERY_TIMEOUT: case MSG_PARAM_FILL_TIMEOUT: @@ -296,11 +297,16 @@ function getMessageParamKind(id: bigint): MessageParamKind { case MSG_PARAM_HOP_PATH: return "bytes"; default: - if (MSG_PARAM_REPEATABLE.includes(id)) return "bytes"; - throw new Error(`unknown message parameter id: ${id.toString()}`); + return MSG_PARAM_REPEATABLE.includes(id) ? "bytes" : undefined; } } +function getMessageParamKind(id: bigint): MessageParamKind { + const kind = messageParamKind(id); + if (kind === undefined) throw new Error(`unknown message parameter id: ${id.toString()}`); + return kind; +} + /** * The draft-16 and earlier form: draft-16 section 9.2 serializes every Message Parameter as * a Key-Value-Pair, and section 9.2.2.7 calls LARGEST_OBJECT "a length-prefixed Location @@ -640,11 +646,12 @@ export class Parameters { prevType = id; } - // These definitions postdate the legacy drafts. Consume their KVP framing - // without interpreting a future value or exposing it to message validation. + // Drafts 14 and 15 have a receiver ignore unrecognized parameters, repeats included, + // and the later definitions are unrecognized there. Consume their KVP framing + // without interpreting a value or exposing it to message validation. if ( (version === Version.DRAFT_14 || version === Version.DRAFT_15) && - MSG_PARAM_AFTER_DRAFT15.includes(id) + (MSG_PARAM_AFTER_DRAFT15.includes(id) || messageParamKind(id) === undefined) ) { if (id % 2n === 0n) await r.u62(); else await r.read(await r.u53()); diff --git a/rs/moq-net/src/ietf/parameters.rs b/rs/moq-net/src/ietf/parameters.rs index 26eb85affe..b51ff74962 100644 --- a/rs/moq-net/src/ietf/parameters.rs +++ b/rs/moq-net/src/ietf/parameters.rs @@ -1140,7 +1140,13 @@ mod tests { #[test] fn legacy_unknown_parameter_is_ignored() { for version in [Version::Draft14, Version::Draft15] { - for data in [&[2, 0x3e, 1, 0x20, 7][..], &[2, 0x3d, 1, 42, 0x20, 7][..]] { + for data in [ + &[2, 0x3e, 1, 0x20, 7][..], + &[2, 0x3d, 1, 42, 0x20, 7][..], + // Unknown ids may repeat. + &[3, 0x3e, 1, 0x3e, 2, 0x20, 7][..], + &[3, 0x3d, 1, 42, 0x3d, 1, 43, 0x20, 7][..], + ] { let mut bytes = data; let result: Result, DecodeError> = (|| { decode_params!(&mut bytes, version, 0x20 => priority: Option); From 78072648ee62a8a09e534b5fd45c976bfb1d28ff Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sun, 11 Oct 2026 10:46:52 -0700 Subject: [PATCH 6/6] fix(net): recognize legacy request parameters by their draft Draft 14 defines only DELIVERY TIMEOUT, AUTHORIZATION TOKEN and MAX CACHE DURATION, so JS now treats every other id as unrecognized there, letting it repeat. The cluster extension's HOP_PATH and ROUTE_COST exist only where it can be negotiated, so both languages ignore them on drafts 14 and 15 instead of refusing a legacy PUBLISH_NAMESPACE that carries them. Co-Authored-By: Claude Opus 5.5 --- js/net/src/ietf/ietf.test.ts | 35 ++++++++++++++---------- js/net/src/ietf/parameters.ts | 33 +++++++++++++++------- rs/moq-net/src/ietf/publish_namespace.rs | 24 ++++++++++++++-- 3 files changed, 66 insertions(+), 26 deletions(-) diff --git a/js/net/src/ietf/ietf.test.ts b/js/net/src/ietf/ietf.test.ts index 18648eac66..0eb6d5c70b 100644 --- a/js/net/src/ietf/ietf.test.ts +++ b/js/net/src/ietf/ietf.test.ts @@ -242,14 +242,8 @@ test("Message Parameters: Location preserves bigint precision", async () => { expect(params.largest).toEqual(largest); - for (const version of [ - Version.DRAFT_14, - Version.DRAFT_15, - Version.DRAFT_16, - Version.DRAFT_17, - Version.DRAFT_18, - Version.DRAFT_19, - ]) { + // Draft 14 carries Largest in the SUBSCRIBE_OK body, so it defines no LARGEST_OBJECT parameter. + for (const version of [Version.DRAFT_15, Version.DRAFT_16, Version.DRAFT_17, Version.DRAFT_18, Version.DRAFT_19]) { const encoded = await encodeVersioned(params, version); const decoded = await decodeVersioned(encoded, Parameters.decode, version); expect(decoded.largest).toEqual(largest); @@ -1947,26 +1941,39 @@ test("Subscribe v16: rejects INCLUDE_PROPERTIES", async () => { }); for (const version of [Version.DRAFT_14, Version.DRAFT_15]) { + // DELIVERY_TIMEOUT (0x02) is the known key: both drafts define it. test(`Parameters ${version}: unknown key does not hide a later known value`, async () => { - for (const bytes of [new Uint8Array([2, 0x3e, 1, 0x20, 7]), new Uint8Array([2, 0x3d, 1, 42, 0x20, 7])]) { + for (const bytes of [new Uint8Array([2, 0x3e, 1, 0x02, 7]), new Uint8Array([2, 0x3d, 1, 42, 0x02, 7])]) { const params = await Parameters.decode(new Reader(undefined, bytes, version), version); - expect(params.subscriberPriority).toBe(7); + expect(params.deliveryTimeout).toBe(7n); } }); test(`Parameters ${version}: only a recognized key must be unique`, async () => { for (const bytes of [ - new Uint8Array([3, 0x3e, 1, 0x3e, 2, 0x20, 7]), - new Uint8Array([3, 0x3d, 1, 42, 0x3d, 1, 43, 0x20, 7]), + new Uint8Array([3, 0x3e, 1, 0x3e, 2, 0x02, 7]), + new Uint8Array([3, 0x3d, 1, 42, 0x3d, 1, 43, 0x02, 7]), + // ROUTE_COST (0x40b58): the cluster extension is negotiated only from draft 17. + new Uint8Array([3, 0x80, 0x04, 0x0b, 0x58, 5, 0x80, 0x04, 0x0b, 0x58, 6, 0x02, 7]), ]) { const params = await Parameters.decode(new Reader(undefined, bytes, version), version); - expect(params.subscriberPriority).toBe(7); + expect(params.deliveryTimeout).toBe(7n); } - const known = new Uint8Array([2, 0x20, 7, 0x20, 8]); + const known = new Uint8Array([2, 0x02, 7, 0x02, 8]); await expect(Parameters.decode(new Reader(undefined, known, version), version)).rejects.toThrow(/duplicate/); }); } +// Draft 14 defines only DELIVERY_TIMEOUT, AUTHORIZATION TOKEN and MAX CACHE DURATION, so +// later definitions such as SUBSCRIBER_PRIORITY and LOCATION_FILTER are unrecognized and may repeat. +test("Parameters draft14: later definitions are unrecognized", async () => { + for (const bytes of [new Uint8Array([2, 0x20, 7, 0x20, 8]), new Uint8Array([2, 0x21, 0, 0x21, 0])]) { + const params = await Parameters.decode(new Reader(undefined, bytes, Version.DRAFT_14), Version.DRAFT_14); + expect(params.subscriberPriority).toBeUndefined(); + expect(params.subscriptionFilter).toBeUndefined(); + } +}); + test("Subscribe draft15 recognizes only its parameters", async () => { for (const parameter of [ [0x32, 7], diff --git a/js/net/src/ietf/parameters.ts b/js/net/src/ietf/parameters.ts index b20a221b8d..abf91a6ea6 100644 --- a/js/net/src/ietf/parameters.ts +++ b/js/net/src/ietf/parameters.ts @@ -255,7 +255,17 @@ const MSG_PARAM_REPEATABLE: readonly bigint[] = [ MSG_PARAM_TRACK_PROPERTY_FILTER, ]; -// These definitions are unknown KVPs in drafts 14 and 15. +/// The message parameters draft 14 defines, plus HIDDEN, whose extension any draft can +/// negotiate. Every other id is unrecognized there. +const MSG_PARAM_DRAFT14: readonly bigint[] = [ + MSG_PARAM_DELIVERY_TIMEOUT, + MSG_PARAM_AUTHORIZATION_TOKEN, + MSG_PARAM_MAX_CACHE_DURATION, + MSG_PARAM_HIDDEN, +]; + +// These definitions, including the cluster extension negotiated only from draft 17, are +// unknown KVPs in draft 15. const MSG_PARAM_AFTER_DRAFT15: readonly bigint[] = [ MSG_PARAM_SUBGROUP_DELIVERY_TIMEOUT, MSG_PARAM_FILL_TIMEOUT, @@ -263,6 +273,8 @@ const MSG_PARAM_AFTER_DRAFT15: readonly bigint[] = [ ...MSG_PARAM_RANGE_FILTERS, MSG_PARAM_TRACK_PROPERTY_FILTER, MSG_PARAM_INCLUDE_PROPERTIES, + MSG_PARAM_HOP_PATH, + MSG_PARAM_ROUTE_COST, ]; type MessageParamKind = "varint" | "uint8" | "bool" | "location" | "filter" | "bytes"; @@ -646,13 +658,15 @@ export class Parameters { prevType = id; } - // Drafts 14 and 15 have a receiver ignore unrecognized parameters, repeats included, - // and the later definitions are unrecognized there. Consume their KVP framing - // without interpreting a value or exposing it to message validation. - if ( - (version === Version.DRAFT_14 || version === Version.DRAFT_15) && - (MSG_PARAM_AFTER_DRAFT15.includes(id) || messageParamKind(id) === undefined) - ) { + // Drafts 14 and 15 have a receiver ignore unrecognized parameters, repeats included. + // Consume their KVP framing without interpreting a value or exposing it to message + // validation. + const unrecognized = + version === Version.DRAFT_14 + ? !MSG_PARAM_DRAFT14.includes(id) + : version === Version.DRAFT_15 && + (MSG_PARAM_AFTER_DRAFT15.includes(id) || messageParamKind(id) === undefined); + if (unrecognized) { if (id % 2n === 0n) await r.u62(); else await r.read(await r.u53()); continue; @@ -675,8 +689,7 @@ export class Parameters { throw new Error(`duplicate message parameter id: ${id.toString()}`); } params.#locations.set(id, decodeLocation(bytes)); - } else if (id === MSG_PARAM_SUBSCRIPTION_FILTER && version !== Version.DRAFT_14) { - // Draft-14 carries the filter in the SUBSCRIBE body, so 0x21 is undefined there. + } else if (id === MSG_PARAM_SUBSCRIPTION_FILTER) { if (params.#filterBytes !== undefined || params.#filter !== undefined) { throw new Error(`duplicate message parameter id: ${id.toString()}`); } diff --git a/rs/moq-net/src/ietf/publish_namespace.rs b/rs/moq-net/src/ietf/publish_namespace.rs index ae090688bd..a305380bc5 100644 --- a/rs/moq-net/src/ietf/publish_namespace.rs +++ b/rs/moq-net/src/ietf/publish_namespace.rs @@ -38,10 +38,11 @@ impl PublishNamespace<'_> { let track_namespace = decode_namespace(r, version)?; // The token is ignored: the session's grant is what authorizes the request. + // The cluster extension defines its parameters only where it can be negotiated. decode_params!(r, version, 0x03 => _authorization_token: Vec, - cluster::HOP_PATH => hops: Option, - cluster::ROUTE_COST => cost: Option, + cluster::HOP_PATH => hops: Option where cluster::supported(version), + cluster::ROUTE_COST => cost: Option where cluster::supported(version), ); let cluster = match negotiated { @@ -572,6 +573,25 @@ mod tests { assert!(decode_message::(&[0x02, 0x00], Version::Draft16).is_err()); } + /// Drafts 14 and 15 predate the cluster extension, so its parameters are unrecognized + /// there and ignored, repeats included. + #[test] + fn legacy_ignores_cluster_parameters() { + for version in [Version::Draft14, Version::Draft15] { + let msg = PublishNamespace { + request_id: RequestId(1), + track_namespace: Path::new("test"), + cluster: None, + }; + let mut encoded = encode_message(&msg, version); + // Replace the empty parameter block with two ROUTE_COST (0x40B58) parameters. + assert_eq!(encoded.pop(), Some(0)); + encoded.extend_from_slice(&[2, 0x80, 0x04, 0x0B, 0x58, 5, 0x80, 0x04, 0x0B, 0x58, 6]); + let decoded: PublishNamespace = decode_message(&encoded, version).unwrap(); + assert!(decoded.cluster.is_none(), "{version}"); + } + } + #[test] fn test_announce_rejects_parameters() { #[rustfmt::skip]