diff --git a/STATUS.md b/STATUS.md index d325b5fc..6ef3b075 100644 --- a/STATUS.md +++ b/STATUS.md @@ -62,8 +62,8 @@ By package, bottom-up along the dependency stack: - **`relay`** — routes objects through a track registry with per-subscription live fanout under a §8 slow-reader policy, merges multiple upstream publishers per track (§9.5) with §2.1 {Group, Object} dedup and survivor-continues - failover, serves FETCHes from a per-track cache (stitching evicted ranges from - an upstream FETCH), issues on-demand upstream SUBSCRIBEs to every matching + failover, serves FETCHes from a per-track cache (asking an upstream FETCH + about what the cache cannot vouch for), issues on-demand upstream SUBSCRIBEs to every matching publisher (local and, via a `DiscoveryStore` + `Dialer`, remote), reflects remote namespaces to local subscribers, gates requests through an `Authorizer` hook, emits telemetry through a `Metrics` hook, and drains sessions with @@ -108,9 +108,9 @@ By package, bottom-up along the dependency stack: | § | Feature | Status | Notes | |---------|----------------------------------|--------|-------| | 5.1 | Subscriptions | DONE | Subscribe/Publish/OK/Error state machine in `pubsub.go`. | -| 5.1.1 | Subscription state management | DONE | REQUEST_ERROR / STOP_SENDING / PUBLISH_DONE handling + cleanup. | +| 5.1.1 | Subscription state management | DONE | REQUEST_ERROR / STOP_SENDING / PUBLISH_DONE handling + cleanup. The relay resets a cancelled subscription's open subgroup and fill streams. | | 5.1.2 | Location filters | DONE | Every start/end form (unfiltered, Next Object, relative and absolute start, absolute range) + `Matches`. | -| 5.1.3 | Fill semantics | PARTIAL | Fill fetch streams from FILL_PARAMETERS on SUBSCRIBE / REQUEST_UPDATE (`handler_fill.go`), and on SUBSCRIBE_TRACKS, one per forwarded PUBLISH's subscription, keyed to the PUBLISH's Request ID (§10.1). A fill inherits the subscription's Range Filters; the ones inside FILL_PARAMETERS override per type. Not done: scheduling fills against their subscription (§7.2, see Limitations) and resetting open fills when the subscription is cancelled (§5.1.3.1). | +| 5.1.3 | Fill semantics | PARTIAL | Fill fetch streams from FILL_PARAMETERS on SUBSCRIBE / REQUEST_UPDATE (`handler_fill.go`), and on SUBSCRIBE_TRACKS, one per forwarded PUBLISH's subscription, keyed to the PUBLISH's Request ID (§10.1). A fill inherits the subscription's Range Filters; the ones inside FILL_PARAMETERS override per type. A cancelled subscription's open fills are reset (§5.1.3.1). Not done: scheduling fills against their subscription (§7.2, see Limitations). | | 5.1.4 | Range filters | DONE | Object filters (SUBGROUP/OBJECTID/PRIORITY/OBJECT_PROPERTY) enforced on SUBSCRIBE fanout, datagrams, and FETCH; TRACK_PROPERTY_FILTER gates PUBLISH forwarding on SUBSCRIBE_TRACKS; `MAX_FILTER_RANGES`/`INVALID_FILTER` gating in place. Object filters on a SUBSCRIBE_TRACKS apply to the subscriptions its forwarded PUBLISHes open. A zero-length filter is no filter; a subscription REQUEST_UPDATE replaces (or, zero-length, removes) the filter types it names and keeps the others. | | 5.1.5 | Combining filters | DONE | `ForwardDecision` ANDs Forward + Location + Range filters per object (§5.1.5); Range filters combine SetIDs via AND/OR. | | 5.1.6 | Joining an ongoing track | DONE | A Location Filter plus FILL_PARAMETERS, served as a fill fetch stream (draft-20 removed the Joining FETCH). | @@ -163,7 +163,7 @@ By package, bottom-up along the dependency stack: | 10.2.2 | AUTHORIZATION_TOKEN | 0x03 | DONE | 4 alias types; session token cache resolves inbound. | | 10.2.3 | SUBGROUP_DELIVERY_TIMEOUT | 0x06 | PARTIAL| Parsed and resolved; the stream reset is not enforced on the bundled transports, and datagrams are not dropped (see §8). | | 10.2.4 | OBJECT_DELIVERY_TIMEOUT | 0x02 | DONE | | -| 10.2.5 | FILL_TIMEOUT | 0x0A | DONE | | +| 10.2.5 | FILL_TIMEOUT | 0x0A | DONE | The budget for a FETCH's or fill's upstream FETCH, its response included: when it runs out, what arrived is served and the rest is an End of Timed-Out Range; 0 asks no upstream. Default 5s. | | 10.2.6 | RENDEZVOUS_TIMEOUT | 0x04 | DONE | | | 10.2.7 | SUBSCRIBER_PRIORITY | 0x20 | DONE | | | 10.2.8 | GROUP_ORDER | 0x22 | DONE | Ascending/Descending validated. | @@ -197,7 +197,7 @@ By package, bottom-up along the dependency stack: | 10.10 | PUBLISH_STATE_NOTIFY | 0x22 | DONE | Only the publisher may send it; enforced by brokers and the relay. | | 10.11 | PUBLISH | 0x1D | DONE | | | 10.12 | PUBLISH_DONE | 0x0B | DONE | Sent once every stream of the subscription has closed and no datagram send is in progress, with the exact Stream Count; written on its own goroutine, so subscribers do not wait on each other. When a track's last upstream ends, its PUBLISH_DONE code reaches subscribers if it is about the track (TRACK_ENDED, MALFORMED_TRACK); codes about the relay's own upstream subscription become INTERNAL_ERROR. | -| 10.13 | FETCH | 0x16 | DONE | Standalone, the only kind in draft-20. | +| 10.13 | FETCH | 0x16 | DONE | Standalone, the only kind in draft-20. From the cache, a Location is non-existent only on a signal: a Prior Group or Object ID Gap, a Group's or the Track's end, or an upstream's FETCH. Other uncached Locations are FETCHed from a fetch-capable upstream in one span, within FILL_TIMEOUT, or else marked End of Unknown (or Timed-Out) Range. | | 10.14 | FETCH_OK | 0x18 | DONE | An End Location before the FETCH's Start closes the session. A Start relative to the Largest Object is compared through End ≤ Largest; an End of {0,0} is let through, as it cannot be told apart from "no content yet". | | 10.15 | TRACK_STATUS | 0x0D | DONE | Reply via REQUEST_OK, then FIN; any follow-up from the requester closes the session. | | 10.16 | PUBLISH_NAMESPACE | 0x06 | DONE | | @@ -222,7 +222,7 @@ By package, bottom-up along the dependency stack: | 11.4.3 | Closing subgroup streams | DONE | Relay forwards only the next object on a stream, otherwise reset+reopen: the next object is one ID greater, read next from the same upstream stream (only filtered-out objects between), or covered by its Prior Object ID Gap; an object the relay dropped, or one from another upstream, breaks the run. FINs on clean inbound EOF, resets on inbound reset, resets with MALFORMED_TRACK after a terminal EndOfGroup/EndOfTrack object (§2.4.2), marks reliable boundaries for RESET_STREAM_AT (`SetReliableBoundary`, transport-gated on `EnableStreamResetPartialDelivery`), and resets (not FINs) in-flight subgroups whose group falls out of range after a narrowing REQUEST_UPDATE. A subscription that skipped any Object of the Subgroup other than one before its Start Location (a filter, Forward State 0, a Start raised past Objects already sent, an inbox overflow with EXCESSIVE_LOAD, an expiry) gets resets, never a FIN, on that Subgroup's streams. Objects published before a subscription joined are treated as before its Start. | | 11.4.4 | Fetch header | DONE | Serialization Flags of 128 or more that are not an End of Range close the session. | | 11.4.4.1 | Fetch flags | DONE | All subgroup modes + delta/priority/properties/status flags. A first Object that references a prior Object's fields closes the session. | -| 11.4.4.2 | End of range | DONE | Non-existent (0x8C) / unknown (0x10C) handled. An Object after a leading marker that references a prior Subgroup ID or Priority closes the session. | +| 11.4.4.2 | End of range | DONE | Non-existent (0x8C) / unknown (0x10C) / timed-out (0x20C) handled; a marker covers the Locations after the previous element in the order the response carries them (see Limitations). An Object after a leading marker that references a prior Subgroup ID or Priority closes the session. | | 11.5 | Padding streams & datagrams | DONE | Recognised type IDs silently discarded. | ## §12 MOQT properties @@ -483,6 +483,27 @@ Known protocol gaps, roughly ordered by how load-bearing they are: reads only the mutable list, so a LOC Timestamp and the like placed inside Immutable Properties is not found. Filling the fields from there would make `Append` re-emit them in the mutable list, changing what a relay forwards. +- **A Group Order the subscriber cannot learn (§10.2.8, §10.2.21)** — a + SUBSCRIBE that omits GROUP_ORDER takes the publisher's + DEFAULT_PUBLISHER_GROUP_ORDER, and its fill fetch stream is written in that + order (§11.4.4.1). With INCLUDE_PROPERTIES=0 the subscriber gets no Track + Properties, and neither SUBSCRIBE_OK nor the FETCH_HEADER carries a Group + Order, so it cannot tell a Descending fill from an Ascending one. A gap in the + draft; a subscriber that asks for a fill avoids it by sending GROUP_ORDER. +- **FETCH End of Range markers in Descending order (§11.4.4.2)** — an + interpretation. A marker covers "Locations between the last serialized + Object, if any, and this Location"; the relay reads "between" in the order + the response carries Locations (Groups in its Group Order, Object IDs + ascending within a Group), so in Descending order a Group's unknown tail is + marked at {G, 2^64-1} after its Objects. The draft does not say which order + it means. Interop with moxygen and moqtail is unverified. +- **FETCH from the cache and publishers that do not mark Group ends (§2.1, + §10.13)** — the relay treats a Group's tail as unknown unless an + END_OF_GROUP or END_OF_TRACK status, an END_OF_GROUP bit on a FINed + subgroup, or a gap Property says where it ends. A publisher that marks none + gets an End of Unknown Range after every Group in a cached FETCH or fill, or, + behind a fetch-capable upstream, an upstream FETCH for them. Group ends known + only from a subgroup FIN are kept for the last 32 Groups. - **Fill streams are not scheduled against their subscription (§7.2 rules 3 and 4)** — a subscription-delivered Object should go first when the fill's Group Order differs, and the fill-delivered one first within a Group. The @@ -492,3 +513,103 @@ Known protocol gaps, roughly ordered by how load-bearing they are: the first copy of each {Group, Object} is forwarded and later ones are dropped unread. Comparing them would detect a malformed track (§2.4.2 condition 6), at a cost on every Object. + +### Draft-20 compliance review backlog + +A second full review against draft-ietf-moq-transport-20 (2026-09-26, at +`dbe571e`) found the gaps below. Each item names the rule it misses. Items +already listed as Limitations above are not repeated here. + +Session layer: + +- DUPLICATE_TRACK_ALIAS never closes the session: the relay answers REQUEST_ERROR + MALFORMED_TRACK, `Session.Subscribe` and `AcceptPublish` return an error (§11.1). + A session-layer subscriber also never releases an alias when its subscription + ends. +- A GOAWAY on a request stream is ignored: a second one, or one carrying a New + Session URI sent to a server, does not close the session (§10.4). +- A REQUEST_ERROR Redirect is dropped after parsing: a server receiving a Connect + URI, or a Track Name on a namespace-scoped request, does not close the session, + and the application cannot follow it (§10.6.1). +- A first response other than REQUEST_OK / REQUEST_ERROR to SUBSCRIBE_NAMESPACE or + SUBSCRIBE_TRACKS does not close the session (§10.19, §10.20). +- `Publication`'s automatic PUBLISH_DONE UPDATE_FAILED is sent while its subgroup + streams are open, `WriteObject` still succeeds after `Done`, and a subgroup + opened concurrently with `Done` is missing from the Stream Count (§10.12). +- `Publication`'s REQUEST_UPDATE_OK carries LARGEST_OBJECT only for Objects it + wrote itself, not the one its SUBSCRIBE_OK or PUBLISH reported (§10.2.17, + §10.9.1). +- `ReadPublishSkipped` does not close the session on a REQUEST_UPDATE or + PUBLISH_STATE_NOTIFY from the publisher (§10.9, §10.10), and nothing enforces + NAMESPACE_DONE-before-NAMESPACE on a namespace subscription (§10.19). +- A second SUBSCRIBE_OK is handed to the application (§5.1 SHOULD close). +- A rejected request sends STOP_SENDING with INTERNAL_ERROR (§3.3.4 SHOULD use a + relevant code). +- Mandatory Track Property enforcement is off unless configured (§2.5.1). +- FETCH Serialization Flags ≥ 128 are read as field bits before being rejected, + so a reset or oversized length avoids the PROTOCOL_VIOLATION (§11.4.4). +- SETUP options are sorted unstably, so with more than 12 the Token order on the + wire can differ from the order `heldSetupAliases` replays (§10.3.1.4). + +Validation (values that MUST close the session): + +- GROUP_ORDER on PUBLISH, inside FILL_PARAMETERS, and in `AcceptSubscribe` / + `AcceptPublish` (§10.2.8); the FETCH case is the Limitation above. +- FORWARD in PUBLISH_STATE_NOTIFY, in a publisher's REQUEST_UPDATE, and in + `AcceptPublish` (§10.2.18). +- DEFAULT_PUBLISHER_GROUP_ORDER outside {1, 2} and DYNAMIC_GROUPS above 1 in + Track Properties (§12.5, §12.6). +- A LOCATION_FILTER whose StartGroup + EndGroupDelta overflows: REQUEST_ERROR + MALFORMED_TRACK on SUBSCRIBE, REQUEST_UPDATE and SUBSCRIBE_TRACKS, INVALID_FILTER + on FETCH, a fill reset in FILL_PARAMETERS (§5.1.2). + +Relay: + +- Any REQUEST_UPDATE turns INCLUDE_PROPERTIES=0 back off, so the subscriber + resolves the wrong default Publisher Priority. §10.9: a parameter absent from + REQUEST_UPDATE "remains unchanged", and INCLUDE_PROPERTIES cannot appear in + one (§10.2.21, §12.4). +- A merged Subgroup FINs when one contributor ends cleanly although its Objects + began after ones a reset contributor never delivered (§11.4.3). +- Replay streams (joiners, gap and properties reopens) lose the first Object's + delivery-timeout override (§8, §12.1, §12.2). +- FETCH_OK never sets End Of Track (§10.14). +- A cancelled FETCH keeps writing its data stream (§5.2: "MUST reset"). +- Objects from an upstream FETCH are exempt from MAX_CACHE_DURATION, and cached + Objects age from when they were read whole rather than their beginning (§12.3). +- A fill range is evaluated against a later Largest Object than SUBSCRIBE_OK or + REQUEST_UPDATE_OK reported (§5.1.3). +- TRACK_STATUS returns DOES_NOT_EXIST for a PUBLISHed track with no properties + or Objects, which SUBSCRIBE accepts (§10.15: "treats it identically"). +- A client cannot SUBSCRIBE to a track it publishes under its own + PUBLISH_NAMESPACE (§5.1). +- RENDEZVOUS_TIMEOUT is ignored (§10.2.6 SHOULD hold the subscription; §9.5). +- REQUEST_ERROR MALFORMED_TRACK, defined for FETCH, answers SUBSCRIBE, PUBLISH + and REQUEST_UPDATE failures (§10.6.2). +- The relay keeps initiating requests on a session it sent GOAWAY to (§10.4 + SHOULD avoid), and closes with GOAWAY_TIMEOUT when it sent none (§3.5). +- A PUBLISH can follow PUBLISH_SKIPPED for the same upstream PUBLISH after a + prefix update moves away and back (§6.1). +- Upstream FETCHes to a publisher whose track is found malformed are not + cancelled (§2.4.2). +- Filters are not aggregated upstream (§6.3.1 SHOULD). +- A REQUEST_UPDATE's AUTHORIZATION_TOKENs go through the TokenVerifier only on + SUBSCRIBE_NAMESPACE and SUBSCRIBE_TRACKS; on SUBSCRIBE, FETCH and + PUBLISH_NAMESPACE they are resolved but not verified (§10.2.2). + +Documentation: + +- Limitations: "Duplicate Objects … are not compared" is stale; the FETCH + GROUP_ORDER entry omits PUBLISH and FILL_PARAMETERS; the LOC entry names + `PropAudioLevel = 0x0A` (it is 0x0C); "Handles the application reads itself" + says `CheckPeerParams` checks roles; "Inbound GOAWAY" omits request streams. +- Table rows 10.2.6, 10.2.8, 10.2.9, 10.2.15, 10.2.18, 10.2.21, 12.3, + 12.5 and 12.6 overstate what is done (see the items above), and the package + summary still lists joining FETCH. +- `session/namespace.go` says NAMESPACE / NAMESPACE_DONE go on a + PUBLISH_NAMESPACE stream (§10.17, §10.18). +- About a dozen stale `§` citations (padding, grease, fetch ordering, caching). + +Open questions for interop: whether an End of Range marker carries an Object +Payload Length (Figure 28 vs §11.4.4.2), and whether EXPIRES may appear in +TRACK_STATUS_OK (§10.15 vs §10.2.16). diff --git a/pkg/moqt/session/example_test.go b/pkg/moqt/session/example_test.go index 0b056aac..800ff8e3 100644 --- a/pkg/moqt/session/example_test.go +++ b/pkg/moqt/session/example_test.go @@ -228,9 +228,9 @@ func ExampleRequestMux() { _ = r.RejectError(moqt.RequestNotSupported, "unsupported request type") }) - // Run returns when ctx is cancelled or AcceptRequest fails. A session-fatal - // error (e.g. *session.ErrDuplicateRequestID) should be escalated by closing - // the session with the mapped code. + // Run returns when ctx is cancelled or AcceptRequest fails; a protocol + // violation (e.g. *session.ErrDuplicateRequestID) has already closed the + // session with the mapped code. _ = mux.Run(ctx, server) } diff --git a/pkg/moqt/session/helpers_test.go b/pkg/moqt/session/helpers_test.go index 2aed7347..99a63b72 100644 --- a/pkg/moqt/session/helpers_test.go +++ b/pkg/moqt/session/helpers_test.go @@ -166,19 +166,24 @@ func requireRefusedOpen(t *testing.T, want string, open func(context.Context) (* // requireClosedProtocolViolation waits for sess to close and checks the code. func requireClosedProtocolViolation(t *testing.T, sess *session.Session) { + t.Helper() + requireClosedCode(t, sess, moqt.SessionProtocolViolation) +} + +// requireClosedCode checks that sess closes itself with want. +func requireClosedCode(t *testing.T, sess *session.Session, want moqt.SessionErrorCode) { t.Helper() select { case <-sess.Done(): case <-time.After(2 * time.Second): - t.Fatal("session stayed open; want PROTOCOL_VIOLATION close") + t.Fatalf("session stayed open; want close with %#x", uint64(want)) } closed, ok := errors.AsType[*session.ClosedError](sess.Err()) if !ok { t.Fatalf("Err() = %v, want a *session.ClosedError", sess.Err()) } - if closed.Code != moqt.SessionProtocolViolation { - t.Errorf("closed with code %#x, want PROTOCOL_VIOLATION (%#x)", - uint64(closed.Code), uint64(moqt.SessionProtocolViolation)) + if closed.Code != want { + t.Errorf("closed with code %#x, want %#x", uint64(closed.Code), uint64(want)) } } diff --git a/pkg/moqt/session/malformed_test.go b/pkg/moqt/session/malformed_test.go index 393c32bc..6f70da00 100644 --- a/pkg/moqt/session/malformed_test.go +++ b/pkg/moqt/session/malformed_test.go @@ -66,24 +66,43 @@ func TestMalformedOpenerClosesSession(t *testing.T) { } } -// TestTruncatedOpenerResetsOnlyStream: a stream ending before its first frame -// is complete is the peer giving up, not a Length mismatch. +// TestTruncatedOpenerResetsOnlyStream: a stream ending or reset before its +// first frame is complete is the peer giving up on that request (§3.3.2, +// §3.3.3), not a Length mismatch: AcceptRequest moves on to the next request. func TestTruncatedOpenerResetsOnlyStream(t *testing.T) { t.Parallel() - _, server, cliConn, _ := openPairWithConns(t) - stream, err := cliConn.OpenStream() - if err != nil { - t.Fatalf("OpenStream: %v", err) - } - go func() { - // Type SUBSCRIBE, Length 16, then only two body bytes and a FIN. - _, _ = stream.Write([]byte{byte(message.TypeSubscribe), 0x00, 0x10, 0x00, 0x00}) - _ = stream.Close() - }() - if _, err := server.AcceptRequest(t.Context()); err == nil { - t.Fatal("AcceptRequest accepted a truncated opener") + for _, tc := range []struct { + name string + end func(session.Stream) + }{ + {"FIN", func(s session.Stream) { _ = s.Close() }}, + {"reset", func(s session.Stream) { s.CancelWrite(0) }}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + client, server, cliConn, _ := openPairWithConns(t) + stream, err := cliConn.OpenStream() + if err != nil { + t.Fatalf("OpenStream: %v", err) + } + go func() { + // Type SUBSCRIBE, Length 16, then only two body bytes. + _, _ = stream.Write([]byte{byte(message.TypeSubscribe), 0x00, 0x10, 0x00, 0x00}) + tc.end(stream) + _, _ = session.OpenRequestForTest(client, &message.Subscribe{ + RequestID: 0, Namespace: videoNS, Name: []byte("next"), + }) + }() + req, err := server.AcceptRequest(t.Context()) + if err != nil { + t.Fatalf("AcceptRequest: %v, want the request after the truncated one", err) + } + if name := string(req.First.(*message.Subscribe).Name); name != "next" { + t.Fatalf("accepted %q, want the request after the truncated one", name) + } + requireStaysOpen(t, server, 100*time.Millisecond) + }) } - requireStaysOpen(t, server, 100*time.Millisecond) } func TestMalformedResponseClosesSession(t *testing.T) { diff --git a/pkg/moqt/session/request.go b/pkg/moqt/session/request.go index 52c83882..c5927d86 100644 --- a/pkg/moqt/session/request.go +++ b/pkg/moqt/session/request.go @@ -15,9 +15,10 @@ import ( "github.com/floatdrop/moq-go/pkg/moqt/track" ) -// ErrRequestIDParityViolation is returned by AcceptRequest when the peer sends -// a Request ID whose parity does not match the expected value per §10.1. -// The caller MUST close the session with SessionInvalidRequestID. +// ErrRequestIDParityViolation is returned by [Session.CheckPeerRequestID] when +// the peer sends a Request ID whose parity does not match the expected value +// per §10.1. [Session.AcceptRequest] has already closed the session with +// INVALID_REQUEST_ID; another caller of CheckPeerRequestID MUST. type ErrRequestIDParityViolation struct { RequestID uint64 ExpectedEven bool // true = expected even (peer is client), false = expected odd (peer is server) @@ -40,7 +41,8 @@ func (e *ErrRequestIDParityViolation) Error() string { // Request ID" MUST close the session with INVALID_REQUEST_ID). Cross-stream // delivery reordering is tolerated — an ID below the high-water mark counts // as a duplicate only once every unseen ID it could have been is accounted -// for. The caller MUST close the session with SessionInvalidRequestID. +// for. [Session.AcceptRequest] has already closed the session with +// INVALID_REQUEST_ID; another caller of CheckPeerRequestID MUST. type ErrDuplicateRequestID struct { RequestID uint64 MaxSeen uint64 @@ -239,8 +241,11 @@ type Request struct { // malformed (§10, wrapping [message.ErrMalformedMessage]), closes the session // with PROTOCOL_VIOLATION; the error is *ErrUnexpectedRequestOpener, // *ErrUnexpectedRequestUpdate, ErrUnexpectedPublishStateNotify or the parse -// error. A stream that ends before its first message is complete only resets -// that stream. +// error. A Request ID violation (§10.1) closes it with INVALID_REQUEST_ID and +// returns *ErrRequestIDParityViolation or *ErrDuplicateRequestID, and a token +// cache fault (§10.2.2) with the *TokenCacheError's Code. A stream that ends +// or is reset before its first message is complete fails only that request +// (§3.3.2, §3.3.3): it is reset and AcceptRequest moves on. func (s *Session) AcceptRequest(ctx context.Context) (*Request, error) { for { stream, err := s.conn.AcceptStream(ctx) @@ -252,16 +257,24 @@ func (s *Session) AcceptRequest(ctx context.Context) (*Request, error) { // accept loop past cancellation. msg, err := s.readResponse(ctx, stream) if err != nil { - resetStream(stream) if ctx.Err() != nil { + resetStream(stream) return nil, ctx.Err() } // §3.3: readResponse already closed the session; this only shapes // the error. if typ, ok := errors.AsType[message.ErrUnknownType](err); ok { + resetStream(stream) return nil, s.closeProtocolViolation(&ErrUnexpectedRequestOpener{Type: message.Type(typ)}) } - return nil, fmt.Errorf("moqt/session: parse request first message: %w", err) + if errors.Is(err, message.ErrMalformedMessage) { + resetStream(stream) + return nil, fmt.Errorf("moqt/session: parse request first message: %w", err) + } + // §3.3.2, §3.3.3: the peer ended or reset the stream first, which + // fails that request only. A closed session fails AcceptStream. + cancelRequest(stream) + continue } // §10.9, §3.3: REQUEST_UPDATE never opens a stream. @@ -295,16 +308,20 @@ func (s *Session) AcceptRequest(ctx context.Context) (*Request, error) { if m, ok := msg.(message.WithRequestID); ok { if err := s.CheckPeerRequestID(m.GetRequestID()); err != nil { resetStream(stream) + _ = s.Close(moqt.SessionInvalidRequestID, err.Error()) return nil, err } } // §10.2.2: REGISTER tokens commit before any rejection, so the alias // persists even if the request fails. A *TokenCacheError is - // session-fatal; the caller closes the session with its Code. + // session-fatal. tokens, err := s.processRequestTokens(msg) if err != nil { resetStream(stream) + if tce, ok := errors.AsType[*TokenCacheError](err); ok { + _ = s.Close(tce.Code, tce.Error()) + } return nil, err } @@ -350,8 +367,7 @@ func evictLowestGapsLocked(gaps map[uint64]struct{}, n int) { // for those). // // Two violations are session-fatal per §10.1, and the caller MUST close the -// session with [moqt.SessionInvalidRequestID] (AcceptRequest instead returns -// the error to its caller, which owns that decision): +// session with [moqt.SessionInvalidRequestID] (AcceptRequest does so itself): // // - wrong parity for the sender (*ErrRequestIDParityViolation); // - a duplicate ID (*ErrDuplicateRequestID). diff --git a/pkg/moqt/session/request_test.go b/pkg/moqt/session/request_test.go index 8358b78c..17ea51a6 100644 --- a/pkg/moqt/session/request_test.go +++ b/pkg/moqt/session/request_test.go @@ -237,6 +237,7 @@ func TestAcceptRequestDuplicateID(t *testing.T) { if dupErr.RequestID != 0 { t.Errorf("ErrDuplicateRequestID.RequestID = %d, want 0", dupErr.RequestID) } + requireClosedCode(t, server, moqt.SessionInvalidRequestID) } // TestAcceptRequestOutOfOrderID verifies §10.1's receiver rules under @@ -330,6 +331,7 @@ func TestAcceptRequestOutOfOrderID(t *testing.T) { if dupErr.MaxSeen != 4 { t.Errorf("ErrDuplicateRequestID.MaxSeen = %d, want 4", dupErr.MaxSeen) } + requireClosedCode(t, server, moqt.SessionInvalidRequestID) } // TestAcceptRequestMonotonicHappyPath verifies that multiple requests with @@ -428,6 +430,7 @@ func TestAcceptRequestParityViolation_ServerReceivesOddID(t *testing.T) { if !parityErr.ExpectedEven { t.Errorf("ErrRequestIDParityViolation.ExpectedEven = false, want true (server expects even IDs from client)") } + requireClosedCode(t, server, moqt.SessionInvalidRequestID) } // TestAcceptRequestParityViolation_ClientReceivesEvenID verifies that when the @@ -476,6 +479,7 @@ func TestAcceptRequestParityViolation_ClientReceivesEvenID(t *testing.T) { if parityErr.ExpectedEven { t.Errorf("ErrRequestIDParityViolation.ExpectedEven = true, want false (client expects odd IDs from server)") } + requireClosedCode(t, client, moqt.SessionInvalidRequestID) } // TestAcceptRequestParityHappyPath verifies that correct-parity IDs are diff --git a/pkg/moqt/session/requestmux.go b/pkg/moqt/session/requestmux.go index b908f970..3135ba6c 100644 --- a/pkg/moqt/session/requestmux.go +++ b/pkg/moqt/session/requestmux.go @@ -86,14 +86,9 @@ func (m *RequestMux) OnUnknown(f func(*Request)) { // until ctx is cancelled or [Session.AcceptRequest] returns an error, which Run // returns. // -// Some AcceptRequest errors are session-fatal protocol violations — a §10.1 -// Request-ID parity/monotonicity violation (*ErrRequestIDParityViolation / -// *ErrDuplicateRequestID) or a token-cache fault (*TokenCacheError) — that the -// caller MUST escalate by closing the session with the mapped code (see -// [Session.AcceptRequest]). Run surfaces the error unchanged so the caller can -// inspect it with errors.As and Close accordingly. A request stream opened by -// anything but a request message (§3.3, *ErrUnexpectedRequestOpener and -// friends) arrives with the session already closed. +// Run surfaces an AcceptRequest error unchanged. Any but ctx's means the +// session has ended: a protocol violation (§3.3, §10, §10.1, §10.2.2) arrives +// already closed with the mapped code (see [Session.AcceptRequest]). // // Dispatch is synchronous: a handler runs to completion before Run accepts the // next request, mirroring a hand-written accept loop and [Demux.Run]. A handler diff --git a/pkg/moqt/session/token_verify.go b/pkg/moqt/session/token_verify.go index 8d775c59..e80ee3f5 100644 --- a/pkg/moqt/session/token_verify.go +++ b/pkg/moqt/session/token_verify.go @@ -65,8 +65,9 @@ func (f TokenVerifierFunc) VerifyToken(ctx context.Context, sess *Session, tok R // inbound request's AUTHORIZATION_TOKEN parameters fails at the cache layer // (§10.2.2). These are session-level faults: a malformed token, a duplicate // REGISTER alias, a cache overflow, or a USE_ALIAS / DELETE referencing an -// unknown alias. Code is the SESSION_ERROR the caller should close the -// session with. +// unknown alias. Code is the SESSION_ERROR the session is closed with: +// AcceptRequest has already done so, and a caller of +// [Session.ProcessFollowupTokens] MUST. type TokenCacheError struct { // Code is the §10.2.2 SESSION_ERROR code to terminate the session with. Code moqt.SessionErrorCode @@ -212,10 +213,16 @@ func (s *Session) applyToken(t *message.Token) (tok ResolvedToken, ok bool, err // can call it from their own request loop. It is safe to call with a req whose // Tokens slice is empty. func (s *Session) VerifyRequestTokens(ctx context.Context, req *Request) error { - if s.tokenVerifier == nil || len(req.Tokens) == 0 { + return s.VerifyTokens(ctx, req.Tokens) +} + +// VerifyTokens is [Session.VerifyRequestTokens] for tokens resolved by +// [Session.ProcessFollowupTokens], such as a REQUEST_UPDATE's (§10.2.2). +func (s *Session) VerifyTokens(ctx context.Context, toks []ResolvedToken) error { + if s.tokenVerifier == nil || len(toks) == 0 { return nil } - for _, tok := range req.Tokens { + for _, tok := range toks { if err := s.tokenVerifier.VerifyToken(ctx, s, tok); err != nil { if denied, ok := errors.AsType[*TokenDeniedError](err); ok { return denied diff --git a/pkg/moqt/session/token_verify_test.go b/pkg/moqt/session/token_verify_test.go index 308d309e..23da3611 100644 --- a/pkg/moqt/session/token_verify_test.go +++ b/pkg/moqt/session/token_verify_test.go @@ -137,6 +137,7 @@ func TestAcceptRequestDuplicateAliasIsSessionError(t *testing.T) { if tce.Code != moqt.SessionDuplicateAuthTokenAlias { t.Errorf("Code = 0x%X, want SessionDuplicateAuthTokenAlias", uint64(tce.Code)) } + requireClosedCode(t, server, moqt.SessionDuplicateAuthTokenAlias) } // TestAcceptRequestUnknownAliasIsSessionError verifies that USE_ALIAS for an @@ -159,6 +160,7 @@ func TestAcceptRequestUnknownAliasIsSessionError(t *testing.T) { if tce.Code != moqt.SessionUnknownAuthTokenAlias { t.Errorf("Code = 0x%X, want SessionUnknownAuthTokenAlias", uint64(tce.Code)) } + requireClosedCode(t, server, moqt.SessionUnknownAuthTokenAlias) } // TestAcceptRequestRegisterPersistsWhenAliasingProhibited verifies that with @@ -184,6 +186,7 @@ func TestAcceptRequestRegisterProhibitedIsOverflow(t *testing.T) { if tce.Code != moqt.SessionAuthTokenCacheOverflow { t.Errorf("Code = 0x%X, want SessionAuthTokenCacheOverflow", uint64(tce.Code)) } + requireClosedCode(t, server, moqt.SessionAuthTokenCacheOverflow) } // TestVerifyRequestTokensAllow verifies that a verifier returning nil diff --git a/pkg/relay/auth.go b/pkg/relay/auth.go index 279cb12c..deb61060 100644 --- a/pkg/relay/auth.go +++ b/pkg/relay/auth.go @@ -15,6 +15,13 @@ import ( // mutation; a non-nil return causes the relay to reply REQUEST_ERROR with // the [DeniedError]'s mapped code (see [DeniedError.RequestErrorCode]). // +// A REQUEST_UPDATE that changes a SUBSCRIBE_NAMESPACE's or SUBSCRIBE_TRACKS's +// TRACK_NAMESPACE_PREFIX is authorized again (§10.19, §10.20): the method +// receives the subscription as updated: the new prefix, and the +// AUTHORIZATION_TOKENs of the latest request or update that carried any (a +// DELETE authorizes nothing). A denial refuses the update and ends the +// subscription (§10.9.1). +// // The interface is split per request type for two reasons: // // - It lets a policy reject categories of request without having to @@ -130,9 +137,9 @@ func ReasonForAuthorizerError(err error) string { // // Production deployments SHOULD replace this with a token- or // session-attestation-aware implementation via [Config.Authorizer]. The relay -// only invokes the authorizer once per request before any state mutation, so -// the cost of policy evaluation is bounded by the request rate rather than -// the object rate. +// invokes the authorizer once per request, and per prefix update, before any +// state mutation, so the cost of policy evaluation is bounded by the request +// rate rather than the object rate. type AllowAllAuthorizer struct{} var _ Authorizer = AllowAllAuthorizer{} diff --git a/pkg/relay/cache/cache.go b/pkg/relay/cache/cache.go index 231343d6..07ee9000 100644 --- a/pkg/relay/cache/cache.go +++ b/pkg/relay/cache/cache.go @@ -1,5 +1,5 @@ -// Package cache holds the relay's per-track Object Cache (§9.4 fetch -// support). Storage is a fixed-capacity circular ring buffer (FIFO) with +// Package cache holds the relay's per-track Object Cache (§9.1 caching +// relays). Storage is a fixed-capacity circular ring buffer (FIFO) with // an auxiliary {GroupID, ObjectID} → ring-slot index map for O(1) point // lookup and overwrite-in-place. // @@ -115,7 +115,7 @@ type cacheKey struct { // cache. Each [TrackEntry] holds one. // // Concurrency: an RWMutex guards the ring. Writes (Put / Delete) take the -// write lock and are O(1); reads (Get / GetRange / OldestRetained / Len) +// write lock and are O(1); reads (Get / GetRange / Len) // take the read lock. FETCH reads are // O(capacity) (default 1024), so the read lock lets concurrent FETCHes — the // flash-crowd-of-joining-subscribers case the relay is built for — scan in @@ -339,7 +339,6 @@ func (c *ObjectCache) GetRange(start, end message.Location, order message.GroupO return nil } c.mu.RLock() - floor, hasFloor := c.oldestRetainedLocked() out := make([]*CachedObject, 0) for _, obj := range c.ring { if obj == nil { @@ -350,13 +349,7 @@ func (c *ObjectCache) GetRange(start, end message.Location, order message.GroupO continue } if !c.notExpiredLocked(obj) { - // §12.3: expired state "becomes unknown". Above the floor a - // plain gap would assert non-existence (§11.4.4), so mark it; - // below it the caller accounts for the span (OldestRetained). - if hasFloor && floor.Less(loc) { - out = append(out, &CachedObject{GroupID: obj.GroupID, ObjectID: obj.ObjectID, EndOfUnknownRange: true}) - } - continue + continue // §12.3: its state "becomes unknown"; the caller treats it so } // Append the stored pointer directly — Put never recycles or // mutates a stored struct, so this never aliases storage a later @@ -368,46 +361,6 @@ func (c *ObjectCache) GetRange(start, end message.Location, order message.GroupO return out } -// OldestRetained returns the lowest Location currently held by the cache — -// the eviction floor — and a bool that is false when the cache holds no live -// object. -// -// Because the ring evicts oldest-first and objects are stored in (broadly -// increasing) arrival order, the retained set is a suffix of the track by -// Location: everything below OldestRetained has either been evicted by size -// or TTL pressure, or was never cached by this relay. Either way the relay -// does not hold it. A FETCH responder uses this boundary to decide which part -// of a requested range it can answer from cache and which part it must stitch -// from upstream — a gap below the floor is "maybe exists upstream", whereas a -// gap at or above the floor is ground-truth non-existence. -// -// Like [ObjectCache.GetRange], this is an O(capacity) scan; FETCH is not the -// hot path. -func (c *ObjectCache) OldestRetained() (message.Location, bool) { - c.mu.RLock() - defer c.mu.RUnlock() - return c.oldestRetainedLocked() -} - -// oldestRetainedLocked is OldestRetained with c.mu held. -func (c *ObjectCache) oldestRetainedLocked() (message.Location, bool) { - var ( - oldest message.Location - found bool - ) - for _, obj := range c.ring { - if obj == nil || !c.notExpiredLocked(obj) { - continue - } - loc := message.Location{Group: obj.GroupID, Object: obj.ObjectID} - if !found || loc.Less(oldest) { - oldest = loc - found = true - } - } - return oldest, found -} - // sortObjects sorts in-place by (group, object). Group direction is // controlled by order; objects within a group are always ascending. // An unknown GroupOrder falls back to ascending. diff --git a/pkg/relay/cache/cache_test.go b/pkg/relay/cache/cache_test.go index 846fc41f..0f3921f4 100644 --- a/pkg/relay/cache/cache_test.go +++ b/pkg/relay/cache/cache_test.go @@ -24,36 +24,6 @@ func locs(objs []*cache.CachedObject) []message.Location { return out } -// TestObjectCache_OldestRetained pins the eviction-floor accessor: false on an -// empty cache, the minimum live Location otherwise, and an advancing floor as -// size pressure evicts the oldest entries. -func TestObjectCache_OldestRetained(t *testing.T) { - t.Parallel() - - c := cache.NewObjectCache(0, 0) - if _, ok := c.OldestRetained(); ok { - t.Fatal("empty cache must report no oldest retained") - } - - // Insert out of order; the floor is the minimum Location regardless. - putAt(c, 5, 2) - putAt(c, 3, 0) - putAt(c, 7, 1) - if got, ok := c.OldestRetained(); !ok || got != (message.Location{Group: 3, Object: 0}) { - t.Fatalf("OldestRetained = %v, %v; want {3 0}, true", got, ok) - } - - // A size-bounded cache evicts oldest-first, so the floor advances as - // newer groups push the earliest insert out of the ring. - small := cache.NewObjectCache(2, 0) - putAt(small, 1, 0) - putAt(small, 2, 0) - putAt(small, 3, 0) // evicts {1,0} - if got, ok := small.OldestRetained(); !ok || got != (message.Location{Group: 2, Object: 0}) { - t.Fatalf("after eviction OldestRetained = %v, %v; want {2 0}, true", got, ok) - } -} - // TestObjectCache_GetRange_Order pins GetRange's sort: groups in the requested // order, Objects ascending within a group (§10.13). Inserts are scrambled so // FIFO order cannot pass. @@ -168,8 +138,8 @@ func TestObjectCache_Delete(t *testing.T) { } // TestPerObjectMaxCacheDuration: each Object keeps its upstream's -// MAX_CACHE_DURATION (§12.3). Expired above the oldest served Object it reads -// as End of Unknown Range; a present 0 is never served; absent means relay TTL. +// MAX_CACHE_DURATION (§12.3). An expired one is not returned; a present 0 is +// never served; absent means relay TTL. func TestPerObjectMaxCacheDuration(t *testing.T) { c := cache.NewObjectCache(16, 0) put := func(group uint64, maxAge time.Duration, has bool) *cache.CachedObject { @@ -182,7 +152,7 @@ func TestPerObjectMaxCacheDuration(t *testing.T) { c.Put(o) return o } - put(0, 10*time.Millisecond, true) // expires, below the floor + put(0, 10*time.Millisecond, true) // expires put(1, 0, false) // never expires short := put(2, 10*time.Millisecond, true) put(3, 0, false) @@ -197,7 +167,7 @@ func TestPerObjectMaxCacheDuration(t *testing.T) { } got = append(got, fmt.Sprintf("%d:%s", o.GroupID, kind)) } - want := []string{"1:obj", "2:unknown", "3:obj", "4:unknown"} + want := []string{"1:obj", "3:obj"} if !slices.Equal(got, want) { t.Fatalf("GetRange = %v, want %v", got, want) } diff --git a/pkg/relay/cache_test.go b/pkg/relay/cache_test.go index 03ddf918..c4d586c2 100644 --- a/pkg/relay/cache_test.go +++ b/pkg/relay/cache_test.go @@ -266,9 +266,18 @@ func TestRelay_MaxCacheDurationExpiredObjectIsUnknown(t *testing.T) { pubB := dialAnotherClient(t, pubA) publishVideoTrackProps(t, pubB, "cam1", 9, nil) newCam1Subscriber(t, pubA) - publishObjects(t, pubB, 9, 1, 1) - publishObjects(t, pubA, aliasA, 2, 1) - publishObjects(t, pubB, 9, 3, 1) + // END_OF_GROUP, so the Groups' ends are known and only the expired + // Object is unknown. + for _, p := range []struct { + sess *session.Session + alias, group uint64 + }{{pubB, 9, 1}, {pubA, aliasA, 2}, {pubB, 9, 3}} { + hdr := subgroupHeader(p.alias, p.group) + hdr.EndOfGroup = true + if err := writeSubgroup(p.sess, hdr, 1); err != nil { + t.Fatal(err) + } + } time.Sleep(3 * maxCacheMs * time.Millisecond) elems := fetchCam1(t, dialAnotherClient(t, pubA), 3) diff --git a/pkg/relay/dedup_window_test.go b/pkg/relay/dedup_window_test.go new file mode 100644 index 00000000..6560f159 --- /dev/null +++ b/pkg/relay/dedup_window_test.go @@ -0,0 +1,247 @@ +package relay_test + +import ( + "errors" + "io" + "slices" + "testing" + "time" + + "github.com/floatdrop/moq-go/pkg/moqt/message" + "github.com/floatdrop/moq-go/pkg/moqt/session" +) + +// The relay dedups Objects from redundant upstreams against the last 32 +// Groups (§9.3). Past that it forwards an Object only above every one it +// forwarded in its Subgroup; one it drops leaves its stream missing an Object, +// so that stream is reset, not FINed (§11.4.3). + +// readSubgroupsConcurrently reads every subgroup stream sess accepts to its +// end, each on its own goroutine, so an open stream does not hold up the +// others. +func readSubgroupsConcurrently(t *testing.T, sess *session.Session) <-chan subgroupRead { + t.Helper() + out := make(chan subgroupRead, 64) + go func() { + for { + ds, err := sess.AcceptDataStream(t.Context()) + if err != nil { + return + } + in, ok := ds.(*session.IncomingSubgroupStream) + if !ok { + continue + } + go func() { + r := subgroupRead{header: in.Header} + for { + o, err := in.ReadDecoded() + if err != nil { + r.end = err + out <- r + return + } + r.ids = append(r.ids, o.ObjectID) + } + }() + } + }() + return out +} + +// awaitGroupStream returns the first stream of group read from reads, +// failing after 5s. +func awaitGroupStream(t *testing.T, reads <-chan subgroupRead, group uint64) subgroupRead { + t.Helper() + deadline := time.After(5 * time.Second) + for { + select { + case r := <-reads: + if r.header.GroupID == group { + return r + } + case <-deadline: + t.Fatalf("no stream of Group %d ended within 5s", group) + } + } +} + +// writeOpenGroup0 opens Group 0's subgroup (END_OF_GROUP set, so its FIN also +// ends the Group) and writes Objects ids, leaving it open. +func writeOpenGroup0( + t *testing.T, + pubSess *session.Session, + alias uint64, + ids ...uint64, +) *session.OutgoingSubgroupStream { + t.Helper() + hdr := subgroupHeader(alias, 0) + hdr.EndOfGroup = true + sg, err := openSubgroupWaiting(t, pubSess, hdr) + if err != nil { + t.Fatalf("OpenSubgroup: %v", err) + } + writeIDs(t, sg, -1, ids...) + return sg +} + +// writeIDs writes Objects ids (ascending) on sg after Object prev, -1 for +// none (§11.4.2 delta encoding). +func writeIDs(t *testing.T, sg *session.OutgoingSubgroupStream, prev int64, ids ...uint64) { + t.Helper() + for _, id := range ids { + delta := id + if prev >= 0 { + delta = id - uint64(prev) - 1 + } + prev = int64(id) + if err := sg.WriteObject(&message.SubgroupObject{ObjectIDDelta: delta, Payload: []byte("x")}); err != nil { + t.Fatalf("WriteObject(%d): %v", id, err) + } + } +} + +// publishGroups publishes one Object in each of Groups 1..n, waiting for each +// to reach the subscriber. +func publishGroups(t *testing.T, pubSess *session.Session, alias uint64, reads <-chan subgroupRead, n uint64) { + t.Helper() + for g := uint64(1); g <= n; g++ { + publishObjects(t, pubSess, alias, g, 1) + awaitGroupStream(t, reads, g) + } +} + +// TestFanout_GroupIDJumpKeepsOpenSubgroup: Group IDs need not be consecutive +// (§2.3.1, §12.8), so a jump to Group 100 while Group 0's stream is still open +// does not age Group 0 out; its later Objects are forwarded. +func TestFanout_GroupIDJumpKeepsOpenSubgroup(t *testing.T) { + t.Parallel() + pubSess, alias := newCam1Publisher(t, nil) + subSess := dialAnotherClient(t, pubSess) + subscribeCam1(t, subSess) + reads := readSubgroupsConcurrently(t, subSess) + + sg := writeOpenGroup0(t, pubSess, alias, 0, 1) + publishObjects(t, pubSess, alias, 100, 1) + awaitGroupStream(t, reads, 100) + writeIDs(t, sg, 1, 2) + if err := sg.Close(); err != nil { + t.Fatalf("Close: %v", err) + } + + r := awaitGroupStream(t, reads, 0) + if !slices.Equal(r.ids, []uint64{0, 1, 2}) || !errors.Is(r.end, io.EOF) { + t.Fatalf("Group 0 stream carried %v and ended with %v, want [0 1 2] and a FIN", r.ids, r.end) + } +} + +// TestFanout_AgedOutNextObjectForwarded: once 32 newer Groups push Group 0 +// out of the dedup window, the next Object on its open stream is still +// forwarded: it is above every Object forwarded in its Subgroup, so it +// repeats none (§9.4: "MUST NOT reorder or drop objects received on a +// multi-object stream"). +func TestFanout_AgedOutNextObjectForwarded(t *testing.T) { + t.Parallel() + pubSess, alias := newCam1Publisher(t, nil) + subSess := dialAnotherClient(t, pubSess) + subscribeCam1(t, subSess) + reads := readSubgroupsConcurrently(t, subSess) + + sg := writeOpenGroup0(t, pubSess, alias, 0, 1) + publishGroups(t, pubSess, alias, reads, 32) + writeIDs(t, sg, 1, 2) + if err := sg.Close(); err != nil { + t.Fatalf("Close: %v", err) + } + + r := awaitGroupStream(t, reads, 0) + if !slices.Equal(r.ids, []uint64{0, 1, 2}) || !errors.Is(r.end, io.EOF) { + t.Fatalf("Group 0 stream carried %v and ended with %v, want [0 1 2] and a FIN", r.ids, r.end) + } +} + +// TestFanout_AgedOutObjectResetsStream: an aged-out Object below one already +// forwarded in its Subgroup (here Object 1, from a second stream, after 0 and +// 2) may be a repeat the relay can no longer detect, so it is dropped; the +// stream missing it is reset rather than FINed, which with END_OF_GROUP would +// say it does not exist (§11.4.2, §11.4.3). A subscription whose Start +// Location is past the dropped Object was not owed it, so its stream FINs. +func TestFanout_AgedOutObjectResetsStream(t *testing.T) { + t.Parallel() + pubSess, alias := newCam1Publisher(t, nil) + subSess := dialAnotherClient(t, pubSess) + subscribeCam1(t, subSess) + reads := readSubgroupsConcurrently(t, subSess) + lateSess := dialAnotherClient(t, pubSess) + subscribeCam1(t, lateSess, message.AbsoluteStartFilter(message.Location{Group: 0, Object: 2})) + lateReads := readSubgroupsConcurrently(t, lateSess) + // Its OBJECTID_FILTER omits Object 1, which resets its stream as it would + // had Object 1 been forwarded (§11.4.3). + filteredSess := dialAnotherClient(t, pubSess) + subscribeCam1(t, filteredSess, message.RangeFilterParam(&message.RangeFilter{ + Type: message.ParamObjectIDFilter, Ranges: []message.Range{{Start: 0, End: 0}, {Start: 2, End: 2}}, + })) + filteredReads := readSubgroupsConcurrently(t, filteredSess) + + first := writeOpenGroup0(t, pubSess, alias, 0, 2) + publishGroups(t, pubSess, alias, reads, 32) + hdr := subgroupHeader(alias, 0) + hdr.EndOfGroup, hdr.ReplayingSubgroup = true, true + second, err := openSubgroupWaiting(t, pubSess, hdr) + if err != nil { + t.Fatalf("OpenSubgroup: %v", err) + } + writeIDs(t, second, -1, 1) + for _, sg := range []*session.OutgoingSubgroupStream{second, first} { + if err := sg.Close(); err != nil { + t.Fatalf("Close: %v", err) + } + } + + r := awaitGroupStream(t, reads, 0) + if !slices.Equal(r.ids, []uint64{0, 2}) || errors.Is(r.end, io.EOF) { + t.Fatalf("Group 0 stream carried %v and ended with %v, want [0 2] and a reset", r.ids, r.end) + } + late := awaitGroupStream(t, lateReads, 0) + if !slices.Equal(late.ids, []uint64{2}) || !errors.Is(late.end, io.EOF) { + t.Fatalf("Group 0 stream from {0, 2} carried %v and ended with %v, want [2] and a FIN", late.ids, late.end) + } + filtered := awaitGroupStream(t, filteredReads, 0) + if !slices.Equal(filtered.ids, []uint64{0, 2}) || errors.Is(filtered.end, io.EOF) { + t.Fatalf("filtered Group 0 stream carried %v and ended with %v, want [0 2] and a reset", + filtered.ids, filtered.end) + } +} + +// TestFanout_AgedOutRepeatDropped: an aged-out Object this Subgroup already +// forwarded, here Objects 1 and 2 again from a second stream, is a redundant +// copy (§9.3): dropped without resetting the streams that got the first. +func TestFanout_AgedOutRepeatDropped(t *testing.T) { + t.Parallel() + pubSess, alias := newCam1Publisher(t, nil) + subSess := dialAnotherClient(t, pubSess) + subscribeCam1(t, subSess) + reads := readSubgroupsConcurrently(t, subSess) + + first := writeOpenGroup0(t, pubSess, alias, 0, 1, 2) + publishGroups(t, pubSess, alias, reads, 32) + hdr := subgroupHeader(alias, 0) + hdr.EndOfGroup, hdr.ReplayingSubgroup = true, true + second, err := openSubgroupWaiting(t, pubSess, hdr) + if err != nil { + t.Fatalf("OpenSubgroup: %v", err) + } + writeIDs(t, second, -1, 1, 2) + if err := second.Close(); err != nil { + t.Fatalf("Close: %v", err) + } + writeIDs(t, first, 2, 3) + if err := first.Close(); err != nil { + t.Fatalf("Close: %v", err) + } + + r := awaitGroupStream(t, reads, 0) + if !slices.Equal(r.ids, []uint64{0, 1, 2, 3}) || !errors.Is(r.end, io.EOF) { + t.Fatalf("Group 0 stream carried %v and ended with %v, want [0 1 2 3] and a FIN", r.ids, r.end) + } +} diff --git a/pkg/relay/fetch_holes_test.go b/pkg/relay/fetch_holes_test.go new file mode 100644 index 00000000..51b2a925 --- /dev/null +++ b/pkg/relay/fetch_holes_test.go @@ -0,0 +1,396 @@ +package relay_test + +import ( + "math" + "slices" + "sync/atomic" + "testing" + "time" + + "github.com/floatdrop/moq-go/pkg/moqt" + "github.com/floatdrop/moq-go/pkg/moqt/message" + "github.com/floatdrop/moq-go/pkg/moqt/session" + "github.com/floatdrop/moq-go/pkg/relay" +) + +// A FETCH served from the relay's cache says an Object does not exist only +// when the relay knows it (§2.1: "A gap in the observed Object IDs does not by +// itself convey any information about the skipped Objects"). Any other +// uncached Location is asked of an upstream (§10.13: "the relay MUST pause +// subsequent delivery until it has confirmed the object's status upstream"), +// or, with none to ask, marked End of Unknown Range (§11.4.4.2). + +// cam1Object is one Object of video/cam1 to publish: its Location, and its +// Object Properties. +type cam1Object struct { + group, object uint64 + props []byte +} + +// publishCam1Group writes objs, all of one Group and ascending, on one +// subgroup stream of alias, with END_OF_GROUP when ended, and FINs it. +func publishCam1Group(t *testing.T, sess *session.Session, alias uint64, ended bool, objs ...cam1Object) { + t.Helper() + hdr := subgroupHeader(alias, objs[0].group) + hdr.EndOfGroup = ended + hdr.Properties = slices.ContainsFunc(objs, func(o cam1Object) bool { return o.props != nil }) + sg, err := openSubgroupWaiting(t, sess, hdr) + if err != nil { + t.Fatalf("OpenSubgroup: %v", err) + } + for i, o := range objs { + delta := o.object + if i > 0 { + delta = o.object - objs[i-1].object - 1 + } + if err := sg.WriteObject(&message.SubgroupObject{ + ObjectIDDelta: delta, Properties: o.props, Payload: []byte("x"), + }); err != nil { + t.Fatalf("WriteObject: %v", err) + } + } + if err := sg.Close(); err != nil { + t.Fatalf("Close: %v", err) + } +} + +// fetchCam1Range FETCHes [start, end] of video/cam1 in order and returns the +// response elements. +func fetchCam1Range( + t *testing.T, + sess *session.Session, + start, end message.Location, + order message.GroupOrder, + extra ...message.Parameter, +) []fetchElem { + t.Helper() + params := append(message.Parameters{fetchRangeFilter(start, end)}, extra...) + if order == message.GroupOrderDescending { + params = append(params, message.GroupOrderParam(order)) + } + fr, err := sess.Fetch(t.Context(), &message.Fetch{Namespace: ns("video"), Name: []byte("cam1"), Parameters: params}) + if err != nil { + t.Fatalf("Fetch: %v", err) + } + defer fr.Close() + return collectFetchElems(t, sess, order, 3*time.Second) +} + +// obj and unknownAt are the fetchElems of an Object and of an End of Unknown +// Range marker. +func obj(g, o uint64) fetchElem { return fetchElem{Group: g, Object: o} } +func unknownAt(g, o uint64) fetchElem { + return fetchElem{Group: g, Object: o, Unknown: true, Marker: true} +} + +// timedOutAt is the fetchElem of an End of Timed-Out Range marker. +func timedOutAt(g, o uint64) fetchElem { return fetchElem{Group: g, Object: o, Marker: true} } + +// TestFetch_CacheHolesWithoutUpstream: with no upstream to ask, an uncached +// Location the relay knows nothing of is marked unknown, in the order the +// response carries it (Groups descending, Objects ascending within one); a +// known one is a plain gap. +func TestFetch_CacheHolesWithoutUpstream(t *testing.T) { + t.Parallel() + const maxID = math.MaxUint64 + for _, tc := range []struct { + name string + publish func(t *testing.T, sess *session.Session, alias uint64) + end message.Location + order message.GroupOrder + want []fetchElem + }{ + { + name: "hole inside a Group", + publish: func(t *testing.T, sess *session.Session, alias uint64) { + publishCam1Group(t, sess, alias, true, cam1Object{0, 0, nil}, cam1Object{0, 1, nil}, cam1Object{0, 3, nil}) + }, + end: message.Location{Group: 0, Object: 3}, + want: []fetchElem{obj(0, 0), obj(0, 1), unknownAt(0, 2), obj(0, 3)}, + }, + { + name: "Group tail with no end", + publish: func(t *testing.T, sess *session.Session, alias uint64) { + publishCam1Group(t, sess, alias, false, cam1Object{0, 0, nil}, cam1Object{0, 1, nil}) + publishCam1Group(t, sess, alias, false, cam1Object{1, 0, nil}) + }, + end: message.Location{Group: 1, Object: 0}, + want: []fetchElem{obj(0, 0), obj(0, 1), unknownAt(0, maxID), obj(1, 0)}, + }, + { + name: "Group tail with no end, Descending", + publish: func(t *testing.T, sess *session.Session, alias uint64) { + publishCam1Group(t, sess, alias, false, cam1Object{0, 0, nil}, cam1Object{0, 1, nil}) + publishCam1Group(t, sess, alias, false, cam1Object{1, 0, nil}) + }, + end: message.Location{Group: 1, Object: 0}, + order: message.GroupOrderDescending, + want: []fetchElem{obj(1, 0), obj(0, 0), obj(0, 1), unknownAt(0, maxID)}, + }, + { + name: "Groups ended by END_OF_GROUP", + publish: func(t *testing.T, sess *session.Session, alias uint64) { + publishCam1Group(t, sess, alias, true, cam1Object{0, 0, nil}, cam1Object{0, 1, nil}) + publishCam1Group(t, sess, alias, true, cam1Object{1, 0, nil}) + }, + end: message.Location{Group: 1, Object: 0}, + want: []fetchElem{obj(0, 0), obj(0, 1), obj(1, 0)}, + }, + { + name: "hole a Prior Object ID Gap announces", + publish: func(t *testing.T, sess *session.Session, alias uint64) { + publishCam1Group(t, sess, alias, true, cam1Object{0, 0, nil}, cam1Object{0, 1, nil}, + cam1Object{0, 3, priorGap(message.PropertyPriorObjectIDGap, 1)}) + }, + end: message.Location{Group: 0, Object: 3}, + want: []fetchElem{obj(0, 0), obj(0, 1), obj(0, 3)}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + pubSess, alias := newCam1Publisher(t, nil) + tc.publish(t, pubSess, alias) + time.Sleep(50 * time.Millisecond) // the relay caches them + + order := tc.order + if order == 0 { + order = message.GroupOrderAscending + } + got := fetchCam1Range(t, dialAnotherClient(t, pubSess), message.Location{}, tc.end, order) + if !slices.Equal(got, tc.want) { + t.Fatalf("FETCH elements %v, want %v", got, tc.want) + } + }) + } +} + +// TestFetch_CacheHoleAskedUpstream: with a fetch-capable upstream, the relay +// FETCHes an uncached Location of unknown status from it, and its answer +// decides: the Object it sends is served, and a gap under its FIN is one. +func TestFetch_CacheHoleAskedUpstream(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + serve bool // whether the upstream has Object {0, 2} + want []fetchElem + }{ + {"upstream has it", true, []fetchElem{obj(0, 0), obj(0, 1), obj(0, 2), obj(0, 3)}}, + {"upstream says it does not exist", false, []fetchElem{obj(0, 0), obj(0, 1), obj(0, 3)}}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + upSess, teardown := connectRelay(t, relay.Config{}) + t.Cleanup(teardown) + if _, err := upSess.PublishNamespace( + t.Context(), + &message.PublishNamespace{Namespace: ns("video")}, + ); err != nil { + t.Fatalf("PublishNamespace: %v", err) + } + var asked atomic.Int32 + go func() { + for { + req, err := upSess.AcceptRequest(t.Context()) + if err != nil { + return + } + switch m := req.First.(type) { + case *message.Subscribe: + if req.Reply(&message.SubscribeOK{TrackAlias: 42}) != nil { + return + } + // The live stream misses Object 2. + publishCam1Group(t, upSess, 42, true, + cam1Object{0, 0, nil}, cam1Object{0, 1, nil}, cam1Object{0, 3, nil}) + case *message.Fetch: + asked.Add(1) + if req.Reply(&message.FetchOK{EndLocation: message.Location{Group: 0, Object: 3}}) != nil { + return + } + out, err := upSess.OpenFetchStream(message.FetchHeader{RequestID: m.RequestID}) + if err != nil { + return + } + if tc.serve { + _ = out.WriteObject(&message.FetchObject{ + SerializationFlags: message.FetchFlagGroupIDDelta | message.FetchFlagObjectIDDelta | + message.FetchFlagPriority | uint64(message.FetchSubgroupIDExplicit), + GroupIDDelta: 0, ObjectIDDelta: 2, ObjectPayload: []byte("x"), + }) + } + _ = out.Close() + } + } + }() + live := dialAnotherClient(t, upSess) + subscribeCam1(t, live) + go drainAll(t.Context(), live) + fc := dialAnotherClient(t, upSess) + waitRelayLargest(t, fc, ns("video"), []byte("cam1"), 0, 3) + + got := fetchCam1Range(t, fc, message.Location{}, message.Location{Group: 0, Object: 3}, + message.GroupOrderAscending) + if !slices.Equal(got, tc.want) { + t.Fatalf("FETCH elements %v, want %v", got, tc.want) + } + if asked.Load() != 1 { + t.Fatalf("the relay asked the upstream %d times, want once", asked.Load()) + } + }) + } +} + +// TestFetch_UpstreamUnknownKeepsWhatTheRelayKnows: an upstream that marks the +// whole span unknown, or timed out, does not unsay what the relay knows: its +// cached Objects are served, and a Group tail an END_OF_GROUP ended stays a +// gap. Only the holes are marked, with the upstream's kind, in either order. +func TestFetch_UpstreamUnknownKeepsWhatTheRelayKnows(t *testing.T) { + t.Parallel() + const maxID = math.MaxUint64 + for _, tc := range []struct { + name string + order message.GroupOrder + marker message.Location // the upstream's, at the span's end in stream order + flags uint64 + want []fetchElem + }{ + { + "Ascending", message.GroupOrderAscending, message.Location{Group: 1, Object: 1}, + message.FetchEndOfUnknownRange, + []fetchElem{obj(0, 0), unknownAt(0, 1), obj(0, 2), obj(1, 0), unknownAt(1, 1), obj(1, 2)}, + }, + { + "Descending", message.GroupOrderDescending, message.Location{Group: 0, Object: maxID}, + message.FetchEndOfUnknownRange, + []fetchElem{obj(1, 0), unknownAt(1, 1), obj(1, 2), obj(0, 0), unknownAt(0, 1), obj(0, 2)}, + }, + { + "Timed-Out", message.GroupOrderAscending, message.Location{Group: 1, Object: 1}, + message.FetchEndOfTimedOutRange, + []fetchElem{obj(0, 0), timedOutAt(0, 1), obj(0, 2), obj(1, 0), timedOutAt(1, 1), obj(1, 2)}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + upSess, teardown := connectRelay(t, relay.Config{}) + t.Cleanup(teardown) + if _, err := upSess.PublishNamespace( + t.Context(), + &message.PublishNamespace{Namespace: ns("video")}, + ); err != nil { + t.Fatalf("PublishNamespace: %v", err) + } + var asked atomic.Int32 + go func() { + for { + req, err := upSess.AcceptRequest(t.Context()) + if err != nil { + return + } + switch m := req.First.(type) { + case *message.Subscribe: + if req.Reply(&message.SubscribeOK{TrackAlias: 42}) != nil { + return + } + // Each Group misses Object 1 and ends after Object 2. + for g := range uint64(2) { + publishCam1Group(t, upSess, 42, true, cam1Object{g, 0, nil}, cam1Object{g, 2, nil}) + } + case *message.Fetch: + asked.Add(1) + _, end, _ := fetchRequestRange(m) + if req.Reply(&message.FetchOK{EndLocation: end}) != nil { + return + } + out, err := upSess.OpenFetchStream(message.FetchHeader{RequestID: m.RequestID}) + if err != nil { + return + } + _ = out.WriteObject(&message.FetchObject{ + SerializationFlags: tc.flags, + GroupIDDelta: tc.marker.Group, ObjectIDDelta: tc.marker.Object, + }) + _ = out.Close() + } + } + }() + live := dialAnotherClient(t, upSess) + subscribeCam1(t, live) + go drainAll(t.Context(), live) + fc := dialAnotherClient(t, upSess) + waitRelayLargest(t, fc, ns("video"), []byte("cam1"), 1, 2) + time.Sleep(50 * time.Millisecond) // the relay caches Group 0 too + + got := fetchCam1Range(t, fc, message.Location{}, message.Location{Group: 1, Object: 2}, tc.order) + if !slices.Equal(got, tc.want) { + t.Fatalf("FETCH elements %v, want %v", got, tc.want) + } + if asked.Load() != 1 { + t.Fatalf("the relay asked the upstream %d times, want once", asked.Load()) + } + }) + } +} + +// TestFetch_FillTimeoutBoundsUpstreamRead: FILL_TIMEOUT is the budget for the +// whole upstream FETCH, its response included (§10.2.5). An upstream that +// sends part of its answer and stalls has what it sent served, and the rest +// reported "as Timed-Out gaps" — between what it sent too, since without a FIN +// its gaps assert nothing (§10.13). +func TestFetch_FillTimeoutBoundsUpstreamRead(t *testing.T) { + t.Parallel() + upSess, teardown := connectRelay(t, relay.Config{}) + t.Cleanup(teardown) + if _, err := upSess.PublishNamespace(t.Context(), &message.PublishNamespace{Namespace: ns("video")}); err != nil { + t.Fatalf("PublishNamespace: %v", err) + } + go func() { + for { + req, err := upSess.AcceptRequest(t.Context()) + if err != nil { + return + } + switch m := req.First.(type) { + case *message.Subscribe: + if req.Reply(&message.SubscribeOK{TrackAlias: 42}) != nil { + return + } + // The live stream misses Objects 2 to 4. + publishCam1Group(t, upSess, 42, true, + cam1Object{0, 0, nil}, cam1Object{0, 1, nil}, cam1Object{0, 5, nil}) + case *message.Fetch: + _, end, _ := fetchRequestRange(m) + if req.Reply(&message.FetchOK{EndLocation: end}) != nil { + return + } + out, err := upSess.OpenFetchStream(message.FetchHeader{RequestID: m.RequestID}) + if err != nil { + return + } + // Objects 2 and 4, then nothing: the stream stays open. + _ = out.WriteObject(&message.FetchObject{ + SerializationFlags: message.FetchFlagGroupIDDelta | message.FetchFlagObjectIDDelta | + message.FetchFlagPriority | uint64(message.FetchSubgroupIDExplicit), + GroupIDDelta: 0, ObjectIDDelta: 2, ObjectPayload: []byte("x"), + }) + _ = out.WriteObject(&message.FetchObject{ + SerializationFlags: message.FetchFlagObjectIDDelta | uint64(message.FetchSubgroupIDPrior), + ObjectIDDelta: 2, ObjectPayload: []byte("x"), + }) + t.Cleanup(func() { out.Cancel(moqt.StreamResetCancelled) }) + } + } + }() + live := dialAnotherClient(t, upSess) + subscribeCam1(t, live) + go drainAll(t.Context(), live) + fc := dialAnotherClient(t, upSess) + waitRelayLargest(t, fc, ns("video"), []byte("cam1"), 0, 5) + + got := fetchCam1Range(t, fc, message.Location{}, message.Location{Group: 0, Object: 5}, + message.GroupOrderAscending, message.FillTimeoutParam(300*time.Millisecond)) + want := []fetchElem{obj(0, 0), obj(0, 1), obj(0, 2), timedOutAt(0, 3), obj(0, 4), obj(0, 5)} + if !slices.Equal(got, want) { + t.Fatalf("FETCH elements %v, want %v", got, want) + } +} diff --git a/pkg/relay/fetch_ranges.go b/pkg/relay/fetch_ranges.go new file mode 100644 index 00000000..6c43e36c --- /dev/null +++ b/pkg/relay/fetch_ranges.go @@ -0,0 +1,175 @@ +package relay + +import ( + "cmp" + "math" + "slices" + + "github.com/floatdrop/moq-go/pkg/moqt/message" + "github.com/floatdrop/moq-go/pkg/relay/cache" + "github.com/floatdrop/moq-go/pkg/relay/internal/registry" +) + +// A FETCH response asserts what it does not carry: "Any gaps in the Group and +// Object IDs in the response stream indicate objects that do not exist" +// (§10.13), and "All signals that an Object does not exist are +// authoritative" (§2.1). The relay knows an Object does not exist only from a +// signal — a Prior Group or Object ID Gap (§12.8, §12.9), a Group's or the +// Track's end (§11.2.1.1, §11.4.2), or an upstream's FETCH response — never from a gap in +// what it happened to receive: "A gap in the observed Object IDs does not by +// itself convey any information about the skipped Objects" (§2.1). So a +// response is built from the requested range classified into Objects, known +// absent Locations, and the rest, whose status is unknown and is either asked +// of an upstream (§10.13) or marked with an End of Unknown Range (§11.4.4.2). +// +// Interpretation: an End of Range marker covers "Locations between the last +// serialized Object, if any, and this Location" (§11.4.4.2) in the order the +// response carries them: Groups in its Group Order, Object IDs ascending +// within a Group. In Descending order that is not Location order. + +// locSucc returns the Location right after l, and false when l is the last. +func locSucc(l message.Location) (message.Location, bool) { + switch { + case l.Object < math.MaxUint64: + return message.Location{Group: l.Group, Object: l.Object + 1}, true + case l.Group < math.MaxUint64: + return message.Location{Group: l.Group + 1}, true + } + return message.Location{}, false +} + +// uncovered returns the parts of [start, end] no range in known covers, in +// ascending order. It sorts known. +func uncovered(start, end message.Location, known []registry.LocRange) []registry.LocRange { + slices.SortFunc(known, func(a, b registry.LocRange) int { return a.Lo.Compare(b.Lo) }) + var out []registry.LocRange + cur := start + for _, k := range known { + if k.Hi.Less(cur) { + continue + } + if end.Less(k.Lo) { + break + } + if cur.Less(k.Lo) { + pred, _ := fetchPredecessor(k.Lo) // k.Lo > cur, so it has one + out = append(out, registry.LocRange{Lo: cur, Hi: pred}) + } + next, ok := locSucc(k.Hi) + if !ok || end.Less(next) { + return out + } + cur = next + } + return append(out, registry.LocRange{Lo: cur, Hi: end}) +} + +// knownFromCache is what objs, Objects the cache holds, establish: each +// Object's Location; from an END_OF_GROUP or END_OF_TRACK status, the rest of +// its Group or of the Track (§11.2.1.1); and the gaps its Prior Group and +// Object ID Gaps announce (§12.8, §12.9). [message.CheckObjectProperties] +// rejected a gap above an Object's ID on receipt. +func knownFromCache(objs []*cache.CachedObject) []registry.LocRange { + out := make([]registry.LocRange, 0, len(objs)) + for _, o := range objs { + loc := message.Location{Group: o.GroupID, Object: o.ObjectID} + r := registry.LocRange{Lo: loc, Hi: loc} + switch o.Status { + case message.ObjectStatusEndOfGroup: + r.Hi.Object = math.MaxUint64 + case message.ObjectStatusEndOfTrack: + r.Hi = message.Location{Group: math.MaxUint64, Object: math.MaxUint64} + } + out = append(out, r) + gaps := message.ObjectPriorGaps(o.Properties) + if gaps.HasObject && gaps.Object > 0 { + out = append(out, registry.LocRange{ + Lo: message.Location{Group: o.GroupID, Object: o.ObjectID - gaps.Object}, + Hi: message.Location{Group: o.GroupID, Object: o.ObjectID - 1}, + }) + } + if gaps.HasGroup && gaps.Group > 0 { + out = append(out, registry.LocRange{ + Lo: message.Location{Group: o.GroupID - gaps.Group}, + Hi: message.Location{Group: o.GroupID - 1, Object: math.MaxUint64}, + }) + } + } + return out +} + +// unknownIn returns the Locations of [start, end] whose status the relay does +// not know: neither among objs, the cached Objects of the range, nor known +// absent from them or entry's ledger. +func unknownIn( + entry *registry.TrackEntry, + objs []*cache.CachedObject, + start, end message.Location, +) []registry.LocRange { + return uncovered(start, end, append(knownFromCache(objs), entry.KnownAbsent()...)) +} + +// streamCompare orders Locations as a FETCH response in order carries them +// (§10.13): Groups in the Group Order, Object IDs ascending within a Group. +func streamCompare(a, b message.Location, order message.GroupOrder) int { + if order == message.GroupOrderDescending && a.Group != b.Group { + return cmp.Compare(b.Group, a.Group) + } + return a.Compare(b) +} + +// runEnds returns where each part of r a response in order carries without +// interruption ends, in stream order. Ascending, r is one run. Descending, a +// range across Groups is up to three: the start of its highest Group, the +// Groups strictly between, and the rest of its lowest Group. +func runEnds(r registry.LocRange, order message.GroupOrder) []message.Location { + if order != message.GroupOrderDescending || r.Lo.Group == r.Hi.Group { + return []message.Location{r.Hi} + } + ends := []message.Location{r.Hi} + if r.Hi.Group-r.Lo.Group > 1 { + ends = append(ends, message.Location{Group: r.Lo.Group + 1, Object: math.MaxUint64}) + } + return append(ends, message.Location{Group: r.Lo.Group, Object: math.MaxUint64}) +} + +// fetchElements orders objs for a response in order and adds, at the end of +// each run of unknown and timedOut Locations, an End of Unknown or Timed-Out +// Range marker (§11.4.4.2), so that a plain gap in the response only ever +// covers Locations known not to exist. unknown and timedOut must be disjoint +// and hold no Object of objs. A marker's run starts after the previous +// element, so it may also cover known-absent Locations: weaker, never false. +func fetchElements( + objs []*cache.CachedObject, + unknown, timedOut []registry.LocRange, + order message.GroupOrder, +) []*cache.CachedObject { + elems := slices.Clone(objs) + for _, r := range unknown { + for _, at := range runEnds(r, order) { + elems = append(elems, unknownRangeMarker(at)) + } + } + for _, r := range timedOut { + for _, at := range runEnds(r, order) { + elems = append(elems, timedOutRangeMarker(at)) + } + } + slices.SortStableFunc(elems, func(a, b *cache.CachedObject) int { + return streamCompare( + message.Location{Group: a.GroupID, Object: a.ObjectID}, + message.Location{Group: b.GroupID, Object: b.ObjectID}, + order) + }) + // A marker's coverage starts after the previous element, so a marker + // right after one of its own kind covers both: keep only the later. + out := elems[:0] + for i, e := range elems { + if i+1 < len(elems) && e.IsRangeMarker() && elems[i+1].IsRangeMarker() && + e.EndOfTimedOutRange == elems[i+1].EndOfTimedOutRange { + continue + } + out = append(out, e) + } + return out +} diff --git a/pkg/relay/fill_group_order_test.go b/pkg/relay/fill_group_order_test.go new file mode 100644 index 00000000..0fa6e846 --- /dev/null +++ b/pkg/relay/fill_group_order_test.go @@ -0,0 +1,92 @@ +package relay_test + +import ( + "slices" + "testing" + "time" + + "github.com/floatdrop/moq-go/pkg/moqt/message" + "github.com/floatdrop/moq-go/pkg/moqt/session" + "github.com/floatdrop/moq-go/pkg/relay" +) + +// A subscription that omits GROUP_ORDER takes the publisher's preference from +// the Track (§10.2.8, §12.5), and so does its fill (§10.2.15). The fill's +// Group ID deltas run in that order (§11.4.4.1), so a subscriber decoding it +// by the draft's rule gets the right Groups only if the relay agrees. + +// publishDescendingCam publishes video/cam on alias 7 with +// DEFAULT_PUBLISHER_GROUP_ORDER Descending and one Object in each of Groups +// 0-2, which the relay caches. +func publishDescendingCam(t *testing.T) *session.Session { + t.Helper() + pubSess, teardown := connectRelay(t, relay.Config{}) + t.Cleanup(teardown) + publishVideoTrackProps(t, pubSess, "cam", 7, + trackProp(message.PropertyDefaultPublisherGroupOrder, uint64(message.GroupOrderDescending))) + for g := range uint64(3) { + sendObjects(pubSess, 7, g, 1) + } + time.Sleep(50 * time.Millisecond) + return pubSess +} + +// requireDescendingFill reads the fill fetch stream sess accepts, decoding it +// Descending, and requires Groups 2, 1, 0. +func requireDescendingFill(t *testing.T, sess *session.Session) { + t.Helper() + ds, ok := tryAcceptDataStream(t, sess, 2*time.Second) + if !ok { + t.Fatal("no fill fetch stream") + } + fs, ok := ds.(*session.IncomingFetchStream) + if !ok { + t.Fatalf("got %T, want the fill fetch stream", ds) + } + var groups []uint64 + for _, o := range decodeFetchStream(t, fs, message.GroupOrderDescending) { + groups = append(groups, o.group) + } + if !slices.Equal(groups, []uint64{2, 1, 0}) { + t.Fatalf("fill decoded Descending gave Groups %v, want [2 1 0]", groups) + } +} + +// fillWholeTrack is a Next Object subscription's FILL_PARAMETERS for the whole +// track, with no GROUP_ORDER anywhere. +var fillWholeTrack = []message.Parameter{ + message.NextObjectFilter(), + message.FillParametersParam(message.Parameters{message.UnfilteredFilter()}), +} + +// TestSubscribe_FillFollowsPublisherGroupOrder: SUBSCRIBE. +func TestSubscribe_FillFollowsPublisherGroupOrder(t *testing.T) { + t.Parallel() + subSess := dialAnotherClient(t, publishDescendingCam(t)) + sub, err := subSess.Subscribe(t.Context(), &message.Subscribe{ + Namespace: ns("video"), Name: []byte("cam"), Parameters: fillWholeTrack, + }) + if err != nil { + t.Fatalf("Subscribe: %v", err) + } + t.Cleanup(func() { _ = sub.Close() }) + requireDescendingFill(t, subSess) +} + +// TestSubscribeTracks_FillFollowsPublisherGroupOrder: a forwarded PUBLISH's +// subscription (§10.20.1). The PUBLISH states the Group Order it resolved to, +// since §10.20.1 says these parameters "are explicitly communicated in +// PUBLISH". +func TestSubscribeTracks_FillFollowsPublisherGroupOrder(t *testing.T) { + t.Parallel() + holder := dialAnotherClient(t, publishDescendingCam(t)) + reqs := forwardedPublishes(t, holder) + subscribeTracks(t, holder, ns("video"), fillWholeTrack...) + fwd := awaitForwarded(t, reqs) + if p, ok := fwd.First.(*message.Publish).Parameters.Find(message.ParamGroupOrder); !ok || + message.GroupOrder(p.Byte) != message.GroupOrderDescending { + t.Errorf("forwarded GROUP_ORDER = %d (present=%v), want Descending (0x2)", p.Byte, ok) + } + acceptForwarded(t, fwd) + requireDescendingFill(t, holder) +} diff --git a/pkg/relay/handler_datagram.go b/pkg/relay/handler_datagram.go index 8e1e9cc8..db08c939 100644 --- a/pkg/relay/handler_datagram.go +++ b/pkg/relay/handler_datagram.go @@ -72,13 +72,15 @@ func (h *sessionHandler) handleDatagram(ctx context.Context, d *message.ObjectDa EndOfGroup: d.HasEndOfGroup(), Gaps: message.ObjectPriorGaps(d.Properties), } - fresh, err := entry.ClaimDelivered(info) + claim, err := entry.ClaimDelivered(info) if err != nil { h.endMalformedTrack(ctx, entry, h.sess, err) return } - if !fresh { - if err := checkDuplicate(entry.Cache, &cache.CachedObject{ + // An aged-out datagram is dropped like a redundant one: it has no stream + // to reset (§9.4 covers multi-object streams). + if claim != registry.ClaimFresh { + if err := recordRedundant(entry, info, &cache.CachedObject{ GroupID: d.GroupID, ObjectID: d.ObjectID, PublisherPriority: d.PublisherPriority, @@ -88,10 +90,6 @@ func (h *sessionHandler) handleDatagram(ctx context.Context, d *message.ObjectDa Payload: d.ObjectPayload, }); err != nil { h.endMalformedTrack(ctx, entry, h.sess, err) - return - } - if err := entry.RecordDuplicate(info); err != nil { - h.endMalformedTrack(ctx, entry, h.sess, err) } return } diff --git a/pkg/relay/handler_duplicate.go b/pkg/relay/handler_duplicate.go index c30bcb63..5e63c213 100644 --- a/pkg/relay/handler_duplicate.go +++ b/pkg/relay/handler_duplicate.go @@ -7,8 +7,21 @@ import ( "github.com/floatdrop/moq-go/pkg/moqt/message" "github.com/floatdrop/moq-go/pkg/moqt/session" "github.com/floatdrop/moq-go/pkg/relay/cache" + "github.com/floatdrop/moq-go/pkg/relay/internal/registry" ) +// recordRedundant handles dup, a copy [registry.TrackEntry.ClaimDelivered] +// found redundant, described to the ledger by info: it is checked against the +// first copy ([checkDuplicate]) and then recorded +// ([registry.TrackEntry.RecordDuplicate]). An error wraps +// [session.ErrMalformedTrack]. +func recordRedundant(entry *registry.TrackEntry, info registry.ObjectInfo, dup *cache.CachedObject) error { + if err := checkDuplicate(entry.Cache, dup); err != nil { + return err + } + return entry.RecordDuplicate(info) +} + // checkDuplicate compares dup, a copy that lost the §9.3 dedup claim, with the // first copy in c. A different Forwarding Preference, Subgroup ID, Priority or // Payload (§9.1), or different Immutable Properties (§2.4.2, §12.7; one copy diff --git a/pkg/relay/handler_fanout.go b/pkg/relay/handler_fanout.go index 0e8a02f5..3f5f844f 100644 --- a/pkg/relay/handler_fanout.go +++ b/pkg/relay/handler_fanout.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "log/slog" + "math" "sync" "sync/atomic" "time" @@ -71,6 +72,11 @@ type subgroupWriterSet struct { // within the ledger's window. lowest uint64 forwarded bool + // runLo and runHi are the lowest and highest Object IDs forwarded + // through this set, if hasRun; unbroken reports that every ID between + // them was (see [subgroupWriterSet.admitAgedOut]). + runLo, runHi uint64 + hasRun, unbroken bool // sawClean records that some contributor ended cleanly, so the merged // stream FINs even if a peer reset; resetCode is used only when every @@ -87,9 +93,63 @@ func (s *subgroupWriterSet) claimFirst(objectID uint64, claimed bool) bool { if lowest { s.lowest, s.forwarded = objectID, true } + switch { + case !s.hasRun: + s.runLo, s.runHi, s.hasRun, s.unbroken = objectID, objectID, true, true + case s.runHi < math.MaxUint64 && objectID == s.runHi+1: + s.runHi = objectID + case s.runLo > 0 && objectID == s.runLo-1: + s.runLo = objectID + default: + s.runLo, s.runHi, s.unbroken = min(s.runLo, objectID), max(s.runHi, objectID), false + } return claimed && lowest } +// admitAgedOut reports whether an Object at objectID of the Subgroup hdr +// names, which [registry.TrackEntry.ClaimDelivered] returned as claim, may be +// forwarded. Only a [registry.ClaimAgedOut] one may not: with its Group out of +// the dedup window, it goes on only above every Object forwarded through this +// set, where it can neither repeat one nor arrive out of order (§2.2). That +// covers a single upstream's open stream. One this set is known to have +// forwarded (an end of its run, or inside an unbroken one) is a redundant +// copy, dropped as [registry.ClaimRedundant] would be. +// +// Any other is dropped, and each stream it would have gone on is marked +// incomplete as [subgroupWriter.admit] would mark it, so it resets rather +// than FIN (§11.4.3). Deviation: §9.4 says a relay "MUST NOT reorder or drop +// objects received on a multi-object stream"; the relay drops such an Object +// rather than risk forwarding a duplicate it can no longer detect (§9.3). A +// Subgroup whose Object IDs are not consecutive (its Group split across +// Subgroups) has no unbroken run, so there a redundant copy resets too. +// Callers hold sg.Mu. +func (s *subgroupWriterSet) admitAgedOut( + claim registry.Claim, + hdr message.SubgroupHeader, + objectID uint64, + props []byte, +) bool { + switch { + case claim != registry.ClaimAgedOut, s.hasRun && objectID > s.runHi: + return true + case s.hasRun && (objectID == s.runLo || objectID == s.runHi || + s.unbroken && s.runLo < objectID && objectID < s.runHi): + return false + } + for _, w := range s.writers { + if w == nil { + continue + } + v := w.sub.ForwardDecision(hdr.GroupID, objectID, hdr.SubgroupID, hdr.PublisherPriority, props) + if v == registry.Forward { + w.markIncomplete(moqt.StreamResetInternalError) + } else { + w.skip(v, objectID) + } + } + return false +} + // resolveImplicitSubgroupID handles §11.4.2 SUBGROUP_ID_MODE 0b01 (Subgroup ID // = first Object ID): it reads the first object and rewrites hdr to the // explicit form. The returned pending object must be processed as the @@ -362,13 +422,13 @@ func (h *sessionHandler) runFanout(ctx context.Context, stream *session.Incoming Status: obj.ObjectStatus, Gaps: message.ObjectPriorGaps(obj.Properties), } - fresh, err := entry.ClaimDelivered(info) + claim, err := entry.ClaimDelivered(info) if err != nil { malformed(err) return } - if !fresh { - if err := checkDuplicate(entry.Cache, &cache.CachedObject{ + if claim == registry.ClaimRedundant { + if err := recordRedundant(entry, info, &cache.CachedObject{ GroupID: hdr.GroupID, ObjectID: objectID, SubgroupID: hdr.SubgroupID, @@ -381,19 +441,19 @@ func (h *sessionHandler) runFanout(ctx context.Context, stream *session.Incoming malformed(err) return } - if err := entry.RecordDuplicate(info); err != nil { - malformed(err) - return - } continue // redundant copy already forwarded by a peer upstream. } - // Counted after the dedup claim, so redundant copies don't count. - h.metrics.ObjectReceived(ref, hdr.SubgroupID) - // Under sg.Mu: joiner detection, writer open and publish are atomic // against other contributors and the last-contributor teardown. sg.Mu.Lock() + if !set.admitAgedOut(claim, hdr, objectID, obj.Properties) { + sg.Mu.Unlock() + continue + } + + // Counted after the dedup claim, so redundant copies don't count. + h.metrics.ObjectReceived(ref, hdr.SubgroupID) // Cache before bumping LARGEST_OBJECT, so a FETCH that snapshots the // new watermark finds the object cached. @@ -516,12 +576,14 @@ func (h *sessionHandler) openWriterForSub( subHdr.InlinePriority = true } // cancelIO unblocks a writer wedged on a subscriber that stopped - // reading (see [joinWriters]). + // reading (see [joinWriters]), and resets the open stream of one that + // cancelled (§5.1.1). ioCtx, cancelIO := context.WithCancel(ctx) w := &subgroupWriter{ sub: sub, ctx: ioCtx, cancelIO: cancelIO, + unwatchSub: context.AfterFunc(sub.Cancelled(), cancelIO), hdr: subHdr, inbox: make(chan fwdObject, h.sendQueueSize), done: make(chan struct{}), @@ -555,15 +617,17 @@ type subgroupWriter struct { sub *registry.DownstreamSub // ctx bounds every blocking stream operation; cancelIO resets the // in-flight stream, unwedging a writer blocked on a stalled subscriber. - ctx context.Context - cancelIO context.CancelFunc - hdr message.SubgroupHeader // template; TrackAlias already remapped - out *session.OutgoingSubgroupStream // nil until run opens it lazily - unbridge func() bool // stops the current stream's ctx→Cancel bridge - inbox chan fwdObject - done chan struct{} - log *slog.Logger - metrics Metrics + // The subscriber's cancellation calls it too (§5.1.1), until unwatchSub. + ctx context.Context + cancelIO context.CancelFunc + unwatchSub func() bool + hdr message.SubgroupHeader // template; TrackAlias already remapped + out *session.OutgoingSubgroupStream // nil until run opens it lazily + unbridge func() bool // stops the current stream's ctx→Cancel bridge + inbox chan fwdObject + done chan struct{} + log *slog.Logger + metrics Metrics // ref labels every Metrics call; built once, since it allocates. ref TrackRef maxDropsBeforeReset int @@ -580,9 +644,11 @@ type subgroupWriter struct { inboundReset bool // set under dropsMu inside close inboundResetCode moqt.StreamResetCode // §3.3.4 reset code when inboundReset; set inside close // incomplete records that this subscription skipped an Object after its - // Start Location (filter, Forward State 0, overflow or expiry), so its - // streams end with a reset, not a FIN (§11.4.3), with incompleteCode: - // EXCESSIVE_LOAD after any overflow, else CANCELLED. Set under dropsMu. + // Start Location (filter, Forward State 0, overflow, expiry, or an Object + // dropped as aged out of the dedup window), so its streams end with a + // reset, not a FIN (§11.4.3), with incompleteCode: EXCESSIVE_LOAD after any + // overflow, else the first recorded (INTERNAL_ERROR for an aged-out drop, + // else CANCELLED). Set under dropsMu. // // Interpretation: Objects published before the subscription joined count // as before its Start Location, so a joiner's stream may still FIN. @@ -612,11 +678,26 @@ func (w *subgroupWriter) admit( props []byte, ) (take, follows bool) { follows = w.lastPos.src == pos.src && w.lastPos.seq+1 == pos.seq - switch w.sub.ForwardDecision(hdr.GroupID, objectID, hdr.SubgroupID, hdr.PublisherPriority, props) { - case registry.Forward: + v := w.sub.ForwardDecision(hdr.GroupID, objectID, hdr.SubgroupID, hdr.PublisherPriority, props) + if v == registry.Forward { w.lastAdmitted, w.hasAdmitted = objectID, true w.lastPos = pos return true, follows + } + w.skip(v, objectID) + // §11.4.3: an Object that "did not pass the subscriber's filters" does + // not separate the ones either side of it. Forward State is one of them + // (§5.1.5). + if follows { + w.lastPos = pos + } + return false, false +} + +// skip records that w omits the Object at objectID for verdict v, other than +// [registry.Forward], closing w when it will take none again. +func (w *subgroupWriter) skip(v registry.ForwardVerdict, objectID uint64) { + switch v { case registry.SkipObject, registry.SkipPaused: // The stream stays open for later Objects, but the Subgroup is now // incomplete (§11.4.3). @@ -631,14 +712,8 @@ func (w *subgroupWriter) admit( if w.hasAdmitted && objectID > w.lastAdmitted { w.markIncomplete(moqt.StreamResetCancelled) } + case registry.Forward: } - // §11.4.3: an Object that "did not pass the subscriber's filters" does - // not separate the ones either side of it. Forward State is one of them - // (§5.1.5). - if follows { - w.lastPos = pos - } - return false, false } // markIncomplete sets subgroupWriter.incomplete; the first code recorded @@ -760,6 +835,7 @@ func (w *subgroupWriter) dropOut() { // draining, so publish never blocks. func (w *subgroupWriter) run() { defer close(w.done) + defer w.unwatchSub() var ( prevID uint64 diff --git a/pkg/relay/handler_fanout_join_test.go b/pkg/relay/handler_fanout_join_test.go index 1edd1c4c..adc4bcca 100644 --- a/pkg/relay/handler_fanout_join_test.go +++ b/pkg/relay/handler_fanout_join_test.go @@ -17,10 +17,12 @@ import ( func newWedgeableWriter(t *testing.T, cli *session.Session) *subgroupWriter { t.Helper() ioCtx, cancelIO := context.WithCancel(t.Context()) + sub := registry.NewDownstreamSub(1, cli, nil, 42) return &subgroupWriter{ - sub: registry.NewDownstreamSub(1, cli, nil, 42), - ctx: ioCtx, - cancelIO: cancelIO, + sub: sub, + ctx: ioCtx, + cancelIO: cancelIO, + unwatchSub: context.AfterFunc(sub.Cancelled(), cancelIO), hdr: message.SubgroupHeader{ SubgroupIDMode: message.SubgroupIDImplicitZero, TrackAlias: 42, diff --git a/pkg/relay/handler_fetch.go b/pkg/relay/handler_fetch.go index f0ff9d18..94b99403 100644 --- a/pkg/relay/handler_fetch.go +++ b/pkg/relay/handler_fetch.go @@ -21,19 +21,10 @@ import ( // defaultUpstreamFetchTimeout bounds an upstream stitch FETCH when the // downstream supplied no FILL_TIMEOUT. It keeps a fetch-capable upstream that // nonetheless stalls (or never answers FETCH) from wedging the downstream -// handler: the stitch degrades to cache-only once it elapses. +// handler: once it elapses, the cache is served with the unknown Locations +// marked Timed-Out. const defaultUpstreamFetchTimeout = 5 * time.Second -// handleFetch implements FETCH (§9.4, §10.13): validate the requested range, -// reply FETCH_OK, open a FETCH_HEADER uni-stream, and serialise the cached -// objects in the requested group order. Gaps in the response stream are how -// the spec signals "objects do not exist" (§11.4.4). -// -// The below-floor portion of the range — objects the relay evicted or never -// cached — is stitched from an upstream FETCH when one is reachable; see -// [sessionHandler.stitchedFetchObjects]. Whatever no source could vouch for -// is covered by §11.4.4.2 End of Unknown Range markers, so a gap always means -// authoritative non-existence. // trackKnown reports whether entry stands for a track the relay actually knows // of. Bare existence does not say so: subscribeUpstreamOnSession creates the // entry before the upstream round trip that would confirm the track, because @@ -59,6 +50,12 @@ func trackKnown(entry *registry.TrackEntry) bool { return len(entry.CopyUpstream()) > 0 || len(entry.CopyDownstream()) > 0 } +// handleFetch implements FETCH (§10.13): validate the requested range, reply +// FETCH_OK, open a FETCH_HEADER uni-stream, and serialise the cached objects +// in the requested group order. Gaps in the response stream are how the spec +// signals "objects do not exist" (§10.13), so what the cache cannot vouch for +// is asked of an upstream FETCH when one is reachable, or covered by §11.4.4.2 +// End of Range markers; see [sessionHandler.stitchedFetchObjects]. func (h *sessionHandler) handleFetch(ctx context.Context, req *session.Request, msg *message.Fetch) { if err := h.auth.AuthorizeFetch(ctx, h.sess, msg); err != nil { h.rejectAuth(ctx, req, "Fetch", err) @@ -198,7 +195,7 @@ func (h *sessionHandler) readFetchUpdates(ctx context.Context, req *session.Requ } // §10.2.2: an update may REGISTER/DELETE token aliases; // a cache fault there is session-fatal. - if !h.handleFollowupTokens(ctx, upd) { + if _, ok := h.handleFollowupTokens(ctx, upd); !ok { return false } h.handleFetchUpdate(ctx, req) @@ -259,22 +256,23 @@ func capFetchEndLocation(filter *message.LocationFilter, largest message.Locatio return end } -// stitchedFetchObjects answers a FETCH range from the relay's cache, filling -// the below-floor portion the relay does not hold from an upstream FETCH when -// one is reachable (§9.4 upstream stitching). +// stitchedFetchObjects answers a FETCH range [start, end] from the relay's +// cache, asking an upstream about the Locations whose status it does not know +// (§10.13: "If it encounters an object in the requested range that is not +// cached and has unknown status, the relay MUST pause subsequent delivery +// until it has confirmed the object's status upstream"). See fetch_ranges.go +// for what the relay knows. // -// Everything below the cache's eviction floor (see -// [cache.ObjectCache.OldestRetained]) was evicted or never cached, so a gap -// there might still exist upstream whereas a gap at/above the floor is -// ground-truth non-existence. The handler splits the request at the floor, -// fetches [requestStart, floor) from an established upstream, and concatenates -// it with the cached part — the two are disjoint by Location, so the result is -// correctly ordered. With no FETCH-able upstream (or on error/timeout) it -// serves what the cache has and covers the below-floor remainder with a -// §11.4.4.2 End of Unknown Range marker, since a plain gap would falsely -// assert non-existence (§11.4.4). Upstream-fetched objects are NOT cached -// back: the FIFO ring is keyed by arrival, so old backfill would evict live -// objects. +// With a fetch-capable upstream, one FETCH covers the span from the first +// unknown Location to the last, within the FILL_TIMEOUT budget (§10.2.5). Its +// Objects fill the holes, and what it marks unknown or timed out stays so where +// the relay does not know better; the cached Objects are served either way, +// and what the relay knows does not exist stays a gap. With no such upstream, or +// when its FETCH fails or times out, the unknown Locations are marked End of +// Unknown or Timed-Out Range (§11.4.4.2) and the cached Objects served: the +// relay can "indicate the range of unknown Objects and continue serving other +// known Objects" (§10.13). Upstream-fetched objects are NOT cached back: the +// FIFO ring is keyed by arrival, so old backfill would evict live objects. // // A non-nil refusal (see fetchUpstreamRange) means the track must not be // forwarded; no objects are returned. @@ -282,68 +280,71 @@ func (h *sessionHandler) stitchedFetchObjects( ctx context.Context, entry *registry.TrackEntry, fullName track.FullTrackName, - requestStart message.Location, - requestEndIncl message.Location, + start, end message.Location, order message.GroupOrder, fillTimeout time.Duration, ) (objs []*cache.CachedObject, refusal error) { - cacheObjs := entry.Cache.GetRange(requestStart, requestEndIncl, order) - - // Determine the inclusive upper bound of the below-floor sub-range the - // relay cannot answer from cache. - upEndIncl := requestEndIncl - if floor, hasFloor := entry.Cache.OldestRetained(); hasFloor { - pred, ok := fetchPredecessor(floor) - if !ok { - return cacheObjs, nil // floor == {0,0}: nothing exists below it - } - if pred.Less(upEndIncl) { - upEndIncl = pred - } + // An expired Object (§12.3) is not returned: its status is unknown. + cached := entry.Cache.GetRange(start, end, message.GroupOrderAscending) + unknown := unknownIn(entry, cached, start, end) + if len(unknown) == 0 { + return fetchElements(cached, nil, nil, order), nil } - if upEndIncl.Less(requestStart) { - return cacheObjs, nil // the request starts at/above the floor — no gap - } - - // GetRange and OldestRetained are two separate cache reads: an eviction - // or TTL expiry between them can raise the floor above snapshot entries, - // making the upstream sub-range [requestStart, upEndIncl] overlap the - // snapshot. mergeFetchObjects relies on the two sources being disjoint - // by Location (a duplicate would serialize a non-ascending Object ID), - // so clip the snapshot to strictly above the sub-range. - cacheObjs = slices.DeleteFunc(cacheObjs, func(o *cache.CachedObject) bool { - return !upEndIncl.Less(message.Location{Group: o.GroupID, Object: o.ObjectID}) - }) - up := h.pickFetchUpstream(entry) if up == nil { - // No reachable upstream: the below-floor sub-range has unknown - // status, not ground-truth non-existence. A plain gap in a - // FIN-terminated response asserts the latter (§11.4.4), so cover - // the sub-range with an End of Unknown Range marker instead. This - // is the unknown-status case, not the §10.2.5 budget case — nothing - // timed out, we simply have no source to ask. - return mergeFetchObjects(order, - unknownWholeRange(requestStart, upEndIncl, order), cacheObjs), nil - } - - upstreamObjs, refusal := h.fetchUpstreamRange( - ctx, up, fullName, requestStart, upEndIncl, order, fillTimeout, - ) + return fetchElements(cached, unknown, nil, order), nil + } + span := registry.LocRange{Lo: unknown[0].Lo, Hi: unknown[len(unknown)-1].Hi} + ans, refusal := h.fetchUpstreamRange(ctx, up, fullName, span, order, fillTimeout) if errors.Is(refusal, session.ErrMalformedTrack) { h.endMalformedTrack(ctx, entry, up.Session, refusal) } if refusal != nil { return nil, refusal } - if len(upstreamObjs) == 0 { - // A clean-FIN, uncapped, empty upstream response: the upstream - // authoritatively asserted the whole sub-range non-existent, which - // a plain gap encodes exactly. (Every unknown outcome returns at - // least a marker element.) - return cacheObjs, nil + switch ans.failed { + case upstreamUnknown: + return fetchElements(cached, unknown, nil, order), nil + case upstreamTimedOut: + return fetchElements(cached, nil, unknown, order), nil + case upstreamAnswered: + } + + // The upstream answered for the span: its Objects fill the holes, and + // under its FIN the rest does not exist, except what it marked unknown or + // timed out and the relay has no signal for either. + have := make(map[message.Location]bool, len(cached)) + for _, o := range cached { + have[message.Location{Group: o.GroupID, Object: o.ObjectID}] = true + } + merged := cached + for _, o := range ans.objs { + if !have[message.Location{Group: o.GroupID, Object: o.ObjectID}] { + merged = append(merged, o) + } } - return mergeFetchObjects(order, upstreamObjs, cacheObjs), nil + return fetchElements(merged, intersect(ans.unknown, unknown), intersect(ans.timedOut, unknown), order), nil +} + +// intersect returns the Locations both a and b hold, each a set of disjoint +// ranges. +func intersect(a, b []registry.LocRange) []registry.LocRange { + var out []registry.LocRange + for _, x := range a { + for _, y := range b { + lo, hi := x.Lo, x.Hi + if lo.Less(y.Lo) { + lo = y.Lo + } + if y.Hi.Less(hi) { + hi = y.Hi + } + if !hi.Less(lo) { + out = append(out, registry.LocRange{Lo: lo, Hi: hi}) + } + } + } + return out } // pickFetchUpstream returns an Established, fetch-capable upstream on a @@ -364,29 +365,32 @@ func (h *sessionHandler) pickFetchUpstream(entry *registry.TrackEntry) *registry return nil } -// fetchUpstreamRange issues a standalone FETCH for the inclusive range -// [start, endIncl] on the upstream's session, awaits the response stream via -// the relay's fetch router, and returns the decoded objects in the requested -// group order (the upstream FETCH carries the same GROUP_ORDER parameter). -// -// The returned slice preserves what the upstream did and did not vouch for, -// so the downstream response stays truthful under §11.4.4's gap rule (a gap -// in a FIN-terminated response asserts non-existence): -// -// - Upstream End of Unknown Range markers (§11.4.4.2, 0x10C) are kept as -// [cache.CachedObject] marker elements and re-emitted downstream. -// - End of Non-Existent Range markers (0x8C) are dropped: a plain gap in -// our FIN-terminated response is the semantically equivalent encoding -// (§9.1 lets relays re-represent missing ranges), and §11.4.4.2 prefers -// it outside known/unknown splits. -// - When the upstream vouches for less than the whole sub-range — FETCH -// rejected, response timeout, a mid-stream error (no FIN, so its gaps -// assert nothing), or a clean FIN whose FETCH_OK EndLocation was capped -// below endIncl — the unvouched-for remainder is covered by an unknown -// marker. The mid-stream-error and descending capped cases collapse to -// "whole sub-range unknown": exact per-gap markers are inexpressible in -// §11.4.4's delta encoding wherever the element after a marker would be -// a same-group, lower-Object-ID transition. +// upstreamFailure is how an upstream FETCH failed to answer at all. +type upstreamFailure uint8 + +const ( + upstreamAnswered upstreamFailure = iota + // upstreamUnknown: refused, reset, malformed or out of order; nothing it + // sent is vouched for. + upstreamUnknown + // upstreamTimedOut: the FILL_TIMEOUT budget ran out (§10.2.5). + upstreamTimedOut +) + +// upstreamAnswer is an upstream FETCH response for a span, as Location ranges: +// its Objects; the parts its End of Unknown and Timed-Out Range markers +// covered, and any past a capped FETCH_OK End Location; every other Location +// of the span, a gap under a clean FIN, is known not to exist (§10.13). +type upstreamAnswer struct { + objs []*cache.CachedObject + unknown, timedOut []registry.LocRange + failed upstreamFailure +} + +// fetchUpstreamRange issues a standalone FETCH for span on the upstream's +// session, awaits the response stream via the relay's fetch router, and reads +// it into an upstreamAnswer. End of Non-Existent Range markers need no +// record: under a clean FIN a gap already says so. // // It returns a refusal instead when the track MUST NOT be forwarded: a // FETCH_OK with unacceptable Track Properties (§2.5.1), or a response Object @@ -395,17 +399,14 @@ func (h *sessionHandler) fetchUpstreamRange( ctx context.Context, up *registry.UpstreamSub, fullName track.FullTrackName, - start, endIncl message.Location, + span registry.LocRange, order message.GroupOrder, fillTimeout time.Duration, -) (objs []*cache.CachedObject, refusal error) { - unknownWhole := unknownWholeRange(start, endIncl, order) - timedOutWhole := timedOutWholeRange(start, endIncl, order) - +) (ans upstreamAnswer, refusal error) { // §10.2.5: an explicit 0 means "MUST NOT wait for upstream delivery" // (fillTimeout is already resolved, see [resolveFillBudget]). if fillTimeout == 0 { - return timedOutWhole, nil + return upstreamAnswer{failed: upstreamTimedOut}, nil } params := message.Parameters{} @@ -415,7 +416,7 @@ func (h *sessionHandler) fetchUpstreamRange( // §5.1.2: the range rides in LOCATION_FILTER. EndGroupDelta is delta-encoded // from the start group, and EndObject makes the end Object-precise. params = append(params, message.AbsoluteRangeObjectFilter( - start, endIncl.Group-start.Group, endIncl.Object)) + span.Lo, span.Hi.Group-span.Lo.Group, span.Hi.Object)) fmsg := &message.Fetch{ Namespace: fullName.Namespace, Name: fullName.Name, @@ -423,7 +424,7 @@ func (h *sessionHandler) fetchUpstreamRange( } // Bound the upstream round-trip so a silent or non-FETCH-answering - // upstream degrades to cache-plus-unknown-gap instead of wedging the + // upstream degrades to cache-plus-marked-unknown instead of wedging the // downstream handler. FILL_TIMEOUT, when present, is the subscriber's // explicit budget; otherwise fall back to a default. fctx, cancel := context.WithTimeout(ctx, fillTimeout) @@ -436,12 +437,12 @@ func (h *sessionHandler) fetchUpstreamRange( // §2.5.1: Session.Fetch has cancelled it; the caller resets the // downstream stream. if isTrackPropertiesErr(err) { - return nil, err + return upstreamAnswer{}, err } if fctx.Err() != nil { - return timedOutWhole, nil + return upstreamAnswer{failed: upstreamTimedOut}, nil } - return unknownWhole, nil + return upstreamAnswer{failed: upstreamUnknown}, nil } defer fr.Close() @@ -457,129 +458,162 @@ func (h *sessionHandler) fetchUpstreamRange( case fs = <-ch: case <-fctx.Done(): h.log.LogAttrs(ctx, slog.LevelDebug, "upstream FETCH response timed out") - return timedOutWhole, nil + return upstreamAnswer{failed: upstreamTimedOut}, nil } if fs == nil { - return unknownWhole, nil + return upstreamAnswer{failed: upstreamUnknown}, nil } // ReadDecoded needs the response's group order to resolve cross-group // deltas (§11.4.4.1); the upstream serves in the order our FETCH asked // for. fs.GroupOrder = order + // §10.2.5: the budget covers the response too; when it runs out, what + // has arrived is kept and the rest reported Timed-Out. + defer context.AfterFunc(fctx, func() { fs.Cancel(moqt.StreamResetCancelled) })() - var ( - out []*cache.CachedObject - prevLoc message.Location - havePrev bool - ) + var prev *message.Location for { obj, err := fs.ReadDecoded() if errors.Is(err, io.EOF) { - break // clean FIN: the upstream's gaps are authoritative (§11.4.4) + break // clean FIN: the upstream's gaps are authoritative (§10.13) } if errors.Is(err, session.ErrMalformedTrack) { // §2.4.2: fr.Close (deferred) cancels the fetch; the caller // resets the downstream stream. fs.Cancel(moqt.StreamResetMalformedTrack) - return nil, err + return upstreamAnswer{}, err + } + if err != nil && fctx.Err() != nil { + // Without a FIN its gaps assert nothing (§10.13): all of the span + // it did not send or mark is Timed-Out. + h.log.LogAttrs(ctx, slog.LevelDebug, "upstream FETCH response timed out mid-read") + known := slices.Concat(ans.unknown, ans.timedOut) + for _, o := range ans.objs { + loc := message.Location{Group: o.GroupID, Object: o.ObjectID} + known = append(known, registry.LocRange{Lo: loc, Hi: loc}) + } + ans.timedOut = append(ans.timedOut, uncovered(span.Lo, span.Hi, known)...) + return ans, nil } if err != nil { // No FIN (or a FIN mid-object), so the gaps in what arrived - // assert nothing; declare the whole sub-range unknown rather - // than serve partial objects whose gaps would read as - // non-existence. + // assert nothing. h.log.LogAttrs(ctx, slog.LevelDebug, "upstream FETCH stream failed mid-read", slog.String("err", err.Error())) - return unknownWhole, nil - } - if obj.EndOfNonExistentRange { - // Dropped: a plain gap in our FIN-terminated response is the - // semantically equivalent encoding (§9.1). - continue + return upstreamAnswer{failed: upstreamUnknown}, nil } loc := message.Location{Group: obj.GroupID, Object: obj.ObjectID} - if !upstreamFetchElemOK(loc, prevLoc, havePrev, start, endIncl, order, - obj.EndOfUnknownRange || obj.EndOfTimedOutRange) { + // §10.14: nothing past its own End Location. + if !upstreamFetchElemOK(loc, prev, span, order) || fr.OK.EndLocation.Less(loc) { h.log.LogAttrs(ctx, slog.LevelDebug, "upstream FETCH element out of range or order", slog.Uint64("group", loc.Group), slog.Uint64("object", loc.Object)) - return unknownWhole, nil - } - prevLoc, havePrev = loc, true - if obj.EndOfUnknownRange { - out = append(out, unknownRangeMarker(loc)) - continue - } - if obj.EndOfTimedOutRange { - out = append(out, timedOutRangeMarker(loc)) - continue + return upstreamAnswer{failed: upstreamUnknown}, nil } - // The §11.4.4.1 Datagram bit carries the original wire shape - // across this relay hop, so the object is re-emitted downstream - // with the same forwarding preference it was published with. - // (Stitched objects are merged into the response only — they are - // not written back into the cache.) - pref := cache.ForwardingSubgroup - if obj.Datagram { - pref = cache.ForwardingDatagram + switch { + case obj.EndOfUnknownRange: + ans.unknown = append(ans.unknown, streamCovered(prev, loc, span, order)...) + case obj.EndOfTimedOutRange: + ans.timedOut = append(ans.timedOut, streamCovered(prev, loc, span, order)...) + case obj.EndOfNonExistentRange: + default: + // The §11.4.4.1 Datagram bit carries the original wire shape + // across this relay hop. + pref := cache.ForwardingSubgroup + if obj.Datagram { + pref = cache.ForwardingDatagram + } + ans.objs = append(ans.objs, &cache.CachedObject{ + GroupID: obj.GroupID, + ObjectID: obj.ObjectID, + SubgroupID: obj.SubgroupID, + PublisherPriority: obj.PublisherPriority, + ForwardingPref: pref, + Properties: obj.Properties, + Payload: obj.Payload, + }) } - out = append(out, &cache.CachedObject{ - GroupID: obj.GroupID, - ObjectID: obj.ObjectID, - SubgroupID: obj.SubgroupID, - PublisherPriority: obj.PublisherPriority, - ForwardingPref: pref, - Properties: obj.Properties, - Payload: obj.Payload, - }) + prev = &loc } - // A clean FIN asserts gaps only up to the FETCH_OK EndLocation (§11.4.4). - // If the upstream capped it below our sub-range end (§10.13: End beyond - // its Largest), the remainder has unknown status. - if authEnd := fr.OK.EndLocation; authEnd.Less(endIncl) { - if order == message.GroupOrderDescending { - // The unknown remainder precedes every object in descending - // stream order, and a leading marker cannot in general be - // followed by a same-group object with a lower ID (see the - // doc comment) — fall back to whole-sub-range unknown. - return unknownWhole, nil - } - out = append(out, unknownRangeMarker(endIncl)) + // A clean FIN asserts gaps only up to the FETCH_OK End Location (§10.13). + // If the upstream capped it below the span (§10.13: End beyond its + // Largest), what lies past it has unknown status. + // No element lay past it, and Session.Fetch refused one before the span. + if authEnd := fr.OK.EndLocation; authEnd.Less(span.Hi) { + next, _ := locSucc(authEnd) // below span.Hi, so it has one + ans.unknown = append(ans.unknown, registry.LocRange{Lo: next, Hi: span.Hi}) } - return out, nil + return ans, nil } -// upstreamFetchElemOK validates one kept element of an upstream FETCH -// response before it is re-serialized downstream. Every element must lie -// inside the requested sub-range [start, endIncl] — the merge with the -// cached part relies on Location disjointness — and an object must advance -// from the previous kept element the way §11.4.4's delta encoding can -// express: within a group, Object IDs strictly ascend; across groups, the -// Group ID moves in the response's order direction. Unknown-range markers -// carry absolute IDs and merely re-anchor the encoding, so only the range -// check applies to them. A violation means the upstream is nonconformant; -// trusting the element would corrupt the downstream delta stream (e.g. flip -// its group-direction inference), so the caller discards the response. +// upstreamFetchElemOK validates one element of an upstream FETCH response for +// span before it is re-served downstream: it lies inside span, and after the +// previous element prev (nil for the first) in the order the response carries +// them (see [streamCompare]), as §11.4.4's delta encoding requires. A +// violation means the upstream is nonconformant; trusting the element would +// corrupt the downstream stream, so the caller discards the response. func upstreamFetchElemOK( - loc, prev message.Location, - havePrev bool, - start, endIncl message.Location, + loc message.Location, + prev *message.Location, + span registry.LocRange, order message.GroupOrder, - isMarker bool, ) bool { - if loc.Less(start) || endIncl.Less(loc) { + if loc.Less(span.Lo) || span.Hi.Less(loc) { return false } - if isMarker || !havePrev { - return true + return prev == nil || streamCompare(*prev, loc, order) < 0 +} + +// streamCovered returns, as Location ranges, what an End of Range marker at at +// covers in a response to a FETCH of span in order: the Locations after the +// previous element prev (nil for the first) up to at, in the order the +// response carries them (see fetch_ranges.go). at is after prev. +func streamCovered( + prev *message.Location, + at message.Location, + span registry.LocRange, + order message.GroupOrder, +) []registry.LocRange { + if order != message.GroupOrderDescending { + from := span.Lo + if prev != nil { + from, _ = locSucc(*prev) // at is after prev, so it has one + } + return []registry.LocRange{{Lo: from, Hi: at}} } - if loc.Group == prev.Group { - return prev.Object < loc.Object + // Descending: Group g of span carries Objects lo(g) through hi(g). + lo := func(g uint64) uint64 { + if g == span.Lo.Group { + return span.Lo.Object + } + return 0 } - if order == message.GroupOrderDescending { - return loc.Group < prev.Group + hi := func(g uint64) uint64 { + if g == span.Hi.Group { + return span.Hi.Object + } + return math.MaxUint64 + } + var from message.Location + switch { + case prev == nil: + from = message.Location{Group: span.Hi.Group, Object: lo(span.Hi.Group)} + case prev.Object < hi(prev.Group): + from = message.Location{Group: prev.Group, Object: prev.Object + 1} + default: + from = message.Location{Group: prev.Group - 1, Object: lo(prev.Group - 1)} + } + if from.Group == at.Group { + return []registry.LocRange{{Lo: from, Hi: at}} } - return prev.Group < loc.Group + out := []registry.LocRange{{Lo: message.Location{Group: at.Group, Object: lo(at.Group)}, Hi: at}} + if from.Group-at.Group > 1 { + out = append(out, registry.LocRange{ + Lo: message.Location{Group: at.Group + 1}, + Hi: message.Location{Group: from.Group - 1, Object: math.MaxUint64}, + }) + } + return append(out, registry.LocRange{Lo: from, Hi: message.Location{Group: from.Group, Object: hi(from.Group)}}) } // unknownRangeMarker returns the serve-path element that streamFetchObjects @@ -603,99 +637,6 @@ func timedOutRangeMarker(loc message.Location) *cache.CachedObject { } } -// unknownWholeRange declares the whole inclusive sub-range [start, endIncl] -// unknown with a single marker, positioned for the response's stream order. -// The marker Location is the range's far end in stream direction (endIncl -// when ascending, start when descending), so §11.4.4.2's "between the last -// serialized Object, if any, and this Location, inclusive" coverage spans -// the sub-range. -func unknownWholeRange(start, endIncl message.Location, order message.GroupOrder) []*cache.CachedObject { - return wholeRange(unknownRangeMarker, start, endIncl, order) -} - -// timedOutWholeRange is [unknownWholeRange] with the §11.4.4.2 End of -// Timed-Out Range marker, for when the FILL_TIMEOUT budget is what stopped us -// (§10.2.5) rather than an unreachable or unhelpful upstream. -func timedOutWholeRange(start, endIncl message.Location, order message.GroupOrder) []*cache.CachedObject { - return wholeRange(timedOutRangeMarker, start, endIncl, order) -} - -// wholeRange covers [start, endIncl] with a single marker built by mark. The -// marker names the far end of the range in delivery order, since §11.4.4.2 -// markers cover everything from the previous element up to their own Location. -func wholeRange( - mark func(message.Location) *cache.CachedObject, - start, endIncl message.Location, - order message.GroupOrder, -) []*cache.CachedObject { - if order == message.GroupOrderDescending { - return []*cache.CachedObject{mark(start)} - } - return []*cache.CachedObject{mark(endIncl)} -} - -// mergeFetchObjects merges the below-floor (upstream) and at/above-floor -// (cache) slices in group order. The two are disjoint by Location and each is -// already sorted in order, so for ascending the lower range leads and for -// descending the higher (cache) range leads. -// -// Descending needs one more step: within a group, Object IDs always ascend -// (§11.4.3), and §11.4.4's delta encoding cannot express a same-group -// transition to a lower Object ID — so when the eviction floor splits a -// group across the two sources, the seam group's runs must be spliced into -// one contiguous ascending run, upstream part (lower Object IDs) first. -// Plain concatenation would put the cache's high-object run before the -// upstream's low-object run of the same group and serialize a wrapped -// delta. Unknown-range markers interleaved with the seam run's objects move -// with them (their coverage and delta re-anchoring stay as the upstream -// meant them); a marker-only prefix — the whole-sub-range unknown marker, -// whose coverage spans everything below the cache — stays after it. -func mergeFetchObjects(order message.GroupOrder, lower, upper []*cache.CachedObject) []*cache.CachedObject { - switch { - case len(lower) == 0: - return upper - case len(upper) == 0: - return lower - } - out := make([]*cache.CachedObject, 0, len(lower)+len(upper)) - if order != message.GroupOrderDescending { - out = append(out, lower...) - out = append(out, upper...) - return out - } - - // The only group the two sources can share is the cache's lowest - // (upper's last element) — the floor group. splice is the length of - // lower's leading seam-group run, markers included: an interleaved - // upstream 0x10C marker belongs with its neighbouring objects (its - // coverage and the delta re-anchoring stay exactly as the upstream - // meant them, and every spliced Location is below the cache's seam - // objects). A prefix with no objects at all is NOT spliced — that is - // the whole-sub-range unknown marker, whose coverage spans everything - // below the cache and must stay after it. - seamG := upper[len(upper)-1].GroupID - splice, seamHasObject := 0, false - for splice < len(lower) && lower[splice].GroupID == seamG { - seamHasObject = seamHasObject || !lower[splice].IsRangeMarker() - splice++ - } - if !seamHasObject { - splice = 0 - } - // cut is where upper's trailing seam-group run starts. A plain group - // comparison suffices: upper's only markers are GetRange's expired-Object - // markers, each at its own Location. - cut := len(upper) - for cut > 0 && upper[cut-1].GroupID == seamG { - cut-- - } - out = append(out, upper[:cut]...) - out = append(out, lower[:splice]...) - out = append(out, upper[cut:]...) - out = append(out, lower[splice:]...) - return out -} - // fetchPredecessor returns the Location immediately below loc in (group, // object) order, and false when loc is {0, 0} (nothing precedes it). The // object-underflow case rolls back to the end of the previous group. @@ -829,8 +770,8 @@ func streamFetchObjects( default: // Same group. §11.4.4 cannot express a non-ascending Object ID - // here — the delta only ever adds. The inputs are sorted and - // seam-spliced (mergeFetchObjects), so hitting this is an + // here — the delta only ever adds. The inputs are sorted in + // stream order (fetchElements), so hitting this is an // internal invariant violation; fail rather than emit a wrapped // delta the subscriber must treat as a session-fatal overflow. if o.ObjectID <= prevObject { diff --git a/pkg/relay/handler_fetch_elem_test.go b/pkg/relay/handler_fetch_elem_test.go index 8c4b8f0b..c419a426 100644 --- a/pkg/relay/handler_fetch_elem_test.go +++ b/pkg/relay/handler_fetch_elem_test.go @@ -4,105 +4,63 @@ import ( "testing" "github.com/floatdrop/moq-go/pkg/moqt/message" + "github.com/floatdrop/moq-go/pkg/relay/internal/registry" ) -// TestUpstreamFetchElemOK: an upstream FETCH element may be re-serialized only -// if it lies in [start, endIncl], Object IDs ascend within a group, and Group -// IDs move in the response's order; markers get the range check only -// (§11.4.4). A wrong one would re-encode to well-formed but wrong IDs. +// TestUpstreamFetchElemOK: an upstream FETCH element, Object or marker, may be +// re-served only if it lies in the span and comes after the previous element +// in the order the response carries them: Object IDs ascending within a +// Group, Groups in the response's order (§11.4.4). A wrong one would re-encode +// to well-formed but wrong IDs. func TestUpstreamFetchElemOK(t *testing.T) { t.Parallel() loc := func(g, o uint64) message.Location { return message.Location{Group: g, Object: o} } - // The requested sub-range for every case below. - start, endIncl := loc(10, 0), loc(20, 5) + span := registry.LocRange{Lo: loc(10, 0), Hi: loc(20, 5)} + at := func(g, o uint64) *message.Location { l := loc(g, o); return &l } for _, tc := range []struct { - name string - loc message.Location - prev message.Location - havePrev bool - order message.GroupOrder - isMarker bool - want bool + name string + loc message.Location + prev *message.Location + order message.GroupOrder + want bool }{ - // Range bounds. Inclusive at both ends: endIncl is the last - // serviceable Location, not one past it. - {name: "first element at start", loc: loc(10, 0), want: true}, - {name: "first element at endIncl", loc: loc(20, 5), want: true}, - {name: "below start by one object", loc: loc(9, 9), want: false}, - {name: "above endIncl by one object", loc: loc(20, 6), want: false}, - {name: "above endIncl by one group", loc: loc(21, 0), want: false}, + // Span bounds, inclusive at both ends. + {name: "first element at the start", loc: loc(10, 0), want: true}, + {name: "first element at the end", loc: loc(20, 5), want: true}, + {name: "below the start by one object", loc: loc(9, 9), want: false}, + {name: "above the end by one object", loc: loc(20, 6), want: false}, + {name: "above the end by one group", loc: loc(21, 0), want: false}, + {name: "no prev, mid-span", loc: loc(15, 3), want: true}, - // No predecessor: only the range check can apply. - {name: "no prev, mid-range", loc: loc(15, 3), want: true}, - - // Within one group, Object IDs must strictly ascend — in BOTH - // order directions. GROUP_ORDER sequences groups, not the objects - // inside them, so descending must not loosen this. - { - name: "same group ascending object", loc: loc(15, 4), - prev: loc(15, 3), havePrev: true, want: true, - }, - { - name: "same group repeated object", loc: loc(15, 3), - prev: loc(15, 3), havePrev: true, want: false, - }, + // Within a Group, Object IDs strictly ascend in both orders. + {name: "same group ascending object", loc: loc(15, 4), prev: at(15, 3), want: true}, + {name: "same group repeated object", loc: loc(15, 3), prev: at(15, 3), want: false}, + {name: "same group descending object", loc: loc(15, 2), prev: at(15, 3), want: false}, { - name: "same group descending object", loc: loc(15, 2), - prev: loc(15, 3), havePrev: true, want: false, - }, - { - name: "same group descending object, descending order", loc: loc(15, 2), - prev: loc(15, 3), havePrev: true, + name: "same group descending object, descending order", loc: loc(15, 2), prev: at(15, 3), order: message.GroupOrderDescending, want: false, }, - // Across groups the direction must match the response order. - { - name: "ascending order, group advances", loc: loc(16, 0), - prev: loc(15, 3), havePrev: true, want: true, - }, + // Across Groups the direction must match the response order. + {name: "ascending order, group advances", loc: loc(16, 0), prev: at(15, 3), want: true}, + {name: "ascending order, group goes backwards", loc: loc(14, 0), prev: at(15, 3), want: false}, { - name: "ascending order, group goes backwards", loc: loc(14, 0), - prev: loc(15, 3), havePrev: true, want: false, - }, - { - name: "descending order, group goes backwards", loc: loc(14, 0), - prev: loc(15, 3), havePrev: true, + name: "descending order, group goes backwards", loc: loc(14, 0), prev: at(15, 3), order: message.GroupOrderDescending, want: true, }, { - name: "descending order, group advances", loc: loc(16, 0), - prev: loc(15, 3), havePrev: true, + name: "descending order, group advances", loc: loc(16, 0), prev: at(15, 3), order: message.GroupOrderDescending, want: false, }, - - // Markers re-anchor the encoding with absolute IDs, so an ordering - // violation is not one for them — but they are still confined to - // the requested range. - { - name: "marker may break group direction", loc: loc(14, 0), - prev: loc(15, 3), havePrev: true, isMarker: true, want: true, - }, - { - name: "marker may repeat a location", loc: loc(15, 3), - prev: loc(15, 3), havePrev: true, isMarker: true, want: true, - }, - { - name: "marker outside the range is still rejected", loc: loc(21, 0), - prev: loc(15, 3), havePrev: true, isMarker: true, want: false, - }, } { t.Run(tc.name, func(t *testing.T) { t.Parallel() - got := upstreamFetchElemOK( - tc.loc, tc.prev, tc.havePrev, start, endIncl, tc.order, tc.isMarker) - if got != tc.want { - t.Errorf("upstreamFetchElemOK(loc=%v prev=%v havePrev=%v order=%v marker=%v) = %v, want %v", - tc.loc, tc.prev, tc.havePrev, tc.order, tc.isMarker, got, tc.want) + if got := upstreamFetchElemOK(tc.loc, tc.prev, span, tc.order); got != tc.want { + t.Errorf("upstreamFetchElemOK(%v, prev %v, %v) = %v, want %v", tc.loc, tc.prev, tc.order, got, tc.want) } }) } diff --git a/pkg/relay/handler_fetch_merge_test.go b/pkg/relay/handler_fetch_merge_test.go deleted file mode 100644 index e8197b0f..00000000 --- a/pkg/relay/handler_fetch_merge_test.go +++ /dev/null @@ -1,189 +0,0 @@ -package relay - -import ( - "errors" - "io" - "slices" - "testing" - - "github.com/floatdrop/moq-go/pkg/moqt" - - "github.com/floatdrop/moq-go/pkg/moqt/message" - "github.com/floatdrop/moq-go/pkg/moqt/session" - "github.com/floatdrop/moq-go/pkg/moqt/session/sessiontest" - "github.com/floatdrop/moq-go/pkg/relay/cache" -) - -// obj is a cached Object at {g, o}. -func obj(g, o uint64) *cache.CachedObject { - return &cache.CachedObject{GroupID: g, ObjectID: o, Payload: []byte{byte(o)}} -} - -// marker is a cached End of Unknown Range marker at {g, o}. -func marker(g, o uint64) *cache.CachedObject { - return &cache.CachedObject{GroupID: g, ObjectID: o, EndOfUnknownRange: true} -} - -// loc is a {Group, Object} Location. -type loc struct{ G, O uint64 } - -// locsOf returns the Locations of objs. -func locsOf(objs []*cache.CachedObject) []loc { - out := make([]loc, 0, len(objs)) - for _, o := range objs { - out = append(out, loc{o.GroupID, o.ObjectID}) - } - return out -} - -// TestMergeFetchObjects_DescendingSeamSplice: when the eviction floor splits a -// group, the descending merge splices it into one ascending run, upstream's -// lower Object IDs first, which §11.4.4's delta encoding can express. -func TestMergeFetchObjects_DescendingSeamSplice(t *testing.T) { - t.Parallel() - desc := message.GroupOrderDescending - - cases := []struct { - name string - lower, upper []*cache.CachedObject - want []loc - }{ - { - name: "seam group split across sources", - upper: []*cache.CachedObject{obj(7, 0), obj(6, 2), obj(6, 3)}, - lower: []*cache.CachedObject{obj(6, 0), obj(6, 1), obj(5, 0)}, - want: []loc{{7, 0}, {6, 0}, {6, 1}, {6, 2}, {6, 3}, {5, 0}}, - }, - { - name: "trailing unknown marker stays after everything", - upper: []*cache.CachedObject{obj(7, 0), obj(6, 2), obj(6, 3)}, - lower: []*cache.CachedObject{obj(6, 0), obj(6, 1), marker(2, 0)}, - want: []loc{{7, 0}, {6, 0}, {6, 1}, {6, 2}, {6, 3}, {2, 0}}, - }, - { - name: "marker-only lower is appended, never spliced", - upper: []*cache.CachedObject{obj(6, 2), obj(6, 3)}, - lower: []*cache.CachedObject{marker(6, 1)}, - want: []loc{{6, 2}, {6, 3}, {6, 1}}, - }, - { - name: "group-aligned floor concatenates", - upper: []*cache.CachedObject{obj(7, 0), obj(6, 0)}, - lower: []*cache.CachedObject{obj(5, 3), obj(4, 0)}, - want: []loc{{7, 0}, {6, 0}, {5, 3}, {4, 0}}, - }, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - got := locsOf(mergeFetchObjects(desc, tc.lower, tc.upper)) - if !slices.Equal(got, tc.want) { - t.Errorf("merged %v, want %v", got, tc.want) - } - }) - } - - // Ascending stays a plain lower-then-upper concatenation. - asc := locsOf(mergeFetchObjects(message.GroupOrderAscending, - []*cache.CachedObject{obj(6, 0), obj(6, 1)}, - []*cache.CachedObject{obj(6, 2), obj(7, 0)})) - if !slices.Equal(asc, []loc{{6, 0}, {6, 1}, {6, 2}, {7, 0}}) { - t.Errorf("ascending merge = %v", asc) - } -} - -// TestStreamFetchObjects_DescendingSeamRoundTrip: a descending stitched -// response with a split group decodes back to the merged Locations. -func TestStreamFetchObjects_DescendingSeamRoundTrip(t *testing.T) { - t.Parallel() - cli, srv := sessiontest.NewSessionPair(t) - - merged := mergeFetchObjects(message.GroupOrderDescending, - []*cache.CachedObject{obj(6, 0), obj(6, 1), obj(5, 0)}, // upstream stitch - []*cache.CachedObject{obj(7, 0), obj(6, 2), obj(6, 3)}, // cache tail - ) - - writeErr := make(chan error, 1) - go func() { - out, err := cli.OpenFetchStream(message.FetchHeader{RequestID: 0}) - if err != nil { - writeErr <- err - return - } - if _, err := streamFetchObjects(out, merged, nil); err != nil { - // Reset so the reader fails fast instead of hanging on a - // never-FIN'd stream. - out.Cancel(moqt.StreamResetInternalError) - writeErr <- err - return - } - writeErr <- out.Close() - }() - - ds, err := srv.AcceptDataStream(t.Context()) - if err != nil { - t.Fatalf("AcceptDataStream: %v", err) - } - fs, ok := ds.(*session.IncomingFetchStream) - if !ok { - t.Fatalf("AcceptDataStream returned %T", ds) - } - fs.GroupOrder = message.GroupOrderDescending - - var got []loc - for { - o, err := fs.ReadDecoded() - if errors.Is(err, io.EOF) { - break - } - if err != nil { - t.Fatalf("ReadDecoded: %v", err) - } - got = append(got, loc{o.GroupID, o.ObjectID}) - } - if err := <-writeErr; err != nil { - t.Fatalf("writer: %v", err) - } - - want := []loc{{7, 0}, {6, 0}, {6, 1}, {6, 2}, {6, 3}, {5, 0}} - if !slices.Equal(got, want) { - t.Fatalf("decoded %v, want %v", got, want) - } -} - -// TestMergeFetchObjects_SeamMarkersSpliced: an upstream marker among the seam -// group's Objects moves with them; a marker-only prefix stays after the cache. -func TestMergeFetchObjects_SeamMarkersSpliced(t *testing.T) { - t.Parallel() - desc := message.GroupOrderDescending - upper := []*cache.CachedObject{obj(7, 0), obj(6, 3), obj(6, 4)} - - cases := []struct { - name string - lower []*cache.CachedObject - want []loc - }{ - { - name: "marker between seam objects", - lower: []*cache.CachedObject{obj(6, 0), marker(6, 1), obj(6, 2), obj(5, 0)}, - want: []loc{{7, 0}, {6, 0}, {6, 1}, {6, 2}, {6, 3}, {6, 4}, {5, 0}}, - }, - { - name: "marker trailing the seam run", - lower: []*cache.CachedObject{obj(6, 0), marker(6, 2), obj(5, 0)}, - want: []loc{{7, 0}, {6, 0}, {6, 2}, {6, 3}, {6, 4}, {5, 0}}, - }, - { - name: "marker-only seam prefix stays after the cache", - lower: []*cache.CachedObject{marker(6, 2)}, - want: []loc{{7, 0}, {6, 3}, {6, 4}, {6, 2}}, - }, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - got := locsOf(mergeFetchObjects(desc, tc.lower, upper)) - if !slices.Equal(got, tc.want) { - t.Errorf("merged %v, want %v", got, tc.want) - } - }) - } -} diff --git a/pkg/relay/handler_fetch_ranges_test.go b/pkg/relay/handler_fetch_ranges_test.go new file mode 100644 index 00000000..4fc4ab0b --- /dev/null +++ b/pkg/relay/handler_fetch_ranges_test.go @@ -0,0 +1,193 @@ +package relay + +import ( + "errors" + "io" + "math" + "slices" + "testing" + + "github.com/floatdrop/moq-go/pkg/moqt" + "github.com/floatdrop/moq-go/pkg/moqt/message" + "github.com/floatdrop/moq-go/pkg/moqt/session" + "github.com/floatdrop/moq-go/pkg/moqt/session/sessiontest" + "github.com/floatdrop/moq-go/pkg/moqt/wire" + "github.com/floatdrop/moq-go/pkg/relay/cache" + "github.com/floatdrop/moq-go/pkg/relay/internal/registry" +) + +// at is the Location {g, o}; rng the range [lo, hi]. +func at(g, o uint64) message.Location { return message.Location{Group: g, Object: o} } +func rng(lo, hi message.Location) registry.LocRange { + return registry.LocRange{Lo: lo, Hi: hi} +} + +const maxID = math.MaxUint64 + +// TestUncovered: the parts of a range no known range covers, with overlapping, +// touching and out-of-range known ranges, across a Group boundary. +func TestUncovered(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + known []registry.LocRange + want []registry.LocRange + }{ + {"nothing known", nil, []registry.LocRange{rng(at(1, 0), at(3, 5))}}, + { + "points and a range", []registry.LocRange{rng(at(1, 0), at(1, 0)), rng(at(1, 2), at(1, maxID)), rng(at(3, 5), at(3, 5))}, + []registry.LocRange{rng(at(1, 1), at(1, 1)), rng(at(2, 0), at(3, 4))}, + }, + { + "overlapping and touching", []registry.LocRange{rng(at(0, 0), at(1, 3)), rng(at(1, 2), at(1, 5)), rng(at(1, 6), at(9, 0))}, + nil, + }, + {"covering the end exactly", []registry.LocRange{rng(at(2, 0), at(3, 5))}, []registry.LocRange{rng(at(1, 0), at(1, maxID))}}, + { + "known ranges outside", []registry.LocRange{rng(at(0, 0), at(0, maxID)), rng(at(4, 0), at(4, 0))}, + []registry.LocRange{rng(at(1, 0), at(3, 5))}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + if got := uncovered(at(1, 0), at(3, 5), tc.known); !slices.Equal(got, tc.want) { + t.Fatalf("uncovered = %v, want %v", got, tc.want) + } + }) + } +} + +// TestKnownFromCache: what cached Objects establish, from status Objects and +// gap Properties (§11.2.1.1, §12.8, §12.9). +func TestKnownFromCache(t *testing.T) { + t.Parallel() + gapProps := func(typ message.PropertyType, n uint64) []byte { + return message.AppendTrackProperties([]wire.KVPair{{Type: typ, IntVal: n}}) + } + got := knownFromCache([]*cache.CachedObject{ + {GroupID: 1, ObjectID: 0, Payload: []byte("x")}, + {GroupID: 1, ObjectID: 4, Status: message.ObjectStatusEndOfGroup}, + {GroupID: 5, ObjectID: 3, Payload: []byte("x"), Properties: gapProps(message.PropertyPriorObjectIDGap, 2)}, + {GroupID: 8, ObjectID: 0, Payload: []byte("x"), Properties: gapProps(message.PropertyPriorGroupIDGap, 3)}, + {GroupID: 9, ObjectID: 2, Status: message.ObjectStatusEndOfTrack}, + }) + want := []registry.LocRange{ + rng(at(1, 0), at(1, 0)), + rng(at(1, 4), at(1, maxID)), + rng(at(5, 3), at(5, 3)), rng(at(5, 1), at(5, 2)), + rng(at(8, 0), at(8, 0)), rng(at(5, 0), at(7, maxID)), + rng(at(9, 2), at(maxID, maxID)), + } + if !slices.Equal(got, want) { + t.Fatalf("knownFromCache = %v, want %v", got, want) + } +} + +// TestStreamCovered: what an upstream's End of Range marker covers, in the +// order the response carries Locations (see fetch_ranges.go). +func TestStreamCovered(t *testing.T) { + t.Parallel() + span := rng(at(2, 3), at(5, 1)) + prev := func(g, o uint64) *message.Location { l := at(g, o); return &l } + for _, tc := range []struct { + name string + prev *message.Location + at message.Location + order message.GroupOrder + want []registry.LocRange + }{ + {"ascending, first element", nil, at(3, 4), message.GroupOrderAscending, []registry.LocRange{rng(at(2, 3), at(3, 4))}}, + {"ascending, after one", prev(3, 4), at(4, 0), message.GroupOrderAscending, []registry.LocRange{rng(at(3, 5), at(4, 0))}}, + { + "descending, first element, same Group", nil, at(5, 0), message.GroupOrderDescending, + []registry.LocRange{rng(at(5, 0), at(5, 0))}, + }, + { + "descending, across Groups", prev(5, 0), at(2, 7), message.GroupOrderDescending, + []registry.LocRange{rng(at(2, 3), at(2, 7)), rng(at(3, 0), at(4, maxID)), rng(at(5, 1), at(5, 1))}, + }, + { + "descending, from a Group's last Object", prev(5, 1), at(4, 2), message.GroupOrderDescending, + []registry.LocRange{rng(at(4, 0), at(4, 2))}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got := streamCovered(tc.prev, tc.at, span, tc.order) + slices.SortFunc(got, func(a, b registry.LocRange) int { return a.Lo.Compare(b.Lo) }) + if !slices.Equal(got, tc.want) { + t.Fatalf("streamCovered = %v, want %v", got, tc.want) + } + }) + } +} + +// TestFetchElements_DescendingRoundTrip: a Descending response with unknown +// runs inside, at the head of, and across Groups encodes (§11.4.4) and decodes +// back to its elements, markers included, in stream order. +func TestFetchElements_DescendingRoundTrip(t *testing.T) { + t.Parallel() + o := func(g, id uint64) *cache.CachedObject { + return &cache.CachedObject{GroupID: g, ObjectID: id, Payload: []byte{byte(id)}} + } + elems := fetchElements( + []*cache.CachedObject{o(1, 0), o(3, 2), o(3, 4), o(6, 0)}, + []registry.LocRange{ + rng(at(1, 1), at(3, 1)), // tail of 1, Group 2, head of 3 + rng(at(3, 3), at(3, 3)), // inside 3 + rng(at(4, 0), at(5, maxID)), + }, + nil, message.GroupOrderDescending) + + type el struct { + g, o uint64 + unknown bool + } + want := []el{ + {6, 0, false}, + {3, 1, true}, // Groups 5 and 4 and the head of 3: one run, no Object between + {3, 2, false}, {3, 3, true}, {3, 4, false}, + {2, maxID, true}, // Group 2 (the tail of 1 follows Object {1, 0}) + {1, 0, false}, + {1, maxID, true}, + } + + cli, srv := sessiontest.NewSessionPair(t) + writeErr := make(chan error, 1) + go func() { + out, err := cli.OpenFetchStream(message.FetchHeader{RequestID: 0}) + if err != nil { + writeErr <- err + return + } + if _, err := streamFetchObjects(out, elems, nil); err != nil { + out.Cancel(moqt.StreamResetInternalError) + writeErr <- err + return + } + writeErr <- out.Close() + }() + ds, err := srv.AcceptDataStream(t.Context()) + if err != nil { + t.Fatalf("AcceptDataStream: %v", err) + } + fs := ds.(*session.IncomingFetchStream) + fs.GroupOrder = message.GroupOrderDescending + var got []el + for { + d, err := fs.ReadDecoded() + if errors.Is(err, io.EOF) { + break + } + if err != nil { + t.Fatalf("ReadDecoded: %v", err) + } + got = append(got, el{d.GroupID, d.ObjectID, d.EndOfUnknownRange}) + } + if err := <-writeErr; err != nil { + t.Fatalf("writer: %v", err) + } + if !slices.Equal(got, want) { + t.Fatalf("decoded %v, want %v", got, want) + } +} diff --git a/pkg/relay/handler_fetch_split_test.go b/pkg/relay/handler_fetch_split_test.go index f7f4727e..aaa4c2ae 100644 --- a/pkg/relay/handler_fetch_split_test.go +++ b/pkg/relay/handler_fetch_split_test.go @@ -2,11 +2,9 @@ package relay import ( "math" - "slices" "testing" "github.com/floatdrop/moq-go/pkg/moqt/message" - "github.com/floatdrop/moq-go/pkg/relay/cache" ) func TestFetchPredecessor(t *testing.T) { @@ -82,36 +80,3 @@ func TestCapFetchEndLocation(t *testing.T) { t.Errorf("whole end group = %v, want the capped %v", got, largest) } } - -func TestMergeFetchObjects(t *testing.T) { - lower := []*cache.CachedObject{{GroupID: 0}, {GroupID: 1}} - upper := []*cache.CachedObject{{GroupID: 5}, {GroupID: 6}} - - asc := groupIDs(mergeFetchObjects(message.GroupOrderAscending, lower, upper)) - if !slices.Equal(asc, []uint64{0, 1, 5, 6}) { - t.Errorf("ascending merge = %v; want [0 1 5 6] (lower leads)", asc) - } - desc := groupIDs(mergeFetchObjects(message.GroupOrderDescending, lower, upper)) - if !slices.Equal(desc, []uint64{5, 6, 0, 1}) { - t.Errorf("descending merge = %v; want [5 6 0 1] (upper leads)", desc) - } - - // Degenerate inputs pass through unchanged. - emptyLower := groupIDs(mergeFetchObjects(message.GroupOrderAscending, nil, upper)) - if !slices.Equal(emptyLower, []uint64{5, 6}) { - t.Errorf("empty lower = %v; want [5 6]", emptyLower) - } - emptyUpper := groupIDs(mergeFetchObjects(message.GroupOrderAscending, lower, nil)) - if !slices.Equal(emptyUpper, []uint64{0, 1}) { - t.Errorf("empty upper = %v; want [0 1]", emptyUpper) - } -} - -// groupIDs returns the Group IDs of objs. -func groupIDs(objs []*cache.CachedObject) []uint64 { - out := make([]uint64, len(objs)) - for i, o := range objs { - out[i] = o.GroupID - } - return out -} diff --git a/pkg/relay/handler_fetch_stitch_test.go b/pkg/relay/handler_fetch_stitch_test.go index 8980f035..ee0f1f56 100644 --- a/pkg/relay/handler_fetch_stitch_test.go +++ b/pkg/relay/handler_fetch_stitch_test.go @@ -22,7 +22,7 @@ func TestFetch_StitchesEvictedRangeFromUpstream(t *testing.T) { video := ns("video") name := []byte("cam1") - const liveLo, liveHi = uint64(5), uint64(9) // cached → floor = group 5 + const liveLo, liveHi = uint64(5), uint64(9) // cached from group 5 const upstreamAlias = uint64(42) if _, err := upSess.PublishNamespace(t.Context(), &message.PublishNamespace{Namespace: video}); err != nil { @@ -31,7 +31,7 @@ func TestFetch_StitchesEvictedRangeFromUpstream(t *testing.T) { // Upstream loop: answer the on-demand SUBSCRIBE by pushing the live tail // (which the relay caches), and answer the stitch FETCH with the older, - // below-floor range. + // uncached range. go func() { for { req, err := upSess.AcceptRequest(t.Context()) @@ -61,11 +61,10 @@ func TestFetch_StitchesEvictedRangeFromUpstream(t *testing.T) { } case *message.Fetch: // Serve exactly the group range the relay asks for. The relay - // requests precisely the below-floor part, and the floor isn't - // fixed (the upstream subscription uses the Next Object filter, so - // the relay may not cache the upstream's first pushed group) — - // honouring the requested range keeps the split gapless whatever - // the floor turns out to be. + // requests precisely the uncached part, which isn't fixed (the + // upstream subscription uses the Next Object filter, so the relay + // may not cache the upstream's first pushed group) — honouring the + // requested range keeps the answer gapless whatever it is. // draft-20 carries the range in LOCATION_FILTER (§5.1.2), and the // relay always sends the absolute four-field form. f, ferr := message.LocationFilterFromParam(m.Parameters) diff --git a/pkg/relay/handler_fetch_unknown_test.go b/pkg/relay/handler_fetch_unknown_test.go index 8c958357..cf08f840 100644 --- a/pkg/relay/handler_fetch_unknown_test.go +++ b/pkg/relay/handler_fetch_unknown_test.go @@ -2,6 +2,7 @@ package relay_test import ( "math" + "sync" "testing" "time" @@ -15,7 +16,7 @@ import ( // unknownGapTopology wires upstream publisher → relay ← live subscriber, with // onFetch answering the relay's upstream FETCH, and returns a fetch-only // client. The upstream pushes single-object groups liveLo..liveHi, so the cache -// floor is liveLo and a FETCH from group 0 has a below-floor part. +// cached tail starts at liveLo, so a FETCH from group 0 has an uncached part. func unknownGapTopology( t *testing.T, ns wire.TrackNamespace, @@ -32,6 +33,8 @@ func unknownGapTopology( t.Fatalf("PublishNamespace: %v", err) } + written := make(chan struct{}) + tailWritten := sync.OnceFunc(func() { close(written) }) go func() { for { req, err := upSess.AcceptRequest(t.Context()) @@ -46,6 +49,7 @@ func unknownGapTopology( for g := liveLo; g <= liveHi; g++ { sg, err := upSess.OpenSubgroup(message.SubgroupHeader{ SubgroupIDMode: message.SubgroupIDImplicitZero, + EndOfGroup: true, // its FIN ends the Group TrackAlias: upstreamAlias, GroupID: g, }) @@ -55,6 +59,7 @@ func unknownGapTopology( _ = sg.WriteObject(&message.SubgroupObject{Payload: []byte{byte('a' + g)}}) _ = sg.Close() } + tailWritten() case *message.Fetch: onFetch(upSess, req, m) } @@ -75,19 +80,20 @@ func unknownGapTopology( // would hand back a fetch client while the cache is still filling, and // every caller's expected answer is stated in terms of a *full* tail — so // a fetch that lands early gets a legitimately different answer, with the - // unknown-range floor sitting wherever the cache happened to reach. + // unknown range ending wherever the cache happened to reach. // // Wait on the relay's watermark (TRACK_STATUS, §10.2.17), not on the // subscriber: the relay may drop a lagging subscriber (§3.3.4) while the // cache is fully populated. go drainAll(t.Context(), live) + awaitTailCached(t, written) fetchClient := dialAnotherClient(t, upSess) waitRelayLargest(t, fetchClient, ns, name, liveHi, 0) return fetchClient } -// TestFetch_UnknownRangeMarkerWhenUpstreamRejects: a below-floor part the +// TestFetch_UnknownRangeMarkerWhenUpstreamRejects: an uncached part the // upstream refuses to serve is covered by an End of Unknown Range marker before // the cached Objects, not left as a gap (§11.4.4). func TestFetch_UnknownRangeMarkerWhenUpstreamRejects(t *testing.T) { @@ -104,8 +110,8 @@ func TestFetch_UnknownRangeMarkerWhenUpstreamRejects(t *testing.T) { for { elems := tryFetchElems(t, fc, video, name, liveHi, nil) if len(elems) > 0 && elems[0].Unknown && groupsEqual(realGroups(elems), liveLo, liveHi) { - // The marker covers [request start, cache floor): its Location - // is the floor's predecessor. + // The marker covers the uncached part: its Location is the end of + // the Group before the cached tail. if wantG := liveLo - 1; elems[0].Group != wantG || elems[0].Object != math.MaxUint64 { t.Fatalf("unknown marker at {%d,%d}, want {%d,%d}", elems[0].Group, elems[0].Object, wantG, uint64(math.MaxUint64)) @@ -126,8 +132,10 @@ func TestFetch_UnknownRangeMarkerWhenUpstreamRejects(t *testing.T) { } // TestFetch_UnknownRangeMarkerDescending is the descending-order counterpart: -// the unserviceable below-floor range comes last in stream order, so the -// marker must trail the cached objects, at the range's start Location. +// the unserviceable uncached range comes last in stream order, so the +// marker trails the cached objects, at the range's last Location in stream +// order: Group 0's last Object (Groups descending, Objects ascending within +// one; see fetch_ranges.go). func TestFetch_UnknownRangeMarkerDescending(t *testing.T) { video := ns("video") name := []byte("cam-unknown-desc") @@ -151,8 +159,8 @@ func TestFetch_UnknownRangeMarkerDescending(t *testing.T) { } if len(elems) > 0 && descOK { last := elems[len(elems)-1] - if !last.Unknown || last.Group != 0 || last.Object != 0 { - t.Fatalf("want trailing unknown marker at {0,0}, got %+v (elems %v)", last, elems) + if !last.Unknown || last.Group != 0 || last.Object != math.MaxUint64 { + t.Fatalf("want trailing unknown marker at {0, 2^64-1}, got %+v (elems %v)", last, elems) } return } @@ -265,7 +273,7 @@ func TestFetch_UnknownMarkerWhenUpstreamCapsEndLocation(t *testing.T) { groupsEqual(got[:upstreamHi+1], 0, upstreamHi) && groupsEqual(got[upstreamHi+1:], liveLo, liveHi) { // One unknown marker, between the stitched head and the cached - // tail, at the below-floor sub-range's inclusive end. + // tail, at the uncached sub-range's inclusive end. if len(elems) != len(got)+1 || !elems[upstreamHi+1].Unknown { t.Fatalf("want single unknown marker after group %d, got elems %v", upstreamHi, elems) } @@ -306,7 +314,7 @@ func TestFetch_DiscardsOutOfRangeUpstreamElements(t *testing.T) { return } // Rogue marker beyond the requested range (the relay asked for - // the below-floor part only, ending before group liveLo). + // the uncached part only, ending before group liveLo). _ = out.WriteObject(&message.FetchObject{ SerializationFlags: message.FetchEndOfUnknownRange, GroupIDDelta: liveHi - 2, @@ -319,7 +327,7 @@ func TestFetch_DiscardsOutOfRangeUpstreamElements(t *testing.T) { for { elems := tryFetchElems(t, fc, video, name, liveHi, nil) if len(elems) > 0 && elems[0].Unknown && groupsEqual(realGroups(elems), liveLo, liveHi) { - // The rogue marker must not appear; the below-floor range is + // The rogue marker must not appear; the uncached range is // covered by the relay's own whole-sub-range marker instead. if wantG := liveLo - 1; elems[0].Group != wantG || elems[0].Object != math.MaxUint64 { t.Fatalf("marker at {%d,%d}, want relay's own at {%d,%d}", diff --git a/pkg/relay/handler_fetch_upstream_fail_test.go b/pkg/relay/handler_fetch_upstream_fail_test.go index 4ebcdbe0..df91210b 100644 --- a/pkg/relay/handler_fetch_upstream_fail_test.go +++ b/pkg/relay/handler_fetch_upstream_fail_test.go @@ -2,6 +2,7 @@ package relay_test import ( "fmt" + "math" "sync" "testing" "time" @@ -79,7 +80,7 @@ func TestFetch_UpstreamOutcomeDecidesGapOrUnknown(t *testing.T) { if got := stitchMarkerOf(objs); got != tc.wantMarker { t.Fatalf("stitched response encoded %s, want %s", got, tc.wantMarker) } - // The below-floor part must never be served from an upstream + // The uncached part must never be served from an upstream // response the relay could not read to completion. Groups below // the cached tail would be exactly that. if groups := stitchedGroups( @@ -93,11 +94,11 @@ func TestFetch_UpstreamOutcomeDecidesGapOrUnknown(t *testing.T) { } } -// TestFetch_DescendingCappedUpstreamFallsBackToWholeUnknown: when the upstream -// caps FETCH_OK below the requested sub-range (§10.13), a descending response -// marks the whole sub-range unknown, anchored at its start, since a trailing -// marker cannot be placed. -func TestFetch_DescendingCappedUpstreamFallsBackToWholeUnknown(t *testing.T) { +// TestFetch_DescendingCappedUpstreamMarksRemainder: when the upstream caps +// FETCH_OK below the requested sub-range (§10.13), a descending response marks +// what lies past the cap unknown, and nothing below it: under the upstream's +// clean FIN the rest does not exist. +func TestFetch_DescendingCappedUpstreamMarksRemainder(t *testing.T) { t.Parallel() objs := runStitch(t, stitchOpts{ order: message.GroupOrderDescending, @@ -121,29 +122,18 @@ func TestFetch_DescendingCappedUpstreamFallsBackToWholeUnknown(t *testing.T) { }, }) - // Presence of a marker is not enough to tell the two encodings apart: the - // ascending path also emits one here. What separates them is WHERE it is - // anchored. unknownWholeRange anchors a descending marker at the - // sub-range START (group 0 — the whole below-floor range is unknown), - // while the per-remainder path appends one at endIncl, the top of that - // range. A marker in the wrong place is a well-formed response making a - // false claim about which objects are undetermined. - var marker *session.DecodedFetchObject + // The sub-range is Groups 0-4 (the cache holds 5-9); the cap is {3, 0}. + // Past it lie {3, 1} through Group 4, whose run ends, in stream order, + // with Group 3's last Object. Groups 0-2 and {3, 0} are a plain gap. + var markers []*session.DecodedFetchObject for _, o := range objs { if o.EndOfUnknownRange { - marker = o - break + markers = append(markers, o) } } - if marker == nil { - t.Fatalf("a capped descending upstream response produced no unknown marker; "+ - "the uncovered remainder was encoded as an authoritative gap (objects: %v)", - stitchedGroups(objs)) - } - if marker.GroupID != 0 { - t.Errorf("unknown marker anchored at group %d, want 0 — descending must fall back "+ - "to marking the WHOLE sub-range unknown, not just the uncovered remainder", - marker.GroupID) + if len(markers) != 1 || markers[0].GroupID != 3 || markers[0].ObjectID != math.MaxUint64 { + t.Fatalf("unknown markers %v, want one at {3, 2^64-1} covering what lies past the cap "+ + "(objects: %v)", markers, stitchedGroups(objs)) } } @@ -209,7 +199,7 @@ func TestFetch_StitchedObjectKeepsDatagramForwardingPreference(t *testing.T) { } const ( - stitchLiveLo = uint64(5) // cached live tail: groups 5..9, so the eviction floor is 5 + stitchLiveLo = uint64(5) // cached live tail: groups 5..9, so Groups 0-4 are uncached stitchLiveHi = uint64(9) ) @@ -246,8 +236,8 @@ func replyThen(end func(*session.OutgoingFetchStream)) func(*session.Session, *s } // runStitch runs the stitch topology of TestFetch_StitchesEvictedRangeFromUpstream -// (an upstream feeding a cached live tail, and a FETCH reaching below the -// eviction floor) and returns the stitched response's elements. +// (an upstream feeding a cached live tail, and a FETCH reaching the uncached +// Groups below it) and returns the stitched response's elements. func runStitch(t *testing.T, opts stitchOpts) []*session.DecodedFetchObject { t.Helper() upSess, teardown := connectRelay(t, relay.Config{}) @@ -288,6 +278,8 @@ func runStitch(t *testing.T, opts stitchOpts) []*session.DecodedFetchObject { return fmt.Sprintf("; the upstream goroutine had already stopped: %v", upFail) } + written := make(chan struct{}) + tailWritten := sync.OnceFunc(func() { close(written) }) go func() { for { req, err := upSess.AcceptRequest(t.Context()) @@ -306,6 +298,7 @@ func runStitch(t *testing.T, opts stitchOpts) []*session.DecodedFetchObject { for g := stitchLiveLo; g <= stitchLiveHi; g++ { sg, err := openSubgroupWaiting(t, upSess, message.SubgroupHeader{ SubgroupIDMode: message.SubgroupIDImplicitZero, + EndOfGroup: true, // its FIN ends the Group TrackAlias: upstreamAlias, GroupID: g, }) @@ -322,6 +315,7 @@ func runStitch(t *testing.T, opts stitchOpts) []*session.DecodedFetchObject { return } } + tailWritten() case *message.Fetch: if opts.onFetch == nil { continue // never answer: the relay must time out @@ -339,9 +333,10 @@ func runStitch(t *testing.T, opts stitchOpts) []*session.DecodedFetchObject { } t.Cleanup(func() { _ = liveReq.Close() }) go drainAll(t.Context(), live) + awaitTailCached(t, written) // Retry until the cached tail is present: before that the FETCH is either - // rejected or answers from an empty cache, and the below-floor split this + // rejected or answers from an empty cache, and the uncached part this // test is about has not happened yet. fc := dialAnotherClient(t, upSess) deadline := time.Now().Add(10 * time.Second) @@ -468,7 +463,7 @@ func TestFetch_RangeFilterKeepsTimedOutMarker(t *testing.T) { }) // onFetch nil + a short FILL_TIMEOUT is the §10.2.5 budget-exhausted path, - // which reports the below-floor span as an End of Timed-Out Range. + // which reports the uncached span as an End of Timed-Out Range. objs := runStitch(t, stitchOpts{ onFetch: nil, fillTimeout: 300 * time.Millisecond, diff --git a/pkg/relay/handler_fill.go b/pkg/relay/handler_fill.go index b6c7278d..ddd002fb 100644 --- a/pkg/relay/handler_fill.go +++ b/pkg/relay/handler_fill.go @@ -104,15 +104,16 @@ func (h *sessionHandler) maybeServeFill( } h.relayGo(func() { - h.serveFill(ctx, sub, requestID, entry, fullName, start, end, order, fillTimeout, rangeFilters) + // §5.1.3.1: "When the subscription is cancelled, the publisher MUST + // reset any open fill fetch streams". + fillCtx, cancel := context.WithCancelCause(ctx) + defer cancel(nil) + defer context.AfterFunc(sub.Cancelled(), func() { cancel(errRequestCancelled) })() + h.serveFill(fillCtx, sub, requestID, entry, fullName, start, end, order, fillTimeout, rangeFilters) }) return nil } -// TODO(draft-20): §5.1.3.1 "When the subscription is cancelled, the publisher -// MUST reset any open fill fetch streams" needs a watchdog on ctx cancellation -// mid-write. - // serveFill writes one fill fetch stream; the FIN signals completion // (§5.1.3.1), and [sessionHandler.streamFetchRange] resets it on a write error. func (h *sessionHandler) serveFill( diff --git a/pkg/relay/handler_forward.go b/pkg/relay/handler_forward.go index 1381c7fd..7abb9e4c 100644 --- a/pkg/relay/handler_forward.go +++ b/pkg/relay/handler_forward.go @@ -85,6 +85,11 @@ func (h *sessionHandler) serveForwardedPublish( sub.OpenedByPublish() // handleSubscribeTracks already refused parameters this would reject. _ = installSubscribeParams(sub, params) + // The Group Order the PUBLISH stated (see publishParamsForSubscriber), so + // the subscription's fills follow what it was told (§10.20.1). + if p, ok := fwd.Parameters.Find(message.ParamGroupOrder); ok { + sub.SetGroupOrder(p.Byte) + } _, largest, has, added := h.tracks.AddDownstreamSnapshotLargest(fullName, sub) registered() if !added { @@ -96,6 +101,9 @@ func (h *sessionHandler) serveForwardedPublish( ref := h.trackRef(fullName) h.metrics.SubscriptionOpened(ref) defer h.metrics.SubscriptionClosed(ref) + // §5.1.1: once the subscriber cancels, or the session ends, reset the + // streams still open for it. + defer sub.Cancel() // §9.2: a forwarding subscriber resumes a paused upstream. if sub.ForwardState() == 1 { h.propagateForwardUpstream(ctx, fullName) diff --git a/pkg/relay/handler_namespace.go b/pkg/relay/handler_namespace.go index a7ede1d3..ee829f62 100644 --- a/pkg/relay/handler_namespace.go +++ b/pkg/relay/handler_namespace.go @@ -201,7 +201,7 @@ func (h *sessionHandler) handleSubscribeNamespace( h.spawn(entry.RunWriter) // Replies share the entry's queue, keeping their order with NAMESPACE. - h.serveNamespaceFollowups(ctx, req, enqueueReply(entry), h.namespaceUpdate(entry, &prefix)) + h.serveNamespaceFollowups(ctx, req, enqueueReply(entry), h.namespaceUpdate(entry, &prefix, msg)) } // handleSubscribeTracks implements SUBSCRIBE_TRACKS (§6.1, §10.20): @@ -265,7 +265,7 @@ func (h *sessionHandler) handleSubscribeTracks( } // Replies share the entry's queue with PUBLISH_SKIPPED, so each // PUBLISH_SKIPPED suffix matches the prefix the subscriber last saw. - h.serveNamespaceFollowups(ctx, req, enqueueReply(entry), h.tracksUpdate(entry, &prefix)) + h.serveNamespaceFollowups(ctx, req, enqueueReply(entry), h.tracksUpdate(entry, &prefix, msg)) } // subscribeTracksForwarding resolves a SUBSCRIBE_TRACKS's FORWARD (§10.2.18, @@ -290,15 +290,15 @@ func subscribeTracksForwarding(ps message.Parameters) (forward bool, groupOrder } // serveNamespaceFollowups holds a namespace request stream open and answers -// each REQUEST_UPDATE (§10.9), validating its Request ID (§10.1) and tokens. -// The subscriptions pass update, which applies it and replies; with update -// nil (PUBLISH_NAMESPACE) write sends a plain REQUEST_OK. Other follow-ups -// are ignored. +// each REQUEST_UPDATE (§10.9), validating its Request ID (§10.1) and resolving +// its tokens. The subscriptions pass update, which authorizes and applies it +// and replies; with update nil (PUBLISH_NAMESPACE) write sends a plain +// REQUEST_OK. Other follow-ups are ignored. func (h *sessionHandler) serveNamespaceFollowups( ctx context.Context, req *session.Request, write func(message.Message) error, - update func(context.Context, *message.RequestUpdate) bool, + update func(context.Context, *message.RequestUpdate, []session.ResolvedToken) bool, ) { stream := req.Stream scope := message.ScopeOfUpdate(req.First.Type()) @@ -323,12 +323,13 @@ func (h *sessionHandler) serveNamespaceFollowups( if !h.handleRequestUpdateLimit(ctx, updates) { return false } - if !h.handleFollowupTokens(ctx, upd) { + toks, ok := h.handleFollowupTokens(ctx, upd) + if !ok { return false } // false from update ends the request. if update != nil { - if !update(ctx, upd) { + if !update(ctx, upd, toks) { return false } updates.Responded() @@ -376,18 +377,33 @@ func (h *sessionHandler) updatePrefixParam(upd *message.RequestUpdate) (prefix w return prefix, true, true } -// namespaceUpdate answers a REQUEST_UPDATE on a SUBSCRIBE_NAMESPACE: a -// TRACK_NAMESPACE_PREFIX is applied (§10.9.2); anything else is acknowledged. +// namespaceUpdate answers a REQUEST_UPDATE on the SUBSCRIBE_NAMESPACE msg: a +// TRACK_NAMESPACE_PREFIX is authorized and applied (§10.9.2); anything else is +// acknowledged. A refused update ends the request (see [endAfterFinish]). func (h *sessionHandler) namespaceUpdate( e *registry.SubscriberEntry, cur *wire.TrackNamespace, -) func(context.Context, *message.RequestUpdate) bool { + msg *message.SubscribeNamespace, +) func(context.Context, *message.RequestUpdate, []session.ResolvedToken) bool { updatePrefix := h.prefixUpdater(e, &h.nsPrefixes, cur) - return func(ctx context.Context, upd *message.RequestUpdate) bool { + tokens := authorizingTokens(msg.Parameters) + return func(ctx context.Context, upd *message.RequestUpdate, toks []session.ResolvedToken) bool { prefix, found, ok := h.updatePrefixParam(upd) if !ok { return false } + updTokens := updatedTokens(tokens, upd.Parameters) + if rej := h.refuseUpdate(ctx, toks, found, func() error { + return h.auth.AuthorizeSubscribeNamespace(ctx, h.sess, &message.SubscribeNamespace{ + RequestID: msg.RequestID, + TrackNamespacePrefix: prefix, + Parameters: withTokens(msg.Parameters, updTokens), + }) + }); rej != nil { + e.Finish(rej) + return endAfterFinish(ctx, e) + } + tokens = updTokens if !found { e.Enqueue(&message.RequestOK{}) return true @@ -409,14 +425,29 @@ func (h *sessionHandler) namespaceUpdate( func (h *sessionHandler) tracksUpdate( e *registry.SubscriberEntry, cur *wire.TrackNamespace, -) func(context.Context, *message.RequestUpdate) bool { - return func(ctx context.Context, upd *message.RequestUpdate) bool { + msg *message.SubscribeTracks, +) func(context.Context, *message.RequestUpdate, []session.ResolvedToken) bool { + tokens := authorizingTokens(msg.Parameters) + return func(ctx context.Context, upd *message.RequestUpdate, toks []session.ResolvedToken) bool { prefix, hasPrefix, ok := h.updatePrefixParam(upd) if !ok { return false } before := e.TracksParams() - params, err := h.resolveTracksParams(mergeTracksUpdate(before.Params, upd.Parameters)) + merged := mergeTracksUpdate(before.Params, upd.Parameters) + updTokens := updatedTokens(tokens, upd.Parameters) + if rej := h.refuseUpdate(ctx, toks, hasPrefix, func() error { + return h.auth.AuthorizeSubscribeTracks(ctx, h.sess, &message.SubscribeTracks{ + RequestID: msg.RequestID, + TrackNamespacePrefix: prefix, + Parameters: withTokens(merged, updTokens), + }) + }); rej != nil { + e.Finish(rej) + return endAfterFinish(ctx, e) + } + tokens = updTokens + params, err := h.resolveTracksParams(merged) if err != nil { if _, ok := errors.AsType[*paramProtocolViolation](err); ok { _ = h.sess.Close(moqt.SessionProtocolViolation, err.Error()) @@ -457,6 +488,69 @@ func (h *sessionHandler) tracksUpdate( } } +// refuseUpdate authorizes a REQUEST_UPDATE on a namespace subscription, +// returning the REQUEST_ERROR that refuses it, or nil. Its tokens (§10.2.2) +// go through the TokenVerifier as an opener's do. When it changes the prefix, +// authorize runs the Authorizer on the subscription it would become: §10.19 +// and §10.20 require that "the subscriber is authorized to perform this +// namespace subscription". +func (h *sessionHandler) refuseUpdate( + ctx context.Context, + toks []session.ResolvedToken, + prefixChanged bool, + authorize func() error, +) *message.RequestError { + if err := h.sess.VerifyTokens(ctx, toks); err != nil { + code, reason := tokenDenial(err) + return &message.RequestError{ErrorCode: code, ErrorReason: reason} + } + if !prefixChanged { + return nil + } + if err := authorize(); err != nil { + return &message.RequestError{ + ErrorCode: CodeForAuthorizerError(err), + ErrorReason: ReasonForAuthorizerError(err), + } + } + return nil +} + +// authorizingTokens is the AUTHORIZATION_TOKENs in ps that can authorize a +// request: all but DELETEs, which only retire an alias (§10.2.2). +func authorizingTokens(ps message.Parameters) message.Parameters { + var out message.Parameters + for _, p := range ps { + if p.Type != message.ParamAuthorizationToken { + continue + } + var tok message.Token + if tok.Parse(p.Bytes) == nil && tok.AliasType != message.AliasTypeDelete { + out = append(out, p) + } + } + return out +} + +// updatedTokens is the tokens a namespace subscription holds after an update +// carrying upd: its authorizing tokens replace cur when it has any; otherwise +// cur "remains unchanged" (§10.9). +func updatedTokens(cur, upd message.Parameters) message.Parameters { + if toks := authorizingTokens(upd); len(toks) > 0 { + return toks + } + return cur +} + +// withTokens is ps with its AUTHORIZATION_TOKENs replaced by toks: the +// subscription an update's Authorizer call judges. +func withTokens(ps, toks message.Parameters) message.Parameters { + out := slices.DeleteFunc(slices.Clone(ps), func(p message.Parameter) bool { + return p.Type == message.ParamAuthorizationToken + }) + return append(out, toks...) +} + // mergeTracksUpdate applies a REQUEST_UPDATE's parameters to a // SUBSCRIBE_TRACKS's: each type present in upd replaces every stored one of // that type (§10.9); a zero-length Range Filter removes it (§5.1.4). diff --git a/pkg/relay/handler_publish.go b/pkg/relay/handler_publish.go index 1efdc5c2..34d8c57e 100644 --- a/pkg/relay/handler_publish.go +++ b/pkg/relay/handler_publish.go @@ -149,7 +149,10 @@ var notEchoedInPublish = []message.ParamID{ // Relays"): // - SUBSCRIBE_TRACKS parameters valid on PUBLISH are echoed (§10.20.1), // except AUTHORIZATION_TOKEN (§10.2.2); -// - FORWARD=0 and GROUP_ORDER only when the subscriber set them; +// - FORWARD=0 only when the subscriber set it; +// - GROUP_ORDER always: the subscriber's, else the publisher's preference +// (§10.2.8), which the subscription's fills follow; §10.20.1 has these +// "explicitly communicated in PUBLISH"; // - LARGEST_OBJECT is the relay's own watermark (§10.2.17). func publishParamsForSubscriber(tp *registry.TracksParams, entry *registry.TrackEntry) message.Parameters { var out message.Parameters @@ -164,9 +167,11 @@ func publishParamsForSubscriber(tp *registry.TracksParams, entry *registry.Track if !tp.Forward { out = append(out, message.ForwardParam(false)) } - if tp.GroupOrder != 0 { - out = append(out, message.GroupOrderParam(message.GroupOrder(tp.GroupOrder))) + order := message.GroupOrder(tp.GroupOrder) + if order == 0 { + order = entry.DefaultGroupOrder() } + out = append(out, message.GroupOrderParam(order)) if largest, ok := entry.GetLargest(); ok { out = append(out, message.LargestObjectParam(largest.Group, largest.Object)) } diff --git a/pkg/relay/handler_subscribe.go b/pkg/relay/handler_subscribe.go index 9211345d..30cbfd72 100644 --- a/pkg/relay/handler_subscribe.go +++ b/pkg/relay/handler_subscribe.go @@ -94,6 +94,11 @@ func (h *sessionHandler) handleSubscribe(ctx context.Context, req *session.Reque h.log.LogAttrs(ctx, slog.LevelDebug, "SUBSCRIBE serving from existing upstream") } + // Before registration, so the first stream opened for it already + // schedules in its Group Order (§7.2). + if cur, ok := h.tracks.Get(fullName.Key()); ok { + resolveGroupOrder(sub, cur) + } // Register and snapshot Largest atomically, so no object falls between // live delivery and the fill fetch stream. entry, snapshotLargest, snapshotHas, added = h.tracks.AddDownstreamSnapshotLargest(fullName, sub) @@ -120,6 +125,9 @@ func (h *sessionHandler) handleSubscribe(ctx context.Context, req *session.Reque defer h.metrics.SubscriptionClosed(subRef) defer h.tracks.RemoveDownstream(fullName, sub.ID) + // §5.1.1: once the subscriber cancels, or the session ends, reset the + // streams still open for it. + defer sub.Cancel() // §10.2.17: "If Objects have been published on this Track the Publisher // MUST include this parameter." @@ -206,7 +214,7 @@ func (h *sessionHandler) readSubscribeUpdates( } // §10.2.2: an update may REGISTER/DELETE token aliases; // a cache fault there is session-fatal. - if !h.handleFollowupTokens(ctx, upd) { + if _, ok := h.handleFollowupTokens(ctx, upd); !ok { return false } h.handleSubscribeUpdate(ctx, sub, fullName, upd) @@ -554,6 +562,18 @@ func anyDownstreamForwards(entry *registry.TrackEntry) bool { func(d *registry.DownstreamSub) bool { return d.ForwardState() == 1 }) } +// resolveGroupOrder gives sub, when its request omitted GROUP_ORDER, the +// publisher's preference from entry's Track Properties (§10.2.8: "If omitted +// from SUBSCRIBE or SUBSCRIBE_TRACKS, the publisher's preference from the +// Track is used"; §12.5). Its fills (§10.2.15) and stream scheduling (§7.2) +// then follow it. GROUP_ORDER cannot appear in REQUEST_UPDATE, so this holds +// for the subscription's life. +func resolveGroupOrder(sub *registry.DownstreamSub, entry *registry.TrackEntry) { + if sub.GroupOrder == 0 { + sub.SetGroupOrder(uint8(entry.DefaultGroupOrder())) + } +} + // installSubscribeParams records the subscription parameters present in ps // (§10.2) on sub, leaving absent ones unchanged. The Largest snapshot is the // caller's (see [registry.TrackRegistry.AddDownstreamSnapshotLargest]). diff --git a/pkg/relay/helpers_fetch_test.go b/pkg/relay/helpers_fetch_test.go index 1df96f88..e61fd08a 100644 --- a/pkg/relay/helpers_fetch_test.go +++ b/pkg/relay/helpers_fetch_test.go @@ -122,8 +122,22 @@ func fetchAndDrain( return reqStream.OK, decodeFetchStream(t, fs, order) } +// awaitTailCached waits, up to 5s, for an upstream to close written once it +// has written its live tail, then lets the relay cache the last of it. Writes +// over the test transport return once the relay has read them. +func awaitTailCached(t *testing.T, written <-chan struct{}) { + t.Helper() + select { + case <-written: + case <-time.After(5 * time.Second): + t.Fatal("the upstream never wrote its live tail") + } + time.Sleep(50 * time.Millisecond) +} + // decodeFetchStream reads fs to EOF, reversing the §11.4.4.1 delta encoding -// itself rather than through the session decoder. +// itself rather than through the session decoder. End of Range markers +// (§11.4.4.2) are skipped, but anchor the Object after them. func decodeFetchStream(t *testing.T, fs *session.IncomingFetchStream, order message.GroupOrder) []decodedFetchObject { t.Helper() var ( @@ -141,6 +155,10 @@ func decodeFetchStream(t *testing.T, fs *session.IncomingFetchStream, order mess } t.Fatalf("ReadObject: %v", err) } + if isEndOfRange(fo.SerializationFlags) { + prevGroup, prevObject, havePrev = fo.GroupIDDelta, fo.ObjectIDDelta, true + continue + } var g, o uint64 switch { @@ -171,6 +189,15 @@ func decodeFetchStream(t *testing.T, fs *session.IncomingFetchStream, order mess } } +// isEndOfRange reports whether flags are a §11.4.4.2 End of Range marker's. +func isEndOfRange(flags uint64) bool { + switch flags { + case message.FetchEndOfNonExistentRange, message.FetchEndOfUnknownRange, message.FetchEndOfTimedOutRange: + return true + } + return false +} + // fetchElem is one element of a FETCH response: an Object or a §11.4.4.2 // End of Range marker. type fetchElem struct { diff --git a/pkg/relay/inbound_goaway_test.go b/pkg/relay/inbound_goaway_test.go index e7bcb9b8..6234e0ae 100644 --- a/pkg/relay/inbound_goaway_test.go +++ b/pkg/relay/inbound_goaway_test.go @@ -1,9 +1,12 @@ package relay_test import ( + "sync" + "sync/atomic" "testing" "time" + "github.com/floatdrop/moq-go/pkg/moqt" "github.com/floatdrop/moq-go/pkg/moqt/message" "github.com/floatdrop/moq-go/pkg/moqt/session" "github.com/floatdrop/moq-go/pkg/relay" @@ -108,7 +111,12 @@ func TestRelay_NoUpstreamFetchToGoingAwayPublisher(t *testing.T) { if _, err := pubSess.PublishNamespace(t.Context(), &message.PublishNamespace{Namespace: video}); err != nil { t.Fatalf("PublishNamespace: %v", err) } + // A FETCH before the GOAWAY may be stitched from the publisher (a Group + // still in flight is unknown to the relay); only one after it counts. + var goneAway atomic.Bool fetches := make(chan *session.Request, 4) + written := make(chan struct{}) + tailWritten := sync.OnceFunc(func() { close(written) }) go func() { for { req, err := pubSess.AcceptRequest(t.Context()) @@ -123,6 +131,7 @@ func TestRelay_NoUpstreamFetchToGoingAwayPublisher(t *testing.T) { for g := stitchLiveLo; g <= stitchLiveHi; g++ { sg, err := openSubgroupWaiting(t, pubSess, message.SubgroupHeader{ SubgroupIDMode: message.SubgroupIDImplicitZero, TrackAlias: 42, GroupID: g, + EndOfGroup: true, // so the cache alone answers the tail }) if err != nil { return @@ -130,7 +139,12 @@ func TestRelay_NoUpstreamFetchToGoingAwayPublisher(t *testing.T) { _ = sg.WriteObject(&message.SubgroupObject{Payload: []byte{byte('a' + g)}}) _ = sg.Close() } + tailWritten() case *message.Fetch: + if !goneAway.Load() { + _ = req.RejectError(moqt.RequestDoesNotExist, "not yet") + continue + } fetches <- req } } @@ -141,6 +155,7 @@ func TestRelay_NoUpstreamFetchToGoingAwayPublisher(t *testing.T) { t.Fatalf("Subscribe: %v", err) } go drainAll(t.Context(), live) + awaitTailCached(t, written) // Wait until the cache holds the live tail, with a FETCH the cache // answers alone. @@ -158,6 +173,7 @@ func TestRelay_NoUpstreamFetchToGoingAwayPublisher(t *testing.T) { time.Sleep(20 * time.Millisecond) } + goneAway.Store(true) if err := pubSess.SendGoaway(10*time.Second, ""); err != nil { t.Fatalf("SendGoaway: %v", err) } diff --git a/pkg/relay/internal/registry/dynamic_groups_test.go b/pkg/relay/internal/registry/dynamic_groups_test.go index 5c198b32..eea72b96 100644 --- a/pkg/relay/internal/registry/dynamic_groups_test.go +++ b/pkg/relay/internal/registry/dynamic_groups_test.go @@ -67,6 +67,43 @@ func TestTrackEntry_DynamicGroups(t *testing.T) { }) } +// TestTrackEntry_DefaultGroupOrder pins the §12.5 DEFAULT_PUBLISHER_GROUP_ORDER +// decode: Ascending when omitted, the value when allowed, and Ascending for a +// value outside {1, 2} or a malformed block, including one that would +// truncate to an allowed byte. +func TestTrackEntry_DefaultGroupOrder(t *testing.T) { + t.Parallel() + order := func(v uint64) []byte { + return message.AppendTrackProperties([]wire.KVPair{ + {Type: message.PropertyDefaultPublisherGroupOrder, IntVal: v}, + }) + } + for _, tc := range []struct { + name string + props []byte + want message.GroupOrder + }{ + {"absent", nil, message.GroupOrderAscending}, + {"Ascending", order(1), message.GroupOrderAscending}, + {"Descending", order(2), message.GroupOrderDescending}, + {"out of range", order(3), message.GroupOrderAscending}, + {"truncates to Descending", order(0x102), message.GroupOrderAscending}, + {"malformed block", []byte{0x40}, message.GroupOrderAscending}, + {"inside Immutable Properties", message.AppendTrackProperties([]wire.KVPair{ + {Type: message.PropertyImmutableProperties, ByteVal: order(2)}, + }), message.GroupOrderDescending}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + e := ®istry.TrackEntry{} + e.SetProperties(tc.props) + if got := e.DefaultGroupOrder(); got != tc.want { + t.Fatalf("DefaultGroupOrder() = %v, want %v", got, tc.want) + } + }) + } +} + // TestConsiderNewGroupRequest pins the §10.2.19 relay decision and its // outstanding-request bookkeeping. func TestConsiderNewGroupRequest(t *testing.T) { diff --git a/pkg/relay/internal/registry/ledger_test.go b/pkg/relay/internal/registry/ledger_test.go index 0ce7deb9..33514cf9 100644 --- a/pkg/relay/internal/registry/ledger_test.go +++ b/pkg/relay/internal/registry/ledger_test.go @@ -2,6 +2,7 @@ package registry_test import ( "errors" + "math" "testing" "github.com/floatdrop/moq-go/pkg/moqt/message" @@ -15,11 +16,11 @@ func newTestEntry(name string) *registry.TrackEntry { return registry.NewTrackRegistry().GetOrCreate(newTestTrackName(name)) } -// mustClaim claims o and fails unless ClaimDelivered returns (wantFresh, nil). -func mustClaim(t *testing.T, e *registry.TrackEntry, o registry.ObjectInfo, wantFresh bool) { +// mustClaim claims o and fails unless ClaimDelivered returns (want, nil). +func mustClaim(t *testing.T, e *registry.TrackEntry, o registry.ObjectInfo, want registry.Claim) { t.Helper() - if fresh, err := e.ClaimDelivered(o); err != nil || fresh != wantFresh { - t.Fatalf("ClaimDelivered(%+v) = (%v, %v), want (%v, nil)", o, fresh, err, wantFresh) + if got, err := e.ClaimDelivered(o); err != nil || got != want { + t.Fatalf("ClaimDelivered(%+v) = (%v, %v), want (%v, nil)", o, got, err, want) } } @@ -40,32 +41,74 @@ func cacheAndClaim(t *testing.T, e *registry.TrackEntry, objs ...registry.Object } // TestTrackEntry_ClaimDelivered pins the §2.1 dedup ledger: the first claim of a -// {GroupID, ObjectID} wins, a repeat loses, distinct objects/groups are -// independent, and a group that has aged out of the window is treated as already -// delivered. +// {GroupID, ObjectID} wins, a repeat loses, and distinct objects/groups are +// independent. func TestTrackEntry_ClaimDelivered(t *testing.T) { t.Parallel() e := newTestEntry("dedup") for _, c := range []struct { group, object uint64 - fresh bool + want registry.Claim }{ - {0, 5, true}, - {0, 5, false}, - {0, 2, true}, // a distinct Object in a Group already seen - {1, 5, true}, // a distinct Group - // Group 1000 moves the window past Group 0, so a late straggler from - // Group 0 counts as already delivered. - {1000, 0, true}, - {0, 9, false}, - // The current Group still dedups after the window advanced. - {1000, 1, true}, - {1000, 1, false}, + {0, 5, registry.ClaimFresh}, + {0, 5, registry.ClaimRedundant}, + {0, 2, registry.ClaimFresh}, // a distinct Object in a Group already seen + {1, 5, registry.ClaimFresh}, // a distinct Group + {1, 5, registry.ClaimRedundant}, } { - mustClaim(t, e, registry.ObjectInfo{Group: c.group, Object: c.object}, c.fresh) + mustClaim(t, e, registry.ObjectInfo{Group: c.group, Object: c.object}, c.want) } } +// TestTrackEntry_ClaimDeliveredWindow: the ledger holds 32 Groups, counted +// rather than spanned by ID (Group IDs need not be consecutive, §2.3.1). A +// jump in Group ID keeps the older Group; an Object of a Group older than the +// 32 held is ClaimAgedOut, not taken for a duplicate. +func TestTrackEntry_ClaimDeliveredWindow(t *testing.T) { + t.Parallel() + e := newTestEntry("window") + mustClaim(t, e, registry.ObjectInfo{Group: 0, Object: 0}, registry.ClaimFresh) + mustClaim(t, e, registry.ObjectInfo{Group: 1000, Object: 0}, registry.ClaimFresh) + mustClaim(t, e, registry.ObjectInfo{Group: 0, Object: 1}, registry.ClaimFresh) + mustClaim(t, e, registry.ObjectInfo{Group: 0, Object: 1}, registry.ClaimRedundant) + + // 30 more Groups fill the window; the next pushes out Group 0. + for g := uint64(2000); g < 2030; g++ { + mustClaim(t, e, registry.ObjectInfo{Group: g, Object: 0}, registry.ClaimFresh) + } + mustClaim(t, e, registry.ObjectInfo{Group: 0, Object: 2}, registry.ClaimFresh) + mustClaim(t, e, registry.ObjectInfo{Group: 2030, Object: 0}, registry.ClaimFresh) + for _, o := range []registry.ObjectInfo{ + {Group: 0, Object: 1}, // held before, now pruned + {Group: 0, Object: 3}, + {Group: 5, Object: 0}, // never held, below the window + } { + mustClaim(t, e, o, registry.ClaimAgedOut) + } + // Group 1000 is now the lowest held, and still dedups. + mustClaim(t, e, registry.ObjectInfo{Group: 1000, Object: 0}, registry.ClaimRedundant) + // A new Group between those held pushes out the lowest, Group 1000. + mustClaim(t, e, registry.ObjectInfo{Group: 1500, Object: 0}, registry.ClaimFresh) + mustClaim(t, e, registry.ObjectInfo{Group: 1000, Object: 0}, registry.ClaimAgedOut) + mustClaim(t, e, registry.ObjectInfo{Group: 1500, Object: 0}, registry.ClaimRedundant) +} + +// TestTrackEntry_ClaimDeliveredWindowNotFull: until the window holds 32 +// Groups, a Group below every one held is added, and is then the first pruned. +func TestTrackEntry_ClaimDeliveredWindowNotFull(t *testing.T) { + t.Parallel() + e := newTestEntry("window-low") + mustClaim(t, e, registry.ObjectInfo{Group: 10, Object: 0}, registry.ClaimFresh) + mustClaim(t, e, registry.ObjectInfo{Group: 3, Object: 0}, registry.ClaimFresh) + for g := uint64(11); g < 41; g++ { // 30 more fill the window + mustClaim(t, e, registry.ObjectInfo{Group: g, Object: 0}, registry.ClaimFresh) + } + mustClaim(t, e, registry.ObjectInfo{Group: 3, Object: 0}, registry.ClaimRedundant) + mustClaim(t, e, registry.ObjectInfo{Group: 41, Object: 0}, registry.ClaimFresh) + mustClaim(t, e, registry.ObjectInfo{Group: 3, Object: 0}, registry.ClaimAgedOut) + mustClaim(t, e, registry.ObjectInfo{Group: 10, Object: 0}, registry.ClaimRedundant) +} + // TestTrackEntry_ClaimDeliveredGapProperties pins how the ledger treats the // Prior Group and Object ID Gaps (§12.8, §12.9) of earlier Objects. An Object // inside an announced gap is known not to exist (§2.1) and dropped (§9.1); a gap @@ -111,7 +154,7 @@ func TestTrackEntry_ClaimDeliveredGapProperties(t *testing.T) { e := newTestEntry("gaps") last := len(tc.claims) - 1 for i, c := range tc.claims { - fresh, err := e.ClaimDelivered(registry.ObjectInfo{Group: c.group, Object: c.object, Gaps: c.gaps}) + claim, err := e.ClaimDelivered(registry.ObjectInfo{Group: c.group, Object: c.object, Gaps: c.gaps}) got := forwarded switch { case err != nil: @@ -119,7 +162,7 @@ func TestTrackEntry_ClaimDeliveredGapProperties(t *testing.T) { if !errors.Is(err, session.ErrMalformedTrack) { t.Fatalf("claim %d %+v: %v, want it to wrap session.ErrMalformedTrack", i, c, err) } - case !fresh: + case claim != registry.ClaimFresh: got = dropped } want := forwarded @@ -142,23 +185,28 @@ func TestTrackEntry_ClaimDeliveredMalformedLeavesNoTrace(t *testing.T) { e := newTestEntry("gaps") groupGap := func(n uint64) message.PriorGaps { return message.PriorGaps{Group: n, HasGroup: true} } - mustClaim(t, e, registry.ObjectInfo{Group: 9, Object: 0, Gaps: groupGap(2)}, true) // Groups 7-8 absent + mustClaim( + t, + e, + registry.ObjectInfo{Group: 9, Object: 0, Gaps: groupGap(2)}, + registry.ClaimFresh, + ) // Groups 7-8 absent if _, err := e.ClaimDelivered(registry.ObjectInfo{Group: 9, Object: 1, Gaps: groupGap(3)}); err == nil { t.Fatal("a second Prior Group ID Gap value in Group 9 is not malformed") } - mustClaim(t, e, registry.ObjectInfo{Group: 9, Object: 1}, true) // Object 1 was not recorded - mustClaim(t, e, registry.ObjectInfo{Group: 6, Object: 0}, true) // nor the gap of 3 (Groups 6-8) - mustClaim(t, e, registry.ObjectInfo{Group: 8, Object: 0}, false) + mustClaim(t, e, registry.ObjectInfo{Group: 9, Object: 1}, registry.ClaimFresh) // Object 1 was not recorded + mustClaim(t, e, registry.ObjectInfo{Group: 6, Object: 0}, registry.ClaimFresh) // nor the gap of 3 (Groups 6-8) + mustClaim(t, e, registry.ObjectInfo{Group: 8, Object: 0}, registry.ClaimRedundant) if _, err := e.ClaimDelivered(registry.ObjectInfo{Group: 9, Object: 5, Priority: 7}); err == nil { t.Fatal("another Publisher Priority in Subgroup 0 of Group 9 is not malformed") } - mustClaim(t, e, registry.ObjectInfo{Group: 9, Object: 5}, true) + mustClaim(t, e, registry.ObjectInfo{Group: 9, Object: 5}, registry.ClaimFresh) // A duplicate records no Subgroup state: the §9.1 check may still reject // it (the caller's), so Subgroup 3 keeps no priority from it. - mustClaim(t, e, registry.ObjectInfo{Group: 9, Object: 5, Subgroup: 3, Priority: 9}, false) - mustClaim(t, e, registry.ObjectInfo{Group: 9, Object: 6, Subgroup: 3, Priority: 1}, true) + mustClaim(t, e, registry.ObjectInfo{Group: 9, Object: 5, Subgroup: 3, Priority: 9}, registry.ClaimRedundant) + mustClaim(t, e, registry.ObjectInfo{Group: 9, Object: 6, Subgroup: 3, Priority: 1}, registry.ClaimFresh) } // TestTrackEntry_FinalObjects pins the §2.4.2 conditions that need earlier @@ -174,8 +222,8 @@ func TestTrackEntry_FinalObjects(t *testing.T) { // passed, is recorded too. claim := func(o registry.ObjectInfo) step { return func(e *registry.TrackEntry) error { - fresh, err := e.ClaimDelivered(o) - if err == nil && !fresh { + claim, err := e.ClaimDelivered(o) + if err == nil && claim != registry.ClaimFresh { err = e.RecordDuplicate(o) } return err @@ -358,10 +406,15 @@ func TestTrackEntry_FinalObjects(t *testing.T) { func TestTrackEntry_RecordDuplicateChecksAgain(t *testing.T) { t.Parallel() e := newTestEntry("race") - mustClaim(t, e, registry.ObjectInfo{Group: 1, Object: 2}, true) + mustClaim(t, e, registry.ObjectInfo{Group: 1, Object: 2}, registry.ClaimFresh) eog := registry.ObjectInfo{Group: 1, Object: 2, Status: message.ObjectStatusEndOfGroup} - mustClaim(t, e, eog, false) // a duplicate; the caller's §9.1 check runs now - mustClaim(t, e, registry.ObjectInfo{Group: 1, Object: 5}, true) // meanwhile, another upstream + mustClaim( + t, + e, + eog, + registry.ClaimRedundant, + ) // a duplicate; the caller's §9.1 check runs now + mustClaim(t, e, registry.ObjectInfo{Group: 1, Object: 5}, registry.ClaimFresh) // meanwhile, another upstream if err := e.RecordDuplicate(eog); !errors.Is(err, session.ErrMalformedTrack) { t.Fatalf("RecordDuplicate = %v, want the Group ending below Object 5 to be malformed", err) } @@ -458,14 +511,35 @@ func TestTrackEntry_LowestForwarded(t *testing.T) { if _, ok := e.LowestForwarded(1, 0); ok { t.Fatal("a Subgroup nothing was forwarded in has a lowest Object") } - mustClaim(t, e, registry.ObjectInfo{Group: 1, Object: 5}, true) - mustClaim(t, e, registry.ObjectInfo{Group: 1, Object: 3}, true) - mustClaim(t, e, registry.ObjectInfo{Group: 1, Object: 1, Datagram: true}, true) - mustClaim(t, e, registry.ObjectInfo{Group: 1, Object: 0, Subgroup: 1}, true) - mustClaim(t, e, registry.ObjectInfo{Group: 1, Object: 9, Subgroup: 2, Status: message.ObjectStatusEndOfGroup}, true) + mustClaim(t, e, registry.ObjectInfo{Group: 1, Object: 5}, registry.ClaimFresh) + mustClaim(t, e, registry.ObjectInfo{Group: 1, Object: 3}, registry.ClaimFresh) + mustClaim(t, e, registry.ObjectInfo{Group: 1, Object: 1, Datagram: true}, registry.ClaimFresh) + mustClaim(t, e, registry.ObjectInfo{Group: 1, Object: 0, Subgroup: 1}, registry.ClaimFresh) + mustClaim( + t, + e, + registry.ObjectInfo{Group: 1, Object: 9, Subgroup: 2, Status: message.ObjectStatusEndOfGroup}, + registry.ClaimFresh, + ) for _, tc := range []struct{ subgroup, want uint64 }{{0, 3}, {1, 0}, {2, 9}} { if low, ok := e.LowestForwarded(1, tc.subgroup); !ok || low != tc.want { t.Errorf("LowestForwarded(1, %d) = (%d, %v), want (%d, true)", tc.subgroup, low, ok, tc.want) } } } + +// TestTrackEntry_EndAfterLastObjectID: a hard end after Object 2^64-1 has +// nothing past it, so it records no absence rather than wrapping to Object 0. +func TestTrackEntry_EndAfterLastObjectID(t *testing.T) { + t.Parallel() + const last = math.MaxUint64 + e := newTestEntry("last-id") + mustClaim(t, e, registry.ObjectInfo{Group: 1, Object: last, Datagram: true, EndOfGroup: true}, registry.ClaimFresh) + mustClaim(t, e, registry.ObjectInfo{Group: 2, Object: last}, registry.ClaimFresh) + if err := e.SubgroupEnded(registry.ObjectInfo{Group: 2, Object: last}, true); err != nil { + t.Fatalf("SubgroupEnded: %v", err) + } + if got := e.KnownAbsent(); len(got) != 0 { + t.Fatalf("KnownAbsent = %v, want nothing", got) + } +} diff --git a/pkg/relay/internal/registry/properties.go b/pkg/relay/internal/registry/properties.go index 19c9e433..48b61c89 100644 --- a/pkg/relay/internal/registry/properties.go +++ b/pkg/relay/internal/registry/properties.go @@ -35,6 +35,10 @@ type decodedProperties struct { // property decodes to — so the zero DeliveryTimeouts is the correct // reading of a track that declares neither. deliveryTimeouts message.DeliveryTimeouts + + // groupOrder is DEFAULT_PUBLISHER_GROUP_ORDER (§12.5) when it holds an + // allowed value, else zero. + groupOrder message.GroupOrder } // decodeTrackProperties parses the raw Track Properties block once and pulls @@ -61,6 +65,11 @@ func decodeTrackProperties(raw []byte) decodedProperties { d.deliveryTimeouts.Object = message.MillisecondTimeout(kv.IntVal) case message.PropertySubgroupDeliveryTimeout: d.deliveryTimeouts.Subgroup = message.MillisecondTimeout(kv.IntVal) + case message.PropertyDefaultPublisherGroupOrder: + d.groupOrder = 0 + if kv.IntVal == uint64(message.GroupOrderAscending) || kv.IntVal == uint64(message.GroupOrderDescending) { + d.groupOrder = message.GroupOrder(kv.IntVal) + } } } return d @@ -112,3 +121,16 @@ func (e *TrackEntry) DeliveryTimeouts() message.DeliveryTimeouts { } return e.decoded.deliveryTimeouts } + +// DefaultGroupOrder is the publisher's Group Order preference, its +// DEFAULT_PUBLISHER_GROUP_ORDER Track Property (§12.5): Ascending when it is +// omitted ("If omitted, the publisher's preference is Ascending"), and when +// it is not an allowed value or the Properties are malformed. +func (e *TrackEntry) DefaultGroupOrder() message.GroupOrder { + e.mu.RLock() + defer e.mu.RUnlock() + if e.decoded.parseErr != nil || e.decoded.groupOrder == 0 { + return message.GroupOrderAscending + } + return e.decoded.groupOrder +} diff --git a/pkg/relay/internal/registry/subscription.go b/pkg/relay/internal/registry/subscription.go index b67b876f..2f293aef 100644 --- a/pkg/relay/internal/registry/subscription.go +++ b/pkg/relay/internal/registry/subscription.go @@ -423,9 +423,9 @@ type DownstreamSub struct { // 0x1 = ascending, 0x2 = descending. It drives the group-order // tie-breaker in both reorder-capable paths (FETCH responses) and the // §7.2 rule-3 GroupKey of the subgroup-stream scheduling priority. - // Default per §7.1: the publisher's preference, which the relay does not - // currently track, so an unset GroupOrder is left at zero and treated as - // Ascending. + // Default per §7.1: the publisher's preference, which the relay fills in + // from the Track Properties before registering the subscription; zero + // until then, read as Ascending. GroupOrder uint8 // omitProperties records INCLUDE_PROPERTIES=0; see @@ -443,6 +443,10 @@ type DownstreamSub struct { // datagramsSending counts datagram sends in flight. datagramsSending int pendingDone *pendingPublishDone + + // cancelled is done once the request ends; see [DownstreamSub.Cancel]. + cancelled context.Context + cancel context.CancelFunc } // pendingPublishDone is a termination's PUBLISH_DONE, held until the last of @@ -533,7 +537,10 @@ func (d *DownstreamSub) takeReadyDoneLocked() (*pendingPublishDone, uint64) { // installSubscribeParams overrides this to 0 only when the peer explicitly // sends FORWARD=0, and REQUEST_UPDATE can flip it later (§9.2 / §10.9). func NewDownstreamSub(id uint64, sess *session.Session, stream session.Stream, trackAlias uint64) *DownstreamSub { + cancelled, cancel := context.WithCancel(context.Background()) return &DownstreamSub{ + cancelled: cancelled, + cancel: cancel, state: SubEstablished, ID: id, Session: sess, @@ -666,8 +673,7 @@ func (d *DownstreamSub) EffectiveStreamPriority( // §7.2 rule 3: Descending order means higher Group IDs are scheduled // first. Complementing the Group ID flips the numeric comparison so the // same "lower GroupKey is higher priority" rule yields that direction. - // An unset GROUP_ORDER (zero value) defaults to Ascending — §7.1 says - // the publisher's preference applies, which the relay does not track. + // An unresolved GROUP_ORDER (zero value) is read as Ascending. groupKey := groupID if order == message.GroupOrderDescending { groupKey = ^groupID @@ -773,6 +779,24 @@ func GroupOutOfRange(group uint64, f *message.LocationFilter) bool { return ok && group > end.Group } +// Cancel ends the subscription once its request has ended: the subscriber +// cancelled it (§3.3.3), or the session is ending. [DownstreamSub.Cancelled] +// is done, so the streams still open for it are reset ("It MUST reset any open +// streams associated with the SUBSCRIBE", §5.1.1; its fill fetch streams, +// §5.1.3.1). No new PUBLISH_DONE follows; one a termination left pending still +// goes out once those streams close, since it may be the request's only +// response (§3.3.2). Safe to call more than once, and after another +// termination. +func (d *DownstreamSub) Cancel() { + d.mu.Lock() + d.state = SubTerminated + d.mu.Unlock() + d.cancel() +} + +// Cancelled is done once [DownstreamSub.Cancel] has run. +func (d *DownstreamSub) Cancelled() context.Context { return d.cancelled } + // TerminateWithPublishDone ends this downstream subscription (§10.12): the // relay writes PUBLISH_DONE on the subscriber's request stream and FINs the // send side. If SUBSCRIBE_OK never went out, it answers with REQUEST_ERROR diff --git a/pkg/relay/internal/registry/track_entry.go b/pkg/relay/internal/registry/track_entry.go index d3e6a323..9946e9ad 100644 --- a/pkg/relay/internal/registry/track_entry.go +++ b/pkg/relay/internal/registry/track_entry.go @@ -135,11 +135,9 @@ type TrackEntry struct { // identity). It lives on the entry (not on a SharedSubgroup) so peers whose // streams do not temporally overlap — e.g. one origin's subgroup FINs before // the redundant origin's arrives — still dedup. Memory is bounded by - // [deliveredGroupWindow]: state for a group more than that many groups behind - // the largest seen group is pruned, and a stray object from such an aged-out - // group is treated as already-delivered (a peer lagging by that many groups - // is beyond any useful reorder window). deliveredMax/HasMax track the largest - // group seen, for the pruning window. + // [deliveredGroupWindow]: it holds that many Groups, the lowest ID pruned + // first, and an Object of a pruned or lower Group is [ClaimAgedOut]. + // deliveredFloor is the lowest Group held, while delivered is non-empty. // // It also holds the Prior Group and Object ID Gaps (§12.8, §12.9) seen: // each Group its Object ID gaps and Group gap value, and groupGaps the Group @@ -148,10 +146,9 @@ type TrackEntry struct { groupGaps []idRange // trackEnd is where the Track ends, its END_OF_TRACK (§2.4.2), if // hasTrackEnd; kept past the window. - trackEnd message.Location - hasTrackEnd bool - deliveredMax uint64 - deliveredHasMax bool + trackEnd message.Location + hasTrackEnd bool + deliveredFloor uint64 // subgroups holds the shared outbound fan-out state for each // (GroupID, SubgroupID) currently being produced by one or more upstreams. @@ -224,27 +221,46 @@ func (e *TrackEntry) CopySubgroups() []*SharedSubgroup { return slices.Collect(maps.Values(e.subgroups)) } -// deliveredGroupWindow bounds the §2.1 dedup ledger ([TrackEntry.delivered]): -// dedup state is retained for the most recent deliveredGroupWindow groups. An -// object whose group is more than this many groups behind the largest group -// seen is assumed already delivered. The window must comfortably exceed any -// realistic inter-publisher group lag (a redundant origin or relay running a -// few groups behind) while keeping per-track dedup memory bounded. +// deliveredGroupWindow bounds the §2.1 dedup ledger ([TrackEntry.delivered]) +// to that many distinct Groups, counted rather than spanned by ID, since Group +// IDs need not be consecutive (§2.3.1, §12.8). The window must comfortably +// exceed any realistic inter-publisher group lag (a redundant origin or relay +// running a few groups behind) while keeping per-track dedup memory bounded. +// +// It assumes Group IDs mostly increase, as §2.3.1 lets a publisher choose: +// the lowest ID is pruned first, and a Group below all 32 held is taken as +// old. A publisher whose IDs decrease loses its Objects past the first 32 +// Groups to [ClaimAgedOut]. const deliveredGroupWindow = 32 +// Claim is [TrackEntry.ClaimDelivered]'s verdict on an Object. +type Claim int + +const ( + // ClaimFresh: the first copy; forward it. + ClaimFresh Claim = iota + // ClaimRedundant: a copy of an Object already forwarded, or one inside a + // gap announced earlier; drop it. + ClaimRedundant + // ClaimAgedOut: its Group is older than every Group the window holds, so + // whether it was forwarded is unknown, and nothing is recorded. The + // caller forwards it only where it cannot be a repeat. + ClaimAgedOut +) + // ClaimDelivered is the dedup gate across multiple upstream publishers (§9.3). // It records (group, object) as forwarded and reports whether the caller is -// the first to do so (true → forward it) or it was already forwarded by a peer -// upstream (false → drop it). The ledger persists on the entry (not on a +// the first to do so ([ClaimFresh]) or it was already forwarded by a peer +// upstream ([ClaimRedundant]). The ledger persists on the entry (not on a // per-Subgroup structure) and is independent of the size-bounded Object Cache, // so redundant streams that do not temporally overlap, or peers lagging by more -// than the cache capacity, still dedup correctly. Memory is bounded to the most -// recent [deliveredGroupWindow] groups. +// than the cache capacity, still dedup correctly. Memory is bounded to +// [deliveredGroupWindow] Groups; an Object of an older one is [ClaimAgedOut]. // // gaps are the Object's Prior Group and Object ID Gaps (§12.8, §12.9), which // the ledger records for the whole track. An Object inside a gap announced -// earlier is known not to exist, and that is permanent (§2.1): false, since a -// caching relay "SHOULD NOT cache or forward" it (§9.1). A gap covering an +// earlier is known not to exist, and that is permanent (§2.1): ClaimRedundant, +// since a caching relay "SHOULD NOT cache or forward" it (§9.1). A gap covering an // Object already received is accepted: an Object may go from existing to not // existing (§2.1). // @@ -258,56 +274,60 @@ const deliveredGroupWindow = 32 // carrying two Prior Group ID Gap values (§12.8). An Object ClaimDelivered // rejects leaves no state in the ledger, and a duplicate records only its // gaps, since the caller's §9.1 check may still reject it. -func (e *TrackEntry) ClaimDelivered(o ObjectInfo) (bool, error) { +func (e *TrackEntry) ClaimDelivered(o ObjectInfo) (Claim, error) { group, object, gaps := o.Group, o.Object, o.Gaps e.deliveredMu.Lock() defer e.deliveredMu.Unlock() - // An object from a group already aged out of the window is treated as - // already delivered — a peer lagging that far behind is past any useful - // reorder window, and re-forwarding it would be a large out-of-order break. - if e.deliveredHasMax && group <= e.deliveredMax && e.deliveredMax-group >= deliveredGroupWindow { - return false, nil - } g := e.delivered[group] + if g == nil && len(e.delivered) >= deliveredGroupWindow && group < e.deliveredFloor { + return ClaimAgedOut, nil + } if gaps.HasGroup && g != nil && g.hasGroupGap && g.groupGap != gaps.Group { - return false, fmt.Errorf("%w: Group %d carries Prior Group ID Gaps %d and %d (§12.8)", + return ClaimRedundant, fmt.Errorf("%w: Group %d carries Prior Group ID Gaps %d and %d (§12.8)", session.ErrMalformedTrack, group, g.groupGap, gaps.Group) } if err := e.checkEndsLocked(g, o); err != nil { - return false, err + return ClaimRedundant, err } if e.announcedAbsentLocked(g, group, object) { - return false, nil - } - - if e.delivered == nil { - e.delivered = make(map[uint64]*deliveredGroup) - } - // Advance the window when a newer group appears, pruning groups that have - // fallen out of it. - if !e.deliveredHasMax || group > e.deliveredMax { - e.deliveredMax = group - e.deliveredHasMax = true - maps.DeleteFunc(e.delivered, func(id uint64, _ *deliveredGroup) bool { - return e.deliveredMax-id >= deliveredGroupWindow - }) - e.groupGaps = slices.DeleteFunc(e.groupGaps, func(r idRange) bool { - return e.deliveredMax-r.hi >= deliveredGroupWindow - }) + return ClaimRedundant, nil } if g == nil { g = &deliveredGroup{objects: make(map[uint64]struct{})} - e.delivered[group] = g + e.addDeliveredGroupLocked(group, g) } e.recordGapsLocked(g, group, object, gaps) if _, ok := g.objects[object]; ok { - return false, nil + return ClaimRedundant, nil } e.recordEndsLocked(g, o) g.objects[object] = struct{}{} - return true, nil + return ClaimFresh, nil +} + +// addDeliveredGroupLocked adds group's ledger entry g, first pruning the +// lowest Group, and the gaps announced below the new lowest, when the window +// is full. Runs once per Group, so its scan of the window is not per Object. +func (e *TrackEntry) addDeliveredGroupLocked(group uint64, g *deliveredGroup) { + if e.delivered == nil { + e.delivered = make(map[uint64]*deliveredGroup, deliveredGroupWindow) + } + if len(e.delivered) >= deliveredGroupWindow { + delete(e.delivered, e.deliveredFloor) + e.deliveredFloor = math.MaxUint64 + for id := range e.delivered { + e.deliveredFloor = min(e.deliveredFloor, id) + } + e.groupGaps = slices.DeleteFunc(e.groupGaps, func(r idRange) bool { + return r.hi < e.deliveredFloor + }) + } + if len(e.delivered) == 0 || group < e.deliveredFloor { + e.deliveredFloor = group + } + e.delivered[group] = g } // ObjectInfo is what [TrackEntry.ClaimDelivered] checks of an Object against @@ -412,6 +432,9 @@ func (e *TrackEntry) SubgroupEnded(lastObj ObjectInfo, endOfGroup bool) error { hard := lastObj.Status == message.ObjectStatusNormal at := lastObj.Object if hard { + if at == math.MaxUint64 { + return nil // nothing lies past it + } at++ } ends := end{at: at, set: true, hard: hard} @@ -499,15 +522,53 @@ func (e *TrackEntry) LowestForwarded(group, subgroup uint64) (uint64, bool) { return sg.minObject, sg.hasObject } +// LocRange is an inclusive range of Locations, Lo through Hi. +type LocRange struct{ Lo, Hi message.Location } + +// KnownAbsent reports the Locations the ledger knows do not exist (§2.1: +// "All signals that an Object does not exist are authoritative"), within the +// Groups it holds: each Group's end onward (§11.2.1.1, §11.4.2; a status end +// at M from M, since a status Object is not one a FETCH serializes), and the Object +// and Group ID gaps announced (§12.8, §12.9). The ranges may overlap and are +// in no particular order. A FIN or END_OF_GROUP bit that ended a Group is +// known only here, not from the cache. +func (e *TrackEntry) KnownAbsent() []LocRange { + e.deliveredMu.Lock() + defer e.deliveredMu.Unlock() + var out []LocRange + for id, g := range e.delivered { + if g.end.set { + out = append(out, LocRange{ + Lo: message.Location{Group: id, Object: g.end.at}, + Hi: message.Location{Group: id, Object: math.MaxUint64}, + }) + } + for _, r := range g.objectGaps { + out = append(out, LocRange{ + Lo: message.Location{Group: id, Object: r.lo}, + Hi: message.Location{Group: id, Object: r.hi}, + }) + } + } + for _, r := range e.groupGaps { + out = append(out, LocRange{ + Lo: message.Location{Group: r.lo}, + Hi: message.Location{Group: r.hi, Object: math.MaxUint64}, + }) + } + return out +} + // groupEnd reports where o ends its Group (see [end]), if it does: an // END_OF_GROUP or END_OF_TRACK status at M at M (§11.2.1.1); a datagram's // END_OF_GROUP bit on Object N at N+1, which §2.4.2's non-exhaustive list does -// not name but §11.3.1 defines alike. +// not name but §11.3.1 defines alike. After Object 2^64-1 nothing lies past, +// so that bit ends nothing. func groupEnd(o ObjectInfo) (at uint64, hard, ok bool) { switch { case o.Status == message.ObjectStatusEndOfGroup, o.Status == message.ObjectStatusEndOfTrack: return o.Object, false, true - case o.Datagram && o.EndOfGroup: + case o.Datagram && o.EndOfGroup && o.Object < math.MaxUint64: return o.Object + 1, true, true } return 0, false, false diff --git a/pkg/relay/malformed_track_test.go b/pkg/relay/malformed_track_test.go index 609a29f3..8cd14b4a 100644 --- a/pkg/relay/malformed_track_test.go +++ b/pkg/relay/malformed_track_test.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "io" + "sync" "sync/atomic" "testing" "time" @@ -455,6 +456,8 @@ func refusedFetchResetsStream(t *testing.T, upstreamProps, objProps []byte) { if _, err := pubSess.PublishNamespace(t.Context(), &message.PublishNamespace{Namespace: video}); err != nil { t.Fatalf("PublishNamespace: %v", err) } + written := make(chan struct{}) + tailWritten := sync.OnceFunc(func() { close(written) }) go func() { for { req, err := pubSess.AcceptRequest(t.Context()) @@ -468,7 +471,7 @@ func refusedFetchResetsStream(t *testing.T, upstreamProps, objProps []byte) { } for g := stitchLiveLo; g <= stitchLiveHi; g++ { sg, err := openSubgroupWaiting(t, pubSess, message.SubgroupHeader{ - SubgroupIDMode: message.SubgroupIDImplicitZero, TrackAlias: 42, GroupID: g, + SubgroupIDMode: message.SubgroupIDImplicitZero, TrackAlias: 42, GroupID: g, EndOfGroup: true, }) if err != nil { return @@ -476,6 +479,7 @@ func refusedFetchResetsStream(t *testing.T, upstreamProps, objProps []byte) { _ = sg.WriteObject(&message.SubgroupObject{Payload: []byte{byte('a' + g)}}) _ = sg.Close() } + tailWritten() case *message.Fetch: _ = req.Reply(&message.FetchOK{ EndLocation: message.Location{Group: stitchLiveLo - 1}, @@ -503,6 +507,7 @@ func refusedFetchResetsStream(t *testing.T, upstreamProps, objProps []byte) { t.Fatalf("Subscribe: %v", err) } go drainAll(t.Context(), live) + awaitTailCached(t, written) fc := dialAnotherClient(t, pubSess) deadline := time.Now().Add(5 * time.Second) for { diff --git a/pkg/relay/namespace_update_auth_test.go b/pkg/relay/namespace_update_auth_test.go new file mode 100644 index 00000000..2bcb4537 --- /dev/null +++ b/pkg/relay/namespace_update_auth_test.go @@ -0,0 +1,238 @@ +package relay_test + +import ( + "context" + "slices" + "sync" + "testing" + + "github.com/floatdrop/moq-go/pkg/moqt" + "github.com/floatdrop/moq-go/pkg/moqt/message" + "github.com/floatdrop/moq-go/pkg/moqt/session" + "github.com/floatdrop/moq-go/pkg/moqt/wire" + "github.com/floatdrop/moq-go/pkg/relay" +) + +// A TRACK_NAMESPACE_PREFIX update changes what a namespace subscription +// covers, so it is authorized like the subscription itself (§10.19, §10.20: +// "The publisher MUST ensure the subscriber is authorized to perform this +// namespace subscription"), with the update's own tokens (§10.2.2). + +// prefixAuthorizer admits namespace subscriptions under allowed only. +type prefixAuthorizer struct { + relay.AllowAllAuthorizer + + allowed wire.TrackNamespace +} + +func (a *prefixAuthorizer) check(prefix wire.TrackNamespace) error { + if !prefix.HasPrefix(a.allowed) { + return relay.DenyReason("outside the tenant") + } + return nil +} + +func (a *prefixAuthorizer) AuthorizeSubscribeNamespace( + _ context.Context, + _ *session.Session, + m *message.SubscribeNamespace, +) error { + return a.check(m.TrackNamespacePrefix) +} + +func (a *prefixAuthorizer) AuthorizeSubscribeTracks( + _ context.Context, + _ *session.Session, + m *message.SubscribeTracks, +) error { + return a.check(m.TrackNamespacePrefix) +} + +// TestNamespaceUpdate_PrefixOutsideAuthorizationRefused: a SUBSCRIBE_NAMESPACE +// cannot widen itself past what the Authorizer admits; the refused update ends +// the request (§10.9.1) without announcing the namespace. +func TestNamespaceUpdate_PrefixOutsideAuthorizationRefused(t *testing.T) { + t.Parallel() + pubSess, teardown := connectRelay(t, relay.Config{ + Authorizer: &prefixAuthorizer{allowed: ns("tenantA")}, + }) + defer teardown() + publishNS(t, pubSess, "secret", "x") + subSess := dialAnotherClient(t, pubSess) + nsSub, msgs := subscribeNS(t, subSess, "tenantA") + + sendPrefixUpdate(t, subSess, nsSub.Stream, "secret") + m := nextMessage(t, msgs) + rej, ok := m.(*message.RequestError) + if !ok || rej.ErrorCode != moqt.RequestUnauthorized { + t.Fatalf("got %T %+v, want REQUEST_ERROR UNAUTHORIZED", m, m) + } + requireStreamEnds(t, msgs) +} + +// TestSubscribeTracksUpdate_PrefixOutsideAuthorizationRefused: the same for +// SUBSCRIBE_TRACKS, whose widened prefix would forward every track under it. +func TestSubscribeTracksUpdate_PrefixOutsideAuthorizationRefused(t *testing.T) { + t.Parallel() + subSess, teardown := connectRelay(t, relay.Config{ + Authorizer: &prefixAuthorizer{allowed: ns("tenantA")}, + }) + defer teardown() + reqs := forwardedPublishes(t, subSess) + stream := subscribeTracks(t, subSess, ns("tenantA")) + publishVideoTrack(t, dialAnotherClient(t, subSess), "cam", 7) + + _, err := subSess.UpdateRequest(t.Context(), stream, + message.Parameters{message.TrackNamespacePrefixParam(ns("video"))}) + requireRejectedWithCode(t, err, moqt.RequestUnauthorized) + requireNoForward(t, reqs, "a refused prefix update") +} + +// openNamespaceSub opens a SUBSCRIBE_NAMESPACE, or a SUBSCRIBE_TRACKS when +// tracks, with params, and returns its request stream; it closes at cleanup. +func openNamespaceSub( + t *testing.T, + sess *session.Session, + tracks bool, + prefix wire.TrackNamespace, + params ...message.Parameter, +) session.Stream { + t.Helper() + if tracks { + return subscribeTracks(t, sess, prefix, params...) + } + s, err := sess.SubscribeNamespace(t.Context(), &message.SubscribeNamespace{ + TrackNamespacePrefix: prefix, Parameters: params, + }) + if err != nil { + t.Fatalf("SubscribeNamespace: %v", err) + } + t.Cleanup(func() { _ = s.Close() }) + return s.Stream +} + +// tokenParam is a USE_VALUE AUTHORIZATION_TOKEN carrying v. +func tokenParam(v string) message.Parameter { + return message.AuthorizationTokenParam(message.Token{ + AliasType: message.AliasTypeUseValue, TokenType: 1, TokenValue: []byte(v), + }) +} + +// TestNamespaceUpdate_TokenVerified: an update's AUTHORIZATION_TOKEN goes +// through the TokenVerifier, with or without a prefix change, and its denial +// refuses the update with the verifier's code. +func TestNamespaceUpdate_TokenVerified(t *testing.T) { + t.Parallel() + verifier := session.TokenVerifierFunc( + func(_ context.Context, _ *session.Session, tok session.ResolvedToken) error { + if string(tok.Value) == "expired" { + return session.DenyToken(moqt.RequestExpiredAuthToken, "token expired") + } + return nil + }) + for _, tc := range []struct { + name string + tracks bool + update message.Parameters + }{ + {"SUBSCRIBE_NAMESPACE prefix", false, + message.Parameters{message.TrackNamespacePrefixParam(ns("video")), tokenParam("expired")}}, + {"SUBSCRIBE_NAMESPACE token only", false, message.Parameters{tokenParam("expired")}}, + {"SUBSCRIBE_TRACKS prefix", true, + message.Parameters{message.TrackNamespacePrefixParam(ns("video")), tokenParam("expired")}}, + {"SUBSCRIBE_TRACKS token only", true, message.Parameters{tokenParam("expired")}}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + subSess, teardown := connectRelay(t, relay.Config{ + SessionOptions: []session.Option{session.WithTokenVerifier(verifier)}, + }) + defer teardown() + stream := openNamespaceSub(t, subSess, tc.tracks, ns("audio")) + _, err := subSess.UpdateRequest(t.Context(), stream, tc.update) + requireRejectedWithCode(t, err, moqt.RequestExpiredAuthToken) + }) + } +} + +// tokenRecorder records the AUTHORIZATION_TOKEN values each namespace +// subscription authorization carried. +type tokenRecorder struct { + relay.AllowAllAuthorizer + + mu sync.Mutex + seen [][]string +} + +func (a *tokenRecorder) record(ps message.Parameters) error { + toks, err := message.TokensFromParam(ps) + if err != nil { + return err + } + var vals []string + for _, tok := range toks { + vals = append(vals, string(tok.TokenValue)) + } + a.mu.Lock() + a.seen = append(a.seen, vals) + a.mu.Unlock() + return nil +} + +func (a *tokenRecorder) AuthorizeSubscribeNamespace( + _ context.Context, + _ *session.Session, + m *message.SubscribeNamespace, +) error { + return a.record(m.Parameters) +} + +func (a *tokenRecorder) AuthorizeSubscribeTracks( + _ context.Context, + _ *session.Session, + m *message.SubscribeTracks, +) error { + return a.record(m.Parameters) +} + +// TestNamespaceUpdate_AuthorizerSeesLatestTokens: a prefix update is +// authorized with the tokens of the latest request or update that carried +// any, since a parameter absent from REQUEST_UPDATE "remains unchanged" +// (§10.9). A DELETE only retires an alias, so it replaces nothing (§10.2.2). +func TestNamespaceUpdate_AuthorizerSeesLatestTokens(t *testing.T) { + t.Parallel() + for _, tracks := range []bool{false, true} { + t.Run(map[bool]string{false: "SUBSCRIBE_NAMESPACE", true: "SUBSCRIBE_TRACKS"}[tracks], func(t *testing.T) { + t.Parallel() + auth := &tokenRecorder{} + subSess, teardown := connectRelay(t, relay.Config{ + Authorizer: auth, + SessionOptions: []session.Option{session.WithMaxAuthTokenCacheSize(1024)}, + }) + defer teardown() + stream := openNamespaceSub(t, subSess, tracks, ns("a"), tokenParam("t0")) + for _, ps := range []message.Parameters{ + {message.TrackNamespacePrefixParam(ns("b"))}, + {tokenParam("t1")}, + {message.TrackNamespacePrefixParam(ns("c"))}, + {message.TrackNamespacePrefixParam(ns("d")), tokenParam("t2")}, + {message.AuthorizationTokenParam(message.Token{ + AliasType: message.AliasTypeRegister, TokenAlias: 7, TokenType: 1, TokenValue: []byte("t3"), + })}, + {message.TrackNamespacePrefixParam(ns("e")), message.AuthorizationTokenParam(message.Token{ + AliasType: message.AliasTypeDelete, TokenAlias: 7, + })}, + } { + if _, err := subSess.UpdateRequest(t.Context(), stream, ps); err != nil { + t.Fatalf("REQUEST_UPDATE %v: %v", ps, err) + } + } + auth.mu.Lock() + defer auth.mu.Unlock() + want := [][]string{{"t0"}, {"t0"}, {"t1"}, {"t2"}, {"t3"}} + if !slices.EqualFunc(auth.seen, want, slices.Equal) { + t.Fatalf("Authorizer saw tokens %q, want %q", auth.seen, want) + } + }) + } +} diff --git a/pkg/relay/session_handler.go b/pkg/relay/session_handler.go index 2e802eaa..5ab66fb7 100644 --- a/pkg/relay/session_handler.go +++ b/pkg/relay/session_handler.go @@ -250,22 +250,11 @@ func (h *sessionHandler) run(ctx context.Context) error { // - the session emits an unrecoverable error from AcceptRequest, // - a non-shutdown read failure occurs. // -// Per-request failures (auth, rejected requests) do NOT terminate the loop. -// A stream opened by anything but a request message is session-fatal (§3.3); -// AcceptRequest has already closed the session. +// Per-request failures (auth, rejected requests, a stream ended before its +// request) do NOT terminate the loop. A protocol violation does, and +// AcceptRequest has already closed the session with its code. func (h *sessionHandler) runRequestLoop(ctx context.Context) error { - err := h.requestMux(ctx).Run(ctx, h.sess) - // A malformed / duplicate / overflowing / unknown AUTHORIZATION_TOKEN alias - // surfaces from AcceptRequest as a session-level fault per §10.2.2: close the - // session with the mapped SESSION_ERROR code rather than just tearing down - // the request loop. - if tce, ok := errors.AsType[*session.TokenCacheError](err); ok { - h.log.LogAttrs(ctx, slog.LevelDebug, "relay closing session on token cache error", - slog.String("err", err.Error()), - slog.Uint64("code", uint64(tce.Code))) - _ = h.sess.Close(tce.Code, tce.Error()) - } - return err + return h.requestMux(ctx).Run(ctx, h.sess) } // runDataLoop accepts inbound data streams and routes each by type: subgroup @@ -456,14 +445,7 @@ func (h *sessionHandler) rejectExcessiveLoad(ctx context.Context, req *session.R // [moqt.RequestUnauthorized] — maps straight onto the wire reply. Like // rejectAuth, a write failure is logged and otherwise swallowed. func (h *sessionHandler) rejectTokenDenied(ctx context.Context, req *session.Request, denyErr error) { - code := moqt.RequestUnauthorized - reason := denyErr.Error() - if denied, ok := errors.AsType[*session.TokenDeniedError](denyErr); ok { - code = denied.RequestErrorCode() - if denied.Reason != "" { - reason = denied.Reason - } - } + code, reason := tokenDenial(denyErr) h.log.LogAttrs(ctx, slog.LevelDebug, "relay rejecting request on token verification", slog.String("err", denyErr.Error()), slog.Uint64("code", uint64(code))) if err := req.RejectError(code, reason); err != nil && !errors.Is(err, context.Canceled) { @@ -472,6 +454,18 @@ func (h *sessionHandler) rejectTokenDenied(ctx context.Context, req *session.Req } } +// tokenDenial is the REQUEST_ERROR code and reason for a token-verification +// denial (see [sessionHandler.rejectTokenDenied]). +func tokenDenial(denyErr error) (moqt.RequestErrorCode, string) { + if denied, ok := errors.AsType[*session.TokenDeniedError](denyErr); ok { + if denied.Reason != "" { + return denied.RequestErrorCode(), denied.Reason + } + return denied.RequestErrorCode(), denyErr.Error() + } + return moqt.RequestUnauthorized, denyErr.Error() +} + // handleFollowupRequestID validates a peer REQUEST_UPDATE's Request ID — // §10.1: an update consumes an ID from the sender's space, and the readers // that parse follow-ups directly bypass AcceptRequest's checking. A @@ -509,24 +503,40 @@ func (h *sessionHandler) handleRequestUpdateLimit(ctx context.Context, lim *sess // handleFollowupTokens routes a follow-up message's AUTHORIZATION_TOKEN // parameters through the session token cache — §10.2.2 allows REQUEST_UPDATE // to REGISTER or DELETE aliases, and the readers that parse follow-ups -// directly bypass AcceptRequest's processing. Returns false when a token -// fault closed the session, in which case the caller's read loop should -// stop. -func (h *sessionHandler) handleFollowupTokens(ctx context.Context, msg message.Message) bool { - _, err := h.sess.ProcessFollowupTokens(msg) +// directly bypass AcceptRequest's processing. It returns the resolved tokens, +// and ok false when a token fault closed the session, in which case the +// caller's read loop should stop. +func (h *sessionHandler) handleFollowupTokens( + ctx context.Context, + msg message.Message, +) (toks []session.ResolvedToken, ok bool) { + toks, err := h.sess.ProcessFollowupTokens(msg) if err == nil { - return true + return toks, true } if tce, ok := errors.AsType[*session.TokenCacheError](err); ok { h.log.LogAttrs(ctx, slog.LevelDebug, "relay closing session on follow-up token cache error", slog.String("err", err.Error()), slog.Uint64("code", uint64(tce.Code))) _ = h.sess.Close(tce.Code, tce.Error()) - return false + return nil, false } h.log.LogAttrs(ctx, slog.LevelDebug, "follow-up token processing failed", slog.String("err", err.Error())) - return false + return nil, false +} + +// errRequestCancelled is the cancellation cause of a context bound to a +// request its peer cancelled (§3.3.3); see [ctxResetCode]. +var errRequestCancelled = errors.New("relay: request cancelled") + +// ctxResetCode is the §3.3.4 code for a stream reset because ctx ended: +// CANCELLED for a cancelled request, SESSION_CLOSED otherwise. +func ctxResetCode(ctx context.Context) moqt.StreamResetCode { + if errors.Is(context.Cause(ctx), errRequestCancelled) { + return moqt.StreamResetCancelled + } + return moqt.StreamResetSessionClosed } // readRequestStream owns all reads on an established request stream: it @@ -661,10 +671,20 @@ func (h *sessionHandler) streamFetchRange( // stream closes (§10.12). defer sub.StreamClosed() } - - // Gather cached objects, stitching the below-floor portion from upstream - // when the cache doesn't cover the whole range (§9.4). + // ctx ending resets the stream rather than completing it: CANCELLED when + // its cause is errRequestCancelled (a fill's cancelled subscription, + // §5.1.3.1), else SESSION_CLOSED (§3.3.4). + cancelOut := func() { out.Cancel(ctxResetCode(ctx)) } + unwatch := context.AfterFunc(ctx, cancelOut) + defer unwatch() + + // Gather cached objects, asking an upstream about what the cache cannot + // vouch for (§10.13). objs, refusal := h.stitchedFetchObjects(ctx, entry, fullName, start, end, order, fillTimeout) + if ctx.Err() != nil { + cancelOut() // before the deferred StreamClosed (§10.12) + return false + } if refusal != nil { // §2.5.1: with FETCH_OK (or SUBSCRIBE_OK) already sent, only a // reset is left (an interpretation: no Object was forwarded yet). @@ -700,6 +720,10 @@ func (h *sessionHandler) streamFetchRange( out.Cancel(moqt.StreamResetInternalError) return false } + if !unwatch() { + cancelOut() // ctx ended first; reset before the deferred StreamClosed + return false + } _ = out.Close() return true } diff --git a/pkg/relay/session_handler_test.go b/pkg/relay/session_handler_test.go index c7f7b39e..192bd63e 100644 --- a/pkg/relay/session_handler_test.go +++ b/pkg/relay/session_handler_test.go @@ -5,6 +5,7 @@ import ( "errors" "sync/atomic" "testing" + "time" "github.com/floatdrop/moq-go/pkg/moqt" "github.com/floatdrop/moq-go/pkg/moqt/message" @@ -224,3 +225,63 @@ func (a *denyAuthorizer) AuthorizeTrackStatus(context.Context, *session.Session, a.trackStatusCalls.Add(1) return a.err } + +// TestSessionHandler_TruncatedOpenerKeepsServing: a request stream that ends +// or is reset before its first message is complete fails that request only +// (§3.3.2, §3.3.3); the relay goes on serving the session. +func TestSessionHandler_TruncatedOpenerKeepsServing(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + end func(session.Stream) + }{ + {"FIN", func(s session.Stream) { _ = s.Close() }}, + {"reset", func(s session.Stream) { s.CancelWrite(0) }}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + l := newPipeListener() + _, teardown := connectRelayOn(t, relay.Config{}, l) + defer teardown() + peer, conn := dialRaw(t, l) + stream, err := conn.OpenStream() + if err != nil { + t.Fatalf("OpenStream: %v", err) + } + // Type SUBSCRIBE, Length 16, then only two body bytes. + _, _ = stream.Write([]byte{byte(message.TypeSubscribe), 0x00, 0x10, 0x00, 0x00}) + tc.end(stream) + + // The pipe transport blocks the opener's write until the relay + // reads it, which no context bounds, so wait here instead. + errc := make(chan error, 1) + go func() { + _, err := peer.Subscribe(t.Context(), &message.Subscribe{Namespace: ns("video"), Name: []byte("cam1")}) + errc <- err + }() + select { + case err := <-errc: + requireRejectedWithCode(t, err, moqt.RequestDoesNotExist) + case <-time.After(2 * time.Second): + t.Fatal("relay stopped reading requests after the truncated one") + } + }) + } +} + +// TestSessionHandler_BadRequestIDClosesSession: a request opener with a +// wrong-parity Request ID closes the session (§10.1). +func TestSessionHandler_BadRequestIDClosesSession(t *testing.T) { + t.Parallel() + l := newPipeListener() + _, teardown := connectRelayOn(t, relay.Config{}, l) + defer teardown() + peer, conn := dialRaw(t, l) + stream, err := conn.OpenStream() + if err != nil { + t.Fatalf("OpenStream: %v", err) + } + // A client's Request IDs are even. + _ = message.Marshal(stream, &message.Subscribe{RequestID: 1, Namespace: ns("video"), Name: []byte("cam1")}) + requireSessionClosed(t, peer, "a SUBSCRIBE with an odd Request ID from a client") +} diff --git a/pkg/relay/session_publish_test.go b/pkg/relay/session_publish_test.go index fd72b87d..06a4187b 100644 --- a/pkg/relay/session_publish_test.go +++ b/pkg/relay/session_publish_test.go @@ -119,14 +119,15 @@ func TestPublish_ForwardsToSubscribeTracks(t *testing.T) { if pub.TrackAlias == 0 { t.Fatal("forwarded TrackAlias is 0; want one allocated on the subscriber's session") } - // §10.20.1: the SUBSCRIBE_TRACKS omitted FORWARD and GROUP_ORDER, so the - // forwarded PUBLISH carries neither (FORWARD defaults to 1, GROUP_ORDER to - // the publisher's preference). + // §10.20.1: the SUBSCRIBE_TRACKS omitted FORWARD and GROUP_ORDER. + // FORWARD defaults to 1 and is omitted; GROUP_ORDER is the publisher's + // preference (§10.2.8), Ascending for this track (§12.5), and stated. if p, ok := pub.Parameters.Find(message.ParamForward); ok { t.Errorf("forwarded FORWARD present (=%d), want omitted", p.Byte) } - if p, ok := pub.Parameters.Find(message.ParamGroupOrder); ok { - t.Errorf("forwarded GROUP_ORDER present (=%d), want omitted", p.Byte) + if p, ok := pub.Parameters.Find(message.ParamGroupOrder); !ok || + message.GroupOrder(p.Byte) != message.GroupOrderAscending { + t.Errorf("forwarded GROUP_ORDER = %d (present=%v), want Ascending (0x1)", p.Byte, ok) } } diff --git a/pkg/relay/subscribe_cancel_test.go b/pkg/relay/subscribe_cancel_test.go new file mode 100644 index 00000000..dcf41df1 --- /dev/null +++ b/pkg/relay/subscribe_cancel_test.go @@ -0,0 +1,200 @@ +package relay_test + +import ( + "bytes" + "errors" + "fmt" + "io" + "testing" + "time" + + "github.com/floatdrop/moq-go/pkg/moqt/message" + "github.com/floatdrop/moq-go/pkg/moqt/session" + "github.com/floatdrop/moq-go/pkg/relay" +) + +// A subscriber that cancels its SUBSCRIBE gets the subscription's open +// streams reset: §5.1.1 "It MUST reset any open streams associated with the +// SUBSCRIBE", and §5.1.3.1 "When the subscription is cancelled, the publisher +// MUST reset any open fill fetch streams". + +// readEnd reads next until it fails, returning that error, or fails the test +// after 2s. +func readEnd(t *testing.T, next func() error) error { + t.Helper() + end := make(chan error, 1) + go func() { + for { + if err := next(); err != nil { + end <- err + return + } + } + }() + select { + case err := <-end: + return err + case <-time.After(2 * time.Second): + t.Fatal("the stream stayed open after the subscription was cancelled") + return nil + } +} + +// subClosedMetrics signals each SubscriptionClosed, which the relay reports +// after it has acted on a subscription's end. +type subClosedMetrics struct { + relay.NopMetrics + + closed chan struct{} +} + +func (m *subClosedMetrics) SubscriptionClosed(relay.TrackRef) { m.closed <- struct{}{} } + +// newCancelTestRelay is [newCam1Publisher] on a relay whose closed channel +// receives once for each subscription it has finished. +func newCancelTestRelay(t *testing.T) (pubSess *session.Session, alias uint64, closed <-chan struct{}) { + t.Helper() + m := &subClosedMetrics{closed: make(chan struct{}, 8)} + pubSess, teardown := connectRelay(t, relay.Config{Metrics: m}) + t.Cleanup(teardown) + alias = 7 + publishVideoTrackProps(t, pubSess, "cam1", alias, nil) + return pubSess, alias, m.closed +} + +// cancelAndAwait cancels the subscription and waits for the relay to finish +// it, so what the test does next cannot overtake the cancellation. +func cancelAndAwait(t *testing.T, cancel func(), closed <-chan struct{}) { + t.Helper() + cancel() + select { + case <-closed: + case <-time.After(2 * time.Second): + t.Fatal("the relay did not finish the cancelled subscription") + } +} + +// subscribeVia subscribes sess to video/cam1 with params, directly or as the +// subscription a SUBSCRIBE_TRACKS on video gets through a forwarded PUBLISH, +// and returns what cancels it. +type subscribeVia func(t *testing.T, sess *session.Session, params ...message.Parameter) (cancel func()) + +var subscribeVias = []struct { + name string + sub subscribeVia +}{ + {"SUBSCRIBE", func(t *testing.T, sess *session.Session, params ...message.Parameter) func() { + sub := subscribeCam1(t, sess, params...) + return func() { _ = sub.Close() } + }}, + {"SUBSCRIBE_TRACKS", func(t *testing.T, sess *session.Session, params ...message.Parameter) func() { + reqs := forwardedPublishes(t, sess) + subscribeTracks(t, sess, ns("video"), params...) + in := acceptForwarded(t, awaitForwarded(t, reqs)) + return func() { _ = in.Close() } + }}, +} + +// TestSubscribe_CancelResetsOpenSubgroup: the subgroup stream the relay has +// open for a cancelled subscription is reset, whether the publisher goes on +// writing to it or not. +func TestSubscribe_CancelResetsOpenSubgroup(t *testing.T) { + t.Parallel() + for _, via := range subscribeVias { + for _, continues := range []bool{false, true} { + t.Run(fmt.Sprintf("%s/publisher continues=%v", via.name, continues), func(t *testing.T) { + t.Parallel() + pubSess, alias, closed := newCancelTestRelay(t) + subSess := dialAnotherClient(t, pubSess) + cancel := via.sub(t, subSess) + + sg, err := openSubgroupWaiting(t, pubSess, subgroupHeader(alias, 0)) + if err != nil { + t.Fatalf("OpenSubgroup: %v", err) + } + wrote := make(chan struct{}) + go func() { + defer close(wrote) + _ = sg.WriteObject(&message.SubgroupObject{Payload: []byte("x")}) + }() + ds, err := subSess.AcceptDataStream(t.Context()) + if err != nil { + t.Fatalf("AcceptDataStream: %v", err) + } + in := ds.(*session.IncomingSubgroupStream) + if _, err := in.ReadDecoded(); err != nil { + t.Fatalf("ReadDecoded: %v", err) + } + + <-wrote + cancelAndAwait(t, cancel, closed) + if continues { + go func() { + for range 3 { + if sg.WriteObject(&message.SubgroupObject{Payload: []byte("x")}) != nil { + return + } + } + _ = sg.Close() + }() + } + if err := readEnd(t, func() error { _, err := in.ReadDecoded(); return err }); errors.Is(err, io.EOF) { + t.Fatal("the subgroup stream was FINed after the subscription was cancelled, want a reset") + } + }) + } + } +} + +// TestSubscribe_CancelResetsFill: a fill fetch stream still being written +// when its subscription is cancelled is reset, not completed. +func TestSubscribe_CancelResetsFill(t *testing.T) { + t.Parallel() + for _, via := range subscribeVias { + t.Run(via.name, func(t *testing.T) { + t.Parallel() + pubSess, alias, closed := newCancelTestRelay(t) + sg, err := openSubgroupWaiting(t, pubSess, subgroupHeader(alias, 0)) + if err != nil { + t.Fatalf("OpenSubgroup: %v", err) + } + // Enough bytes that the fill cannot be written ahead of the reader. + payload := bytes.Repeat([]byte("x"), 4096) + for range 32 { + if err := sg.WriteObject(&message.SubgroupObject{Payload: payload}); err != nil { + t.Fatalf("WriteObject: %v", err) + } + } + if err := sg.Close(); err != nil { + t.Fatalf("Close: %v", err) + } + time.Sleep(50 * time.Millisecond) + + subSess := dialAnotherClient(t, pubSess) + cancel := via.sub(t, subSess, + message.NextObjectFilter(), + message.FillParametersParam(message.Parameters{message.UnfilteredFilter()}), + ) + ds, err := subSess.AcceptDataStream(t.Context()) + if err != nil { + t.Fatalf("AcceptDataStream: %v", err) + } + fs, ok := ds.(*session.IncomingFetchStream) + if !ok { + t.Fatalf("got %T, want the fill fetch stream", ds) + } + if _, err := fs.ReadDecoded(); err != nil { + t.Fatalf("ReadDecoded: %v", err) + } + + cancelAndAwait(t, cancel, closed) + // The fill is reset from a goroutine the cancellation starts; not + // reading meanwhile holds the writer on its next Object, which the + // synchronous test transport would otherwise let it race past. + time.Sleep(100 * time.Millisecond) + if err := readEnd(t, func() error { _, err := fs.ReadDecoded(); return err }); errors.Is(err, io.EOF) { + t.Fatal("the fill fetch stream was completed after the subscription was cancelled, want a reset") + } + }) + } +}