aboutsummaryrefslogtreecommitdiff
path: root/tests
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 /tests
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>
Diffstat (limited to 'tests')
-rw-r--r--tests/Jellyfin.Api.Tests/Controllers/ItemUpdateControllerTests.cs87
1 files changed, 87 insertions, 0 deletions
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 })!;
+ }
+}