From 554d1b2ca8868e7aeb1fa9acac75cb8ce423796a Mon Sep 17 00:00:00 2001
From: Stanislav Ionascu <stanislav.ionascu@gmail.com>
Date: Tue, 4 Jan 2022 21:52:52 +0000
Subject: [PATCH] Fix #7100 by catching the exception on opening invalid UDF
 images

When an invalid UDF image is opened by the UdfReader, it may throw
and exception. This change is to catch and log the exception.
---
 .../Library/LibraryManager.cs                 |  9 +++---
 .../Library/Resolvers/BaseVideoResolver.cs    | 31 +++++++++++++------
 .../Library/Resolvers/ExtraResolver.cs        |  8 +++--
 .../Library/Resolvers/GenericVideoResolver.cs |  8 +++--
 .../Library/Resolvers/Movies/MovieResolver.cs |  6 ++--
 .../Library/Resolvers/TV/EpisodeResolver.cs   |  6 ++--
 .../Library/EpisodeResolverTest.cs            |  9 +++---
 .../Library/LibraryManager/FindExtrasTests.cs |  2 +-
 .../Library/MovieResolverTests.cs             |  3 +-
 9 files changed, 52 insertions(+), 30 deletions(-)

diff --git a/Emby.Server.Implementations/Library/LibraryManager.cs b/Emby.Server.Implementations/Library/LibraryManager.cs
index e3be5627fb..0770bdbc3d 100644
--- a/Emby.Server.Implementations/Library/LibraryManager.cs
+++ b/Emby.Server.Implementations/Library/LibraryManager.cs
@@ -46,6 +46,7 @@ using MediaBrowser.Model.Library;
 using MediaBrowser.Model.Querying;
 using MediaBrowser.Model.Tasks;
 using Microsoft.Extensions.Caching.Memory;
+using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
 using Episode = MediaBrowser.Controller.Entities.TV.Episode;
 using EpisodeInfo = Emby.Naming.TV.EpisodeInfo;
@@ -99,7 +100,7 @@ namespace Emby.Server.Implementations.Library
         /// Initializes a new instance of the <see cref="LibraryManager" /> class.
         /// </summary>
         /// <param name="appHost">The application host.</param>
-        /// <param name="logger">The logger.</param>
+        /// <param name="loggerFactory">The logger factory.</param>
         /// <param name="taskManager">The task manager.</param>
         /// <param name="userManager">The user manager.</param>
         /// <param name="configurationManager">The configuration manager.</param>
@@ -115,7 +116,7 @@ namespace Emby.Server.Implementations.Library
         /// <param name="namingOptions">The naming options.</param>
         public LibraryManager(
             IServerApplicationHost appHost,
-            ILogger<LibraryManager> logger,
+            ILoggerFactory loggerFactory,
             ITaskManager taskManager,
             IUserManager userManager,
             IServerConfigurationManager configurationManager,
@@ -131,7 +132,7 @@ namespace Emby.Server.Implementations.Library
             NamingOptions namingOptions)
         {
             _appHost = appHost;
-            _logger = logger;
+            _logger = loggerFactory.CreateLogger<LibraryManager>();
             _taskManager = taskManager;
             _userManager = userManager;
             _configurationManager = configurationManager;
@@ -146,7 +147,7 @@ namespace Emby.Server.Implementations.Library
             _memoryCache = memoryCache;
             _namingOptions = namingOptions;
 
-            _extraResolver = new ExtraResolver(namingOptions);
+            _extraResolver = new ExtraResolver(loggerFactory.CreateLogger<ExtraResolver>(), namingOptions);
 
             _configurationManager.ConfigurationUpdated += ConfigurationUpdated;
 
diff --git a/Emby.Server.Implementations/Library/Resolvers/BaseVideoResolver.cs b/Emby.Server.Implementations/Library/Resolvers/BaseVideoResolver.cs
index 9222a94797..3d6b9f3b62 100644
--- a/Emby.Server.Implementations/Library/Resolvers/BaseVideoResolver.cs
+++ b/Emby.Server.Implementations/Library/Resolvers/BaseVideoResolver.cs
@@ -12,6 +12,7 @@ using MediaBrowser.Controller.Entities;
 using MediaBrowser.Controller.Library;
 using MediaBrowser.Controller.Providers;
 using MediaBrowser.Model.Entities;
+using Microsoft.Extensions.Logging;
 
 namespace Emby.Server.Implementations.Library.Resolvers
 {
@@ -22,8 +23,11 @@ namespace Emby.Server.Implementations.Library.Resolvers
     public abstract class BaseVideoResolver<T> : MediaBrowser.Controller.Resolvers.ItemResolver<T>
         where T : Video, new()
     {
-        protected BaseVideoResolver(NamingOptions namingOptions)
+        private readonly ILogger _logger;
+
+        protected BaseVideoResolver(ILogger logger, NamingOptions namingOptions)
         {
+            _logger = logger;
             NamingOptions = namingOptions;
         }
 
@@ -156,19 +160,26 @@ namespace Emby.Server.Implementations.Library.Resolvers
                 }
                 else
                 {
-                    // use disc-utils, both DVDs and BDs use UDF filesystem
-                    using (var videoFileStream = File.Open(video.Path, FileMode.Open, FileAccess.Read))
-                    using (UdfReader udfReader = new UdfReader(videoFileStream))
+                    try
                     {
-                        if (udfReader.DirectoryExists("VIDEO_TS"))
-                        {
-                            video.IsoType = IsoType.Dvd;
-                        }
-                        else if (udfReader.DirectoryExists("BDMV"))
+                        // use disc-utils, both DVDs and BDs use UDF filesystem
+                        using (var videoFileStream = File.Open(video.Path, FileMode.Open, FileAccess.Read))
+                        using (UdfReader udfReader = new UdfReader(videoFileStream))
                         {
-                            video.IsoType = IsoType.BluRay;
+                            if (udfReader.DirectoryExists("VIDEO_TS"))
+                            {
+                                video.IsoType = IsoType.Dvd;
+                            }
+                            else if (udfReader.DirectoryExists("BDMV"))
+                            {
+                                video.IsoType = IsoType.BluRay;
+                            }
                         }
                     }
+                    catch (Exception ex)
+                    {
+                        _logger.LogError(ex, "Error opening UDF/ISO image: {Value}", video.Path ?? video.Name);
+                    }
                 }
             }
         }
diff --git a/Emby.Server.Implementations/Library/Resolvers/ExtraResolver.cs b/Emby.Server.Implementations/Library/Resolvers/ExtraResolver.cs
index 807913b5d6..408e640f9d 100644
--- a/Emby.Server.Implementations/Library/Resolvers/ExtraResolver.cs
+++ b/Emby.Server.Implementations/Library/Resolvers/ExtraResolver.cs
@@ -6,6 +6,7 @@ using Emby.Naming.Video;
 using MediaBrowser.Controller.Entities;
 using MediaBrowser.Controller.Resolvers;
 using MediaBrowser.Model.Entities;
+using Microsoft.Extensions.Logging;
 using static Emby.Naming.Video.ExtraRuleResolver;
 
 namespace Emby.Server.Implementations.Library.Resolvers
@@ -22,12 +23,13 @@ namespace Emby.Server.Implementations.Library.Resolvers
         /// <summary>
         /// Initializes a new instance of the <see cref="ExtraResolver"/> class.
         /// </summary>
+        /// <param name="logger">The logger.</param>
         /// <param name="namingOptions">An instance of <see cref="NamingOptions"/>.</param>
-        public ExtraResolver(NamingOptions namingOptions)
+        public ExtraResolver(ILogger<ExtraResolver> logger, NamingOptions namingOptions)
         {
             _namingOptions = namingOptions;
-            _trailerResolvers = new IItemResolver[] { new GenericVideoResolver<Trailer>(namingOptions) };
-            _videoResolvers = new IItemResolver[] { new GenericVideoResolver<Video>(namingOptions) };
+            _trailerResolvers = new IItemResolver[] { new GenericVideoResolver<Trailer>(logger, namingOptions) };
+            _videoResolvers = new IItemResolver[] { new GenericVideoResolver<Video>(logger, namingOptions) };
         }
 
         /// <summary>
diff --git a/Emby.Server.Implementations/Library/Resolvers/GenericVideoResolver.cs b/Emby.Server.Implementations/Library/Resolvers/GenericVideoResolver.cs
index b8554bd51c..5e33b402d5 100644
--- a/Emby.Server.Implementations/Library/Resolvers/GenericVideoResolver.cs
+++ b/Emby.Server.Implementations/Library/Resolvers/GenericVideoResolver.cs
@@ -1,7 +1,8 @@
-#nullable disable
+#nullable disable
 
 using Emby.Naming.Common;
 using MediaBrowser.Controller.Entities;
+using Microsoft.Extensions.Logging;
 
 namespace Emby.Server.Implementations.Library.Resolvers
 {
@@ -15,9 +16,10 @@ namespace Emby.Server.Implementations.Library.Resolvers
         /// <summary>
         /// Initializes a new instance of the <see cref="GenericVideoResolver{T}"/> class.
         /// </summary>
+        /// <param name="logger">The logger.</param>
         /// <param name="namingOptions">The naming options.</param>
-        public GenericVideoResolver(NamingOptions namingOptions)
-            : base(namingOptions)
+        public GenericVideoResolver(ILogger logger, NamingOptions namingOptions)
+            : base(logger, namingOptions)
         {
         }
     }
diff --git a/Emby.Server.Implementations/Library/Resolvers/Movies/MovieResolver.cs b/Emby.Server.Implementations/Library/Resolvers/Movies/MovieResolver.cs
index be14609289..140c4272e1 100644
--- a/Emby.Server.Implementations/Library/Resolvers/Movies/MovieResolver.cs
+++ b/Emby.Server.Implementations/Library/Resolvers/Movies/MovieResolver.cs
@@ -17,6 +17,7 @@ using MediaBrowser.Controller.Providers;
 using MediaBrowser.Controller.Resolvers;
 using MediaBrowser.Model.Entities;
 using MediaBrowser.Model.IO;
+using Microsoft.Extensions.Logging;
 
 namespace Emby.Server.Implementations.Library.Resolvers.Movies
 {
@@ -40,9 +41,10 @@ namespace Emby.Server.Implementations.Library.Resolvers.Movies
         /// Initializes a new instance of the <see cref="MovieResolver"/> class.
         /// </summary>
         /// <param name="imageProcessor">The image processor.</param>
+        /// <param name="logger">The logger.</param>
         /// <param name="namingOptions">The naming options.</param>
-        public MovieResolver(IImageProcessor imageProcessor, NamingOptions namingOptions)
-            : base(namingOptions)
+        public MovieResolver(IImageProcessor imageProcessor, ILogger<MovieResolver> logger, NamingOptions namingOptions)
+            : base(logger, namingOptions)
         {
             _imageProcessor = imageProcessor;
         }
diff --git a/Emby.Server.Implementations/Library/Resolvers/TV/EpisodeResolver.cs b/Emby.Server.Implementations/Library/Resolvers/TV/EpisodeResolver.cs
index be99056470..bfa73af2f4 100644
--- a/Emby.Server.Implementations/Library/Resolvers/TV/EpisodeResolver.cs
+++ b/Emby.Server.Implementations/Library/Resolvers/TV/EpisodeResolver.cs
@@ -6,6 +6,7 @@ using Emby.Naming.Common;
 using MediaBrowser.Controller.Entities.TV;
 using MediaBrowser.Controller.Library;
 using MediaBrowser.Model.Entities;
+using Microsoft.Extensions.Logging;
 
 namespace Emby.Server.Implementations.Library.Resolvers.TV
 {
@@ -17,9 +18,10 @@ namespace Emby.Server.Implementations.Library.Resolvers.TV
         /// <summary>
         /// Initializes a new instance of the <see cref="EpisodeResolver"/> class.
         /// </summary>
+        /// <param name="logger">The logger.</param>
         /// <param name="namingOptions">The naming options.</param>
-        public EpisodeResolver(NamingOptions namingOptions)
-            : base(namingOptions)
+        public EpisodeResolver(ILogger<EpisodeResolver> logger, NamingOptions namingOptions)
+            : base(logger, namingOptions)
         {
         }
 
diff --git a/tests/Jellyfin.Server.Implementations.Tests/Library/EpisodeResolverTest.cs b/tests/Jellyfin.Server.Implementations.Tests/Library/EpisodeResolverTest.cs
index 5c7c983c22..c21871297d 100644
--- a/tests/Jellyfin.Server.Implementations.Tests/Library/EpisodeResolverTest.cs
+++ b/tests/Jellyfin.Server.Implementations.Tests/Library/EpisodeResolverTest.cs
@@ -1,4 +1,4 @@
-using Emby.Naming.Common;
+using Emby.Naming.Common;
 using Emby.Server.Implementations.Library.Resolvers.TV;
 using MediaBrowser.Controller;
 using MediaBrowser.Controller.Entities;
@@ -7,6 +7,7 @@ using MediaBrowser.Controller.Library;
 using MediaBrowser.Controller.Providers;
 using MediaBrowser.Model.Entities;
 using MediaBrowser.Model.IO;
+using Microsoft.Extensions.Logging;
 using Moq;
 using Xunit;
 
@@ -21,7 +22,7 @@ namespace Jellyfin.Server.Implementations.Tests.Library
         {
             var parent = new Folder { Name = "extras" };
 
-            var episodeResolver = new EpisodeResolver(_namingOptions);
+            var episodeResolver = new EpisodeResolver(Mock.Of<ILogger<EpisodeResolver>>(), _namingOptions);
             var itemResolveArgs = new ItemResolveArgs(
                 Mock.Of<IServerApplicationPaths>(),
                 Mock.Of<IDirectoryService>())
@@ -44,7 +45,7 @@ namespace Jellyfin.Server.Implementations.Tests.Library
 
             // Have to create a mock because of moq proxies not being castable to a concrete implementation
             // https://github.com/jellyfin/jellyfin/blob/ab0cff8556403e123642dc9717ba778329554634/Emby.Server.Implementations/Library/Resolvers/BaseVideoResolver.cs#L48
-            var episodeResolver = new EpisodeResolverMock(_namingOptions);
+            var episodeResolver = new EpisodeResolverMock(Mock.Of<ILogger<EpisodeResolver>>(), _namingOptions);
             var itemResolveArgs = new ItemResolveArgs(
                 Mock.Of<IServerApplicationPaths>(),
                 Mock.Of<IDirectoryService>())
@@ -61,7 +62,7 @@ namespace Jellyfin.Server.Implementations.Tests.Library
 
         private class EpisodeResolverMock : EpisodeResolver
         {
-            public EpisodeResolverMock(NamingOptions namingOptions) : base(namingOptions)
+            public EpisodeResolverMock(ILogger<EpisodeResolver> logger, NamingOptions namingOptions) : base(logger, namingOptions)
             {
             }
 
diff --git a/tests/Jellyfin.Server.Implementations.Tests/Library/LibraryManager/FindExtrasTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Library/LibraryManager/FindExtrasTests.cs
index f5c8cc9708..5995990711 100644
--- a/tests/Jellyfin.Server.Implementations.Tests/Library/LibraryManager/FindExtrasTests.cs
+++ b/tests/Jellyfin.Server.Implementations.Tests/Library/LibraryManager/FindExtrasTests.cs
@@ -1,4 +1,4 @@
-using System;
+using System;
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
diff --git a/tests/Jellyfin.Server.Implementations.Tests/Library/MovieResolverTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Library/MovieResolverTests.cs
index f2efcddbae..efc3ac0c2a 100644
--- a/tests/Jellyfin.Server.Implementations.Tests/Library/MovieResolverTests.cs
+++ b/tests/Jellyfin.Server.Implementations.Tests/Library/MovieResolverTests.cs
@@ -5,6 +5,7 @@ using MediaBrowser.Controller.Drawing;
 using MediaBrowser.Controller.Library;
 using MediaBrowser.Controller.Providers;
 using MediaBrowser.Model.IO;
+using Microsoft.Extensions.Logging;
 using Moq;
 using Xunit;
 
@@ -17,7 +18,7 @@ public class MovieResolverTests
     [Fact]
     public void Resolve_GivenLocalAlternateVersion_ResolvesToVideo()
     {
-        var movieResolver = new MovieResolver(Mock.Of<IImageProcessor>(), _namingOptions);
+        var movieResolver = new MovieResolver(Mock.Of<IImageProcessor>(), Mock.Of<ILogger<MovieResolver>>(), _namingOptions);
         var itemResolveArgs = new ItemResolveArgs(
             Mock.Of<IServerApplicationPaths>(),
             Mock.Of<IDirectoryService>())