fix: close the session on the draft-20 protocol violations from the compliance backlog

pkg/moqt/message/fill.go

@@ -1,6 +1,7 @@
 package message
 
 import (
+	"errors"
 	"fmt"
 	"slices"
 
@@ -47,7 +48,13 @@ func FillParametersFromParam(ps Parameters) (inner Parameters, ok bool, err erro
 		return nil, false, nil
 	}
 	if err := inner.parse(wire.NewReader(p.Bytes)); err != nil {
-		return nil, true, fmt.Errorf("moqt/message: FILL_PARAMETERS: %w", err)
+		// §10.2: an unknown parameter, or a Type past 2^64-1, is a
+		// PROTOCOL_VIOLATION here as in any message; anything else is a
+		// value that does not parse (§1.4.3).
+		if errors.Is(err, ErrUnknownParameter) || errors.Is(err, errParamTypeOverflow) {
+			return nil, true, fmt.Errorf("moqt/message: FILL_PARAMETERS: %w", err)
+		}
+		return nil, true, fmt.Errorf("%w: FILL_PARAMETERS: %w", ErrValueFormatting, err)
 	}
 	for _, ip := range inner {
 		if !slices.Contains(fillParamsAllowed, ip.Type) {

pkg/moqt/message/filter.go

@@ -1,6 +1,7 @@
 package message
 
 import (
+	"errors"
 	"fmt"
 	"math"
 
@@ -78,13 +79,16 @@ func (f *LocationFilter) Validate() error {
 		return fmt.Errorf("moqt/message: LOCATION_FILTER has %d fields, want 0-4 (§5.1.2)", f.Fields)
 	}
 	if f.HasEnd() && f.StartGroup > math.MaxUint64-f.EndGroupDelta {
-		return fmt.Errorf(
-			"moqt/message: LOCATION_FILTER end group overflow (start=%d delta=%d) (PROTOCOL_VIOLATION §5.1.2)",
-			f.StartGroup, f.EndGroupDelta)
+		return fmt.Errorf("%w (start=%d delta=%d)", errEndGroupOverflow, f.StartGroup, f.EndGroupDelta)
 	}
 	return nil
 }
 
+// errEndGroupOverflow is a LOCATION_FILTER whose StartGroup + EndGroupDelta
+// exceeds 2^64 - 1: a PROTOCOL_VIOLATION (§5.1.2), unlike a value that does
+// not parse ([ErrValueFormatting]).
+var errEndGroupOverflow = errors.New("moqt/message: LOCATION_FILTER end group overflow (PROTOCOL_VIOLATION §5.1.2)")
+
 // Start resolves the first Location that passes the filter, given the
 // publisher's current Largest Object. hasLargest is false before anything has
 // been published on the track, which §5.1.2 pins to {0, 0}.

pkg/moqt/message/param_scope.go

@@ -218,10 +218,18 @@ func (e *ParamScopeError) Error() string {
 	return fmt.Sprintf("moqt/message: %s not allowed in %s (PROTOCOL_VIOLATION §10.2.1)", e.Type, e.Scope)
 }
 
+// ErrValueFormatting marks a parameter whose value does not match the
+// serialization its type defines. §1.4.3: the receiver "MUST close the session
+// with error code KEY_VALUE_FORMATTING_ERROR".
+var ErrValueFormatting = errors.New("moqt/message: value does not match its type's serialization")
+
 // 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.
+// the values a receiver must close the session for, in ps and inside its
+// FILL_PARAMETERS (§10.2.15): GROUP_ORDER (§10.2.8), FORWARD (§10.2.18),
+// INCLUDE_PROPERTIES (§10.2.21) and LOCATION_FILTER (§5.1.2). Every error is a
+// session-level PROTOCOL_VIOLATION, except one wrapping [ErrValueFormatting],
+// a KEY_VALUE_FORMATTING_ERROR.
 func (ps Parameters) CheckScope(scope ParamScope) error {
 	for _, p := range ps {
 		allowed := paramScopes[p.Type]
@@ -236,13 +244,35 @@ func (ps Parameters) CheckScope(scope ParamScope) error {
 	if t, dup := ps.firstDuplicate(); dup {
 		return &ParamScopeError{Type: t, Scope: scope, Duplicate: true}
 	}
-	if _, _, err := FillParametersFromParam(ps); err != nil {
+	inner, _, err := FillParametersFromParam(ps)
+	if err != nil {
 		return err
 	}
+	if err := inner.checkValues(); err != nil {
+		return fmt.Errorf("FILL_PARAMETERS: %w", err)
+	}
+	return ps.checkValues()
+}
+
+// checkValues validates the values in ps a receiver must close the session
+// for; see [Parameters.CheckScope].
+func (ps Parameters) checkValues() error {
+	if p, ok := ps.Find(ParamGroupOrder); ok &&
+		GroupOrder(p.Byte) != GroupOrderAscending && GroupOrder(p.Byte) != GroupOrderDescending {
+		return fmt.Errorf("moqt/message: GROUP_ORDER value %d outside {1,2} (PROTOCOL_VIOLATION §10.2.8)", p.Byte)
+	}
+	if p, ok := ps.Find(ParamForward); ok && p.Byte > 1 {
+		return fmt.Errorf("moqt/message: FORWARD value %d outside {0,1} (PROTOCOL_VIOLATION §10.2.18)", p.Byte)
+	}
 	if _, err := IncludePropertiesFromParam(ps); err != nil {
 		return err
 	}
-	return nil
+	switch _, err := LocationFilterFromParam(ps); {
+	case err == nil, errors.Is(err, errEndGroupOverflow):
+		return err
+	default:
+		return fmt.Errorf("%w: LOCATION_FILTER: %w", ErrValueFormatting, err)
+	}
 }
 
 // firstDuplicate reports the first parameter type repeated in ps where its

pkg/moqt/message/params.go

@@ -358,6 +358,10 @@ func (ps Parameters) append(w *wire.Writer) {
 	}
 }
 
+// errParamTypeOverflow is a parameter Type delta taking the Type past 2^64-1:
+// a PROTOCOL_VIOLATION (§10.2), unlike a value that does not parse.
+var errParamTypeOverflow = errors.New("moqt/message: parameter type delta overflow")
+
 // parse reads a Number-of-Parameters varint followed by that many parameters
 // from r.
 func (ps *Parameters) parse(r *wire.Reader) error {
@@ -376,7 +380,7 @@ func (ps *Parameters) parse(r *wire.Reader) error {
 			return err
 		}
 		if delta > ^uint64(0)-prev {
-			return errors.New("moqt/message: parameter type delta overflow")
+			return errParamTypeOverflow
 		}
 		t := prev + delta
 		p := Parameter{Type: ParamID(t)}

pkg/moqt/message/properties.go

@@ -195,6 +195,29 @@ func AppendTrackProperties(pairs []wire.KVPair) []byte {
 	return w.Bytes()
 }
 
+// CheckTrackPropertyValues reports a Track Property in pairs (Immutable
+// Properties expanded, see [ExpandImmutable]) whose value the draft makes
+// session-fatal: DEFAULT_PUBLISHER_GROUP_ORDER outside {1, 2} (§12.5) or
+// DYNAMIC_GROUPS above 1 (§12.6). The caller MUST close the session with
+// PROTOCOL_VIOLATION.
+func CheckTrackPropertyValues(pairs []wire.KVPair) error {
+	for _, kv := range pairs {
+		switch kv.Type {
+		case PropertyDefaultPublisherGroupOrder:
+			if kv.IntVal != uint64(GroupOrderAscending) && kv.IntVal != uint64(GroupOrderDescending) {
+				return fmt.Errorf(
+					"moqt/message: DEFAULT_PUBLISHER_GROUP_ORDER value %d outside {1,2} (PROTOCOL_VIOLATION §12.5)",
+					kv.IntVal)
+			}
+		case PropertyDynamicGroups:
+			if kv.IntVal > 1 {
+				return fmt.Errorf("moqt/message: DYNAMIC_GROUPS value %d above 1 (PROTOCOL_VIOLATION §12.6)", kv.IntVal)
+			}
+		}
+	}
+	return nil
+}
+
 // FirstUnknownMandatoryTrackProperty returns the first Mandatory Track
 // Property (range 0x4000–0x7FFF) in pairs whose type is not in knownTypes,
 // and whether one was found — the offending type is what callers need to