aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorzerafachris <christopher.zerafa@blocklabs.io>2026-07-17 17:20:30 +0200
committerzerafachris <christopher.zerafa@blocklabs.io>2026-07-17 17:20:30 +0200
commit5cd3d7ebb7c8ce5b7cf68b0696543780215a6fff (patch)
tree2be721b5a44996332e80608fa9991a40cb872b45
parenta96dc8bd9b1e2fc2a897a7d73839abf5bc5b81d4 (diff)
fix: don't throw ArgumentNullException on partial UpdateItem payloads (#17366)
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>
-rw-r--r--Jellyfin.Api/Controllers/ItemUpdateController.cs37
-rw-r--r--tests/Jellyfin.Api.Tests/Controllers/ItemUpdateControllerTests.cs87
2 files changed, 114 insertions, 10 deletions
diff --git a/Jellyfin.Api/Controllers/ItemUpdateController.cs b/Jellyfin.Api/Controllers/ItemUpdateController.cs
index 70b09a4a29..b0ea1dd911 100644
--- a/Jellyfin.Api/Controllers/ItemUpdateController.cs
+++ b/Jellyfin.Api/Controllers/ItemUpdateController.cs
@@ -250,7 +250,11 @@ public class ItemUpdateController : BaseJellyfinApiController
item.IndexNumber = request.IndexNumber;
item.ParentIndexNumber = request.ParentIndexNumber;
item.Overview = request.Overview;
- item.Genres = request.Genres.Distinct(StringComparer.OrdinalIgnoreCase).ToArray();
+
+ if (request.Genres is not null)
+ {
+ item.Genres = request.Genres.Distinct(StringComparer.OrdinalIgnoreCase).ToArray();
+ }
if (item is Episode episode)
{
@@ -293,10 +297,20 @@ public class ItemUpdateController : BaseJellyfinApiController
item.CustomRating = request.CustomRating;
var currentTags = item.Tags;
- var newTags = request.Tags.Select(t => t.Trim()).Distinct(StringComparer.OrdinalIgnoreCase).ToArray();
- var removedTags = currentTags.Except(newTags).ToList();
- var addedTags = newTags.Except(currentTags).ToList();
- item.Tags = newTags;
+ List<string> removedTags;
+ List<string> addedTags;
+ if (request.Tags is not null)
+ {
+ var newTags = request.Tags.Select(t => t.Trim()).Distinct(StringComparer.OrdinalIgnoreCase).ToArray();
+ removedTags = currentTags.Except(newTags).ToList();
+ addedTags = newTags.Except(currentTags).ToList();
+ item.Tags = newTags;
+ }
+ else
+ {
+ removedTags = [];
+ addedTags = [];
+ }
if (item is Series rseries)
{
@@ -408,15 +422,18 @@ public class ItemUpdateController : BaseJellyfinApiController
item.RunTimeTicks = request.RunTimeTicks;
}
- foreach (var pair in request.ProviderIds.ToList())
+ if (request.ProviderIds is not null)
{
- if (string.IsNullOrEmpty(pair.Value))
+ foreach (var pair in request.ProviderIds.ToList())
{
- request.ProviderIds.Remove(pair.Key);
+ if (string.IsNullOrEmpty(pair.Value))
+ {
+ request.ProviderIds.Remove(pair.Key);
+ }
}
- }
- item.ProviderIds = request.ProviderIds;
+ item.ProviderIds = request.ProviderIds;
+ }
if (item is Video video)
{
diff --git a/tests/Jellyfin.Api.Tests/Controllers/ItemUpdateControllerTests.cs b/tests/Jellyfin.Api.Tests/Controllers/ItemUpdateControllerTests.cs
new file mode 100644
index 0000000000..fa167203dd
--- /dev/null
+++ b/tests/Jellyfin.Api.Tests/Controllers/ItemUpdateControllerTests.cs
@@ -0,0 +1,87 @@
+using System;
+using System.Reflection;
+using System.Threading.Tasks;
+using Jellyfin.Api.Controllers;
+using MediaBrowser.Controller.Configuration;
+using MediaBrowser.Controller.Entities;
+using MediaBrowser.Controller.Entities.Movies;
+using MediaBrowser.Controller.Library;
+using MediaBrowser.Controller.Providers;
+using MediaBrowser.Model.Dto;
+using MediaBrowser.Model.Globalization;
+using MediaBrowser.Model.IO;
+using Moq;
+using Xunit;
+
+namespace Jellyfin.Api.Tests.Controllers;
+
+public class ItemUpdateControllerTests
+{
+ private readonly ItemUpdateController _subject;
+
+ public ItemUpdateControllerTests()
+ {
+ _subject = new ItemUpdateController(
+ Mock.Of<IFileSystem>(),
+ Mock.Of<ILibraryManager>(),
+ Mock.Of<IProviderManager>(),
+ Mock.Of<ILocalizationManager>(),
+ Mock.Of<IServerConfigurationManager>());
+ }
+
+ [Fact]
+ public async Task UpdateItem_WhenOnlyTagsFieldSupplied_DoesNotThrowAndAppliesTags()
+ {
+ // Regression test for https://github.com/jellyfin/jellyfin/issues/17366
+ // A partial update payload that only sets "Tags" leaves every other
+ // BaseItemDto collection property null (they have no default
+ // initializer). Genres and ProviderIds used to be fed straight into
+ // Distinct()/ToList() without a null check, so this call used to throw
+ // ArgumentNullException before the fix below was applied.
+ var movie = new Movie();
+ var request = new BaseItemDto
+ {
+ Tags = new[] { "new-tag-1", "new-tag-2" }
+ };
+
+ await InvokeUpdateItem(request, movie);
+
+ Assert.Equal(new[] { "new-tag-1", "new-tag-2" }, movie.Tags);
+ Assert.Empty(movie.Genres);
+ Assert.Empty(movie.ProviderIds);
+ }
+
+ [Fact]
+ public async Task UpdateItem_WhenGenresAndProviderIdsOmitted_LeavesExistingValuesUnchanged()
+ {
+ var movie = new Movie
+ {
+ Genres = new[] { "Action" }
+ };
+ movie.ProviderIds["Imdb"] = "tt1234567";
+
+ var request = new BaseItemDto
+ {
+ Tags = Array.Empty<string>()
+ };
+
+ await InvokeUpdateItem(request, movie);
+
+ Assert.Equal(new[] { "Action" }, movie.Genres);
+ Assert.Equal("tt1234567", movie.ProviderIds["Imdb"]);
+ }
+
+ private Task InvokeUpdateItem(BaseItemDto request, BaseItem item)
+ {
+ var method = typeof(ItemUpdateController).GetMethod(
+ "UpdateItem",
+ BindingFlags.NonPublic | BindingFlags.Instance,
+ null,
+ new[] { typeof(BaseItemDto), typeof(BaseItem) },
+ null);
+
+ Assert.NotNull(method);
+
+ return (Task)method!.Invoke(_subject, new object[] { request, item })!;
+ }
+}