From 9e39a0cf16a2be03634a514e7ee868d7fa985b00 Mon Sep 17 00:00:00 2001 From: gmipf Date: Mon, 26 Oct 2026 19:19:16 +0100 Subject: [PATCH] Close the cabinet files opened for a cabinet set (#105) * Close the cabinet files opened for a cabinet set MicrosoftCabinet.Extract opened the cabinet file again for the set, together with the previous and next cabinets, and never closed any of them, so they stayed open until their streams were finalized. Co-Authored-By: Claude Opus 5.5 * Keep the original comment on the previous cabinet Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- .../MicrosoftCabinetTests.cs | 41 +++++++++++ .../TestData/MicrosoftCabinet/part1.cab | Bin 0 -> 95 bytes .../TestData/MicrosoftCabinet/part2.cab | Bin 0 -> 111 bytes .../TestData/MicrosoftCabinet/part3.cab | Bin 0 -> 97 bytes .../TestData/MicrosoftCabinet/single.cab | Bin 0 -> 79 bytes .../MicrosoftCabinet.Extraction.cs | 69 +++++++++++++++--- 6 files changed, 100 insertions(+), 10 deletions(-) create mode 100644 SabreTools.Wrappers.Test/TestData/MicrosoftCabinet/part1.cab create mode 100644 SabreTools.Wrappers.Test/TestData/MicrosoftCabinet/part2.cab create mode 100644 SabreTools.Wrappers.Test/TestData/MicrosoftCabinet/part3.cab create mode 100644 SabreTools.Wrappers.Test/TestData/MicrosoftCabinet/single.cab diff --git a/SabreTools.Wrappers.Test/MicrosoftCabinetTests.cs b/SabreTools.Wrappers.Test/MicrosoftCabinetTests.cs index 27366d1d..58e6d3f2 100644 --- a/SabreTools.Wrappers.Test/MicrosoftCabinetTests.cs +++ b/SabreTools.Wrappers.Test/MicrosoftCabinetTests.cs @@ -76,6 +76,47 @@ namespace SabreTools.Wrappers.Test Directory.Delete(outputDirectory, recursive: true); } + [Theory] + [InlineData("single.cab", null)] + [InlineData("part1.cab", null)] + [InlineData("part2.cab", null)] + [InlineData("part1.cab", "part2.cab")] + public void Extract_FilesNotKeptOpen(string filename, string? invalidPart) + { + // Copy of the test cabinets, so they can be checked and deleted afterwards + string directory = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + string outputDirectory = Path.Combine(directory, "output"); + Directory.CreateDirectory(directory); + foreach (string file in Directory.GetFiles(Path.Combine(Environment.CurrentDirectory, "TestData", "MicrosoftCabinet"))) + { + File.Copy(file, Path.Combine(directory, Path.GetFileName(file))); + } + + if (invalidPart is not null) + File.WriteAllBytes(Path.Combine(directory, invalidPart), [.. Enumerable.Repeat(0xFF, 1024)]); + + try + { + using (var stream = File.Open(Path.Combine(directory, filename), FileMode.Open, FileAccess.Read, FileShare.ReadWrite)) + { + var cabinet = MicrosoftCabinet.Create(stream); + Assert.NotNull(cabinet); + cabinet.Extract(outputDirectory, includeDebug: false); + } + + // Opening a file exclusively fails while another handle to it is still open + foreach (string file in Directory.GetFiles(directory)) + { + using var exclusive = File.Open(file, FileMode.Open, FileAccess.Read, FileShare.None); + Assert.True(exclusive.CanRead); + } + } + finally + { + Directory.Delete(directory, recursive: true); + } + } + #endregion } } diff --git a/SabreTools.Wrappers.Test/TestData/MicrosoftCabinet/part1.cab b/SabreTools.Wrappers.Test/TestData/MicrosoftCabinet/part1.cab new file mode 100644 index 0000000000000000000000000000000000000000..1672d20b2e587ffc6b864c371c2d0329e8d43590 GIT binary patch literal 95 zcmeYbc6MW6U|@&`5ey6rHc*&9TkeE}Dkq7_*i3}eA literal 0 HcmV?d00001 diff --git a/SabreTools.Wrappers.Test/TestData/MicrosoftCabinet/part2.cab b/SabreTools.Wrappers.Test/TestData/MicrosoftCabinet/part2.cab new file mode 100644 index 0000000000000000000000000000000000000000..3800450245794c21741ee0328dab8a21c970a6bf GIT binary patch literal 111 zcmeYbc6MW6U|`4x5ey6rK2VyOk&%IsftkTXh>@Wnv8cpQFF7%ZAtkdo+Yrn*hVzXX nk{B2m7#SECSfP3u7!;!z7!(+i^hzp9z+$WntPDv-iDmf!j?WQ* literal 0 HcmV?d00001 diff --git a/SabreTools.Wrappers.Test/TestData/MicrosoftCabinet/part3.cab b/SabreTools.Wrappers.Test/TestData/MicrosoftCabinet/part3.cab new file mode 100644 index 0000000000000000000000000000000000000000..4abfc1e54941a16c044585818a794f99b62e2beb GIT binary patch literal 97 zcmeYbc6MW6U|>iD5ey6rHc*j3KEM-jP#NdlNeGmi?fXwf*2SW7#SEC h*r93}7!;!z7!(+i^-3yAz+&tS>`Il~j~~ O#aJ0w84_~}G7 + /// File opened for this cabinet as part of a cabinet set + /// + /// Only used in multi-file + private Stream? _setFile; + + #endregion + #region Cabinet Set /// @@ -47,8 +57,7 @@ namespace SabreTools.Wrappers filename = Path.GetFullPath(filename); // Read in the current file and try to parse - var stream = File.Open(filename, FileMode.Open, FileAccess.Read, FileShare.ReadWrite); - var current = Create(stream); + var current = OpenSetFile(filename); if (current?.Header is null) return null; @@ -60,6 +69,9 @@ namespace SabreTools.Wrappers if (prev?.Header is null) break; + // Close this part, it is opened again below + current._setFile?.Dispose(); + // Assign previous as new current current = prev; } @@ -116,8 +128,7 @@ namespace SabreTools.Wrappers // Open and return the next cabinet try { - var fs = File.Open(next, FileMode.Open, FileAccess.Read, FileShare.ReadWrite); - return Create(fs); + return OpenSetFile(next!); } catch { @@ -153,8 +164,7 @@ namespace SabreTools.Wrappers // Open and return the previous cabinet try { - var fs = File.Open(prev, FileMode.Open, FileAccess.Read, FileShare.ReadWrite); - return Create(fs); + return OpenSetFile(prev!); } catch { @@ -163,6 +173,36 @@ namespace SabreTools.Wrappers } } + /// + /// Open a cabinet file as part of a cabinet set + /// + /// Full path to the cabinet file + /// Wrapper representing the cabinet, null on error + private static MicrosoftCabinet? OpenSetFile(string filename) + { + var stream = File.Open(filename, FileMode.Open, FileAccess.Read, FileShare.ReadWrite); + var cabinet = Create(stream); + if (cabinet is null) + { + stream.Dispose(); + return null; + } + + cabinet._setFile = stream; + return cabinet; + } + + /// + /// Close the files opened for this cabinet and all following parts of the set + /// + private void CloseSetFiles() + { + for (var cabinet = this; cabinet is not null; cabinet = cabinet.Next) + { + cabinet._setFile?.Dispose(); + } + } + #endregion #region Extraction @@ -191,6 +231,8 @@ namespace SabreTools.Wrappers { string firstCabName = Path.GetFileName(cabinet.Filename) ?? string.Empty; if (includeDebug) Console.WriteLine($"Only the first cabinet {firstCabName} will be extracted!"); + + cabinet.CloseSetFiles(); return false; } } @@ -201,11 +243,18 @@ namespace SabreTools.Wrappers cabinet = this; } - // If the archive is invalid - if (cabinet.Folders.Length == 0) - return false; + try + { + // If the archive is invalid + if (cabinet.Folders.Length == 0) + return false; - return cabinet.ExtractSet(Filename, outputDirectory, includeDebug); + return cabinet.ExtractSet(Filename, outputDirectory, includeDebug); + } + finally + { + cabinet.CloseSetFiles(); + } } ///