diff options
| author | zerafachris <christopher.zerafa@blocklabs.io> | 2026-07-17 17:20:30 +0200 |
|---|---|---|
| committer | zerafachris <christopher.zerafa@blocklabs.io> | 2026-07-17 17:20:30 +0200 |
| commit | 5cd3d7ebb7c8ce5b7cf68b0696543780215a6fff (patch) | |
| tree | 2be721b5a44996332e80608fa9991a40cb872b45 /tests | |
| parent | a96dc8bd9b1e2fc2a897a7d73839abf5bc5b81d4 (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.cs | 87 |
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 })!; + } +} |
