mirror of
https://github.com/SabreTools/SabreTools.Serialization.git
synced 2026-11-15 17:00:38 +00:00
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 <noreply@anthropic.com> * Keep the original comment on the previous cabinet Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -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<byte>(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
|
||||
}
|
||||
}
|
||||
|
||||
BIN
SabreTools.Wrappers.Test/TestData/MicrosoftCabinet/part1.cab
Normal file
BIN
SabreTools.Wrappers.Test/TestData/MicrosoftCabinet/part1.cab
Normal file
Binary file not shown.
BIN
SabreTools.Wrappers.Test/TestData/MicrosoftCabinet/part2.cab
Normal file
BIN
SabreTools.Wrappers.Test/TestData/MicrosoftCabinet/part2.cab
Normal file
Binary file not shown.
BIN
SabreTools.Wrappers.Test/TestData/MicrosoftCabinet/part3.cab
Normal file
BIN
SabreTools.Wrappers.Test/TestData/MicrosoftCabinet/part3.cab
Normal file
Binary file not shown.
BIN
SabreTools.Wrappers.Test/TestData/MicrosoftCabinet/single.cab
Normal file
BIN
SabreTools.Wrappers.Test/TestData/MicrosoftCabinet/single.cab
Normal file
Binary file not shown.
@@ -27,6 +27,16 @@ namespace SabreTools.Wrappers
|
||||
|
||||
#endregion
|
||||
|
||||
#region Instance Variables
|
||||
|
||||
/// <summary>
|
||||
/// File opened for this cabinet as part of a cabinet set
|
||||
/// </summary>
|
||||
/// <remarks>Only used in multi-file</remarks>
|
||||
private Stream? _setFile;
|
||||
|
||||
#endregion
|
||||
|
||||
#region Cabinet Set
|
||||
|
||||
/// <summary>
|
||||
@@ -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
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Open a cabinet file as part of a cabinet set
|
||||
/// </summary>
|
||||
/// <param name="filename">Full path to the cabinet file</param>
|
||||
/// <returns>Wrapper representing the cabinet, null on error</returns>
|
||||
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;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Close the files opened for this cabinet and all following parts of the set
|
||||
/// </summary>
|
||||
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();
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
||||
Reference in New Issue
Block a user