Avoid NRE when sorting by user-dependent keys without a user by paoloantinori · Pull Request #17395 · jellyfin/jellyfin

paoloantinori · 2026-07-21T07:49:09Z

Changes

Same fix as #17394 (release-10.11.z), adapted for master. Fixes a NullReferenceException that 500-ed any anonymous (API-key, no userId) /Items request sorted by a user-dependent key (PlayCount, IsFavoriteOrLiked, DatePlayed, IsPlayed, IsUnplayed).

LibraryManager.GetComparer: when user is null and the requested key requires a user (IUserBaseItemComparer), substitute the SortName comparer so the result stays deterministic instead of crashing. (GetComparer is identical to the release branch; the hunk applies line-for-line.)
UserDataManager.GetUserData: ArgumentNullException.ThrowIfNull(user). 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; same guard.
DateLastMediaAddedComparer: re-tagged from IUserBaseItemComparer to IBaseItemComparer (its GetDate is static and never reads User), and dropped the unused User/UserManager/UserDataManager properties. Without this, the new SortName fallback would wrongly engage for DateLastContentAdded on anonymous queries, returning SortName order instead of date order.

The crash surfaced as System.InvalidOperationException: Failed to compare two elements in the array (wrapping the NRE from the LINQ sort).

Code assistance

None — hand-written null-guard fix.

Tests

UserDataManagerTests.GetUserData_NullUser_ThrowsArgumentNullException — reproduces the crash (NRE → now ArgumentNullException). Added to master's existing UserDataManagerTests (reuses its in-memory Sqlite fixture).
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.

Issues

Fixes #17393

Discovered while working on jellyfin-alexa-plugin; the plugin-side workaround is paoloantinori/jellyfin-alexa-plugin@25e5446.

jellyfin-bot · 2026-07-21T17:50:00Z

Jellyfin API (v 12.0.0)

Result

No differences. Specifications are equivalent

Bond-009 · 2026-07-21T18:05:21Z

            {
+                if (user is null)
+                {
+                    return Comparers.FirstOrDefault(c => c.Type == ItemSortBy.SortName);


This should throw an ArgumentException instead IMO

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 jellyfin#17393

paoloantinori · 2026-07-22T05:44:15Z

Good point — changed to throw new ArgumentException per your feedback. The caller now gets a clear error instead of a silent substitution. Updated both this PR and #17394 (release-10.11.z) for consistency.

Bond-009 · 2026-07-22T17:44:49Z

+        // treated as a user-dependent comparer: with no user it should still sort by date, not fall
+        // back to SortName.


Outdated comment

Bond-009

Not a fan of all the lengthy comments that easily get out of date, but functionality look OK

Bond-009 · 2026-07-22T17:45:04Z

+        // comparer were still tagged IUserBaseItemComparer, the null-user SortName fallback would
+        // return [Zulu, Mike, Alpha] and this assertion would fail.


Bond-009 · 2026-07-22T17:45:44Z

+        // BaseItem.SortName/CreateSortName dereference this static; set it so SortName-fallback
+        // paths don't NRE in-process (mirrors AudioResolverTests in the sibling test project).


crobibero · 2026-07-25T01:34:00Z

@paoloantinori please reduce the comments

paoloantinori · 2026-07-25T10:50:53Z

Thanks @crobibero — removed the added comments in 8b70582. The production message and the test method names carry the intent on their own.

sonarqubecloud · 2026-07-25T10:53:44Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

crobibero

Please review our AI/LLM policies for future contributions.
https://jellyfin.org/docs/general/contributing/llm-policies/

paoloantinori · 2026-07-25T13:40:08Z

Please review our AI/LLM policies for future contributions.
https://jellyfin.org/docs/general/contributing/llm-policies/

I'll do, thanks!

paoloantinori force-pushed the fix/userdata-null-user-nre-master branch from a768eee to 6167b65 Compare July 21, 2026 08:56

Shadowghost approved these changes Jul 21, 2026

View reviewed changes

Bond-009 reviewed Jul 21, 2026

View reviewed changes

Bond-009 requested changes Jul 21, 2026

View reviewed changes

paoloantinori force-pushed the fix/userdata-null-user-nre-master branch from 6167b65 to 5d580ab Compare July 22, 2026 05:44

Bond-009 reviewed Jul 22, 2026

View reviewed changes

Bond-009 approved these changes Jul 22, 2026

View reviewed changes

theguymadmax added this to Jellyfin v12 Jul 23, 2026

github-project-automation Bot moved this to Todo in Jellyfin v12 Jul 23, 2026

theguymadmax moved this from Todo to Approved in Jellyfin v12 Jul 23, 2026

crobibero approved these changes Jul 25, 2026

View reviewed changes

Remove added comments (jellyfin#17395 review)

8b70582

crobibero approved these changes Jul 25, 2026

View reviewed changes

crobibero merged commit ebb66f6 into jellyfin:master Jul 25, 2026
5 checks passed

github-project-automation Bot moved this from Approved to Done in Jellyfin v12 Jul 25, 2026

Navigation Menu

Repository navigation

Avoid NRE when sorting by user-dependent keys without a user - #17395

Avoid NRE when sorting by user-dependent keys without a user#17395
crobibero merged 2 commits into
jellyfin:masterfrom
paoloantinori:fix/userdata-null-user-nre-master

paoloantinori commented Jul 21, 2026 •

edited

Loading

Uh oh!

jellyfin-bot commented Jul 21, 2026

Uh oh!

Bond-009 Jul 21, 2026

Uh oh!

paoloantinori commented Jul 22, 2026

Uh oh!

Bond-009 Jul 22, 2026

Uh oh!

Bond-009 left a comment

Uh oh!

Bond-009 Jul 22, 2026

Uh oh!

Bond-009 Jul 22, 2026

Uh oh!

crobibero commented Jul 25, 2026

Uh oh!

paoloantinori commented Jul 25, 2026

Uh oh!

sonarqubecloud Bot commented Jul 25, 2026

Uh oh!

crobibero left a comment

Uh oh!

Uh oh!

paoloantinori commented Jul 25, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

		// treated as a user-dependent comparer: with no user it should still sort by date, not fall
		// back to SortName.

		// comparer were still tagged IUserBaseItemComparer, the null-user SortName fallback would
		// return [Zulu, Mike, Alpha] and this assertion would fail.

		// BaseItem.SortName/CreateSortName dereference this static; set it so SortName-fallback
		// paths don't NRE in-process (mirrors AudioResolverTests in the sibling test project).

Conversation

paoloantinori commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Code assistance

Tests

Issues

Uh oh!

jellyfin-bot commented Jul 21, 2026

Jellyfin API (v 12.0.0)

Result

Uh oh!

Bond-009 Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

paoloantinori commented Jul 22, 2026

Uh oh!

Bond-009 Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

Bond-009 left a comment

Choose a reason for hiding this comment

Uh oh!

Bond-009 Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

Bond-009 Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

crobibero commented Jul 25, 2026

Uh oh!

paoloantinori commented Jul 25, 2026

Uh oh!

sonarqubecloud Bot commented Jul 25, 2026

Quality Gate passed

Uh oh!

crobibero left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

paoloantinori commented Jul 25, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

paoloantinori commented Jul 21, 2026 •

edited

Loading