diff --git a/cmd/silo/session_sync.go b/cmd/silo/session_sync.go index 70bd1d57..533c45e7 100644 --- a/cmd/silo/session_sync.go +++ b/cmd/silo/session_sync.go @@ -25,6 +25,8 @@ func buildLiveSessionSync(s *playback.Session, reportingNode string) worker.Sess ClientIP: s.ClientIP, ClientName: s.ClientName, ClientVersion: s.ClientVersion, + ClientBuild: s.ClientBuild, + ClientChannel: s.ClientChannel, ClientUserAgent: s.ClientUserAgent, AudioTrackIndex: s.AudioTrackIndex, TranscodeAudio: s.TranscodeAudio, diff --git a/docs/design/schemas/playback-v3/v3/replan-request.schema.json b/docs/design/schemas/playback-v3/v3/replan-request.schema.json index bb4c6d55..3b22567e 100644 --- a/docs/design/schemas/playback-v3/v3/replan-request.schema.json +++ b/docs/design/schemas/playback-v3/v3/replan-request.schema.json @@ -580,6 +580,14 @@ "type": "string", "maxLength": 64 }, + "app_build": { + "type": "string", + "maxLength": 64 + }, + "app_channel": { + "type": "string", + "maxLength": 32 + }, "device": { "type": "object", "description": "Manufacturer and model stay first-class because the device-quirk registry matches on them. Everything platform-specific travels in platform_details as opaque bounded strings.", diff --git a/docs/design/schemas/playback-v3/v3/start-request.schema.json b/docs/design/schemas/playback-v3/v3/start-request.schema.json index d05cfcff..002edae5 100644 --- a/docs/design/schemas/playback-v3/v3/start-request.schema.json +++ b/docs/design/schemas/playback-v3/v3/start-request.schema.json @@ -531,6 +531,14 @@ "type": "string", "maxLength": 64 }, + "app_build": { + "type": "string", + "maxLength": 64 + }, + "app_channel": { + "type": "string", + "maxLength": 32 + }, "device": { "type": "object", "description": "Manufacturer and model stay first-class because the device-quirk registry matches on them. Everything platform-specific travels in platform_details as opaque bounded strings.", diff --git a/docs/settings-api.md b/docs/settings-api.md index fb8ec5ad..e2b76928 100644 --- a/docs/settings-api.md +++ b/docs/settings-api.md @@ -48,6 +48,38 @@ the other. Send the exact lower-case family: | macOS | `desktop` | | Browser | `web` | +### App identity headers + +Separately from the family, every first-party client should send its own app +identity on playback requests. These are server-wide contextual headers, not +settings-specific: the playback session stores them, the admin Activity page +renders them ("Silo Android TV 1.0.0 (build 5)"), and playback decision logs +carry them so a report can be tied to an exact build. + +| Header | Clamp | Meaning | +| ----------------------- | ----- | ------------------------------------------------------------------------------------------------------------- | +| `X-Silo-Client` | 128 | Product name, e.g. `Silo Android TV`, `Silo iOS`. | +| `X-Silo-Client-Version` | 64 | Marketing version, e.g. `1.0.0`. Sent verbatim and displayed verbatim — do not pre-shorten it. | +| `X-Silo-Client-Build` | 64 | Opaque per-platform build identifier (Android `versionCode`, Apple `CFBundleVersion`). Never parsed or compared. | +| `X-Silo-Client-Channel` | 32 | Opaque distribution channel: `release`, `beta`, `sideload`, `dev`. Stored verbatim; `release` is not displayed. | + +Values are trimmed and truncated to the clamp above — never rejected, on either +route, because an identity label must not be able to fail a playback start. The +clamp counts characters, not bytes, matching `maxLength` in the v3 request +schemas, and is applied where the request is read rather than where the session +is created so the decision logs and `playback_route_events` observe it too. +Nothing is validated against an enum either, so a client may introduce a new +channel without a server change. + +Protocol-v3 `POST /playback/start` accepts `client_playback_context.app_version`, +`.app_build`, and `.app_channel` as a body-level fallback for clients that cannot +set the headers on every request. The headers win field by field when both are +present, and the fallback applies **only to a client that sent `X-Silo-Client`**: +`client_playback_context` carries no app name, so nothing in the body can +identify a client that did not name itself — such a session is labeled from its +user agent, and its `app_version` is a free-form platform string rather than the +marketing version `client_version` promises. + ## Remote scopes Every stored value has exactly one identity. Context fields not named by the diff --git a/internal/api/handlers/playback.go b/internal/api/handlers/playback.go index 424fd87b..5e7ac3cb 100644 --- a/internal/api/handlers/playback.go +++ b/internal/api/handlers/playback.go @@ -43,6 +43,11 @@ import ( type SessionManagerInterface interface { StartSession(userID int, profileID string, fileID int, method playback.PlayMethod, transcodeAudio bool) (*playback.Session, error) StartSessionWithFiles(userID int, profileID string, effectiveFileID int, requestedFileID int, method playback.PlayMethod, transcodeAudio bool) (*playback.Session, error) + // StartSessionWithFilesContext is required rather than probed for at run + // time: the context is how the reporting client's identity reaches the new + // session, so an implementation without it would start sessions that + // silently carry no client name, version, build or channel. + StartSessionWithFilesContext(ctx context.Context, userID int, profileID string, effectiveFileID int, requestedFileID int, method playback.PlayMethod, transcodeAudio bool) (*playback.Session, error) UpdateProgress(sessionID string, position float64, isPaused bool) error UpdateAudioTrack(sessionID string, audioTrackIndex int, method playback.PlayMethod) error UpdateStreamState(sessionID string, state playback.SessionStreamState) error @@ -63,10 +68,6 @@ type SessionManagerInterface interface { GetSession(sessionID string) (*playback.Session, error) } -type sessionStarterWithFilesContext interface { - StartSessionWithFilesContext(ctx context.Context, userID int, profileID string, effectiveFileID int, requestedFileID int, method playback.PlayMethod, transcodeAudio bool) (*playback.Session, error) -} - type transcodePermissionChecker interface { CheckTranscodingAllowed(ctx context.Context, userID int, requiresVideoTranscode bool) error } @@ -952,7 +953,9 @@ func (h *PlaybackHandler) handleExpiredSession(session *playback.Session) { } sessionCopy := *session go func() { - slog.Info("expired inactive playback session", "session", sessionCopy.ID, "playback_session_id", sessionCopy.ID) + slog.Info("expired inactive playback session", append([]any{ + "session", sessionCopy.ID, "playback_session_id", sessionCopy.ID, + }, sessionCopy.ClientInfo().LogAttrs()...)...) // Expiry is a liveness reap, not a user stop — keep the recipe card so a // resume reconstructs under the same id (the card's own TTL reaps it if // the session is truly abandoned). @@ -1076,11 +1079,18 @@ func playbackClientInfoFromRequest(r *http.Request) playback.ClientInfo { if r == nil { return playback.ClientInfo{} } + // Clamped here, at the boundary, rather than only where the session stamps + // them: the decision logs and playback_route_events are written from this + // value directly, so a client sending a header-sized build would otherwise + // reach both despite the published bound. Values stay opaque — trimmed and + // length-clamped, never parsed or validated against an enum. return playback.ClientInfo{ - Name: strings.TrimSpace(r.Header.Get("X-Silo-Client")), - Version: strings.TrimSpace(r.Header.Get("X-Silo-Client-Version")), + Name: r.Header.Get("X-Silo-Client"), + Version: r.Header.Get("X-Silo-Client-Version"), + Build: r.Header.Get("X-Silo-Client-Build"), + Channel: r.Header.Get("X-Silo-Client-Channel"), UserAgent: r.UserAgent(), - } + }.Normalized() } // HandleUpdateProgress handles POST /playback/{session_id}/progress. diff --git a/internal/api/handlers/playback_sessions.go b/internal/api/handlers/playback_sessions.go index 017a1f7c..97f4cefc 100644 --- a/internal/api/handlers/playback_sessions.go +++ b/internal/api/handlers/playback_sessions.go @@ -48,7 +48,10 @@ type playbackSessionRow struct { ClientIP string `json:"client_ip,omitempty"` ClientName string `json:"client_name,omitempty"` ClientVersion string `json:"client_version,omitempty"` + ClientBuild string `json:"client_build,omitempty"` + ClientChannel string `json:"client_channel,omitempty"` ClientLabel string `json:"client_label,omitempty"` + ClientLabelFull string `json:"client_label_full,omitempty"` ClientUserAgent string `json:"client_user_agent,omitempty"` AudioTrackIndex int `json:"audio_track_index"` TranscodeAudio bool `json:"transcode_audio"` @@ -89,6 +92,11 @@ type playbackSessionsCapabilitiesResponse struct { EffectivePlayMethodValues []string `json:"effective_play_method_values"` // IsJellyfinClient reports that rows carry is_jellyfin_client. IsJellyfinClient bool `json:"is_jellyfin_client"` + // ClientBuild reports that rows carry client_build (and the exact-version + // client_label_full derived from it). + ClientBuild bool `json:"client_build"` + // ClientChannel reports that rows carry client_channel. + ClientChannel bool `json:"client_channel"` } // HandleGetSessionsCapabilities exposes additive feature support for the live @@ -98,6 +106,8 @@ func (h *AdminHandler) HandleGetSessionsCapabilities(w http.ResponseWriter, _ *h EffectivePlayMethod: true, EffectivePlayMethodValues: []string{"direct", "remux", "transcode", "audio"}, IsJellyfinClient: true, + ClientBuild: true, + ClientChannel: true, }) } @@ -182,6 +192,8 @@ func (l *PlaybackSessionsLoader) Load( COALESCE(HOST(s.client_ip), ''), COALESCE(s.client_name, ''), COALESCE(s.client_version, ''), + COALESCE(s.client_build, ''), + COALESCE(s.client_channel, ''), COALESCE(s.client_user_agent, ''), COALESCE(s.audio_track_index, 0), COALESCE(s.transcode_audio, FALSE), @@ -239,6 +251,7 @@ func (l *PlaybackSessionsLoader) Load( &posterPath, &s.PlayMethod, &s.ReportingNode, &s.NodeDisplayName, &s.FileDuration, &s.StartedAt, &s.UpdatedAt, &s.PositionSeconds, &s.IsPaused, &s.HasPlaybackControl, &s.ClientIP, &s.ClientName, &s.ClientVersion, + &s.ClientBuild, &s.ClientChannel, &s.ClientUserAgent, &s.AudioTrackIndex, &s.TranscodeAudio, &streamBitrateKbps, &s.TranscodeNodeURL, &s.TargetResolution, &s.TargetVideoCodec, &s.TargetAudioCodec, &targetBitrateKbps, &s.TranscodeHWAccel, &s.SourceContainer, &sourceBitrateKbps, &s.SourceVideoCodec, &s.SourceVideoResolution, @@ -253,6 +266,13 @@ func (l *PlaybackSessionsLoader) Load( s.SourceBitrateKbps = sourceBitrateKbps s.SourceAudioChannels = sourceAudioChannels s.ClientLabel = playbackClientDisplayName(s.ClientName, s.ClientVersion, s.ClientUserAgent) + // Only worth the bytes when it says more than the compact label — which + // it does not for any client without a build or a non-release channel, + // i.e. most of a 200-row page. Clients read client_label when + // client_label_full is absent, so omitting the duplicate costs nothing. + if full := playbackClientFullDisplayName(s.ClientName, s.ClientVersion, s.ClientBuild, s.ClientChannel, s.ClientUserAgent); full != s.ClientLabel { + s.ClientLabelFull = full + } enrichPlaybackSessionRow(&s, audioTracksJSON) sessions = append(sessions, s) } @@ -447,11 +467,47 @@ func firstNonEmptyValue(values ...string) string { return "" } +// playbackClientFullDisplayName renders the client's exact identity — version, +// opaque build, and non-default channel — for surfaces that can afford the +// width (expanded session details, tooltips). The name-and-version half is +// whatever the compact label resolved, so a client identified only by its user +// agent still gets its build named: a client that bothered to report a build +// has earned having it displayed, whether or not it also named itself. +func playbackClientFullDisplayName(name, version, build, channel, userAgent string) string { + label := playbackClientDisplayName(name, version, userAgent) + if label == "" { + return "" + } + + build = strings.TrimSpace(build) + channel = strings.TrimSpace(channel) + // "release" is the assumed channel, so naming it adds width without adding + // information; every other channel is worth calling out. + if strings.EqualFold(channel, "release") { + channel = "" + } + var qualifiers []string + if build != "" { + qualifiers = append(qualifiers, "build "+build) + } + if channel != "" { + qualifiers = append(qualifiers, channel) + } + if len(qualifiers) > 0 { + label += " (" + strings.Join(qualifiers, ", ") + ")" + } + return label +} + +// playbackClientDisplayName renders the compact label the session lists show. +// A client that reports its own name keeps its version verbatim — truncating +// "1.0.0" to "1" hid the only field that identifies the running build. Labels +// derived from a user agent stay shortened, where a full browser version +// ("120.0.6099.109") is noise. func playbackClientDisplayName(name, version, userAgent string) string { name = strings.TrimSpace(name) - version = shortPlaybackClientVersion(version) if name != "" { - if version != "" { + if version = strings.TrimSpace(version); version != "" { return name + " " + version } return name diff --git a/internal/api/handlers/playback_sessions_test.go b/internal/api/handlers/playback_sessions_test.go index 06df4c82..07cc459c 100644 --- a/internal/api/handlers/playback_sessions_test.go +++ b/internal/api/handlers/playback_sessions_test.go @@ -4,7 +4,12 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "strings" "testing" + "unicode" + "unicode/utf8" + + "github.com/Silo-Server/silo-server/internal/playback" ) func TestSessionComponentDecisionLabelsCopiedAudioDuringHLSAsRemux(t *testing.T) { @@ -65,8 +70,8 @@ func TestSessionsCapabilitiesAdvertisesActivityFields(t *testing.T) { if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil { t.Fatalf("decode capabilities: %v", err) } - if !resp.EffectivePlayMethod || !resp.IsJellyfinClient { - t.Fatalf("capabilities must advertise both fields: %+v", resp) + if !resp.EffectivePlayMethod || !resp.IsJellyfinClient || !resp.ClientBuild || !resp.ClientChannel { + t.Fatalf("capabilities must advertise every additive field: %+v", resp) } want := []string{"direct", "remux", "transcode", "audio"} if len(resp.EffectivePlayMethodValues) != len(want) { @@ -213,6 +218,273 @@ func TestPlaybackClientDisplayNameAndroidDevices(t *testing.T) { } } +func TestPlaybackClientDisplayNameKeepsNamedClientVersionExact(t *testing.T) { + cases := []struct { + name string + clientName string + clientVersion string + want string + }{ + {name: "patch version survives", clientName: "Silo Android TV", clientVersion: "1.0.0", want: "Silo Android TV 1.0.0"}, + {name: "prerelease suffix survives", clientName: "Silo iOS", clientVersion: "2.1.0-rc.3", want: "Silo iOS 2.1.0-rc.3"}, + {name: "no version", clientName: "Silo Android TV", clientVersion: "", want: "Silo Android TV"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := playbackClientDisplayName(tc.clientName, tc.clientVersion, "") + if got != tc.want { + t.Fatalf("playbackClientDisplayName(%q, %q, \"\") = %q, want %q", tc.clientName, tc.clientVersion, got, tc.want) + } + }) + } +} + +func TestPlaybackClientFullDisplayName(t *testing.T) { + cases := []struct { + name string + client string + version string + build string + channel string + userAgent string + want string + }{ + { + name: "version build and channel", + client: "Silo Android TV", + version: "1.0.0", + build: "5", + channel: "dev", + want: "Silo Android TV 1.0.0 (build 5, dev)", + }, + { + name: "release channel omitted", + client: "Silo Android TV", + version: "1.0.0", + build: "5", + channel: "release", + want: "Silo Android TV 1.0.0 (build 5)", + }, + { + name: "release channel omitted case insensitively", + client: "Silo Android TV", + version: "1.0.0", + build: "5", + channel: "Release", + want: "Silo Android TV 1.0.0 (build 5)", + }, + { + name: "empty channel omitted", + client: "Silo Android TV", + version: "1.0.0", + build: "5", + want: "Silo Android TV 1.0.0 (build 5)", + }, + { + name: "channel without build", + client: "Silo Android TV", + version: "1.0.0", + channel: "dev", + want: "Silo Android TV 1.0.0 (dev)", + }, + { + name: "build and channel empty", + client: "Silo Android TV", + version: "1.0.0", + want: "Silo Android TV 1.0.0", + }, + { + name: "version empty", + client: "Silo Android TV", + want: "Silo Android TV", + }, + { + name: "opaque build is not parsed", + client: "Silo tvOS", + version: "1.0.0", + build: "2026.08.13-abcdef", + want: "Silo tvOS 1.0.0 (build 2026.08.13-abcdef)", + }, + { + // A client that reports a build but not a name is still named by its + // build: the user-agent label supplies the product half, and dropping + // the qualifiers would hide the exact field this surface exists for. + name: "no client name keeps build and channel on the user agent label", + userAgent: "Mozilla/5.0 (X11; Linux x86_64) Chrome/120.0.0.0 Safari/537.36", + build: "5", + channel: "dev", + want: "Chrome 120 (build 5, dev)", + }, + { + name: "no client name and no qualifiers keeps the plain user agent label", + userAgent: "Mozilla/5.0 (X11; Linux x86_64) Chrome/120.0.0.0 Safari/537.36", + want: "Chrome 120", + }, + { + name: "no client name and no user agent", + want: "", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := playbackClientFullDisplayName(tc.client, tc.version, tc.build, tc.channel, tc.userAgent) + if got != tc.want { + t.Fatalf("playbackClientFullDisplayName() = %q, want %q", got, tc.want) + } + }) + } +} + +func TestPlaybackClientInfoForStartV3(t *testing.T) { + cases := []struct { + name string + headers map[string]string + context playback.ClientPlaybackContextV3 + want playback.ClientInfo + }{ + { + name: "headers win over the body", + headers: map[string]string{"X-Silo-Client": "Silo iOS", "X-Silo-Client-Version": "2.1.0", "X-Silo-Client-Build": "9", "X-Silo-Client-Channel": "beta"}, + context: playback.ClientPlaybackContextV3{AppVersion: "1.0.0", AppBuild: "1", AppChannel: "release"}, + want: playback.ClientInfo{Name: "Silo iOS", Version: "2.1.0", Build: "9", Channel: "beta"}, + }, + { + // The fallback is per field, not per struct: a client may set the + // name and version headers on every request and still report the + // build it only knows at start time in the body. + name: "body fills only the fields the headers omit", + headers: map[string]string{"X-Silo-Client": "Silo tvOS", "X-Silo-Client-Version": "2.1.0"}, + context: playback.ClientPlaybackContextV3{AppVersion: "1.0.0", AppBuild: "77", AppChannel: "sideload"}, + want: playback.ClientInfo{Name: "Silo tvOS", Version: "2.1.0", Build: "77", Channel: "sideload"}, + }, + { + name: "body supplies everything but the name", + headers: map[string]string{"X-Silo-Client": "Silo Android TV"}, + context: playback.ClientPlaybackContextV3{AppVersion: "1.0.0", AppBuild: "5", AppChannel: "dev"}, + want: playback.ClientInfo{Name: "Silo Android TV", Version: "1.0.0", Build: "5", Channel: "dev"}, + }, + { + // The regression this guards: the web player reports the literal + // "web" as its app_version and sends no X-Silo-Client. Taking the + // body anyway would stamp "web" onto client_version — the one field + // the contract promises is a marketing version — for every browser + // session, and onto every route event and decision log with it. + name: "nameless client keeps the body out of client_version", + context: playback.ClientPlaybackContextV3{AppVersion: "web"}, + want: playback.ClientInfo{}, + }, + { + name: "nameless client takes no build or channel either", + context: playback.ClientPlaybackContextV3{AppVersion: "web", AppBuild: "5", AppChannel: "dev"}, + want: playback.ClientInfo{}, + }, + { + name: "whitespace-only header falls through to the body", + headers: map[string]string{"X-Silo-Client": "Silo iOS", "X-Silo-Client-Build": " "}, + context: playback.ClientPlaybackContextV3{AppBuild: " 42 "}, + want: playback.ClientInfo{Name: "Silo iOS", Build: "42"}, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + req := httptest.NewRequest(http.MethodPost, "/api/v1/playback/start", nil) + for name, value := range tc.headers { + req.Header.Set(name, value) + } + got := playbackClientInfoForStartV3(req, tc.context) + got.UserAgent = "" + if got != tc.want { + t.Fatalf("playbackClientInfoForStartV3() = %+v, want %+v", got, tc.want) + } + }) + } +} + +// TestPlaybackClientInfoFromRequestClampsHeaders pins the bound at the request +// boundary rather than at session creation. The resolved identity is written +// straight to the plan-decision log and to playback_route_events, so a client +// sending a header-sized build would reach both if the clamp only happened +// where the session stamps its fields. +func TestPlaybackClientInfoFromRequestClampsHeaders(t *testing.T) { + req := httptest.NewRequest(http.MethodPost, "/api/v1/playback/start", nil) + req.Header.Set("X-Silo-Client", " "+strings.Repeat("N", 200)+" ") + req.Header.Set("X-Silo-Client-Version", strings.Repeat("v", 100)) + req.Header.Set("X-Silo-Client-Build", strings.Repeat("b", 100)) + req.Header.Set("X-Silo-Client-Channel", strings.Repeat("c", 100)) + + got := playbackClientInfoFromRequest(req) + + for _, tc := range []struct { + field string + value string + want int + }{ + {"Name", got.Name, 128}, + {"Version", got.Version, 64}, + {"Build", got.Build, 64}, + {"Channel", got.Channel, 32}, + } { + if len(tc.value) != tc.want { + t.Errorf("%s length = %d, want %d", tc.field, len(tc.value), tc.want) + } + } +} + +// TestNormalizeClientMetadataCountsRunes guards the two ways a clamp can go +// wrong on a client-supplied header: the published bound is JSON Schema +// maxLength, which counts characters, and a byte-wise cut through a multi-byte +// rune yields invalid UTF-8 that Postgres refuses — failing the whole per-node +// session-sync transaction, not just the offending row. +func TestNormalizeClientMetadataCountsRunes(t *testing.T) { + req := httptest.NewRequest(http.MethodPost, "/api/v1/playback/start", nil) + // 40 three-byte runes: within the 64-character channel bound as the schema + // counts it, well past it as bytes. + req.Header.Set("X-Silo-Client-Channel", strings.Repeat("δ", 40)) + + got := playbackClientInfoFromRequest(req) + + if runes := utf8.RuneCountInString(got.Channel); runes != 32 { + t.Errorf("Channel runes = %d, want the 32-character bound", runes) + } + if !utf8.ValidString(got.Channel) { + t.Errorf("Channel = %q is not valid UTF-8; a text column would reject it", got.Channel) + } +} + +// TestPlaybackClientInfoStripsControlCharacters guards the one malformed value +// that reaches a text column looking perfectly well-formed. A JSON NUL escape in a +// v3 start body decodes to a real NUL, which is valid UTF-8 — so the UTF-8 +// repair above leaves it and TrimSpace does not consider it whitespace — but +// Postgres refuses NUL in text. The per-node session upserts share one +// transaction, so one such start would stop every live session on that node +// from reconciling. Headers cannot carry it (net/http rejects bytes below +// 0x20), which is exactly why the body path needs its own guard. +func TestPlaybackClientInfoStripsControlCharacters(t *testing.T) { + req := httptest.NewRequest(http.MethodPost, "/api/v1/playback/start", nil) + req.Header.Set("X-Silo-Client", "Silo Android TV") + + got := playbackClientInfoForStartV3(req, playback.ClientPlaybackContextV3{ + AppVersion: "1.0\x00.0", + AppBuild: "5\x00", + AppChannel: "de\nv", + }) + + if want := "1.0.0"; got.Version != want { + t.Errorf("Version = %q, want %q", got.Version, want) + } + if want := "5"; got.Build != want { + t.Errorf("Build = %q, want %q", got.Build, want) + } + if want := "dev"; got.Channel != want { + t.Errorf("Channel = %q, want %q", got.Channel, want) + } + for _, field := range []string{got.Name, got.Version, got.Build, got.Channel} { + if strings.ContainsFunc(field, unicode.IsControl) { + t.Errorf("%q still carries a control character; a text column would reject it", field) + } + } +} + func TestEnrichPlaybackSessionRowUsesCompatOrigin(t *testing.T) { row := playbackSessionRow{ ClientName: "Unrecognized Client", diff --git a/internal/api/handlers/playback_test.go b/internal/api/handlers/playback_test.go index 0a431178..619f42c3 100644 --- a/internal/api/handlers/playback_test.go +++ b/internal/api/handlers/playback_test.go @@ -167,6 +167,10 @@ func (failingSessionManager) StartSessionWithFiles(int, string, int, int, playba return nil, errors.New("boom") } +func (failingSessionManager) StartSessionWithFilesContext(context.Context, int, string, int, int, playback.PlayMethod, bool) (*playback.Session, error) { + return nil, errors.New("boom") +} + func (failingSessionManager) UpdateProgress(string, float64, bool) error { return nil } func (failingSessionManager) UpdateAudioTrack(string, int, playback.PlayMethod) error { return nil } diff --git a/internal/api/handlers/playback_v3.go b/internal/api/handlers/playback_v3.go index 207ce32a..80ff90f0 100644 --- a/internal/api/handlers/playback_v3.go +++ b/internal/api/handlers/playback_v3.go @@ -457,20 +457,25 @@ func (h *PlaybackHandler) handleStartPlaybackV3(w http.ResponseWriter, r *http.R } } h.clarifyOriginalQuality4KTerminalV3(r.Context(), result.Terminal, requestedFile, !shouldTryAlternateFileV3(req.QualityPreference)) + // The exact app identity is logged with every decision so a route or + // terminal reported against one build is attributable without asking the + // user which version they are running. + clientInfo := playbackClientInfoForStartV3(r, req.ClientPlaybackContext) if result.Terminal != nil { - slog.InfoContext(r.Context(), "playback plan decided", "component", "playback", + slog.InfoContext(r.Context(), "playback plan decided", append([]any{ + logComponentKey, "playback", "outcome", "terminal", "reason", result.Terminal.Reason, "file_id", effectiveFile.ID, "quality_preference", req.QualityPreference, - ) + }, clientInfo.LogAttrs()...)...) response, persistErr := h.persistTerminalStartDecisionV3(r.Context(), userID, profileID, req, requestDigests, requestedFile.ID, effectiveFile.ID, playback.NewTerminalResponseV3(result.Terminal.Reason, result.Terminal.Message, result.Terminal.Retryable)) if persistErr != nil { writeStartAttemptPersistenceErrorV3(w, persistErr) return } if response.Terminal != nil { - h.enqueueRouteEventV3(playback.RouteEventRecordV3{RouteEventV3: playback.RouteEventV3{ProtocolVersion: playback.ProtocolV3, PlaybackAttemptID: req.PlaybackAttemptID, Event: playback.RouteEventTerminalV3, FallbackReason: response.Terminal.Reason, OutputContextID: req.ClientPlaybackContext.Output.OutputContextID}, UserID: userID, ProfileID: profileID, ClientName: playbackClientInfoFromRequest(r).Name, ClientVersion: playbackClientInfoFromRequest(r).Version, ClientModel: req.ClientPlaybackContext.Device.Model}) + h.enqueueRouteEventV3(playback.RouteEventRecordV3{RouteEventV3: playback.RouteEventV3{ProtocolVersion: playback.ProtocolV3, PlaybackAttemptID: req.PlaybackAttemptID, Event: playback.RouteEventTerminalV3, FallbackReason: response.Terminal.Reason, OutputContextID: req.ClientPlaybackContext.Output.OutputContextID}, UserID: userID, ProfileID: profileID, ClientName: clientInfo.Name, ClientVersion: clientInfo.Version, ClientBuild: clientInfo.Build, ClientChannel: clientInfo.Channel, ClientModel: req.ClientPlaybackContext.Device.Model}) } writeJSON(w, http.StatusCreated, response) return @@ -478,7 +483,8 @@ func (h *PlaybackHandler) handleStartPlaybackV3(w http.ResponseWriter, r *http.R // One line per plan decision so route selection is reconstructible from // server logs alone (finding a mis-planned route previously required // correlating client logcat, ffmpeg commands, and session rows). - slog.InfoContext(r.Context(), "playback plan decided", "component", "playback", + slog.InfoContext(r.Context(), "playback plan decided", append([]any{ + logComponentKey, "playback", "outcome", "plan", "decision_reason", result.Plan.DecisionReason, "delivery", result.Plan.Delivery, @@ -491,9 +497,9 @@ func (h *PlaybackHandler) handleStartPlaybackV3(w http.ResponseWriter, r *http.R "target_bitrate_kbps", result.TargetBitrateKbps, "quality_preference", req.QualityPreference, "bandwidth_estimate_kbps", intOrZeroHandlerV3(req.BandwidthEstimateKbps), - ) + }, clientInfo.LogAttrs()...)...) result.Plan.DegradationWarnings = append(result.Plan.DegradationWarnings, warnings...) - response, statusErr := h.startPlannedPlaybackV3(r, userID, profileID, req, requestDigests, requestedFile, effectiveFile, audioIndex, result) + response, statusErr := h.startPlannedPlaybackV3(r, userID, profileID, req, requestDigests, requestedFile, effectiveFile, audioIndex, result, clientInfo) if statusErr != nil { if statusErr.reason == "playback_attempt_reused" { writeError(w, http.StatusConflict, "playback_attempt_reused", statusErr.message) @@ -518,6 +524,70 @@ type playbackStartRequestDigestsV3 struct { legacy string } +// playbackClientInfoForStartV3 resolves the client's app identity for a v3 +// start request. The X-Silo-Client-* headers win because they are present on +// every request; the start body's client_playback_context is the fallback for +// clients that report their app identity only there. All values stay opaque — +// they are trimmed and length-clamped when the session stamps them. +// +// The fallback applies only to a client that named itself. client_playback_context +// carries no app name, so nothing in the body can identify a nameless client +// anyway — it is labeled from its user agent, and its app_version is a +// free-form platform string rather than the marketing version client_version +// promises. The web player, for one, reports the literal "web" there; taking it +// unconditionally would write "web" into the one field that is contractually +// semver, on every browser session. +func playbackClientInfoForStartV3(r *http.Request, clientContext playback.ClientPlaybackContextV3) playback.ClientInfo { + info := playbackClientInfoFromRequest(r) + if info.Name == "" { + return info + } + if info.Version == "" { + info.Version = strings.TrimSpace(clientContext.AppVersion) + } + if info.Build == "" { + info.Build = strings.TrimSpace(clientContext.AppBuild) + } + if info.Channel == "" { + info.Channel = strings.TrimSpace(clientContext.AppChannel) + } + // The header half is already normalized; body-sourced values have to be + // clamped too before they reach the decision log and the route event. + return info.Normalized() +} + +// playbackClientInfoWithSessionFallbackV3 completes a header-derived identity +// from the session the event belongs to. Route events posted out of band carry +// no client_playback_context, so without this a client that reports its build +// only in the start body would attribute its plan_selected event to a build and +// every later event of the same attempt to none. +func (h *PlaybackHandler) playbackClientInfoWithSessionFallbackV3(sessionID string, info playback.ClientInfo) playback.ClientInfo { + if sessionID == "" || h.sessionMgr == nil { + return info + } + if info.Name != "" && info.Version != "" && info.Build != "" && info.Channel != "" { + return info + } + session, err := h.sessionMgr.GetSession(sessionID) + if err != nil || session == nil { + return info + } + stamped := session.ClientInfo() + if info.Name == "" { + info.Name = stamped.Name + } + if info.Version == "" { + info.Version = stamped.Version + } + if info.Build == "" { + info.Build = stamped.Build + } + if info.Channel == "" { + info.Channel = stamped.Channel + } + return info +} + // newPlaybackStartRequestDigestsV3 fingerprints both the body and normalized // device identity because either can change the selected playback plan. It // also retains the pre-device digest while attempts written by an older @@ -538,7 +608,7 @@ func (d playbackStartRequestDigestsV3) matches(stored string) bool { return stored == "" || stored == d.current || stored == d.legacy } -func (h *PlaybackHandler) startPlannedPlaybackV3(r *http.Request, userID int, profileID string, req playback.StartRequestV3, requestDigests playbackStartRequestDigestsV3, requestedFile, effectiveFile *models.MediaFile, audioIndex int, result playback.PlannerResultV3) (playback.DecisionResponseV3, *transportErrorV3) { +func (h *PlaybackHandler) startPlannedPlaybackV3(r *http.Request, userID int, profileID string, req playback.StartRequestV3, requestDigests playbackStartRequestDigestsV3, requestedFile, effectiveFile *models.MediaFile, audioIndex int, result playback.PlannerResultV3, clientInfo playback.ClientInfo) (playback.DecisionResponseV3, *transportErrorV3) { if result.Plan == nil { return playback.DecisionResponseV3{}, &transportErrorV3{reason: "internal_error", message: "The server produced no playback plan."} } @@ -551,15 +621,8 @@ func (h *PlaybackHandler) startPlannedPlaybackV3(r *http.Request, userID int, pr return playback.DecisionResponseV3{}, &transportErrorV3{reason: reason, message: "The selected server adaptation is disabled for this user."} } } - clientInfo := playbackClientInfoFromRequest(r) ctx := playback.WithClientInfo(r.Context(), clientInfo) - var session *playback.Session - var err error - if starter, ok := h.sessionMgr.(sessionStarterWithFilesContext); ok { - session, err = starter.StartSessionWithFilesContext(ctx, userID, profileID, effectiveFile.ID, requestedFile.ID, result.PlayMethod, result.TranscodeAudio) - } else { - session, err = h.sessionMgr.StartSessionWithFiles(userID, profileID, effectiveFile.ID, requestedFile.ID, result.PlayMethod, result.TranscodeAudio) - } + session, err := h.sessionMgr.StartSessionWithFilesContext(ctx, userID, profileID, effectiveFile.ID, requestedFile.ID, result.PlayMethod, result.TranscodeAudio) if err != nil { return playback.DecisionResponseV3{}, sessionStartErrorV3(err) } @@ -658,7 +721,7 @@ func (h *PlaybackHandler) startPlannedPlaybackV3(r *http.Request, userID int, pr h.maybeQueueLazyPlaybackMarkers(r.Context(), session, effectiveFile) h.persistSeriesSelectionsV3(r.Context(), userID, profileID, effectiveFile, plannedAudioTrackIndexV3(result, audioIndex)) h.syncSessionsNow(r.Context(), "v3_start") - h.enqueueRouteEventV3(playback.RouteEventRecordV3{RouteEventV3: playback.RouteEventV3{ProtocolVersion: playback.ProtocolV3, PlaybackAttemptID: req.PlaybackAttemptID, SessionID: session.ID, PlanID: result.Plan.PlanID, Event: playback.RouteEventPlanSelectedV3, AppliedQuirkIDs: appliedQuirkIDsV3(result.Plan), QuirkRegistryRevision: appliedQuirkRevisionV3(result.Plan), OutputContextID: req.ClientPlaybackContext.Output.OutputContextID}, UserID: userID, ProfileID: profileID, ClientName: clientInfo.Name, ClientVersion: clientInfo.Version, ClientModel: req.ClientPlaybackContext.Device.Model}) + h.enqueueRouteEventV3(playback.RouteEventRecordV3{RouteEventV3: playback.RouteEventV3{ProtocolVersion: playback.ProtocolV3, PlaybackAttemptID: req.PlaybackAttemptID, SessionID: session.ID, PlanID: result.Plan.PlanID, Event: playback.RouteEventPlanSelectedV3, AppliedQuirkIDs: appliedQuirkIDsV3(result.Plan), QuirkRegistryRevision: appliedQuirkRevisionV3(result.Plan), OutputContextID: req.ClientPlaybackContext.Output.OutputContextID}, UserID: userID, ProfileID: profileID, ClientName: clientInfo.Name, ClientVersion: clientInfo.Version, ClientBuild: clientInfo.Build, ClientChannel: clientInfo.Channel, ClientModel: req.ClientPlaybackContext.Device.Model}) return response, nil } @@ -2096,7 +2159,7 @@ func (h *PlaybackHandler) executeReplanV3(r *http.Request, record *playback.Atte event = playback.RouteEventRuntimeCorrectionSucceededV3 clientModel = start.ClientPlaybackContext.Device.Model } - h.enqueueRouteEventV3(playback.RouteEventRecordV3{RouteEventV3: playback.RouteEventV3{ProtocolVersion: playback.ProtocolV3, PlaybackAttemptID: req.PlaybackAttemptID, SessionID: session.ID, PlanID: result.Plan.PlanID, PlanAttemptID: req.PlanAttemptID, PlanAttemptKey: playback.PlanAttemptKeyV3(*result.Plan, start.ClientPlaybackContext.Output.OutputContextID, nil), Event: event, FallbackReason: req.Failure.Classification, AppliedQuirkIDs: appliedQuirkIDsV3(result.Plan), QuirkRegistryRevision: appliedQuirkRevisionV3(result.Plan), OutputContextID: start.ClientPlaybackContext.Output.OutputContextID}, UserID: session.UserID, ProfileID: session.ProfileID, ClientName: session.ClientName, ClientVersion: session.ClientVersion, ClientModel: clientModel}) + h.enqueueRouteEventV3(playback.RouteEventRecordV3{RouteEventV3: playback.RouteEventV3{ProtocolVersion: playback.ProtocolV3, PlaybackAttemptID: req.PlaybackAttemptID, SessionID: session.ID, PlanID: result.Plan.PlanID, PlanAttemptID: req.PlanAttemptID, PlanAttemptKey: playback.PlanAttemptKeyV3(*result.Plan, start.ClientPlaybackContext.Output.OutputContextID, nil), Event: event, FallbackReason: req.Failure.Classification, AppliedQuirkIDs: appliedQuirkIDsV3(result.Plan), QuirkRegistryRevision: appliedQuirkRevisionV3(result.Plan), OutputContextID: start.ClientPlaybackContext.Output.OutputContextID}, UserID: session.UserID, ProfileID: session.ProfileID, ClientName: session.ClientName, ClientVersion: session.ClientVersion, ClientBuild: session.ClientBuild, ClientChannel: session.ClientChannel, ClientModel: clientModel}) } transport.rollback = func() { originalRollback() @@ -2707,8 +2770,8 @@ func (h *PlaybackHandler) HandlePlaybackRouteEventV3(w http.ResponseWriter, r *h return } event.Diagnostics = sanitizeDiagnosticsV3(event.Diagnostics) - client := playbackClientInfoFromRequest(r) - h.enqueueRouteEventV3(playback.RouteEventRecordV3{RouteEventV3: event, UserID: userID, ProfileID: profileID, ClientName: client.Name, ClientVersion: client.Version, ClientModel: event.Diagnostics["device_model"]}) + client := h.playbackClientInfoWithSessionFallbackV3(firstNonEmptyValue(event.SessionID, identity.SessionID), playbackClientInfoFromRequest(r)) + h.enqueueRouteEventV3(playback.RouteEventRecordV3{RouteEventV3: event, UserID: userID, ProfileID: profileID, ClientName: client.Name, ClientVersion: client.Version, ClientBuild: client.Build, ClientChannel: client.Channel, ClientModel: event.Diagnostics["device_model"]}) w.WriteHeader(http.StatusAccepted) } diff --git a/internal/playback/contract/contract_test.go b/internal/playback/contract/contract_test.go index 5b8414aa..87829c93 100644 --- a/internal/playback/contract/contract_test.go +++ b/internal/playback/contract/contract_test.go @@ -512,6 +512,34 @@ func TestRequestSchemaRequiredFieldsMatchValidators(t *testing.T) { assertStringsEqual(t, "route_event.required", schemaStrings(t, routeEvent, "required"), []string{"protocol_version", "playback_attempt_id", "event"}) } +// TestRequestSchemasShareIdenticalDefs pins the one thing two copies of a +// definition can never be trusted to keep on their own. Start and replan +// deserialize the *same* Go types — ClientCodecCapabilitiesV3 and +// ClientPlaybackContextV3 — through the same validator, so any $def they both +// declare must be byte-identical. Adding a field to one schema and not the +// other compiles, validates, and ships a contract that lies about one of the +// two endpoints; this is what catches it. +func TestRequestSchemasShareIdenticalDefs(t *testing.T) { + start := schemaValue(t, mustReadObject(t, filepath.Join(schemaRootV3, "v3", "start-request.schema.json")), "$defs").(map[string]any) + replan := schemaValue(t, mustReadObject(t, filepath.Join(schemaRootV3, "v3", "replan-request.schema.json")), "$defs").(map[string]any) + + shared := make([]string, 0, len(start)) + for name := range start { + if _, ok := replan[name]; ok { + shared = append(shared, name) + } + } + sort.Strings(shared) + if !slices.Contains(shared, "client_playback_context") || !slices.Contains(shared, "client_capabilities") { + t.Fatalf("shared $defs = %v, want the client contract definitions in both request schemas", shared) + } + for _, name := range shared { + if !reflect.DeepEqual(start[name], replan[name]) { + t.Errorf("$defs.%s differs between start-request and replan-request; both deserialize the same Go type", name) + } + } +} + func compileSchemasV3(t *testing.T) map[string]*jsonschema.Schema { t.Helper() diff --git a/internal/playback/planstore/postgres.go b/internal/playback/planstore/postgres.go index 1e0acac3..72f6896e 100644 --- a/internal/playback/planstore/postgres.go +++ b/internal/playback/planstore/postgres.go @@ -360,13 +360,16 @@ func (s *Postgres) RecordRouteEvent(ctx context.Context, record playback.RouteEv INSERT INTO playback_route_events ( playback_attempt_id, session_id, plan_id, plan_attempt_id, plan_attempt_key, event, failure_classification, fallback_reason, output_context_id, - diagnostics, user_id, profile_id, client_name, client_version, client_model + diagnostics, user_id, profile_id, client_name, client_version, client_build, + client_channel, client_model ) VALUES ($1, NULLIF($2, '')::uuid, NULLIF($3, ''), NULLIF($4, ''), NULLIF($5, ''), $6, NULLIF($7, ''), NULLIF($8, ''), NULLIF($9, ''), $10, $11, $12, - NULLIF($13, ''), NULLIF($14, ''), NULLIF($15, ''))`, + NULLIF($13, ''), NULLIF($14, ''), NULLIF($15, ''), NULLIF($16, ''), + NULLIF($17, ''))`, record.PlaybackAttemptID, record.SessionID, record.PlanID, record.PlanAttemptID, record.PlanAttemptKey, record.Event, record.FailureClassification, record.FallbackReason, record.OutputContextID, - diagnostics, record.UserID, record.ProfileID, record.ClientName, record.ClientVersion, record.ClientModel) + diagnostics, record.UserID, record.ProfileID, record.ClientName, record.ClientVersion, + record.ClientBuild, record.ClientChannel, record.ClientModel) return err } diff --git a/internal/playback/protocol_store_v3.go b/internal/playback/protocol_store_v3.go index 02d8e981..dea5267c 100644 --- a/internal/playback/protocol_store_v3.go +++ b/internal/playback/protocol_store_v3.go @@ -57,6 +57,10 @@ type RouteEventRecordV3 struct { ProfileID string ClientName string ClientVersion string + // ClientBuild and ClientChannel are opaque client-reported identifiers, + // recorded so a route decision can be attributed to an exact app build. + ClientBuild string + ClientChannel string ClientModel string } diff --git a/internal/playback/protocol_v3.go b/internal/playback/protocol_v3.go index 58f20df2..8263d085 100644 --- a/internal/playback/protocol_v3.go +++ b/internal/playback/protocol_v3.go @@ -331,12 +331,17 @@ type DeliveryCapabilityV3 struct { // advertisement lives exclusively in the request's top-level client_features // list; there is deliberately no second features location here. type ClientPlaybackContextV3 struct { - ProtocolVersion int `json:"protocol_version"` - FormFactor string `json:"form_factor"` - AppVersion string `json:"app_version"` - Device DeviceContextV3 `json:"device"` - Output OutputContextV3 `json:"output"` - Deliveries map[string]DeliveryCapabilityV3 `json:"deliveries"` + ProtocolVersion int `json:"protocol_version"` + FormFactor string `json:"form_factor"` + AppVersion string `json:"app_version"` + // AppBuild and AppChannel are the request-body fallback for the + // X-Silo-Client-Build / X-Silo-Client-Channel headers. Both are opaque + // strings the server stores verbatim. + AppBuild string `json:"app_build,omitempty"` + AppChannel string `json:"app_channel,omitempty"` + Device DeviceContextV3 `json:"device"` + Output OutputContextV3 `json:"output"` + Deliveries map[string]DeliveryCapabilityV3 `json:"deliveries"` } type StartRequestV3 struct { @@ -890,6 +895,13 @@ func validateCapabilitiesV3(c *ClientCodecCapabilitiesV3, ctx *ClientPlaybackCon if len(c.CodecsVideo) > 64 || len(c.CodecsVideoHardware) > 64 || len(c.CodecsAudio) > 64 || len(c.Containers) > 64 || len(c.VideoDecode) > 64 || len(ctx.Deliveries) > 16 || len(ctx.Device.Platform) > 32 || len(ctx.FormFactor) > 32 || len(ctx.AppVersion) > 64 { return errors.New("capability list exceeds supported size") } + // Build and channel are opaque diagnostic labels, so an over-long value is + // worth clamping and never worth refusing playback over. The header route + // (X-Silo-Client-Build / -Channel) clamps with the same helper; rejecting + // here would mean the same string plays from a header and 400s from the + // body. + ctx.AppBuild = normalizeClientMetadataValue(ctx.AppBuild, 64) + ctx.AppChannel = normalizeClientMetadataValue(ctx.AppChannel, 32) deviceValues := []string{ ctx.Device.OSVersion, ctx.Device.Manufacturer, ctx.Device.Model, ctx.Output.CurrentSink, ctx.Output.SinkType, ctx.Output.OutputContextID, diff --git a/internal/playback/session.go b/internal/playback/session.go index 0db781fa..de74f0c8 100644 --- a/internal/playback/session.go +++ b/internal/playback/session.go @@ -8,6 +8,8 @@ import ( "strings" "sync" "time" + "unicode" + "unicode/utf8" "github.com/google/uuid" ) @@ -26,6 +28,8 @@ type Session struct { ClientIP string // resolved client IP for the playback session ClientName string // reported playback client name, when available ClientVersion string // reported playback client version, when available + ClientBuild string // opaque reported client build identifier, when available + ClientChannel string // opaque reported client distribution channel, when available ClientUserAgent string // trimmed request user agent for the playback session IsJellyfinCompat bool // immutable origin identity for Jellyfin compatibility sessions @@ -147,12 +151,76 @@ type clientInfoContextKey struct{} // ClientInfo carries best-effort client metadata from request handling into // the playback session manager. type ClientInfo struct { - Name string - Version string + Name string + Version string + // Build is the client's opaque per-platform build identifier (Android + // versionCode, Apple CFBundleVersion, …). The server never parses or + // compares it; it exists so an admin can name the exact build. + Build string + // Channel is the client's opaque distribution channel ("release", "beta", + // "sideload", "dev", …). Stored verbatim — deliberately not validated + // against an enum so a new channel needs no server change. + Channel string UserAgent string IsCompat bool } +// Normalized returns the identity with every field trimmed and clamped to the +// bound published for it (docs/settings-api.md, and maxLength in the v3 request +// schemas). This is the single definition of those bounds, and callers apply it +// at the request boundary: a session stamps normalized values, but the decision +// logs and playback_route_events are written straight from the resolved +// identity, so clamping only at session creation would let a client's oversized +// header through to both. +func (c ClientInfo) Normalized() ClientInfo { + c.Name = normalizeClientMetadataValue(c.Name, 128) + c.Version = normalizeClientMetadataValue(c.Version, 64) + c.Build = normalizeClientMetadataValue(c.Build, 64) + c.Channel = normalizeClientMetadataValue(c.Channel, 32) + c.UserAgent = normalizeClientMetadataValue(c.UserAgent, 512) + return c +} + +// LogAttrs renders the app identity as slog key/value pairs, skipping the +// fields the client did not report. This is the single definition of those log +// keys — every playback decision and the session-expiry line share it, so a +// rename cannot leave one surface keyed differently from another. Skipping +// empty values matters as much: browsers and Jellyfin-ecosystem clients report +// none of them, and opslog persists the attrs it is handed, so emitting four +// empty keys per decision would grow /admin/logs for no diagnostic value. +func (c ClientInfo) LogAttrs() []any { + attrs := make([]any, 0, 8) + for _, pair := range [...]struct{ key, value string }{ + {"client_name", c.Name}, + {"client_version", c.Version}, + {"client_build", c.Build}, + {"client_channel", c.Channel}, + } { + if pair.value != "" { + attrs = append(attrs, pair.key, pair.value) + } + } + return attrs +} + +// ClientInfo returns the app identity stamped on the session when it was +// created. Surfaces that only hold a session — expiry logging, route events +// posted out of band — recover the reporting client through this instead of +// re-reading request headers that may no longer be present. +func (s *Session) ClientInfo() ClientInfo { + if s == nil { + return ClientInfo{} + } + return ClientInfo{ + Name: s.ClientName, + Version: s.ClientVersion, + Build: s.ClientBuild, + Channel: s.ClientChannel, + UserAgent: s.ClientUserAgent, + IsCompat: s.IsJellyfinCompat, + } +} + // WithClientInfo stores playback client metadata on a context. func WithClientInfo(ctx context.Context, info ClientInfo) context.Context { if ctx == nil { @@ -302,9 +370,41 @@ func (m *SessionManager) SetExpirationHook(fn func(*Session)) { } func normalizeClientMetadataValue(value string, maxLen int) string { + // A text column takes neither invalid UTF-8 nor a NUL, and Postgres refuses + // the whole statement for either. The per-node session upserts share one + // transaction, so a single malformed client string would stop that entire + // node from reconciling — not just its own row — until the session goes + // away. Both scrubs are needed: NUL is perfectly valid UTF-8, so + // ToValidUTF8 leaves it, and a v3 start body can carry one as the JSON + // escape (headers cannot — net/http rejects bytes below 0x20). + // Control characters are stripped wholesale rather than just NUL: none of + // them belong in an identity label rendered in the admin UI and written to + // structured logs. + if !utf8.ValidString(value) { + value = strings.ToValidUTF8(value, "") + } + if strings.ContainsFunc(value, unicode.IsControl) { + value = strings.Map(func(r rune) rune { + if unicode.IsControl(r) { + return -1 + } + return r + }, value) + } value = strings.TrimSpace(value) - if maxLen > 0 && len(value) > maxLen { - value = value[:maxLen] + if maxLen <= 0 || len(value) <= maxLen { + return value + } + // Clamp by runes, not bytes. These bounds are published to clients as JSON + // Schema maxLength, which counts characters — a byte clamp would silently + // cut a value the contract calls valid, and cutting mid-rune would produce + // exactly the invalid UTF-8 scrubbed above. + runes := 0 + for offset := range value { + if runes == maxLen { + return value[:offset] + } + runes++ } return value } @@ -439,7 +539,10 @@ func newSession( transcodeAudio bool, ) *Session { now := time.Now() - clientInfo := ClientInfoFromContext(ctx) + // Normalize here as well as at the request boundary: identities also reach + // the manager from the Jellyfin and Audiobookshelf compat surfaces, which + // build a ClientInfo from their own header vocabularies. + clientInfo := ClientInfoFromContext(ctx).Normalized() return &Session{ ID: uuid.New().String(), UserID: userID, @@ -451,9 +554,11 @@ func newSession( TranscodeAudio: transcodeAudio, Position: 0, IsPaused: false, - ClientName: normalizeClientMetadataValue(clientInfo.Name, 128), - ClientVersion: normalizeClientMetadataValue(clientInfo.Version, 64), - ClientUserAgent: normalizeClientMetadataValue(clientInfo.UserAgent, 512), + ClientName: clientInfo.Name, + ClientVersion: clientInfo.Version, + ClientBuild: clientInfo.Build, + ClientChannel: clientInfo.Channel, + ClientUserAgent: clientInfo.UserAgent, IsJellyfinCompat: clientInfo.IsCompat, StartedAt: now, UpdatedAt: now, diff --git a/internal/worker/reconciler.go b/internal/worker/reconciler.go index 54e08b8f..7854a2f7 100644 --- a/internal/worker/reconciler.go +++ b/internal/worker/reconciler.go @@ -29,6 +29,8 @@ type SessionSync struct { ClientIP string ClientName string ClientVersion string + ClientBuild string + ClientChannel string ClientUserAgent string AudioTrackIndex int TranscodeAudio bool @@ -151,11 +153,11 @@ func (r *Reconciler) ReconcileNodeSessions(ctx context.Context, reportingNode st INSERT INTO playback_sessions_sync (session_id, user_id, profile_id, media_file_id, requested_media_file_id, play_method, reporting_node, started_at, updated_at, last_sync_at, client_ip, - client_name, client_version, client_user_agent, + client_name, client_version, client_build, client_channel, client_user_agent, audio_track_index, transcode_audio, stream_bitrate_kbps, transcode_node_url, target_resolution, target_video_codec, target_audio_codec, target_bitrate_kbps, transcode_hw_accel, position_seconds, is_paused, has_websocket, compat_origin) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, NOW(), $10::inet, $11, $12, $13, $14, $15, $16, $17, $18, $19, $20, $21, $22, $23, $24, $25, $26) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, NOW(), $10::inet, $11, $12, $13, $14, $15, $16, $17, $18, $19, $20, $21, $22, $23, $24, $25, $26, $27, $28) ON CONFLICT (session_id) DO UPDATE SET user_id = EXCLUDED.user_id, profile_id = EXCLUDED.profile_id, @@ -168,6 +170,8 @@ func (r *Reconciler) ReconcileNodeSessions(ctx context.Context, reportingNode st client_ip = EXCLUDED.client_ip, client_name = EXCLUDED.client_name, client_version = EXCLUDED.client_version, + client_build = EXCLUDED.client_build, + client_channel = EXCLUDED.client_channel, client_user_agent = EXCLUDED.client_user_agent, audio_track_index = EXCLUDED.audio_track_index, transcode_audio = EXCLUDED.transcode_audio, @@ -185,7 +189,8 @@ func (r *Reconciler) ReconcileNodeSessions(ctx context.Context, reportingNode st last_sync_at = NOW() `, s.SessionID, s.UserID, s.ProfileID, s.MediaFileID, nullableInt(s.RequestedMediaFileID), s.PlayMethod, sessionNode, s.StartedAt, s.UpdatedAt, nullableIP(s.ClientIP), - nullableString(s.ClientName), nullableString(s.ClientVersion), nullableString(s.ClientUserAgent), + nullableString(s.ClientName), nullableString(s.ClientVersion), nullableString(s.ClientBuild), + nullableString(s.ClientChannel), nullableString(s.ClientUserAgent), s.AudioTrackIndex, s.TranscodeAudio, nullableInt(s.StreamBitrateKbps), nullableString(s.TranscodeNodeURL), nullableString(s.TargetResolution), nullableString(s.TargetVideoCodec), nullableString(s.TargetAudioCodec), nullableInt(s.TargetBitrateKbps), @@ -251,6 +256,8 @@ func loadNodeSessionsSnapshot(ctx context.Context, tx pgx.Tx, reportingNode stri COALESCE(HOST(client_ip), ''), COALESCE(client_name, ''), COALESCE(client_version, ''), + COALESCE(client_build, ''), + COALESCE(client_channel, ''), COALESCE(client_user_agent, ''), COALESCE(audio_track_index, 0), COALESCE(transcode_audio, FALSE), @@ -290,6 +297,8 @@ func loadNodeSessionsSnapshot(ctx context.Context, tx pgx.Tx, reportingNode stri &s.ClientIP, &s.ClientName, &s.ClientVersion, + &s.ClientBuild, + &s.ClientChannel, &s.ClientUserAgent, &s.AudioTrackIndex, &s.TranscodeAudio, @@ -347,6 +356,8 @@ func sessionSnapshotsEqual(left, right []SessionSync) bool { left[i].ClientIP != right[i].ClientIP || left[i].ClientName != right[i].ClientName || left[i].ClientVersion != right[i].ClientVersion || + left[i].ClientBuild != right[i].ClientBuild || + left[i].ClientChannel != right[i].ClientChannel || left[i].ClientUserAgent != right[i].ClientUserAgent || left[i].AudioTrackIndex != right[i].AudioTrackIndex || left[i].TranscodeAudio != right[i].TranscodeAudio || diff --git a/migrations/sql/20260813160942_add_playback_session_client_build_channel.sql b/migrations/sql/20260813160942_add_playback_session_client_build_channel.sql new file mode 100644 index 00000000..1fc760a7 --- /dev/null +++ b/migrations/sql/20260813160942_add_playback_session_client_build_channel.sql @@ -0,0 +1,21 @@ +-- +goose Up +-- Client build number and release channel are reported by the app alongside its +-- version, so admin activity and route diagnostics can name the exact build +-- ("Silo Android TV 1.0.0 (build 5)") instead of a version alone. Both are +-- opaque, per-platform strings the server never parses. +ALTER TABLE public.playback_sessions_sync + ADD COLUMN IF NOT EXISTS client_build text, + ADD COLUMN IF NOT EXISTS client_channel text; + +ALTER TABLE public.playback_route_events + ADD COLUMN IF NOT EXISTS client_build text, + ADD COLUMN IF NOT EXISTS client_channel text; + +-- +goose Down +ALTER TABLE public.playback_route_events + DROP COLUMN IF EXISTS client_channel, + DROP COLUMN IF EXISTS client_build; + +ALTER TABLE public.playback_sessions_sync + DROP COLUMN IF EXISTS client_channel, + DROP COLUMN IF EXISTS client_build; diff --git a/web/src/api/types.ts b/web/src/api/types.ts index 374a794a..2f2c46f2 100644 --- a/web/src/api/types.ts +++ b/web/src/api/types.ts @@ -2445,7 +2445,10 @@ export interface AdminSession { client_ip?: string; client_name?: string; client_version?: string; + client_build?: string; + client_channel?: string; client_label?: string; + client_label_full?: string; client_user_agent?: string; audio_track_index: number; transcode_audio: boolean; diff --git a/web/src/pages/AdminActivity.tsx b/web/src/pages/AdminActivity.tsx index 8d2ccbfb..15b3a59d 100644 --- a/web/src/pages/AdminActivity.tsx +++ b/web/src/pages/AdminActivity.tsx @@ -29,6 +29,7 @@ import { formatSourceContainerSummary, formatTranscodeModeSummary, getSessionClientLabel, + getSessionClientLabelFull, formatVideoDetail, formatVideoSummary, normalizeContainerDecision, @@ -149,7 +150,10 @@ export default function AdminActivity() { s.media_title?.toLowerCase().includes(q) || s.series_name?.toLowerCase().includes(q) || s.episode_name?.toLowerCase().includes(q) || - getSessionClientLabel(s).toLowerCase().includes(q) || + // Search the exact label, not the compact one: "which sessions are on + // build 5?" is the question this identity exists to answer, and the + // compact label deliberately omits the build. + getSessionClientLabelFull(s).toLowerCase().includes(q) || s.client_user_agent?.toLowerCase().includes(q) || s.client_ip?.toLowerCase().includes(q), ); @@ -528,6 +532,13 @@ function StreamRow({ const streamMeta = [sourceContainer, streamBitrate].filter(Boolean).join(" · "); const clientIP = session.client_ip?.trim() || ""; const clientLabel = getSessionClientLabel(session); + // The row stays compact; the exact version, build, and channel live in the + // tooltip and in the expanded panel's Client card. + const clientLabelFull = getSessionClientLabelFull(session); + const clientUserAgent = session.client_user_agent?.trim() || ""; + const clientTitle = clientUserAgent + ? `${clientLabelFull || clientLabel} — ${clientUserAgent}` + : clientLabelFull || clientLabel; const playbackPosition = formatPlaybackPosition(session); const transcodeMode = formatTranscodeModeSummary(session); const activityMethod = classifyActivityMethod(session); @@ -602,10 +613,7 @@ function StreamRow({
{clientLabel ? ( - + {clientLabel} ) : null} @@ -801,10 +809,7 @@ function StreamRow({ {(clientLabel || clientIP || streamMeta) && (
{clientLabel ? ( - + {clientLabel} ) : null} @@ -1051,6 +1056,7 @@ function PlaybackExpandedPanel({ audioDecision === "transcode" && videoDecision !== "transcode" ? transcodeMode : null } /> +
{showFFmpeg ? ( @@ -1132,23 +1138,58 @@ function PlaybackDetailCard({ mode?: string | null; }) { return ( -
-
- - {label} - + {formatDecisionLabel(decision)} + } + > + + + {mode ? : null} + + + ); +} + +/** + * The exact streaming client: app name, version, build, and channel, with the + * raw user agent underneath. This is the surface that answers "which build is + * this?" — the session rows only have room for the compact label. + */ +function PlaybackClientCard({ session }: { session: AdminSession }) { + const label = getSessionClientLabelFull(session) || "Unknown client"; + const userAgent = session.client_user_agent?.trim() || ""; + return ( + + + {userAgent ? : null} + + ); +} + +function PlaybackDetailCardShell({ + label, + badge, + children, +}: { + label: string; + badge?: ReactNode; + children: ReactNode; +}) { + return ( +
+
+ + {label} + + {badge}
-
- - - {mode ? : null} - -
+
{children}
); } diff --git a/web/src/pages/adminActivityPresentation.test.ts b/web/src/pages/adminActivityPresentation.test.ts index 6488b9ac..b51580d8 100644 --- a/web/src/pages/adminActivityPresentation.test.ts +++ b/web/src/pages/adminActivityPresentation.test.ts @@ -13,6 +13,8 @@ import { formatTranscodeModeSummary, formatVideoDetail, formatVideoSummary, + getSessionClientLabel, + getSessionClientLabelFull, normalizeContainerDecision, normalizeStreamDecision, } from "./adminActivityPresentation"; @@ -35,6 +37,11 @@ function makeSession(overrides: Partial = {}): AdminSession { position_seconds: overrides.position_seconds ?? 300, is_paused: overrides.is_paused ?? false, client_name: overrides.client_name, + client_version: overrides.client_version, + client_build: overrides.client_build, + client_channel: overrides.client_channel, + client_label: overrides.client_label, + client_label_full: overrides.client_label_full, client_user_agent: overrides.client_user_agent, effective_play_method: overrides.effective_play_method, is_jellyfin_client: overrides.is_jellyfin_client, @@ -293,4 +300,31 @@ describe("adminActivityPresentation", () => { expect(formatDeliveredContainerSummary(session)).toBe("HLS"); expect(formatVideoDetail(session)).toBe("Video stream copied"); }); + + it("prefers the server's exact client label in the full label", () => { + const session = makeSession({ + client_name: "Silo Android TV", + client_version: "1.0.0", + client_build: "5", + client_label: "Silo Android TV 1.0.0", + client_label_full: "Silo Android TV 1.0.0 (build 5)", + }); + + expect(getSessionClientLabelFull(session)).toBe("Silo Android TV 1.0.0 (build 5)"); + // The compact row label keeps its unchanged width. + expect(getSessionClientLabel(session)).toBe("Silo Android TV 1.0.0"); + }); + + it("falls back to the compact label, then to name and version", () => { + expect( + getSessionClientLabelFull( + makeSession({ client_label: "Chrome 120", client_name: "Chrome", client_version: "120.0" }), + ), + ).toBe("Chrome 120"); + expect( + getSessionClientLabelFull(makeSession({ client_name: "Silo iOS", client_version: "2.1.0" })), + ).toBe("Silo iOS 2.1.0"); + expect(getSessionClientLabelFull(makeSession({ client_name: "Silo iOS" }))).toBe("Silo iOS"); + expect(getSessionClientLabelFull(makeSession())).toBe(""); + }); }); diff --git a/web/src/pages/adminActivityPresentation.ts b/web/src/pages/adminActivityPresentation.ts index d9f8b8db..9492a5a7 100644 --- a/web/src/pages/adminActivityPresentation.ts +++ b/web/src/pages/adminActivityPresentation.ts @@ -245,6 +245,18 @@ export function getSessionClientLabel(session: AdminSession): string { return clientName || ""; } +/** + * The exact client identity — version, build, and non-release channel — for + * surfaces that can afford the width (expanded session details, tooltips). + * `getSessionClientLabel` stays the compact list label; this one must never + * replace it in a fixed-width row. + */ +export function getSessionClientLabelFull(session: AdminSession): string { + // The server omits client_label_full when it would repeat client_label, and + // older servers never send it at all — both degrade to the compact label. + return session.client_label_full?.trim() || getSessionClientLabel(session); +} + export function formatSourceContainerSummary(session: AdminSession): string { return formatContainer(session.source_container) || "Unknown source"; } diff --git a/web/src/player/protocol-v3.ts b/web/src/player/protocol-v3.ts index 0f20f942..84d43969 100644 --- a/web/src/player/protocol-v3.ts +++ b/web/src/player/protocol-v3.ts @@ -252,6 +252,14 @@ export interface ClientPlaybackContextV3 { protocol_version: number; form_factor: string; app_version: string; + /** + * Opaque per-platform build identifier and distribution channel — the + * body-level fallback for the `X-Silo-Client-Build` / `X-Silo-Client-Channel` + * headers. The server stores both verbatim and never parses or compares + * them. The web player has no build concept and omits them. + */ + app_build?: string; + app_channel?: string; device: DeviceContextV3; output: OutputContextV3; /**