fix(net): backport complete namespace fills

js/net/src/ietf/publisher.test.ts

@@ -2323,3 +2323,38 @@ for (const version of [Version.DRAFT_15, Version.DRAFT_19] as const) {
 		}
 	}
 }
+
+for (const version of [
+	Version.DRAFT_16,
+	Version.DRAFT_17,
+	Version.DRAFT_18,
+	Version.DRAFT_19,
+	Version.DRAFT_20,
+	Version.DRAFT_21,
+	Version.DRAFT_22,
+] as const) {
+	test(`namespace fill includes visible routes without solicitation in ${version}`, async () => {
+		const pair = createMockTransportPair(ALPN.DRAFT_19);
+		const session = new NativeSession(pair.server, version, true);
+		const { pub, origin } = publisher(pair.server, { session });
+		const broadcast = publish(origin, Path.from("room/cam"));
+		const client = await Stream.open(pair.client, { version });
+		const server = await Stream.accept(pair.server, version);
+		if (!server) throw new Error("missing request stream");
+		const serving = pub.runSubscribeNamespace(
+			new SubscribeNamespace({ requestId: 0n, namespace: Path.from("room") }),
+			server,
+		);
+		try {
+			expect(await client.reader.u53()).toBe(RequestOk.id);
+			await RequestOk.decode(client.reader, version);
+			expect(await client.reader.u53()).toBe(SubscribeNamespaceEntry.id);
+			expect((await SubscribeNamespaceEntry.decode(client.reader, version)).suffix).toBe(Path.from("cam"));
+		} finally {
+			client.close();
+			await serving;
+			broadcast.close();
+			origin.close();
+		}
+	});
+}

js/net/src/ietf/publisher.ts

@@ -795,12 +795,9 @@ export class Publisher {
 	/**
 	 * Handles an incoming SUBSCRIBE_NAMESPACE on a bidi stream.
 	 *
-	 * This carries the advertisements when the peer asked to be told on request (MoQ
-	 * Solicit); otherwise {@link runPublishNamespaces} has already announced everything
-	 * visible and repeating it here would leave the peer holding two sources for one
-	 * broadcast, so only the hidden namespaces it may see ride here (MoQ Hidden).
-	 * Draft-16+ streams Namespace entries inline; draft-14/15 predate those messages, so
-	 * each advertisement is a PUBLISH_NAMESPACE request of its own.
+	 * Draft-16+ fills the stream with every visible match, independently of unsolicited
+	 * discovery. Draft-14/15 use PUBLISH_NAMESPACE requests instead and suppress matches
+	 * already announced on the unsolicited path. Hidden visibility applies to both.
 	 *
 	 * @internal
 	 */
@@ -826,12 +823,11 @@ export class Publisher {
 				await ok.encode(stream.writer, version);
 			}
 
-			// Hidden namespaces are left out unless the peer opted in (MoQ Hidden). Unless the
-			// peer asked to be told only on request, it has already heard everything visible
-			// from the empty prefix unasked, so this stream carries only what that hid.
+			// Only legacy requests create another route; inline entries always fill the query.
+			// Hidden namespaces still require opt-in unless the prefix names them directly.
 			const carries = (covered: Path.Valid) =>
 				(!this.#hidden || msg.hidden || !hiddenBelow(prefix, covered)) &&
-				(this.#requiresSolicitation || (this.#hidden && hiddenBelow(Path.empty(), covered)));
+				(!legacy || this.#requiresSolicitation || (this.#hidden && hiddenBelow(Path.empty(), covered)));
 
 			// Inline entries always land, and the receiver treats a repeated NAMESPACE as a
 			// replacement, so repricing one is just sending it again, a new original
@@ -910,8 +906,8 @@ export class Publisher {
 	 * The peers that never send SUBSCRIBE_NAMESPACE are exactly the ones expecting a
 	 * publisher to announce itself, so announcing is the default. A peer that would
 	 * rather ask says so in its SETUP (MoQ Solicit) and this does nothing, leaving
-	 * {@link runSubscribeNamespace} to carry the advertisements instead. Exactly one of
-	 * the two is live, so the peer never hears a namespace twice.
+	 * {@link runSubscribeNamespace} to carry the advertisements instead. Modern peers
+	 * may also discover the namespace through an inline subscription fill.
 	 *
 	 * @internal
 	 */

rs/moq-net/src/ietf/publisher.rs

@@ -2040,8 +2040,8 @@ where
 	/// ([`solicit`](super::solicit)),
 	/// and then this loop does nothing and
 	/// [`Self::run_subscribe_namespace_stream`] carries the advertisements instead.
-	/// Exactly one of the two is live, which is what keeps the peer from hearing a
-	/// namespace twice.
+	/// On draft 16 and later, an explicit subscription also fills its requested
+	/// namespaces even when this unsolicited loop is active.
 	pub async fn run_publish_namespaces(self) -> Result<(), Error> {
 		if self.requires_solicitation().await {
 			return Ok(());
@@ -2065,8 +2065,9 @@ where
 	///
 	/// All the announce state is local to this task (mirroring `lite::Publisher`'s
 	/// announce handling): whatever this subscription advertised is withdrawn
-	/// when its stream ends. It only advertises anything when the peer asked to be told
-	/// on request; otherwise [`Self::run_publish_namespaces`] has already said it all.
+	/// when its stream ends. Drafts 14 and 15 suppress visible namespaces already
+	/// carried by [`Self::run_publish_namespaces`]; later drafts fill the request
+	/// independently of unsolicited advertisements.
 	async fn run_subscribe_namespace_stream(
 		self,
 		mut stream: Stream<S, Version>,
@@ -2138,15 +2139,16 @@ where
 		let declared = self.peer_setup.get().await.hidden;
 		let origin = origin.discovery(!declared || msg.hidden || self.origin.includes_hidden());
 
-		// Unless the peer asked to be told only on request, it has already heard what an
-		// unsolicited PUBLISH_NAMESPACE can say. Repeating it here would leave it holding
-		// two sources for one namespace, so this stream carries only what that loop hid
-		// from the empty prefix and this request may see, and otherwise simply stays open
-		// until the peer is done with it.
-		let origin = match self.requires_solicitation().await {
-			true => origin,
-			false if !declared || self.origin.includes_hidden() => origin.empty(),
-			false => origin.beyond(&self.origin.clone().discovery(false)),
+		// NAMESPACE is discovery, so draft 16+ fills the request even after an unsolicited
+		// PUBLISH_NAMESPACE. Earlier drafts only send advertisements the other loop hid.
+		let origin = if matches!(self.version, Version::Draft14 | Version::Draft15) {
+			match self.requires_solicitation().await {
+				true => origin,
+				false if !declared || self.origin.includes_hidden() => origin.empty(),
+				false => origin.beyond(&self.origin.clone().discovery(false)),
+			}
+		} else {
+			origin
 		};
 
 		let ns = Namespaces::new(peer, target);
@@ -5431,13 +5433,13 @@ mod tests {
 	}
 
 	/// A hidden namespace reaches only a subscription that opted in or named its dot
-	/// segment. With the unsolicited loop live, the subscription stream carries just
-	/// what that loop hid, so nothing is advertised twice.
+	/// segment. Modern subscriptions include visible namespaces even when the
+	/// unsolicited loop also advertises them; drafts 14 and 15 suppress duplicates.
 	#[tokio::test]
 	async fn hidden_namespaces_need_an_opt_in() {
 		for (solicit, prefix, hidden, cam, stats) in [
-			(Some(false), "", false, 1, 0),
-			(Some(false), "", true, 1, 1),
+			(Some(false), "", false, 2, 0),
+			(Some(false), "", true, 2, 1),
 			(Some(false), ".stats", false, 1, 1),
 			(Some(true), "", false, 1, 0),
 			(Some(true), "", true, 1, 1),
@@ -5479,7 +5481,16 @@ mod tests {
 								scoped,
 							})
 							.await;
-							let expected = usize::from(!declared || hidden || prefix == ".stats");
+							let on_stream = !declared || hidden || prefix == ".stats";
+							let unsolicited = solicit != Some(true) && !declared;
+							let legacy = matches!(version, Version::Draft14 | Version::Draft15);
+							let inline = if legacy {
+								(solicit == Some(true) && on_stream)
+									|| (solicit != Some(true) && declared && (hidden || prefix == ".stats"))
+							} else {
+								on_stream
+							};
+							let expected = usize::from(inline) + usize::from(unsolicited);
 							assert_eq!(
 								occurrences(&log, b"node"),
 								expected,
@@ -5492,20 +5503,24 @@ mod tests {
 		}
 	}
 
-	/// The regression that made announces solicited in the first place: a namespace sent
-	/// as both PUBLISH_NAMESPACE and NAMESPACE leaves the peer holding two sources for
-	/// one broadcast, and whichever arrives second replaces the one the first attached.
-	/// The peer's SETUP picks which loop carries it, so the other stays quiet and the
-	/// namespace goes out exactly once either way.
-	#[tokio::test]
-	async fn each_namespace_is_advertised_exactly_once() {
-		let (unsolicited, streams) = advertise_both_ways(Some(false)).await;
-		assert_eq!(unsolicited, 1, "a peer that required nothing is told once");
-		assert_eq!(streams, 2, "on its own PUBLISH_NAMESPACE request");
-
-		let (solicited, streams) = advertise_both_ways(Some(true)).await;
-		assert_eq!(solicited, 1, "a peer that asked to be told on request is told once");
-		assert_eq!(streams, 1, "inline on the SUBSCRIBE_NAMESPACE stream it asked on");
+	/// Namespace discovery on a request stream also reaches non-SOLICIT peers from draft 16.
+	#[tokio::test(start_paused = true)]
+	async fn a_non_solicit_peer_hears_a_namespace_both_ways() {
+		let (times, streams) = advertise_both_ways(Some(false)).await;
+		assert_eq!(times, 2, "PUBLISH_NAMESPACE and NAMESPACE");
+		assert_eq!(streams, 2, "one subscription and one PUBLISH_NAMESPACE request");
+		let (once, streams) = advertise_both_ways(Some(true)).await;
+		assert_eq!(once, 1);
+		assert_eq!(streams, 1);
+		for version in [Version::Draft14, Version::Draft15] {
+			let log = advertise_with_hidden(Discovery {
+				version,
+				solicit: Some(false),
+				..Discovery::default()
+			})
+			.await;
+			assert_eq!(occurrences(&log, b"cam"), 1, "{version}");
+		}
 	}
 
 	/// A peer out of stream credit parks the open. That must not wedge the loop, because