Backport pull request #17999 from jellyfin/release-12.z

Don't queue by-name items for playback and reject source requests for them

Original-merge: d3b2542f2193e0350e1dd88a2f3395fe8a9ca6f4

Merged-by: crobibero <cody@robibe.ro>

Backported-by: Cody Robibero <cody@robibe.ro>

4 files changed, 111 insertions, 2 deletions

diff --git a/Emby.Server.Implementations/Library/MediaSourceManager.cs b/Emby.Server.Implementations/Library/MediaSourceManager.cs
index 8e428452f3..e9bba05839 100644
--- a/Emby.Server.Implementations/Library/MediaSourceManager.cs
+++ b/Emby.Server.Implementations/Library/MediaSourceManager.cs
@@ -384,7 +384,13 @@ namespace Emby.Server.Implementations.Library
         {
             ArgumentNullException.ThrowIfNull(item);
 
-            var hasMediaSources = (IHasMediaSources)item;
+            // Clients can ask for the sources of an item that has none (a container queued by mistake).
+            if (item is not IHasMediaSources hasMediaSources)
+            {
+                throw new ArgumentException(
+                    string.Format(CultureInfo.InvariantCulture, "{0} {1} has no media sources and cannot be played.", item.GetType().Name, item.Id),
+                    nameof(item));
+            }
 
             var sources = hasMediaSources.GetMediaSources(enablePathSubstitution);
 
diff --git a/Emby.Server.Implementations/Session/SessionManager.cs b/Emby.Server.Implementations/Session/SessionManager.cs
index 94215bed79..08e2578867 100644
--- a/Emby.Server.Implementations/Session/SessionManager.cs
+++ b/Emby.Server.Implementations/Session/SessionManager.cs
@@ -1449,6 +1449,8 @@ namespace Emby.Server.Implementations.Session
 
             if (item is IItemByName byName)
             {
+                // A by-name item tags containers as well as leaves: a music genre tags its artists,
+                // and a by-name artist row is not a folder, so IsFolder does not exclude it here.
                 return byName.GetTaggedItems(new InternalItemsQuery(user)
                 {
                     IsFolder = false,
@@ -1463,7 +1465,7 @@ namespace Emby.Server.Implementations.Session
                     },
                     IsVirtualItem = false,
                     OrderBy = new[] { (ItemSortBy.SortName, SortOrder.Ascending) }
-                });
+                }).Where(i => i is not IItemByName);
             }
 
             if (item.IsFolder)
diff --git a/tests/Jellyfin.Server.Implementations.Tests/Library/MediaSourceManagerTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Library/MediaSourceManagerTests.cs
index 685b769f34..131cb23fa4 100644
--- a/tests/Jellyfin.Server.Implementations.Tests/Library/MediaSourceManagerTests.cs
+++ b/tests/Jellyfin.Server.Implementations.Tests/Library/MediaSourceManagerTests.cs
@@ -264,6 +264,14 @@ namespace Jellyfin.Server.Implementations.Tests.Library
         }
 
         [Fact]
+        public void GetStaticMediaSources_ItemWithoutMediaSources_ThrowsArgumentException()
+        {
+            // A container queued by mistake is a bad request, not a server fault.
+            Assert.Throws<ArgumentException>(
+                () => _mediaSourceManager.GetStaticMediaSources(new MusicArtist { Id = Guid.NewGuid() }, false, _user));
+        }
+
+        [Fact]
         public void GetStaticMediaSources_NoUser_DoesNotTouchUserData()
         {
             var (primary, _, _) = SetupVersionGroup();
diff --git a/tests/Jellyfin.Server.Implementations.Tests/SessionManager/PlayCommandQueueTests.cs b/tests/Jellyfin.Server.Implementations.Tests/SessionManager/PlayCommandQueueTests.cs
new file mode 100644
index 0000000000..a07e79baa3
--- /dev/null
+++ b/tests/Jellyfin.Server.Implementations.Tests/SessionManager/PlayCommandQueueTests.cs
@@ -0,0 +1,93 @@
+using System;
+using System.Collections.Generic;
+using System.Threading;
+using System.Threading.Tasks;
+using MediaBrowser.Controller;
+using MediaBrowser.Controller.Configuration;
+using MediaBrowser.Controller.Devices;
+using MediaBrowser.Controller.Drawing;
+using MediaBrowser.Controller.Dto;
+using MediaBrowser.Controller.Entities;
+using MediaBrowser.Controller.Entities.Audio;
+using MediaBrowser.Controller.Events;
+using MediaBrowser.Controller.Library;
+using MediaBrowser.Controller.Session;
+using MediaBrowser.Model.Session;
+using Microsoft.Extensions.Hosting;
+using Microsoft.Extensions.Logging.Abstractions;
+using Moq;
+using Xunit;
+
+namespace Jellyfin.Server.Implementations.Tests.SessionManager;
+
+public class PlayCommandQueueTests : IDisposable
+{
+    private readonly ILibraryManager? _previousLibraryManager;
+
+    public PlayCommandQueueTests()
+    {
+        _previousLibraryManager = BaseItem.LibraryManager;
+    }
+
+    /// <summary>
+    /// A music genre tags its artists as well as their songs, and a by-name artist row is not a
+    /// folder, so the queue query cannot exclude it. Such an item has no media sources, and a
+    /// client that reaches it in the queue gets an error instead of the next track.
+    /// </summary>
+    /// <returns><placeholder>A <see cref="Task"/> representing the asynchronous unit test.</placeholder></returns>
+    [Fact]
+    public async Task SendPlayCommand_GenreTaggingAnArtist_QueuesOnlyPlayableItems()
+    {
+        var genre = new MusicGenre { Id = Guid.NewGuid(), Name = "Reggaeton" };
+        var song = new Audio { Id = Guid.NewGuid(), Name = "Me Porto Bonito" };
+        var artist = new MusicArtist { Id = Guid.NewGuid(), Name = "NATTI NATASHA" };
+
+        var libraryManager = new Mock<ILibraryManager>();
+        libraryManager.Setup(i => i.GetItemById(genre.Id)).Returns(genre);
+        libraryManager
+            .Setup(i => i.GetItemList(It.IsAny<InternalItemsQuery>()))
+            .Returns(new List<BaseItem> { artist, song });
+        BaseItem.LibraryManager = libraryManager.Object;
+
+        await using var sessionManager = new Emby.Server.Implementations.Session.SessionManager(
+            NullLogger<Emby.Server.Implementations.Session.SessionManager>.Instance,
+            Mock.Of<IEventManager>(),
+            Mock.Of<IUserDataManager>(),
+            Mock.Of<IServerConfigurationManager>(),
+            libraryManager.Object,
+            Mock.Of<IUserManager>(),
+            Mock.Of<IMusicManager>(),
+            Mock.Of<IDtoService>(),
+            Mock.Of<IImageProcessor>(),
+            Mock.Of<IServerApplicationHost>(),
+            Mock.Of<IDeviceManager>(),
+            Mock.Of<IMediaSourceManager>(),
+            Mock.Of<IHostApplicationLifetime>());
+
+        var session = await sessionManager.LogSessionActivity("app_name", "0.0.0", "device_id", "device_name", "127.0.0.1", null);
+
+        var command = new PlayRequest
+        {
+            ItemIds = new[] { genre.Id },
+            PlayCommand = PlayCommand.PlayNow
+        };
+
+        await sessionManager.SendPlayCommand(null, session.Id, command, CancellationToken.None);
+
+        Assert.Equal(new[] { song.Id }, command.ItemIds);
+    }
+
+    public void Dispose()
+    {
+        Dispose(true);
+        GC.SuppressFinalize(this);
+    }
+
+    protected virtual void Dispose(bool disposing)
+    {
+        if (disposing)
+        {
+            BaseItem.LibraryManager = _previousLibraryManager!;
+        }
+    }
+}