diff --git a/drafts/draft-lcurley-moq-lite.md b/drafts/draft-lcurley-moq-lite.md index 09ef864b9fc..49a2714db34 100644 --- a/drafts/draft-lcurley-moq-lite.md +++ b/drafts/draft-lcurley-moq-lite.md @@ -442,7 +442,7 @@ A second ANNOUNCE_START for an already-advertised route is a protocol violation; The subscriber MUST close the session with a PROTOCOL_VIOLATION if it receives an ANNOUNCE_END, ANNOUNCE_UPDATE, or ANNOUNCE_RESTART referencing an Announce ID that was never assigned or already retired, an ANNOUNCE_START for a route that is already advertised, or any announcement before ANNOUNCE_OK. When the stream is closed, the subscriber MUST assume that all routes are now unavailable. -A broadcast path has at most 32 segments and at most 16,384 bytes; a receiver MUST close the session with a PROTOCOL_VIOLATION on a longer one, including one resolved by [Compression](#announce-compression). +A broadcast path has at most 32 segments and at most 4,096 bytes; a receiver MUST close the session with a PROTOCOL_VIOLATION on a longer one, including one resolved by [Compression](#announce-compression). A route covers a path when its prefix is a leading run of the path's segments; matching is per path segment, so a prefix never matches half a segment, and equality is byte-by-byte within each segment. A publisher answering a request stream presents each of its routes clamped to the intersection with the requested prefix: a route above the request's prefix appears as the request prefix itself (an empty suffix), which is exactly the covered set the subscriber may see. There MAY be multiple Announce Streams, potentially containing overlapping prefixes, that get their own ANNOUNCE_OK + announcements. @@ -1826,7 +1826,7 @@ The `Message Length` describes the payload size on the wire. ## moq-lite-07 -- Limited a broadcast path to 32 segments and 16,384 bytes. +- Limited a broadcast path to 32 segments and 4,096 bytes. - A session without a negotiated subprotocol negotiates the version in a draft-14 CLIENT_SETUP and SERVER_SETUP exchange. Implementations apply this to moq-lite-03 through moq-lite-06 too. diff --git a/js/net/MIGRATE.md b/js/net/MIGRATE.md index e43e6f21b6a..ebee10186ff 100644 --- a/js/net/MIGRATE.md +++ b/js/net/MIGRATE.md @@ -4,6 +4,11 @@ How to update your code for each breaking change, newest first. ## Unreleased +### Paths fit moq-transport's 4096-byte namespace ([#6927](https://github.com/moq-dev/moq/pull/6927)) + +`Path.MAX_BYTES` is 4096, moq-transport's namespace limit, so every path fits every wire and an origin refuses a longer one. Keep broadcast paths within 4096 bytes. +A moq-transport namespace whose escaped path would exceed it is refused when decoded, and a SUBSCRIBE, TRACK\_STATUS, or PUBLISH whose namespace and track name exceed 4096 bytes together fails that request instead of being sent. + ### `Path.from` refuses empty segments ([#6648](https://github.com/moq-dev/moq/pull/6648)) `Path.from` throws on a component with a leading, trailing, or doubled `/` instead of collapsing it, so a path has one spelling, and `Path.decode` refuses one off the wire. A moq-transport namespace field ending in `/`, which escapes to an empty segment, is refused too. diff --git a/js/net/api/index.api.md b/js/net/api/index.api.md index 341dd9c39dc..48a1ebaa242 100644 --- a/js/net/api/index.api.md +++ b/js/net/api/index.api.md @@ -427,7 +427,7 @@ function isWebTransportSupported(): boolean; function join(path: Valid, other: Valid): Valid; -const MAX_BYTES = 16384; +const MAX_BYTES = 4096; const MAX_GROUP_CACHE_BYTES: number; diff --git a/js/net/src/ietf/namespace.test.ts b/js/net/src/ietf/namespace.test.ts index fbee61d46ee..12eab9058d8 100644 --- a/js/net/src/ietf/namespace.test.ts +++ b/js/net/src/ietf/namespace.test.ts @@ -1,5 +1,8 @@ import { expect, test } from "bun:test"; -import { fromTuple, reserved } from "./namespace.ts"; +import { ProtocolViolation } from "../error.ts"; +import * as Path from "../path.ts"; +import { Reader, Writer } from "../stream.ts"; +import { decode, decodeTrack, encode, encodeTrack, fromTuple, reserved } from "./namespace.ts"; import { Version } from "./version.ts"; test("reserved namespaces preserve tuple fields and draft boundaries", () => { @@ -17,3 +20,43 @@ test("reserved namespaces preserve tuple fields and draft boundaries", () => { expect(reserved(path, Version.DRAFT_18)).toBe(false); } }); + +test("separators and escapes count toward a path but not its tuple fields", async () => { + const segments = Array(Path.MAX_PARTS - 1) + .fill("a") + .join("/"); + const deepest = Path.from(`${segments}/${"é".repeat(Path.MAX_BYTES / 2 - 31)}`); + expect(await roundTrip((w) => encode(w, deepest), decode)).toBe(deepest); + + expect(fromTuple([`${"/".repeat(Path.MAX_BYTES / 2 - 1)}a`]).length).toBe(Path.MAX_BYTES - 1); + expect(() => fromTuple([`${"/".repeat(Path.MAX_BYTES / 2)}a`])).toThrow(ProtocolViolation); +}); + +test("a full track name counts field bytes and is refused before writing", async () => { + // 4095 path bytes holding 2048 field bytes. + const namespace = Path.from(`${"\\/".repeat(Path.MAX_BYTES / 2 - 1)}a`); + const name = "x".repeat(2048); + expect(await roundTrip((w) => encodeTrack(w, namespace, name), decodeTrack)).toEqual({ namespace, name }); + await expect(roundTrip((w) => encodeTrack(w, namespace, `${name}x`), decodeTrack)).rejects.toThrow( + "full track name exceeds", + ); +}); + +/** Write with `write`, then read the bytes back with `read`. */ +async function roundTrip(write: (w: Writer) => Promise, read: (r: Reader) => Promise): Promise { + const bytes: number[] = []; + const writer = new Writer( + new WritableStream({ + write: (chunk) => { + bytes.push(...chunk); + }, + }), + Version.DRAFT_22, + ); + try { + await write(writer); + } finally { + await writer.close(); + } + return read(new Reader(undefined, Uint8Array.from(bytes), Version.DRAFT_22)); +} diff --git a/js/net/src/ietf/namespace.ts b/js/net/src/ietf/namespace.ts index 4a5d73906ee..8ef9d6c07a1 100644 --- a/js/net/src/ietf/namespace.ts +++ b/js/net/src/ietf/namespace.ts @@ -25,10 +25,12 @@ function toTuple(namespace: Path.Valid): string[] { /** * Convert IETF namespace tuple fields into the escaped moq-net path representation. * - * Throws when a field ends in `/`, which escapes to a path with an empty segment. + * Throws when a field ends in `/`, which escapes to a path with an empty segment, or when the + * separators and escapes push the path past {@link Path.MAX_BYTES}. */ export function fromTuple(parts: string[]): Path.Valid { const escaped = parts.map((part) => part.replaceAll("\\", "\\\\").replaceAll("/", "\\/")).join("/"); + if (tooLong(escaped)) throw new ProtocolViolation(`namespace exceeds ${Path.MAX_BYTES} bytes as a path`); try { return Path.from(escaped); } catch (cause) { @@ -38,21 +40,42 @@ export function fromTuple(parts: string[]): Path.Valid { /** Encode a moq-net namespace as an IETF namespace tuple. */ export async function encode(w: Writer, namespace: Path.Valid): Promise { - const parts = toTuple(namespace); + await writeFields(w, fields(namespace)); +} - // The IETF draft limits namespaces to 32 parts. - if (parts.length > Path.MAX_PARTS) { - throw new Error(`namespace exceeds ${Path.MAX_PARTS} parts`); - } +/** Encode a full track name: a namespace tuple then a track name, bounded together. */ +export async function encodeTrack(w: Writer, namespace: Path.Valid, name: string): Promise { + const parts = fields(namespace); + let size = utf8.encode(name).length; + for (const part of parts) size += utf8.encode(part).length; + if (size > MAX_BYTES) throw new Error(`full track name exceeds ${MAX_BYTES} bytes`); + await writeFields(w, parts); + await w.string(name); +} + +function fields(namespace: Path.Valid): string[] { + // The fields drop the path's separators and escapes, so a path within Path.MAX_BYTES fits. + if (tooLong(namespace)) throw new Error(`namespace exceeds ${Path.MAX_BYTES} bytes`); + const parts = toTuple(namespace); + if (parts.length > Path.MAX_PARTS) throw new Error(`namespace exceeds ${Path.MAX_PARTS} parts`); + return parts; +} +async function writeFields(w: Writer, parts: string[]): Promise { await w.u53(parts.length); for (const part of parts) { await w.string(part); } } +// A UTF-16 unit is at most 3 UTF-8 bytes, so only a long path needs encoding to measure. +function tooLong(path: string): boolean { + return path.length > Path.MAX_BYTES / 3 && utf8.encode(path).length > Path.MAX_BYTES; +} + // Every draft caps a namespace, and a namespace plus track name, at 4096 bytes. const MAX_BYTES = 4096; +const utf8 = new TextEncoder(); /** Decode an IETF namespace tuple into an escaped moq-net namespace. */ export async function decode(r: Reader): Promise { diff --git a/js/net/src/ietf/publish.ts b/js/net/src/ietf/publish.ts index 1d8ca5f77ba..a89a33968e1 100644 --- a/js/net/src/ietf/publish.ts +++ b/js/net/src/ietf/publish.ts @@ -61,8 +61,7 @@ export class Publish { async #encode(w: Writer, version: IetfVersion): Promise { await w.u62(this.requestId); - await Namespace.encode(w, this.trackNamespace); - await w.string(this.trackName); + await Namespace.encodeTrack(w, this.trackNamespace, this.trackName); await w.u62(this.trackAlias); if (this.contentExists !== !!this.largest) { diff --git a/js/net/src/ietf/subscribe.ts b/js/net/src/ietf/subscribe.ts index d6cd89489fa..1668402d816 100644 --- a/js/net/src/ietf/subscribe.ts +++ b/js/net/src/ietf/subscribe.ts @@ -139,8 +139,7 @@ export class Subscribe { async #encode(w: Writer, version: IetfVersion): Promise { await w.u62(this.requestId); - await Namespace.encode(w, this.trackNamespace); - await w.string(this.trackName); + await Namespace.encodeTrack(w, this.trackNamespace, this.trackName); const params = new Parameters(); params.subscriberPriority = this.subscriberPriority; diff --git a/js/net/src/ietf/track.ts b/js/net/src/ietf/track.ts index c06ab1883ab..938c6cf9cdb 100644 --- a/js/net/src/ietf/track.ts +++ b/js/net/src/ietf/track.ts @@ -43,8 +43,7 @@ export class TrackStatusRequest { async #encode(w: Writer, version: IetfVersion): Promise { await w.u62(this.requestId); - await Namespace.encode(w, this.trackNamespace); - await w.string(this.trackName); + await Namespace.encodeTrack(w, this.trackNamespace, this.trackName); // Only the opt-out is worth bytes, and a peer before draft-22 would read // INCLUDE_PROPERTIES as unknown, which is a protocol violation. diff --git a/js/net/src/path.ts b/js/net/src/path.ts index f1002dfa127..5cd36815cb6 100644 --- a/js/net/src/path.ts +++ b/js/net/src/path.ts @@ -54,8 +54,13 @@ export type Relative = string & { __brand: "Relative" }; */ export const MAX_PARTS = 32; -/** Maximum UTF-8 length of a path in bytes, leaving room in a 64 KiB moq-lite message for its other fields. */ -export const MAX_BYTES = 16384; +/** + * Maximum UTF-8 length of a path in bytes. + * + * Matches the IETF moq-transport limit of 4096 bytes for a namespace. Its tuple fields + * drop the path's separators and escapes, so every path fits both wires. + */ +export const MAX_BYTES = 4096; const utf8 = new TextEncoder(); diff --git a/quest/m1/cut-audit/README.md b/quest/m1/cut-audit/README.md index 0542968a292..216371d071d 100644 --- a/quest/m1/cut-audit/README.md +++ b/quest/m1/cut-audit/README.md @@ -56,7 +56,6 @@ leaving JS-only redesigns of `js/net` to [rs2ts](/quest/m2/rs2ts/README.md). - [Counted field-less `trun` entries](/quest/m1/cut-audit/fmp4-sample-resolver.md) - a moof of field-less runs cannot materialize millions of sample entries - [A compressed announce path never outgrows its literal form](/quest/m1/cut-audit/announce-path-bytes.md) - lite-07 announce compression cannot build multi-MiB paths -- [A route too large to re-encode is skipped](/quest/m1/cut-audit/announce-reencode.md) - one oversized route never resets a downstream announce stream - [A reserved group survives a route returning](/quest/m1/cut-audit/reserved-route-return.md) - a route flip A to B to A no longer panics a peeked reader - [JS tracks end clean only on a deliberate finish](/quest/m1/cut-audit/js-close-cause.md) - local close, violations, and unanswered requests end with errors - [Withdrawing a moq-transport FETCH or TRACK_STATUS ends it](/quest/m1/cut-audit/ietf-withdraw-resolve.md) - withdrawal during resolution releases the request diff --git a/quest/m1/cut-audit/announce-path-bytes.md b/quest/m1/cut-audit/announce-path-bytes.md index ef8a86615ad..e917d253947 100644 --- a/quest/m1/cut-audit/announce-path-bytes.md +++ b/quest/m1/cut-audit/announce-path-bytes.md @@ -37,7 +37,3 @@ parts-only check. unpublished, so this changes no published version). Public API: none. Wire: one receiver rule in the lite-07 draft. - -## Related - -- [A route too large to re-encode is skipped](/quest/m1/cut-audit/announce-reencode.md) - the same size bound on the send side diff --git a/quest/m1/cut-audit/announce-reencode.md b/quest/m1/cut-audit/announce-reencode.md deleted file mode 100644 index 3fc30eb0720..00000000000 --- a/quest/m1/cut-audit/announce-reencode.md +++ /dev/null @@ -1,37 +0,0 @@ -# [S] A route too large to re-encode is skipped, not fatal to the stream - -## Goal - -When a relay cannot re-encode one route for a downstream announce stream -(the downstream's shorter prefix or an extra hop pushes the message past the -encode cap), that downstream simply does not see that route. Its other -routes keep flowing, and asking again does not fail the same way. The same -holds for moq-transport's namespace advertisement. - -## Plan - -Found by the 2026-10-24 pre-cut audit at 151a529670. Reproduced: announcing -`"x/" + "a"*65530` on the lite test harness ended the downstream announce -task. New on main: [moq-dev/moq#4820](https://github.com/moq-dev/moq/pull/4820) -lowered the message cap from 64 MiB to 64 KiB. - -`AnnounceRun::start` (`rs/moq-net/src/lite/publisher.rs:1194`) calls -`stream.writer.buffer(..)?`; `EncodeError::TooLarge` -(`lite/announce.rs:297`) propagates to `AnnounceServe::poll` (about line -906), which aborts the whole stream. Cluster peers and `moq ls` ask for the -root prefix, so one hostile publisher withdraws every route a relay sends -them. On lite-06+ the encoder has already spent an announce id. - -- `outgoing()` returns `None` for a route this wire cannot carry, like a - reflected or overlong hop chain. Encode before assigning the id. -- Never abort an announce stream for one route. -- Check the IETF publisher's namespace encoding against its 4096-byte limit - (`ietf/namespace.rs:66`) for the same failure, and fix it the same way. -- A test announces a near-cap suffix under a short downstream prefix and - asserts the sibling routes still arrive. - -Public API: none. Wire: none. - -## Related - -- [A compressed announce path never outgrows its literal form](/quest/m1/cut-audit/announce-path-bytes.md) - the receive-side bound diff --git a/rs/moq-net/MIGRATE.md b/rs/moq-net/MIGRATE.md index f5319291da5..b110770ddcc 100644 --- a/rs/moq-net/MIGRATE.md +++ b/rs/moq-net/MIGRATE.md @@ -4,6 +4,11 @@ How to update your code for each breaking change, newest first. ## Unreleased +### Paths fit moq-transport's 4096-byte namespace ([#6927](https://github.com/moq-dev/moq/pull/6927)) + +`Path::MAX_BYTES` is 4096, moq-transport's namespace limit, so every path fits every wire and building a longer one fails with `path::Invalid::TooLong`. Keep broadcast paths within 4096 bytes. +A moq-transport namespace whose escaped path would exceed it is refused when decoded, and a SUBSCRIBE, TRACK\_STATUS, FETCH, or PUBLISH whose namespace and track name exceed 4096 bytes together fails that request with `Error::Encode` instead of being sent. + ### Building a `Path` refuses empty segments ([#6648](https://github.com/moq-dev/moq/pull/6648)) `Path::new` and `TryFrom` fail with `path::Invalid::EmptySegment` on a leading, trailing, or doubled `/` instead of collapsing it, so a path has one spelling; the empty path is still the root. Origin entry points report it as `Error::EmptySegment`, and `Path::from_static` panics on it. @@ -20,7 +25,7 @@ The moql SETUP (no ALPN, or `moql`) now negotiates only moq-lite versions. ### `Path` and `Relative` have no lifetime -There is one owned `Path` type: a short path is stored inline and a long one shares a reference-counted buffer, so cloning stays cheap. Every path is at most `Path::MAX_BYTES` (16 KiB). +There is one owned `Path` type: a short path is stored inline and a long one shares a reference-counted buffer, so cloning stays cheap. Every path is at most `Path::MAX_BYTES`. - `PathOwned`, `Path<'a>`, and `Path<'static>` are all `Path`; `RelativeOwned` and `Relative<'a>` are `Relative`. - `Path::{to_owned, into_owned, borrow}` and the `Relative` equivalents are gone: drop the call, or `clone()` a `&Path` you need to keep. diff --git a/rs/moq-net/src/fuzz.rs b/rs/moq-net/src/fuzz.rs index 6997c3d5aa7..34560d57f46 100644 --- a/rs/moq-net/src/fuzz.rs +++ b/rs/moq-net/src/fuzz.rs @@ -180,11 +180,11 @@ fn ietf_paths(data: &[u8], version: ietf::Version) { if let Ok(path) = ietf::namespace::decode_namespace(&mut Decoder::new(data, form)) { assert_strict(&path); let mut encoded = Vec::new(); - if ietf::namespace::encode_namespace(&mut Encoder::new(&mut encoded, form), &path).is_ok() { - let echo = ietf::namespace::decode_namespace(&mut Decoder::new(&encoded, form)) - .expect("could not decode our own namespace"); - assert_eq!(echo, path, "namespace did not survive a round trip"); - } + ietf::namespace::encode_namespace(&mut Encoder::new(&mut encoded, form), &path) + .expect("could not encode a decoded namespace"); + let echo = ietf::namespace::decode_namespace(&mut Decoder::new(&encoded, form)) + .expect("could not decode our own namespace"); + assert_eq!(echo, path, "namespace did not survive a round trip"); } if let Ok((path, _)) = ietf::namespace::decode_track(&mut Decoder::new(data, form)) { assert_strict(&path); diff --git a/rs/moq-net/src/ietf/fetch.rs b/rs/moq-net/src/ietf/fetch.rs index ba0b12238e9..de3334ca157 100644 --- a/rs/moq-net/src/ietf/fetch.rs +++ b/rs/moq-net/src/ietf/fetch.rs @@ -5,7 +5,7 @@ use crate::{ coding::{Decode, DecodeError, Decoder, Encode, EncodeError, Encoder}, ietf::{ Filter, GroupOrder, Location, Opaque, RequestId, - namespace::{decode_track, encode_namespace}, + namespace::{decode_track, encode_track}, }, }; @@ -54,8 +54,7 @@ impl Encode for FetchType<'_> { end, } => { w.u8(1); - encode_namespace(w, namespace)?; - w.string(track)?; + encode_track(w, namespace, track)?; start.encode(w, version)?; end.encode(w, version)?; } @@ -171,8 +170,7 @@ impl Message for Fetch<'_> { else { return Err(EncodeError::Version); }; - encode_namespace(w, namespace)?; - w.string(track)?; + encode_track(w, namespace, 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, diff --git a/rs/moq-net/src/ietf/namespace.rs b/rs/moq-net/src/ietf/namespace.rs index aa4b0c05c5b..6f17b30565a 100644 --- a/rs/moq-net/src/ietf/namespace.rs +++ b/rs/moq-net/src/ietf/namespace.rs @@ -28,7 +28,8 @@ fn to_tuple(namespace: &Path) -> Vec { parts } -// A field ending in `/` escapes to a path with an empty segment, so it is refused. +// A field ending in `/` escapes to a path with an empty segment, so it is refused, as is a +// namespace whose separators and escapes push its path past `Path::MAX_BYTES`. fn from_tuple(parts: &[&str]) -> Result { // Sized for the common case of no escapes, so building it rarely grows. let mut path = String::with_capacity(parts.iter().map(|part| part.len() + 1).sum()); @@ -48,18 +49,35 @@ fn from_tuple(parts: &[&str]) -> Result { Path::try_from(path) } -/// Helper function to encode namespace as tuple of strings +/// Encode a Track Namespace. pub fn encode_namespace(w: &mut Encoder<'_>, namespace: &Path) -> Result<(), EncodeError> { - let parts = to_tuple(namespace); + write_fields(w, &fields(namespace)?) +} - // The IETF draft limits namespaces to 32 parts. +/// Encode a Full Track Name: a Track Namespace, then a Track Name, at most 4096 bytes together. +pub fn encode_track(w: &mut Encoder<'_>, namespace: &Path, name: &str) -> Result<(), EncodeError> { + let parts = fields(namespace)?; + if parts.iter().map(String::len).sum::() + name.len() > MAX_BYTES { + return Err(BoundsExceeded.into()); + } + write_fields(w, &parts)?; + w.string(name) +} + +fn fields(namespace: &Path) -> Result, EncodeError> { + let parts = to_tuple(namespace); + // The fields drop the path's separators and escapes, so `Path::MAX_BYTES` already bounds + // their bytes. A path's depth is unbounded until it reaches a wire. if parts.len() > Path::MAX_PARTS { return Err(BoundsExceeded.into()); } + Ok(parts) +} +fn write_fields(w: &mut Encoder<'_>, parts: &[String]) -> Result<(), EncodeError> { w.varint(parts.len() as u64)?; for part in parts { - w.string(&part)?; + w.string(part)?; } Ok(()) } @@ -67,10 +85,14 @@ pub fn encode_namespace(w: &mut Encoder<'_>, namespace: &Path) -> Result<(), Enc /// The most bytes a Track Namespace, or a whole Full Track Name, may hold on the wire. const MAX_BYTES: usize = 4096; +// Every path encodes as a namespace. +const _: () = assert!(Path::MAX_BYTES <= MAX_BYTES); + /// Decode a namespace tuple, along with the sum of its field lengths. /// /// Every limit here closes the session as a PROTOCOL_VIOLATION (draft-22 section 8.7): /// more than 32 fields, an empty field, a field ending in `/`, or more than 4096 bytes. +/// So does a namespace too long as a [`Path`], which the draft allows but no path can hold. fn decode_fields(r: &mut Decoder<'_>) -> Result<(Path, usize), DecodeError> { let count = r.varint()?; @@ -280,4 +302,42 @@ mod tests { assert_eq!(decode_track_slice(&bytes).is_ok(), ok, "{namespace} + {name}"); } } + + /// Separators and escapes count toward a path but not its tuple fields, so every path + /// encodes, while a namespace dense with escaped slashes decodes only if its path fits. + #[test] + fn path_bound_covers_namespace_fields() { + let segments = ["a"; Path::MAX_PARTS - 1].join("/"); + let deepest = format!("{segments}/{}", "é".repeat(Path::MAX_BYTES / 2 - 31)); + let deepest = Path::new(&deepest).unwrap(); + assert_eq!(deepest.len(), Path::MAX_BYTES); + assert_eq!(decode_ns(&encode_path(&deepest)), deepest); + + let fits = format!("{}a", "/".repeat(Path::MAX_BYTES / 2 - 1)); + assert_eq!(decode_ns(&encode_tuple(&[&fits])).len(), Path::MAX_BYTES - 1); + let too_long = format!("{}a", "/".repeat(Path::MAX_BYTES / 2)); + let err = decode_namespace(&mut Decoder::new(&encode_tuple(&[&too_long]), FORM)).unwrap_err(); + assert!(matches!(err, DecodeError::BoundsExceeded), "{err:?}"); + } + + /// A Full Track Name counts its namespace's field bytes plus its name, and one too long + /// is refused before anything is written. + #[test] + fn full_track_name_is_bounded_on_encode() { + // 4095 path bytes holding 2048 field bytes. + let namespace = Path::new(&format!("{}a", r"\/".repeat(Path::MAX_BYTES / 2 - 1))).unwrap(); + for (len, ok) in [(2048, true), (2049, false)] { + let name = "x".repeat(len); + let mut bytes = Vec::new(); + let result = encode_track(&mut Encoder::new(&mut bytes, FORM), &namespace, &name); + assert_eq!(result.is_ok(), ok, "{len}"); + match ok { + true => assert_eq!( + decode_track(&mut Decoder::new(&bytes, FORM)).unwrap(), + (namespace.clone(), name) + ), + false => assert!(bytes.is_empty(), "refuse before writing"), + } + } + } } diff --git a/rs/moq-net/src/ietf/publish.rs b/rs/moq-net/src/ietf/publish.rs index c9610438e0a..6a8cac95485 100644 --- a/rs/moq-net/src/ietf/publish.rs +++ b/rs/moq-net/src/ietf/publish.rs @@ -110,7 +110,7 @@ use crate::{ coding::{Decode, DecodeError, Decoder, Encode, EncodeError, Encoder}, ietf::{ Filter, GroupOrder, Location, Properties, RequestId, - namespace::{decode_track, encode_namespace}, + namespace::{decode_track, encode_track}, }, }; @@ -213,8 +213,7 @@ impl Message for Publish<'_> { fn encode_msg(&self, w: &mut Encoder<'_>, version: Version) -> Result<(), EncodeError> { self.request_id.encode(w, version)?; - encode_namespace(w, &self.track_namespace)?; - w.string(&self.track_name)?; + encode_track(w, &self.track_namespace, &self.track_name)?; w.varint(self.track_alias)?; // GROUP_ORDER is illegal on PUBLISH on draft-18 (a peer closes the session with @@ -359,8 +358,7 @@ mod tests { let mut body = Vec::new(); let w = &mut Encoder::new(&mut body, version.into()); RequestId(1).encode(w, version)?; - super::super::namespace::encode_namespace(w, &crate::Path::from_static("broadcast"))?; - w.string("video")?; + super::super::namespace::encode_track(w, &crate::Path::from_static("broadcast"), "video")?; w.varint(1u64)?; // track alias // SUBSCRIBER_PRIORITY then LOCATION_FILTER, delta encoded from 0. @@ -382,8 +380,7 @@ mod tests { let mut body = Vec::new(); let w = &mut Encoder::new(&mut body, version.into()); RequestId(1).encode(w, version)?; - super::super::namespace::encode_namespace(w, &crate::Path::from_static("broadcast"))?; - w.string("video")?; + super::super::namespace::encode_track(w, &crate::Path::from_static("broadcast"), "video")?; w.varint(1u64)?; encode_params!(w, version, 0x20 => 128u8); Properties::default().encode(w, version)?; @@ -408,8 +405,7 @@ mod tests { let mut buf = Vec::new(); let w = &mut Encoder::new(&mut buf, version.into()); RequestId(1).encode(w, version).unwrap(); - encode_namespace(w, &Path::from_static("t")).unwrap(); - w.string("v").unwrap(); + encode_track(w, &Path::from_static("t"), "v").unwrap(); w.varint(1).unwrap(); w.varint(1).unwrap(); w.varint(key).unwrap(); diff --git a/rs/moq-net/src/ietf/publisher.rs b/rs/moq-net/src/ietf/publisher.rs index 31de6ab187d..7c682cb4b56 100644 --- a/rs/moq-net/src/ietf/publisher.rs +++ b/rs/moq-net/src/ietf/publisher.rs @@ -8709,8 +8709,7 @@ mod serve_tests { let encode = |body: &mut Vec| -> Result<(), crate::coding::EncodeError> { let w = &mut Encoder::new(body, version.into()); RequestId(REQUEST_ID).encode(w, version)?; - super::super::namespace::encode_namespace(w, &crate::Path::from_static("room"))?; - w.string("video")?; + super::super::namespace::encode_track(w, &crate::Path::from_static("room"), "video")?; encode_params!(w, version, 0x04 => 7u64, super::super::epoch::EPOCH_PARAM => Some(stale.clone()), diff --git a/rs/moq-net/src/ietf/subscribe.rs b/rs/moq-net/src/ietf/subscribe.rs index b924dcef53b..9d64d99bbe3 100644 --- a/rs/moq-net/src/ietf/subscribe.rs +++ b/rs/moq-net/src/ietf/subscribe.rs @@ -12,7 +12,7 @@ use super::token::RequestToken; use super::Message; use super::epoch; -use super::namespace::{decode_track, encode_namespace}; +use super::namespace::{decode_track, encode_track}; use super::Version; @@ -128,8 +128,7 @@ impl Message for Subscribe<'_> { fn encode_msg(&self, w: &mut Encoder<'_>, version: Version) -> Result<(), EncodeError> { self.request_id.encode(w, version)?; - encode_namespace(w, &self.track_namespace)?; - w.string(&self.track_name)?; + encode_track(w, &self.track_namespace, &self.track_name)?; // FILL_PARAMETERS arrived in draft-22. Sending it to an older peer would be an // unknown parameter, which is a protocol violation, so it is dropped instead. @@ -419,8 +418,7 @@ mod tests { let mut buf = Vec::new(); let w = &mut Encoder::new(&mut buf, version.into()); RequestId(1).encode(w, version)?; - encode_namespace(w, &Path::from_static("test"))?; - w.string("video")?; + encode_track(w, &Path::from_static("test"), "video")?; encode_params!(w, version, 0x04 => millis, ); @@ -605,8 +603,7 @@ mod tests { let mut body = Vec::new(); let w = &mut Encoder::new(&mut body, version.into()); RequestId(1).encode(w, version)?; - encode_namespace(w, &crate::Path::from_static("broadcast"))?; - w.string("video")?; + encode_track(w, &crate::Path::from_static("broadcast"), "video")?; encode_params!(w, version, 0x02 => 5000u64, 0x06 => 9000u64, diff --git a/rs/moq-net/src/ietf/subscriber.rs b/rs/moq-net/src/ietf/subscriber.rs index fb202d52f97..5d4d9c5d880 100644 --- a/rs/moq-net/src/ietf/subscriber.rs +++ b/rs/moq-net/src/ietf/subscriber.rs @@ -16344,6 +16344,72 @@ mod track_property_tests { } } +#[cfg(test)] +mod full_track_name_tests { + use super::*; + use crate::{model::ProduceTest, transport::memory}; + + /// A Full Track Name too long for the wire fails only its own subscription, and the + /// session carries the next one. + #[moq_clock::test] + async fn an_unencodable_track_name_fails_only_its_subscription() { + for version in [Version::Draft18, Version::Draft22] { + let (session, peer) = memory::pair(memory::Config::default()); + let (tasks, _task_set) = TaskSet::new(); + let subscriber = Subscriber::new( + crate::time::sim(), + crate::driver::Transport::new(session.clone()), + crate::origin::Config::new(crate::Hop::new(1).unwrap()).produce(), + Control::new(false), + None, + peer::PeerSetup::declared(peer::Peer::default()), + crate::Hop::new(1).unwrap(), + None, + version, + tasks, + Default::default(), + ); + // One byte past 4096 with the 9-byte namespace. + let long = "x".repeat(4088); + for (name, fits) in [(long.as_str(), false), ("video", true)] { + let broadcast = crate::broadcast::Info::default().produce(); + let mut dynamic = broadcast.requests(); + let track = broadcast.consume().track(name).unwrap(); + let mut subscribing = std::pin::pin!(track.subscribe(None)); + let request = dynamic.requested_track().await.expect("no track requested"); + let mut subscriber = subscriber.clone(); + let serving = moq_clock::sim::spawn(async move { + subscriber + .run_subscribe(Path::from_static("broadcast"), dynamic, None, request) + .await; + }); + if fits { + // The refused request's stream was reset before its header, which a peer ignores. + let id = loop { + let mut stream = Stream::accept(&mut crate::driver::Transport::new(peer.clone()), version) + .await + .unwrap(); + if let Ok(id) = stream.reader.varint().await { + break id; + } + }; + assert_eq!(id, ietf::Subscribe::ID, "{version}"); + } else { + let refused = moq_clock::timeout(Duration::from_secs(1), &mut subscribing) + .await + .expect("the subscription was never refused"); + assert!( + matches!(refused, Err(Error::Encode(crate::coding::EncodeError::BoundsExceeded))), + "{version}: {:?}", + refused.map(|_| ()) + ); + } + serving.abort(); + } + } + } +} + #[cfg(test)] mod compression_negotiation_tests { use super::*; diff --git a/rs/moq-net/src/ietf/track.rs b/rs/moq-net/src/ietf/track.rs index c9937969f0b..a6dc1675c55 100644 --- a/rs/moq-net/src/ietf/track.rs +++ b/rs/moq-net/src/ietf/track.rs @@ -11,7 +11,7 @@ use crate::{ }; use super::Message; -use super::namespace::encode_namespace; +use super::namespace::encode_track; use super::Version; @@ -34,8 +34,7 @@ impl Message for TrackStatus<'_> { fn encode_msg(&self, w: &mut Encoder<'_>, version: Version) -> Result<(), EncodeError> { self.request_id.encode(w, version)?; - encode_namespace(w, &self.track_namespace)?; - w.string(&self.track_name)?; + encode_track(w, &self.track_namespace, &self.track_name)?; // Only the opt-out is worth bytes, and an older peer would read the // parameter as unknown, which is a protocol violation. diff --git a/rs/moq-net/src/model/origin.rs b/rs/moq-net/src/model/origin.rs index 9551dc406af..0e9e3018f25 100644 --- a/rs/moq-net/src/model/origin.rs +++ b/rs/moq-net/src/model/origin.rs @@ -2149,7 +2149,7 @@ impl Producer { /// producer may publish under (after [`scope`](Self::scope)) or beneath a /// [`mount`](Self::mount), /// [`Error::BoundsExceeded`] if the full rooted path exceeds - /// [`Path::MAX_PARTS`] or 16 KiB, [`Error::InvalidPath`] if it holds a segment no + /// [`Path::MAX_PARTS`] or [`Path::MAX_BYTES`], [`Error::InvalidPath`] if it holds a segment no /// pattern can spell (`*` or `**`), [`Error::NavigationSegment`] for a literal `.` or `..`, /// or [`Error::Closed`] once the origin's /// [`Driver`] has been dropped. @@ -2446,7 +2446,7 @@ impl Producer { /// `at` or `target` overlaps a mount point this producer already has, or `at` /// overlaps an existing target or its own: mounts never chain, in whatever order they are added, /// so a target is always read as the origin holds it. Mounts may share a target. - /// [`Error::BoundsExceeded`] if either rooted path exceeds [`Path::MAX_PARTS`] or 16 KiB, + /// [`Error::BoundsExceeded`] if either rooted path exceeds [`Path::MAX_PARTS`] or [`Path::MAX_BYTES`], /// and [`Error::InvalidPath`] if `at` holds a segment no pattern can spell. pub fn mount( &self, diff --git a/rs/moq-net/src/path/mod.rs b/rs/moq-net/src/path/mod.rs index 63612ca5bb4..d8ffaf11943 100644 --- a/rs/moq-net/src/path/mod.rs +++ b/rs/moq-net/src/path/mod.rs @@ -129,9 +129,9 @@ impl Path { /// Maximum length of a path in bytes. /// - /// A quarter of the 64 KiB moq-lite message limit, leaving room for every other - /// field of a message that carries a path. - pub const MAX_BYTES: usize = 16 * 1024; + /// Matches the IETF moq-transport limit of 4096 bytes for a namespace. Its tuple fields + /// drop the path's separators and escapes, so every path fits both wires. + pub const MAX_BYTES: usize = 4096; /// Create a new Path by copying a string slice. ///