Merge pull request #17367 from zerafachris/fix/backup-skip-corrupt-keyframe-data

fix: skip corrupt KeyframeData rows during full system backup

2 files changed, 213 insertions, 10 deletions

diff --git a/Jellyfin.Server.Implementations/FullSystemBackup/BackupService.cs b/Jellyfin.Server.Implementations/FullSystemBackup/BackupService.cs
index a534fa5fa0..7c10a5dc77 100644
--- a/Jellyfin.Server.Implementations/FullSystemBackup/BackupService.cs
+++ b/Jellyfin.Server.Implementations/FullSystemBackup/BackupService.cs
@@ -359,18 +359,39 @@ public class BackupService : IBackupService
                                     jsonSerializer.WriteStartArray();
 
                                     var set = entityType.ValueFactory().ConfigureAwait(false);
-                                    await foreach (var item in set.ConfigureAwait(false))
+                                    var enumerator = set.GetAsyncEnumerator();
+                                    await using (enumerator)
                                     {
-                                        entities++;
-                                        try
+                                        while (true)
                                         {
-                                            using var document = JsonSerializer.SerializeToDocument(item, _serializerSettings);
-                                            document.WriteTo(jsonSerializer);
-                                        }
-                                        catch (Exception ex)
-                                        {
-                                            _logger.LogError(ex, "Could not load entity {Entity}", item);
-                                            throw;
+                                            bool hasNext;
+                                            try
+                                            {
+                                                hasNext = await enumerator.MoveNextAsync();
+                                            }
+                                            catch (Exception ex)
+                                            {
+                                                _logger.LogError(ex, "Could not read next entity of type {Table}, the underlying data appears to be corrupt. Skipping this row and continuing backup; the affected database row should be inspected and fixed manually", entityType.SourceName);
+                                                continue;
+                                            }
+
+                                            if (!hasNext)
+                                            {
+                                                break;
+                                            }
+
+                                            var item = enumerator.Current;
+                                            entities++;
+                                            try
+                                            {
+                                                using var document = JsonSerializer.SerializeToDocument(item, _serializerSettings);
+                                                document.WriteTo(jsonSerializer);
+                                            }
+                                            catch (Exception ex)
+                                            {
+                                                _logger.LogError(ex, "Could not load entity {Entity}", item);
+                                                throw;
+                                            }
                                         }
                                     }
 
diff --git a/tests/Jellyfin.Server.Implementations.Tests/FullSystemBackup/BackupServiceTests.cs b/tests/Jellyfin.Server.Implementations.Tests/FullSystemBackup/BackupServiceTests.cs
new file mode 100644
index 0000000000..66c392a6ad
--- /dev/null
+++ b/tests/Jellyfin.Server.Implementations.Tests/FullSystemBackup/BackupServiceTests.cs
@@ -0,0 +1,182 @@
+using System;
+using System.IO;
+using System.IO.Compression;
+using System.Linq;
+using System.Text.Json;
+using System.Threading;
+using System.Threading.Tasks;
+using Jellyfin.Database.Implementations;
+using Jellyfin.Database.Implementations.Entities;
+using Jellyfin.Database.Implementations.Locking;
+using Jellyfin.Database.Providers.Sqlite;
+using Jellyfin.Server.Implementations.FullSystemBackup;
+using MediaBrowser.Controller;
+using MediaBrowser.Controller.Library;
+using MediaBrowser.Controller.SystemBackupService;
+using Microsoft.Data.Sqlite;
+using Microsoft.EntityFrameworkCore;
+using Microsoft.Extensions.Hosting;
+using Microsoft.Extensions.Logging.Abstractions;
+using Moq;
+using Xunit;
+using BaseItemKind = Jellyfin.Data.Enums.BaseItemKind;
+
+namespace Jellyfin.Server.Implementations.Tests.FullSystemBackup;
+
+/// <summary>
+/// Tests for <see cref="BackupService"/>, in particular that a single row of corrupt
+/// <see cref="KeyframeData"/> (e.g. malformed <c>KeyframeTicks</c> JSON) does not abort
+/// an otherwise healthy backup. See https://github.com/jellyfin/jellyfin/issues/17216.
+/// </summary>
+public sealed class BackupServiceTests : IDisposable
+{
+    private readonly SqliteConnection _connection;
+    private readonly DbContextOptions<JellyfinDbContext> _dbOptions;
+    private readonly string _testRoot;
+    private readonly string _backupPath;
+    private readonly string _configurationDirectoryPath;
+
+    public BackupServiceTests()
+    {
+        _connection = new SqliteConnection("Data Source=:memory:");
+        _connection.Open();
+
+        _dbOptions = new DbContextOptionsBuilder<JellyfinDbContext>()
+            .UseSqlite(_connection)
+            .Options;
+
+        using (var ctx = CreateDbContext())
+        {
+            ctx.Database.EnsureCreated();
+        }
+
+        // Use the test assembly's own output directory instead of Path.GetTempPath(). On GitHub-hosted
+        // windows-latest runners, the system temp directory lives on the constrained C: drive, which can have
+        // less than the 5GiB BackupService requires free, causing spurious failures. AppContext.BaseDirectory
+        // is under the repo checkout (the much larger D: drive on Windows runners) on all platforms.
+        _testRoot = Path.Combine(AppContext.BaseDirectory, "jellyfin-backup-service-tests-" + Guid.NewGuid().ToString("N"));
+        _backupPath = Path.Combine(_testRoot, "Backup");
+        _configurationDirectoryPath = Path.Combine(_testRoot, "Config");
+        Directory.CreateDirectory(_backupPath);
+        Directory.CreateDirectory(_configurationDirectoryPath);
+    }
+
+    public void Dispose()
+    {
+        _connection.Dispose();
+
+        if (Directory.Exists(_testRoot))
+        {
+            Directory.Delete(_testRoot, true);
+        }
+    }
+
+    [Fact]
+    public async Task CreateBackupAsync_WithCorruptKeyframeDataRow_SkipsRowAndCompletesBackup()
+    {
+        var cancellationToken = TestContext.Current.CancellationToken;
+        var validItemId = Guid.Parse("11111111-1111-1111-1111-111111111111");
+        var corruptItemId = Guid.Parse("22222222-2222-2222-2222-222222222222");
+
+        await using (var ctx = CreateDbContext())
+        {
+            // A healthy item + keyframe row, written the normal way.
+            ctx.BaseItems.Add(CreateMovieEntity(validItemId, "Good Movie"));
+            ctx.BaseItems.Add(CreateMovieEntity(corruptItemId, "Corrupt Movie"));
+            await ctx.SaveChangesAsync(cancellationToken).ConfigureAwait(true);
+
+            ctx.KeyframeData.Add(new KeyframeData
+            {
+                ItemId = validItemId,
+                TotalDuration = 60_000,
+                KeyframeTicks = [0, 1000, 2000]
+            });
+            await ctx.SaveChangesAsync(cancellationToken).ConfigureAwait(true);
+
+            // Simulate a corrupted database row: truncated JSON array for KeyframeTicks,
+            // written directly via SQL to bypass EF's normal (well-formed) write path.
+            await ctx.Database.ExecuteSqlInterpolatedAsync(
+                $"INSERT INTO KeyframeData (ItemId, TotalDuration, KeyframeTicks) VALUES ({corruptItemId.ToString()}, {5000L}, {"[1,2,3"})",
+                cancellationToken).ConfigureAwait(true);
+        }
+
+        var backupService = CreateBackupService();
+
+        var manifest = await backupService.CreateBackupAsync(new BackupOptionsDto()).ConfigureAwait(true);
+
+        Assert.True(File.Exists(manifest.Path));
+
+        using var archive = await ZipFile.OpenReadAsync(manifest.Path, cancellationToken).ConfigureAwait(true);
+        var keyframeEntry = archive.GetEntry("Database/KeyframeData.json");
+        Assert.NotNull(keyframeEntry);
+
+        await using var entryStream = await keyframeEntry!.OpenAsync(cancellationToken).ConfigureAwait(true);
+        using var document = await JsonDocument.ParseAsync(entryStream, cancellationToken: cancellationToken).ConfigureAwait(true);
+
+        var rows = document.RootElement.EnumerateArray().ToList();
+
+        // The corrupt row must be skipped, but the valid row must still make it into the backup.
+        var singleRow = Assert.Single(rows);
+        Assert.Equal(validItemId, singleRow.GetProperty("ItemId").GetGuid());
+    }
+
+    private BackupService CreateBackupService()
+    {
+        var factory = new Mock<IDbContextFactory<JellyfinDbContext>>();
+        factory.Setup(f => f.CreateDbContext()).Returns(CreateDbContext);
+        factory.Setup(f => f.CreateDbContextAsync(It.IsAny<CancellationToken>())).ReturnsAsync(CreateDbContext);
+
+        var applicationHost = new Mock<IServerApplicationHost>();
+        applicationHost.Setup(a => a.ApplicationVersion).Returns(new Version(10, 11, 0));
+
+        var applicationPaths = new Mock<IServerApplicationPaths>();
+        applicationPaths.Setup(a => a.BackupPath).Returns(_backupPath);
+        applicationPaths.Setup(a => a.ConfigurationDirectoryPath).Returns(_configurationDirectoryPath);
+        applicationPaths.Setup(a => a.DataPath).Returns(Path.Combine(_testRoot, "Data"));
+        applicationPaths.Setup(a => a.RootFolderPath).Returns(Path.Combine(_testRoot, "Root"));
+        applicationPaths.Setup(a => a.InternalMetadataPath).Returns(Path.Combine(_testRoot, "Metadata"));
+        applicationPaths.Setup(a => a.DefaultInternalMetadataPath).Returns(Path.Combine(_testRoot, "MetadataDefault"));
+
+        var jellyfinDatabaseProvider = new Mock<IJellyfinDatabaseProvider>();
+        jellyfinDatabaseProvider.Setup(p => p.RunScheduledOptimisation(It.IsAny<CancellationToken>())).Returns(Task.CompletedTask);
+        jellyfinDatabaseProvider.Setup(p => p.PurgeDatabase(It.IsAny<JellyfinDbContext>(), It.IsAny<System.Collections.Generic.IEnumerable<string>>())).Returns(Task.CompletedTask);
+
+        var applicationLifetime = new Mock<IHostApplicationLifetime>();
+
+        var libraryManager = new Mock<ILibraryManager>();
+        libraryManager.Setup(l => l.IsScanRunning).Returns(false);
+
+        return new BackupService(
+            NullLogger<BackupService>.Instance,
+            factory.Object,
+            applicationHost.Object,
+            applicationPaths.Object,
+            jellyfinDatabaseProvider.Object,
+            applicationLifetime.Object,
+            libraryManager.Object);
+    }
+
+    private static BaseItemEntity CreateMovieEntity(Guid id, string name)
+    {
+        return new BaseItemEntity
+        {
+            Id = id,
+            Type = "Movie",
+            Name = name,
+            PresentationUniqueKey = id.ToString("N"),
+            MediaType = "Video",
+            IsMovie = true,
+            IsFolder = false,
+            IsVirtualItem = false
+        };
+    }
+
+    private JellyfinDbContext CreateDbContext()
+    {
+        return new JellyfinDbContext(
+            _dbOptions,
+            NullLogger<JellyfinDbContext>.Instance,
+            new SqliteDatabaseProvider(null!, NullLogger<SqliteDatabaseProvider>.Instance),
+            new NoLockBehavior(NullLogger<NoLockBehavior>.Instance));
+    }
+}