fix(moq-net): bound frame size in create_frame/append_frame

[kixelated]

Summary

Bound the declared frame size at the GroupProducer level so an oversized frame is rejected before FrameProducer::new preallocates its buffer.

• create_frame rejects size > MAX_FRAME_SIZE with Error::FrameTooLarge before produce() allocates.
• append_frame carries the same check as a backstop for direct callers (the buffer is already allocated by then, but it keeps an oversized frame out of the group cache).
• MAX_FRAME_SIZE doc updated to describe where the limit is enforced and the remaining FrameProducer::new gap.
• Test: append_rejects_oversized_frame (over the limit rejected, at the limit accepted).

This is non-breaking: create_frame / append_frame already return Result, and FrameTooLarge is an existing #[non_exhaustive] variant. It guarantees the limit for local callers, not just the wire-decode paths (which already checked).

Relationship to other PRs

• Extracted verbatim from #1870 (the moqsink GstAggregator rewrite), which currently bundles this guard. The frame.rs / group.rs hunks here are byte-identical to moq-gst: replace moqsink with a GstAggregator-based core #1870's, so once this lands moq-gst: replace moqsink with a GstAggregator-based core #1870 can rebase and those hunks drop out cleanly. moq-gst: replace moqsink with a GstAggregator-based core #1870 should drop its copy (or just rebase) to avoid a conflict.
• The breaking follow-up that makes FrameProducer::new itself fallible (closing the direct-constructor gap, the true single chokepoint) is #1881, on dev. This PR is the non-breaking guard for main.

Test plan

Run via nix develop:

• just rs check (check, clippy -D warnings, fmt --check, doc -D warnings, shear, sort)
• cargo test -p moq-net — 347 unit + doctests pass, incl. new append_rejects_oversized_frame
• cargo test -p moq-native -p hang -p moq-mux -p moq-relay — all pass

Cross-package sync

None needed: the wire format and public API signatures are unchanged.

(Written by Claude)

[coderabbitai]

Walkthrough

MAX_FRAME_SIZE is now imported in group.rs and used to add early-exit guards in GroupProducer::create_frame and GroupProducer::append_frame, both returning Error::FrameTooLarge when the frame size exceeds the limit. Documentation for MAX_FRAME_SIZE in frame.rs is updated to describe where enforcement currently occurs and to note that closing the remaining gap in FrameProducer::new would require a breaking, fallible constructor. A unit test verifies that create_frame rejects sizes above the limit and accepts the boundary value.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: bounding frame size validation in GroupProducer methods to prevent oversized frame allocation.
Description check	✅ Passed	The description is directly related to the changeset, providing detailed context about frame size bounds, validation checks, test coverage, and relationship to other PRs.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch claude/frame-size-guard

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

rs/moq-net/src/model/group.rs (1)

454-467: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a direct append_frame oversize-path assertion.

The new test only exercises create_frame; it doesn’t validate the direct-caller backstop added in append_frame. Add a case that builds a FrameProducer with size = MAX_FRAME_SIZE + 1 and calls append_frame to lock this contract down.

Proposed test extension

 #[test]
 fn append_rejects_oversized_frame() {
 	let mut producer = Group { sequence: 0 }.produce();
 	let err = producer.create_frame(Frame {
 		size: MAX_FRAME_SIZE + 1,
 	});
 	assert!(
 		matches!(err, Err(Error::FrameTooLarge)),
 		"a frame over the limit is rejected"
 	);
 	// A frame at the limit is still accepted.
 	assert!(producer.create_frame(Frame { size: MAX_FRAME_SIZE }).is_ok());
+
+	// Direct append callers are also rejected by the append backstop.
+	let direct = Frame { size: MAX_FRAME_SIZE + 1 }.produce();
+	assert!(matches!(
+		producer.append_frame(direct),
+		Err(Error::FrameTooLarge)
+	));
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rs/moq-net/src/model/group.rs` around lines 454 - 467, The test
`append_rejects_oversized_frame` currently only validates the `create_frame`
method path. Add an additional assertion within this test that directly invokes
the `append_frame` method with a FrameProducer containing a frame where size
equals MAX_FRAME_SIZE + 1, and verify that it correctly returns an error
matching Error::FrameTooLarge. This ensures the backstop validation in the
append_frame method itself is properly tested and locked down.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@rs/moq-net/src/model/group.rs`:
- Around line 454-467: The test `append_rejects_oversized_frame` currently only
validates the `create_frame` method path. Add an additional assertion within
this test that directly invokes the `append_frame` method with a FrameProducer
containing a frame where size equals MAX_FRAME_SIZE + 1, and verify that it
correctly returns an error matching Error::FrameTooLarge. This ensures the
backstop validation in the append_frame method itself is properly tested and
locked down.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 384c5eea-ae9f-4ed4-83e9-ce494068e4ce

📥 Commits

Reviewing files that changed from the base of the PR and between a0f0f4a and a01cbf4.

📒 Files selected for processing (2)

• rs/moq-net/src/model/frame.rs
• rs/moq-net/src/model/group.rs