diff --git a/js/net/src/ietf/fetch.ts b/js/net/src/ietf/fetch.ts index 398f111857..bbffd0530e 100644 --- a/js/net/src/ietf/fetch.ts +++ b/js/net/src/ietf/fetch.ts @@ -1,6 +1,6 @@ import type * as Epoch from "../epoch.ts"; import type * as Path from "../path.ts"; -import type { Reader, Writer } from "../stream.ts"; +import { type Reader, Writer } from "../stream.ts"; import { hasRangeFilters, isDraft20 } from "./filter.ts"; import * as Message from "./message.ts"; import * as Namespace from "./namespace.ts"; @@ -22,9 +22,14 @@ export class FetchHeader { this.requestId = requestId; } - /** Write the header, which the stream type must already precede. */ - async encode(w: Writer, _version: IetfVersion): Promise { - await w.u62(this.requestId); + /** Write the stream type and the header in one write, like a subgroup header. */ + async encode(w: Writer, version: IetfVersion): Promise { + await w.write( + await Writer.collect(version, async (h) => { + await h.u53(FetchHeader.type); + await h.u62(this.requestId); + }), + ); } /** Read the header, with the stream type already consumed. */ diff --git a/js/net/src/ietf/object.test.ts b/js/net/src/ietf/object.test.ts index ba6cec1acd..9682517c39 100644 --- a/js/net/src/ietf/object.test.ts +++ b/js/net/src/ietf/object.test.ts @@ -1,7 +1,8 @@ import { expect, test } from "bun:test"; -import { Cursor } from "../stream.ts"; +import { Cursor, Reader, Writer } from "../stream.ts"; import { Timescale, Timestamp } from "../time.ts"; -import { decodeObjectTime, encodeObjectExtensions } from "./object.ts"; +import { FetchHeader } from "./fetch.ts"; +import { decodeObjectTime, encodeObjectExtensions, Group } from "./object.ts"; import { Version } from "./version.ts"; const VERSION = Version.DRAFT_17; @@ -30,3 +31,40 @@ test("a conversion past what a Timestamp holds is refused before anything is wri RangeError, ); }); + +test("a data stream's header goes out in one write, stream type included", async () => { + for (const version of [Version.DRAFT_14, Version.DRAFT_22]) { + const chunks: Uint8Array[] = []; + const writer = new Writer( + new WritableStream({ write: (chunk) => void chunks.push(new Uint8Array(chunk)) }), + version, + ); + + // Every optional field present, with values past a one-byte varint. + const group = new Group({ + trackAlias: 1_000_000n, + groupId: 70_000, + subGroupId: 300, + publisherPriority: 200, + flags: { + hasExtensions: true, + hasSubgroup: true, + hasSubgroupObject: false, + hasEnd: true, + hasPriority: true, + firstObject: true, + }, + }); + await group.encode(writer, version); + await new FetchHeader({ requestId: 1_000_000n }).encode(writer, version); + expect(chunks.length).toBe(2); + + const decoded = await Group.decode(new Reader(undefined, chunks[0], version), version); + expect(decoded).toEqual(group); + + const fetch = new Reader(undefined, chunks[1], version); + expect(await fetch.u53()).toBe(FetchHeader.type); + expect((await FetchHeader.decode(fetch, version)).requestId).toBe(1_000_000n); + expect(await fetch.done()).toBe(true); + } +}); diff --git a/js/net/src/ietf/object.ts b/js/net/src/ietf/object.ts index 4315e666ea..4631aa6b96 100644 --- a/js/net/src/ietf/object.ts +++ b/js/net/src/ietf/object.ts @@ -161,7 +161,15 @@ export class Group { this.publisherPriority = publisherPriority; } + /** + * Write the header, stream type first, in one write: a peer that classifies a stream from + * its first read (imquic) drops a header split across several. + */ async encode(w: Writer, version: IetfVersion): Promise { + await w.write(await Writer.collect(version, (h) => this.#encode(h, version))); + } + + async #encode(w: Writer, version: IetfVersion): Promise { if (!this.flags.hasSubgroup && this.subGroupId !== 0) { throw new Error(`Subgroup ID must be 0 if hasSubgroup is false: ${this.subGroupId}`); } diff --git a/js/net/src/ietf/publisher.ts b/js/net/src/ietf/publisher.ts index 9b2cb8c084..c8b282cd44 100644 --- a/js/net/src/ietf/publisher.ts +++ b/js/net/src/ietf/publisher.ts @@ -1336,7 +1336,6 @@ export class Publisher { streams.opened += 1; if (sequence !== undefined) ranking.add(stream, sequence, object); - await stream.u53(FetchHeader.type); await new FetchHeader({ requestId }).encode(stream, version); if (fill.kind !== "group") { diff --git a/quest/m1/audit-interop.md b/quest/m1/audit-interop.md index 4d93aee879..2adf01abee 100644 --- a/quest/m1/audit-interop.md +++ b/quest/m1/audit-interop.md @@ -86,7 +86,11 @@ Waiting on others: (closes on a request before SETUP completes), [meetecho/imquic#42](https://github.com/meetecho/imquic/issues/42) and [Eyevinn/moqlivemock#157](https://github.com/Eyevinn/moqlivemock/issues/157) - (interop clients drop the URL path), plus moxygen's filter type 250 + (interop clients drop the URL path), + [meetecho/imquic#43](https://github.com/meetecho/imquic/issues/43) + (drops a data stream whose header spans reads; owned by + [JS subgroup data through imquic](/quest/m1/interop-coverage/js-imquic-objects.md)), + plus moxygen's filter type 250 ([facebookexperimental/moxygen#225](https://github.com/facebookexperimental/moxygen/issues/225)), which also fails moqx's client against our relay. - The JS publisher refuses every FETCH, so every relay that fills a late diff --git a/quest/m1/interop-coverage/README.md b/quest/m1/interop-coverage/README.md index c8ef6e10e6..9395ba7b90 100644 --- a/quest/m1/interop-coverage/README.md +++ b/quest/m1/interop-coverage/README.md @@ -41,7 +41,7 @@ coverage on `main` at `3ab6cbf81d`. ## Required - [JS namespace lifecycle](/quest/m1/interop-coverage/js-namespace-done.md) - moq-dev-js subscribe-namespace-lifecycle sees NAMESPACE_DONE with publisher and subscriber in one process -- [JS subgroup data through imquic](/quest/m1/interop-coverage/js-imquic-objects.md) - moq-dev-js data-subgroup-basic receives objects from imquic over WebTransport +- [JS subgroup data through imquic](/quest/m1/interop-coverage/js-imquic-objects.md) - moq-dev-js data-subgroup-basic passes against imquic, or the cell is classified with imquic's split-header bug reported upstream - [Late join and FETCH](/quest/m1/interop-coverage/late-join.md) - a late subscriber gets the current group from object 0, and a standalone FETCH returns a past group exactly ## Related diff --git a/quest/m1/interop-coverage/js-imquic-objects.md b/quest/m1/interop-coverage/js-imquic-objects.md index cc826af5b2..5e7049ffc2 100644 --- a/quest/m1/interop-coverage/js-imquic-objects.md +++ b/quest/m1/interop-coverage/js-imquic-objects.md @@ -1,18 +1,42 @@ -# [S] JS subgroup data reaches the subscriber through imquic +# [XS] JS subgroup data reaches the subscriber through imquic ## Goal The runner's moq-dev-js `data-subgroup-basic` case passes against imquic -(draft-22, WebTransport), where today the subscriber receives no objects. -If the cause is in imquic, it is reported upstream and the cell is -classified instead. +(draft-22, WebTransport), or the cell is classified with the imquic issue +reported upstream. Check the cell on the runner's report. ## Plan -Found in the runs for -[englishm/moq-interop-runner#173](https://github.com/englishm/moq-interop-runner/pull/173) -on 2026-10-22. moq-dev-rs passes against imquic on the same draft, so -compare what the two clients put on the wire (transport, SUBSCRIBE -fields, stream types) before suspecting imquic. +imquic drops a SUBGROUP_HEADER (or FETCH_HEADER) that arrives across more +than one stream read: `imquic_moq_parse_message` creates the data stream on +the first chunk but only buffers bytes once the header has parsed, so the +rest of the header and every object are discarded, and the relay logs +`No track found for that subscription` with alias 0 at the stream's FIN. +QUIC may split a stream anywhere, so this is imquic's bug, reported as +[meetecho/imquic#43](https://github.com/meetecho/imquic/issues/43). +`@moq/net` 0.4.4 wrote the header one field per write, which the polyfill +sent as separate frames; moq-dev-rs buffers it and passes. -Public API: unknown until the cause is found. Wire: none expected. +[moq-dev/moq#6504](https://github.com/moq-dev/moq/pull/6504) writes both +data stream headers in one write. The maintainer kept it on 2026-10-23 +even though the root cause is imquic's: it matches the Rust publisher and +the one-write control messages from +[moq-dev/moq#6255](https://github.com/moq-dev/moq/pull/6255), and also +covers FETCH_HEADER. Reproduced and verified against the runner's imquic +relay image and `lminiero.it:9000`: 0.4.4 fails every run, and passes once +patched. + +Remaining: + +- The cell turns green once imquic buffers a split header, or once the + runner's JS client moves to a `@moq/net` release with + [moq-dev/moq#6504](https://github.com/moq-dev/moq/pull/6504). Delete + this quest then. +- On `main`, `request(path, { announced: true })` asks SUBSCRIBE_NAMESPACE + for the tuple's own namespace, and imquic never answers with the shorter + routing prefix the publisher announced, so the subscriber never + subscribes. 0.4.4 asked for the prefix. The runner's client hits this + when it moves past 0.4.4 unless it requests blindly. + +Public API: none. Wire: none (bytes unchanged, fewer writes).