diff --git a/js/net/src/ietf/ietf.test.ts b/js/net/src/ietf/ietf.test.ts index d16c7b5bee2..5c9dd846325 100644 --- a/js/net/src/ietf/ietf.test.ts +++ b/js/net/src/ietf/ietf.test.ts @@ -2084,3 +2084,105 @@ for (const version of [Version.DRAFT_14, Version.DRAFT_15, Version.DRAFT_16]) { expect(params.groupOrder).toBe(0); }); } + +// Caller-selected SUBSCRIBE group order. +// +// GROUP_ORDER was a module constant written unconditionally on both wire shapes, and +// `Subscribe` had no field for it -- so no caller, internal or public, could ask for +// Ascending. The decoder already read and validated the value, and then dropped it: a relay +// built on this parsed the subscriber's requested order correctly and could not act on it. +test.each([ + ["v14", Version.DRAFT_14], + ["v20", Version.DRAFT_20], +] as const)("Subscribe %s: group order defaults to descending", async (_name, version) => { + const msg = new Subscribe.Subscribe({ + requestId: 1n, + trackNamespace: Path.from("test"), + trackName: "video", + subscriberPriority: 128, + }); + expect(msg.groupOrder).toBe(Subscribe.GroupOrder.Descending); + + const encoded = await encodeVersioned(msg, version); + const decoded = await decodeVersioned(encoded, Subscribe.Subscribe.decode, version); + expect(decoded.groupOrder).toBe(Subscribe.GroupOrder.Descending); +}); + +test.each([ + ["v14", Version.DRAFT_14], + ["v20", Version.DRAFT_20], +] as const)("Subscribe %s: a caller can request ascending group order", async (_name, version) => { + const msg = new Subscribe.Subscribe({ + requestId: 1n, + trackNamespace: Path.from("test"), + trackName: "video", + subscriberPriority: 128, + groupOrder: Subscribe.GroupOrder.Ascending, + }); + + const encoded = await encodeVersioned(msg, version); + + // Differential, not a byte search: `toContain(1)` would pass on requestId 1n + // alone. Encode the same message as Descending and require that exactly one + // byte moved, from 0x02 to 0x01. + const descending = await encodeVersioned( + new Subscribe.Subscribe({ + requestId: 1n, + trackNamespace: Path.from("test"), + trackName: "video", + subscriberPriority: 128, + groupOrder: Subscribe.GroupOrder.Descending, + }), + version, + ); + expect(encoded.length).toBe(descending.length); + const differing = [...encoded].flatMap((byte, i) => (byte === descending[i] ? [] : [i])); + expect(differing).toHaveLength(1); + expect(encoded[differing[0]]).toBe(Subscribe.GroupOrder.Ascending); + expect(descending[differing[0]]).toBe(Subscribe.GroupOrder.Descending); + + const decoded = await decodeVersioned(encoded, Subscribe.Subscribe.decode, version); + expect(decoded.groupOrder).toBe(Subscribe.GroupOrder.Ascending); +}); + +test.each([ + ["v14", Version.DRAFT_14], + ["v20", Version.DRAFT_20], +] as const)("Subscribe %s: a caller asking for Default sends a real order", async (_name, version) => { + // 0x0 is the wire's "no preference". A caller may pass it, but it must normalize to a + // real order rather than reaching the encoders -- from draft-15 GROUP_ORDER is a message + // parameter and Parameters rejects 0 outright, so sending it would be unencodable. + const msg = new Subscribe.Subscribe({ + requestId: 1n, + trackNamespace: Path.from("test"), + trackName: "video", + subscriberPriority: 128, + groupOrder: Subscribe.GroupOrder.Default, + }); + expect(msg.groupOrder).toBe(Subscribe.GroupOrder.Descending); + + const encoded = await encodeVersioned(msg, version); + const decoded = await decodeVersioned(encoded, Subscribe.Subscribe.decode, version); + expect(decoded.groupOrder).toBe(Subscribe.GroupOrder.Descending); +}); + +// Draft-14 carries GROUP_ORDER as an inline u8 rather than a message parameter, so 0 is +// reachable there from a peer and must resolve to the default instead of surfacing as a +// third value a caller has to handle. +test("Subscribe v14: an inline group order of 0 decodes as the default", async () => { + const msg = new Subscribe.Subscribe({ + requestId: 1n, + trackNamespace: Path.from("test"), + trackName: "video", + subscriberPriority: 128, + }); + const encoded = await encodeVersioned(msg, Version.DRAFT_14); + + // Rewrite the encoded inline order byte to 0, which no encoder of ours will emit. + const orderAt = encoded.lastIndexOf(Subscribe.GroupOrder.Descending); + const zeroed = new Uint8Array(encoded); + zeroed[orderAt] = Subscribe.GroupOrder.Default; + + const decoded = await decodeVersioned(zeroed, Subscribe.Subscribe.decode, Version.DRAFT_14); + expect(decoded.groupOrder).toBe(Subscribe.GroupOrder.Descending); +}); diff --git a/js/net/src/ietf/subscribe.ts b/js/net/src/ietf/subscribe.ts index 73d218463ae..fa3414eea6b 100644 --- a/js/net/src/ietf/subscribe.ts +++ b/js/net/src/ietf/subscribe.ts @@ -7,8 +7,27 @@ import { type MessageLocation, Parameters } from "./parameters.ts"; import * as Properties from "./properties.ts"; import { type IetfVersion, Version } from "./version.ts"; -// we only support Group Order descending -const GROUP_ORDER = 0x02; +/** + * The GROUP_ORDER a subscription asks the publisher to deliver in. + * + * `Default` (0x0) is the wire's "no preference": the publisher picks, which in + * practice means its own DEFAULT_PUBLISHER_GROUP_ORDER track property. It is accepted + * from a caller and on a draft-14 decode, and normalized away in both cases -- this + * client never puts it on the wire. From draft-15 it could not: GROUP_ORDER is a + * message parameter there and {@link Parameters} rejects 0 as a protocol violation, so + * only draft-14's inline field can carry it at all. + */ +export const GroupOrder = { + Default: 0x00, + Ascending: 0x01, + Descending: 0x02, +} as const; + +export type GroupOrder = (typeof GroupOrder)[keyof typeof GroupOrder]; + +// The order this client requests unless the caller names one. Descending (newest +// group first) is what every subscription asked for before `groupOrder` existed. +const GROUP_ORDER = GroupOrder.Descending; /** * The filter this implementation joins a live track with. @@ -29,6 +48,14 @@ export class Subscribe { trackName: string; subscriberPriority: number; + /** + * The order the publisher is asked to deliver groups in (GROUP_ORDER). + * + * Defaults to {@link GroupOrder.Descending}, which is what this client sent + * unconditionally before the field existed. + */ + groupOrder: GroupOrder; + /** Which Objects the subscription delivers, resolved by the publisher against the live edge. */ filter: Filter.Filter; @@ -49,6 +76,7 @@ export class Subscribe { trackNamespace, trackName, subscriberPriority, + groupOrder, filter, fill, propertiesWanted, @@ -59,6 +87,7 @@ export class Subscribe { trackNamespace: Path.Valid; trackName: string; subscriberPriority: number; + groupOrder?: GroupOrder; filter?: Filter.Filter; fill?: Filter.Fill; propertiesWanted?: boolean; @@ -69,6 +98,9 @@ export class Subscribe { this.trackNamespace = trackNamespace; this.trackName = trackName; this.subscriberPriority = subscriberPriority; + // Resolve "no preference" to a real order here, so `groupOrder` is never 0 for a + // caller reading it back and 0 can never reach the encoders. + this.groupOrder = groupOrder === undefined || groupOrder === GroupOrder.Default ? GROUP_ORDER : groupOrder; this.filter = filter ?? { kind: "unfiltered" }; this.fill = fill; this.propertiesWanted = propertiesWanted ?? true; @@ -86,7 +118,7 @@ export class Subscribe { if (version === Version.DRAFT_14) { await w.u8(this.subscriberPriority); - await w.u8(GROUP_ORDER); + await w.u8(this.groupOrder); await w.bool(this.forward); await w.write(Filter.encode(this.filter, version)); await w.u53(0); // no parameters @@ -94,7 +126,7 @@ export class Subscribe { // v15+: fields moved into parameters const params = new Parameters(); params.subscriberPriority = this.subscriberPriority; - params.groupOrder = GROUP_ORDER; + params.groupOrder = this.groupOrder; params.forward = this.forward; params.subscriptionFilter = Filter.encode(this.filter, version); @@ -134,31 +166,34 @@ export class Subscribe { if (version === Version.DRAFT_14) { const subscriberPriority = await r.u8(); - let groupOrder = await r.u8(); + const groupOrder = await r.u8(); if (groupOrder > 2) { throw new Error(`unknown group order: ${groupOrder}`); } - if (groupOrder === 0) { - groupOrder = GROUP_ORDER; // default to descending - } + // 0 ("no preference") is resolved to a real order by the constructor. const forward = await r.bool(); const filter = await Filter.decodeInline(r); await Parameters.decode(r, version); // ignore parameters - return new Subscribe({ requestId, trackNamespace, trackName, subscriberPriority, filter, forward }); + return new Subscribe({ + requestId, + trackNamespace, + trackName, + subscriberPriority, + groupOrder: groupOrder as GroupOrder, + filter, + forward, + }); } // v15+: fields are in parameters const params = await Parameters.decode(r, version, "subscribe"); const subscriberPriority = params.subscriberPriority ?? 128; - let groupOrder = params.groupOrder ?? GROUP_ORDER; + const groupOrder = params.groupOrder ?? GROUP_ORDER; if (groupOrder > 2) { throw new Error(`unknown group order: ${groupOrder}`); } - if (groupOrder === 0) { - groupOrder = GROUP_ORDER; // default to descending - } // FILL_PARAMETERS and INCLUDE_PROPERTIES are draft-20 additions, and the Range // Filters draft-19 ones. An unknown message parameter is a protocol violation, so @@ -186,6 +221,7 @@ export class Subscribe { trackNamespace, trackName, subscriberPriority, + groupOrder: groupOrder as GroupOrder, filter, fill, // Defaults to 1, so an absent parameter means the subscriber wants them.