Cleanup: trim draft-20 comments, consolidate tests (no behaviour change)

pkg/moqt/message/datastream.go

@@ -9,8 +9,7 @@ import (
 
 // UnknownDataStreamTypeError is returned when the leading Type of an inbound
 // data uni-stream is not one of the recognized data-stream types. It is
-// session-fatal (§3.4 "An endpoint that receives an unknown stream type MUST
-// close the session"): session.AcceptDataStream closes the session with
+// session-fatal (§3.4): session.AcceptDataStream closes the session with
 // PROTOCOL_VIOLATION before returning it.
 type UnknownDataStreamTypeError struct {
 	Type uint64
@@ -23,9 +22,7 @@ func (e *UnknownDataStreamTypeError) Error() string {
 // ReservedSubgroupIDModeError is returned when the leading Type of an inbound
 // data uni-stream matches the SUBGROUP_HEADER pattern (bit 4 set, bit 7 clear)
 // but carries the reserved SUBGROUP_ID_MODE value 0b11 in bits 1-2. Per
-// §11.4.2, this MUST be treated as a session-level PROTOCOL_VIOLATION. (A truly
-// unknown stream type is session-fatal too, §3.4; the distinct type only
-// names the cause.)
+// §11.4.2, this MUST be treated as a session-level PROTOCOL_VIOLATION.
 type ReservedSubgroupIDModeError struct {
 	Type uint64
 }

pkg/moqt/message/fetch_object.go

@@ -21,9 +21,9 @@ type FetchObject struct {
 	// Only present on the wire when the mode is FetchSubgroupIDExplicit (0x03).
 	SubgroupID uint64
 
-	// ObjectIDDelta is the delta from the previous Object ID, added with no
-	// +1 — or the absolute Object ID on the first object and whenever
-	// GroupIDDelta is present (§11.4.4.1). Present when the
+	// ObjectIDDelta is the delta from the previous Object ID (no +1), or the
+	// absolute Object ID on the first object and whenever GroupIDDelta is
+	// present (§11.4.4.1). Present when the
 	// FetchFlagObjectIDDelta bit (0x04) is set.
 	ObjectIDDelta uint64
 

pkg/moqt/message/fill.go

@@ -55,8 +55,7 @@ func FillParametersFromParam(ps Parameters) (inner Parameters, ok bool, err erro
 				"moqt/message: %s not allowed inside FILL_PARAMETERS (PROTOCOL_VIOLATION §10.2.15)", ip.Type)
 		}
 	}
-	// A separate parameter scope, "encoded as if they were Parameters for a
-	// separate message" (§10.2.15), so §10.2's duplicate rule applies.
+	// A separate message's parameters (§10.2.15): §10.2's duplicate rule applies.
 	if t, dup := inner.firstDuplicate(); dup {
 		return nil, true, fmt.Errorf("moqt/message: duplicate %s inside FILL_PARAMETERS (PROTOCOL_VIOLATION §10.2)", t)
 	}

pkg/moqt/message/goaway_test.go

@@ -32,9 +32,8 @@ func TestGoawayOversizedURIRejected(t *testing.T) {
 	}
 }
 
-// TestGoawayHugeURILengthRejected feeds a GOAWAY whose New Session URI length
-// is 2^63 — a valid draft-20 varint (§1.4.1) that overflows int. Parse must
-// return an error rather than panic, since any peer can send this frame.
+// TestGoawayHugeURILengthRejected: a 2^63 URI length is a valid varint
+// (§1.4.1) that overflows int; Parse must error, not panic.
 func TestGoawayHugeURILengthRejected(t *testing.T) {
 	w := wire.NewWriter(nil)
 	w.Varint(1 << 63)

pkg/moqt/message/message_bench_test.go

@@ -136,6 +136,7 @@ func (b *rwBuffer) Reset() {
 // import collision with the rest of the package's test files.
 var errEOF = rwEOF{}
 
+// rwEOF is the error type behind errEOF.
 type rwEOF struct{}
 
 func (rwEOF) Error() string { return "EOF" }

pkg/moqt/message/object_properties.go

@@ -8,25 +8,14 @@ import (
 )
 
 // CheckObjectProperties reports whether an Object's raw Properties make its
-// track malformed (§2.4.2) by any rule decidable from the Object alone:
+// track malformed (§2.4.2) by a rule decidable from the Object alone: an
+// unparsable pair or nested Immutable Properties (§12.7), a repeated Prior
+// Group/Object ID Gap or one exceeding the Object's ID (§12.8, §12.9), or a
+// Mandatory Track Property (§2.5.1). A repeated Immutable Properties is
+// treated as malformed too, an interpretation: §12.7 forbids it but does not
+// list it as malformed. Rules that need earlier Objects are not checked.
 //
-//   - a Key-Value-Pair that cannot be parsed, in the mutable list or inside
-//     Immutable Properties (§12.7);
-//   - Immutable Properties inside Immutable Properties (§12.7);
-//   - more than one Immutable Properties. §12.7 says only "An Object MUST NOT
-//     contain more than one instance of this property", outside its list of
-//     malformed conditions; treating it as malformed is this package's reading
-//     of §2.4.2's non-exhaustive list;
-//   - more than one Prior Group ID Gap or Prior Object ID Gap, counting both
-//     lists, or one larger than the Object's Group ID / Object ID (§12.8,
-//     §12.9);
-//   - a Mandatory Track Property used as an Object Property (§2.5.1).
-//
-// The §12.8 / §12.9 rules that need earlier Objects — a gap covering an
-// Object already received, an Object inside a gap already communicated,
-// differing Prior Group ID Gaps within a Group — are not checked.
-//
-// It runs once per Object, so it walks the pairs without allocating.
+// Must not allocate: per-Object path.
 func CheckObjectProperties(raw []byte, groupID, objectID uint64) error {
 	var c objectPropertiesCheck
 	if err := c.walk(raw, false); err != nil {
@@ -65,7 +54,7 @@ func (c *objectPropertiesCheck) walk(raw []byte, nested bool) error {
 				return errors.New("moqt/message: Immutable Properties inside Immutable Properties (§12.7)")
 			}
 			if c.immutables++; c.immutables > 1 {
-				// An interpretation: see CheckObjectProperties.
+				// An interpretation, see CheckObjectProperties.
 				return fmt.Errorf("moqt/message: Immutable Properties: %w (§12.7, §2.4.2)", errTooManyInstances)
 			}
 			if err := c.walk(kv.ByteVal, true); err != nil {

pkg/moqt/message/param_scope.go

@@ -8,16 +8,13 @@ import (
 	"strings"
 )
 
-// ErrUnknownParameter is wrapped by the parse error for a Message Parameter
-// type this version does not define. §10.2: an endpoint that receives one
-// "MUST close the session with PROTOCOL_VIOLATION".
+// ErrUnknownParameter is wrapped by the parse error for an undefined Message
+// Parameter type, a PROTOCOL_VIOLATION (§10.2).
 var ErrUnknownParameter = errors.New("moqt/message: unknown parameter type")
 
-// ParamScope is the message form a parameter block arrived in, as the
-// per-parameter scope rules of §10.2.1 tell them apart. A REQUEST_OK takes its
-// form from the request it answers (§10.5: "PUBLISH_OK, REQUEST_UPDATE_OK,
-// TRACK_STATUS_OK, ..."), and a REQUEST_UPDATE from the request it updates.
-// Values are bit flags so a parameter's allowed forms are one mask.
+// ParamScope is the message form a parameter block arrived in (§10.2.1). A
+// REQUEST_OK takes its form from the request it answers (§10.5), and a
+// REQUEST_UPDATE from the request it updates. Values are bit flags.
 type ParamScope uint32
 
 const (
@@ -37,10 +34,9 @@ const (
 	ScopePublishNamespaceOK
 	ScopePublishStateNotify
 	ScopeRequestUpdateOK
-	// ScopeUpdateFromSubscriber is a REQUEST_UPDATE for a subscription —
-	// established by SUBSCRIBE or PUBLISH — sent by its subscriber;
-	// ScopeUpdateFromPublisher one sent by its publisher, on a PUBLISH it
-	// sent. §5.1.4 allows the Range Filters only "from the subscriber".
+	// ScopeUpdateFromSubscriber is a REQUEST_UPDATE on a subscription sent by
+	// its subscriber, ScopeUpdateFromPublisher one sent by the publisher of a
+	// PUBLISH. §5.1.4 allows the Range Filters only "from the subscriber".
 	ScopeUpdateFromSubscriber
 	ScopeUpdateFromPublisher
 	ScopeUpdateFetch
@@ -54,13 +50,8 @@ const (
 	scopeUpdateSubscription = ScopeUpdateFromSubscriber | ScopeUpdateFromPublisher
 	scopeAnyUpdate          = scopeUpdateSubscription | ScopeUpdateFetch | ScopeUpdateTrackStatus |
 		ScopeUpdateSubscribeNamespace | ScopeUpdateSubscribeTracks | ScopeUpdatePublishNamespace
-	// §5.1.4: "All other filter parameters MAY appear multiple times in a
-	// FETCH, SUBSCRIBE, SUBSCRIBE_TRACKS, or REQUEST_UPDATE (on a
-	// subscription, from the subscriber only) message", and the Track
-	// Property filter in "a SUBSCRIBE_TRACKS message or REQUEST_UPDATE for
-	// it". Its opening sentence names SUBSCRIBE, FETCH and SUBSCRIBE_TRACKS
-	// for all five, so all five share one scope here; the text is ambiguous,
-	// and the reading that closes fewer sessions was chosen.
+	// §5.1.4 is ambiguous on where each filter may appear; all five share the
+	// widest reading, which closes fewer sessions.
 	scopeRangeFilter = ScopeSubscribe | ScopeFetch | ScopeSubscribeTracks |
 		ScopeUpdateFromSubscriber | ScopeUpdateSubscribeTracks
 )
@@ -227,18 +218,14 @@ func (e *ParamScopeError) Error() string {
 	return fmt.Sprintf("moqt/message: %s not allowed in %s (PROTOCOL_VIOLATION §10.2.1)", e.Type, e.Scope)
 }
 
-// CheckScope reports the first parameter of ps that may not appear in a
-// message of the given scope, or that repeats where its definition does not
-// allow it (see [Parameters.firstDuplicate]). A FILL_PARAMETERS value is a
-// scope of its own (§10.2.15) and is checked against its table too, and an
-// INCLUDE_PROPERTIES value must be 0 or 1 (§10.2.21). Every error is a
-// session-level PROTOCOL_VIOLATION.
+// CheckScope reports the first parameter of ps not allowed in a message of the
+// given scope (§10.2.1) or repeated where it may not be (§10.2), and validates
+// the FILL_PARAMETERS (§10.2.15) and INCLUDE_PROPERTIES (§10.2.21) values.
+// Every error is a session-level PROTOCOL_VIOLATION.
 func (ps Parameters) CheckScope(scope ParamScope) error {
 	for _, p := range ps {
 		allowed := paramScopes[p.Type]
-		// §10.20.1: "Any Parameter that can be specified on a Subscription
-		// (ie: in SUBSCRIBE) is valid in SUBSCRIBE_TRACKS, unless otherwise
-		// specified." They become the subscriptions' initial parameters.
+		// §10.20.1: SUBSCRIBE parameters are valid in SUBSCRIBE_TRACKS.
 		if allowed&ScopeSubscribe != 0 {
 			allowed |= ScopeSubscribeTracks
 		}
@@ -258,12 +245,9 @@ func (ps Parameters) CheckScope(scope ParamScope) error {
 	return nil
 }
 
-// firstDuplicate reports the first parameter type that appears more than once
-// in ps where its definition does not allow it (§10.2: "Senders MUST NOT
-// repeat the same Parameter Type in a message unless the parameter definition
-// explicitly allows multiple instances"). The Range Filters may repeat
-// (§5.1.4), and so may AUTHORIZATION_TOKEN (§10.2.2: it "MAY be repeated
-// within a message").
+// firstDuplicate reports the first parameter type repeated in ps where its
+// definition does not allow it (§10.2). The Range Filters (§5.1.4) and
+// AUTHORIZATION_TOKEN (§10.2.2) may repeat.
 func (ps Parameters) firstDuplicate() (ParamID, bool) {
 	for i, p := range ps {
 		if IsRangeFilterParam(p.Type) || p.Type == ParamAuthorizationToken {

pkg/moqt/message/params.go

@@ -30,12 +30,9 @@ const (
 	ParamLocationFilter          ParamID = 0x21
 	ParamGroupOrder              ParamID = 0x22
 	ParamFillParameters          ParamID = 0x23
-	// Range Filter parameters (§5.1.4, §10.2.10-14). All five carry a
-	// length-prefixed blob (SetID, optional Property Type, delta-encoded
-	// Ranges) — see rangefilter.go. Message Parameters are not Key-Value-Pairs:
-	// "The encoding is specified by each parameter definition" (§10.2), so type
-	// parity says nothing about them, and all five are length-prefixed as
-	// §5.1.4's figures show (KindBytes, in paramKinds).
+	// Range Filter parameters (§5.1.4, §10.2.10-14), see rangefilter.go. All
+	// five are length-prefixed (KindBytes) whatever their type parity: Message
+	// Parameter encodings are per definition (§10.2), not Key-Value-Pairs.
 	ParamSubgroupFilter       ParamID = 0x25
 	ParamObjectIDFilter       ParamID = 0x26
 	ParamPriorityFilter       ParamID = 0x27
@@ -368,11 +365,8 @@ func (ps *Parameters) parse(r *wire.Reader) error {
 	if err != nil {
 		return err
 	}
-	// count is an untrusted varint (up to 2^64-1, §1.4.1); never preallocate from it
-	// directly or a crafted message triggers an out-of-range makeslice panic.
-	// Each parameter occupies at least one byte on the wire (its type-delta
-	// varint), so the real count cannot exceed the remaining bytes — the loop
-	// surfaces a truncated count as a read error.
+	// count is untrusted (up to 2^64-1, §1.4.1): cap the preallocation by the
+	// remaining bytes, since each parameter takes at least one.
 	//nolint:gosec // G115: Reader.Remaining() = len(buf)-off is always >= 0.
 	out := make(Parameters, 0, min(count, uint64(r.Remaining())))
 	var prev uint64