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 | |
| 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>
| -rw-r--r-- | Jellyfin.Api/Controllers/ItemUpdateController.cs | 37 | ||||
| -rw-r--r-- | tests/Jellyfin.Api.Tests/Controllers/ItemUpdateControllerTests.cs | 87 |
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 })!; + } +} |
