A query sorted by a user-dependent key (PlayCount, IsFavoriteOrLiked,
DatePlayed, IsPlayed, IsUnplayed) but carrying no User caused a
NullReferenceException inside UserDataManager.GetUserData, surfacing as
"Failed to compare two elements in the array" (InvalidOperationException
wrapping the NRE from the LINQ sort) and 500-ing the /Items request.
Root cause: LibraryManager.GetComparer assigned comparer.User = user
without a null guard, so PlayCountComparer.GetValue called
UserDataManager.GetUserData(null, item), dereferencing user.Id.
Two-part fix:
- LibraryManager.GetComparer: when user is null and the sort key requires a
user (IUserBaseItemComparer), substitute the SortName comparer so the
result stays deterministic instead of 500-ing. SortName is the project's
canonical tiebreaker (ItemsController injects it for album-by-artist).
- UserDataManager.GetUserData: ArgumentNullException.ThrowIfNull(user) as
defense in depth (matches the existing guards on the SaveUserData
overloads in the same file). On master this overload was rewritten to use
ResolveUserDataRow, so the NRE dereferences user.Id rather than
user.InternalId as on the release branch — same bug, different line.
Also fixes DateLastMediaAddedComparer being statically mis-tagged as
IUserBaseItemComparer: its GetDate is static and never reads User, so it
does not need one. Without this, the SortName fallback above would wrongly
engage for DateLastContentAdded on anonymous queries (returning SortName
order instead of date order). Re-tagged to IBaseItemComparer and dropped the
unused User/UserManager/UserDataManager properties.
Tests:
- UserDataManagerTests.GetUserData_NullUser_ThrowsArgumentNullException:
reproduces the crash (NRE -> now ArgumentNullException). Added to master's
existing UserDataManagerTests.
- LibraryManagerSortTests.Sort_UserDependentKey_NullUser_FallsBackToSortNameWithoutThrowing:
Sort with a user-dependent key + null user no longer throws and returns
items ordered by the SortName fallback (direction preserved).
- LibraryManagerSortTests.Sort_DateLastContentAdded_NullUser_OrdersByDateNotSortName:
guards that DateLastContentAdded still sorts by date with no user (fixture
chosen so date-desc and SortName-desc disagree, so a revert is caught).
Full Jellyfin.Server.Implementations.Tests suite: 642 passed, 0 failed.
Fixes#17393
BackupServiceTests rooted its temp directory under Path.GetTempPath(), which
on GitHub-hosted windows-latest runners resolves to the constrained system C:
drive. BackupService.CreateBackupAsync requires 5GiB free at the backup path
before starting, and the C: drive's free temp space can dip below that,
failing CreateBackupAsync_WithCorruptKeyframeDataRow_SkipsRowAndCompletesBackup
even though the fix itself is correct. Rooting the test directory under
AppContext.BaseDirectory keeps it on the same (much larger) drive as the repo
checkout on all platforms, without touching the real BackupService free-space
check.
Addresses review feedback from @Bond-009 on PR #17370: the test helper
InvokeUpdateItem was invoking the private UpdateItem(BaseItemDto, BaseItem)
method via reflection. Jellyfin.Api.csproj already grants
InternalsVisibleTo("Jellyfin.Api.Tests"), so the method is changed to
internal and the test now calls it directly, removing the
GetMethod/Invoke boilerplate.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
BaseItemDto.Genres, .Tags, and .ProviderIds are plain auto-properties with
no default initializer, so they deserialize to null when a client omits
them from a partial POST /Items/{itemId} body. The OpenAPI spec documents
every BaseItemDto field as optional, but ItemUpdateController.UpdateItem
fed these three properties straight into Distinct()/Select()/ToList()
without a null check, so a request that (for example) only sets Tags
throws ArgumentNullException("source") once it reaches the unguarded
Genres line, before Tags is even processed.
Guard all three assignments with the same "if (request.X is not null)"
pattern already used for the neighboring Studios/Taglines/ProductionLocations
fields in this method, so omitted fields are left unchanged instead of
crashing the request.
Adds ItemUpdateControllerTests covering the reported repro (only Tags
supplied) and a companion case asserting existing Genres/ProviderIds are
preserved when omitted from the payload.
Signed-off-by: zerafachris <christopher.zerafa@blocklabs.io>
A single row with malformed KeyframeTicks JSON (e.g. a truncated array
from an interrupted write) currently aborts the entire backup, because
the try/catch in BackupService.CreateBackupAsync only wraps
serialization of an already-materialized entity, not the enumeration
itself. EF Core throws JsonReaderException from MoveNextAsync() while
materializing the corrupt row, which propagates past that catch block.
Switch to manual enumerator iteration so MoveNextAsync() failures can
be caught per-row, logged as a warning identifying the affected table,
and skipped, allowing the remaining rows and the rest of the backup to
complete.
Fixes#17216
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
SubtitleEncoder.ConvertSubtitles parsed subtitles with libse's static
Subtitle.Parse, which iterates a statically cached list of shared
SubtitleFormat instances. Format parsers keep mutable per-parse state on
the instance, so concurrent subtitle requests corrupted each other's
output (cues mixed across streams and languages, truncated files) or
failed with NullReferenceException when format detection broke down and
Subtitle.Parse returned null.
Parse through the injected ISubtitleParser instead. SubtitleEditParser
instantiates a fresh format parser per call, so requests no longer share
state. Its Parse method now returns the libse Subtitle directly (the
SubtitleTrackInfo flattening was unused since the SubtitleEdit writer
rework) so the writers keep full fidelity such as ASS styling.
ClearProfileImageAsync removed the ProfileImage instance attached to the
passed-in User, but that instance can carry a stale, never-persisted
(temporary) key because UpdateUserAsync creates the persisted image on a
separately loaded entity and never copies the generated key back.
Removing that detached entity on a fresh DbContext made EF Core throw
InvalidOperationException ('ImageInfo.Id has a temporary value'), leaving
the profile image impossible to delete or replace.
Load the tracked, persisted user and remove its actual ProfileImage,
matching the removal pattern already used in UpdateUserAsync. Adds
regression tests covering the temporary-key case and the no-image no-op
(the first fails before this change and passes after).
Fixes#13137
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds tvdb alias for tvdbid and imdb alias for imdbid.
It also fixes an issue where tmdb alias was being ignored
if it was followed by something like "tmdbidfoo".
The same issue prevented imdb pattern matching from
working, if it was followed by something like "imdbidfoo".
It also allows for detecting the first matching occurence,
whether it was an alias or not.
Finally, it ignores attributes with values consisting of only whitespaces.
Three index bugs in PlayQueueManager, two of which leave
PlayingItemIndex out of bounds, making every subsequent Buffering/Ready
request throw and leaving the group unusable until it empties:
- RemoveFromPlaylist did not compensate for removed items preceding the
playing item: removing the playing item together with earlier items
could select the wrong item or crash with an out-of-bounds index.
- Next/Previous on an empty playlist with RepeatOne/RepeatAll reported
success or set PlayingItemIndex to 0 on an empty list, crashing
downstream in Group and corrupting the index.
- SetPlayingItemByIndex accepted an index equal to the playlist count
(latent off-by-one, callers currently pre-validate).
Cody Robibero