From 990186acc53ee0014496f3f46c5da8dd6c50baa4 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Fri, 6 Nov 2026 16:03:00 +0000 Subject: [PATCH] Fix writable archive duplicate entry checks --- .../Archives/AbstractWritableArchive.Async.cs | 50 +++++-- .../Archives/AbstractWritableArchive.cs | 58 +++++++-- .../Tar/TarArchiveDuplicateTests.cs | 42 ++++++ .../Zip/ZipArchiveDuplicateTests.cs | 122 ++++++++++++++++++ 4 files changed, 250 insertions(+), 22 deletions(-) create mode 100644 tests/SharpCompress.Test/Tar/TarArchiveDuplicateTests.cs create mode 100644 tests/SharpCompress.Test/Zip/ZipArchiveDuplicateTests.cs diff --git a/src/SharpCompress/Archives/AbstractWritableArchive.Async.cs b/src/SharpCompress/Archives/AbstractWritableArchive.Async.cs index 67b8dffc..4b29c4ca 100644 --- a/src/SharpCompress/Archives/AbstractWritableArchive.Async.cs +++ b/src/SharpCompress/Archives/AbstractWritableArchive.Async.cs @@ -2,6 +2,7 @@ using System; using System.Collections.Generic; using System.IO; using System.Linq; +using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; using SharpCompress.Common; @@ -14,6 +15,28 @@ public abstract partial class AbstractWritableArchive where TVolume : IVolume where TOptions : IWriterOptions { + public override IAsyncEnumerable EntriesAsync => GetCurrentEntriesAsync(); + + private async IAsyncEnumerable GetCurrentEntriesAsync( + [EnumeratorCancellation] CancellationToken cancellationToken = default + ) + { + await foreach ( + var entry in OldEntriesAsync.WithCancellation(cancellationToken).ConfigureAwait(false) + ) + { + yield return entry; + } + foreach (var entry in newEntries) + { + cancellationToken.ThrowIfCancellationRequested(); + if (!removedEntries.Contains(entry)) + { + yield return entry; + } + } + } + // Async property moved from main file private IAsyncEnumerable OldEntriesAsync => base.EntriesAsync.Where(x => !removedEntries.Contains(x)); @@ -22,9 +45,12 @@ public abstract partial class AbstractWritableArchive { if (pauseRebuilding) { + // Disposing the synchronous pause must not perform synchronous I/O for async callers. + asyncRebuildPending = true; return; } hasModifications = true; + asyncRebuildPending = false; newEntries.RemoveAll(v => removedEntries.Contains(v)); modifiedEntries.Clear(); await foreach (var entry in OldEntriesAsync.ConfigureAwait(false)) @@ -49,20 +75,21 @@ public abstract partial class AbstractWritableArchive ) { await foreach ( - var entry in EntriesAsync.WithCancellation(cancellationToken).ConfigureAwait(false) + var entry in OldEntriesAsync.WithCancellation(cancellationToken).ConfigureAwait(false) ) { - var path = entry.Key; - if (path is null) + if (entry.Key is { } path && KeysMatch(path, key)) { - continue; + return true; } - var p = path.Replace('/', '\\'); - if (p.Length > 0 && p[0] == '\\') + } + foreach (var entry in newEntries) + { + cancellationToken.ThrowIfCancellationRequested(); + if (!removedEntries.Contains(entry) && entry.Key is { } path && KeysMatch(path, key)) { - p = p.Substring(1); + return true; } - return string.Equals(p, key, StringComparison.OrdinalIgnoreCase); } return false; } @@ -116,9 +143,12 @@ public abstract partial class AbstractWritableArchive CancellationToken cancellationToken = default ) { + var activeNewEntries = newEntries.Where(x => !removedEntries.Contains(x)).ToList(); //reset streams of new entries - newEntries.Cast().ForEach(x => x.Stream.Seek(0, SeekOrigin.Begin)); - await SaveToAsync(stream, options, OldEntriesAsync, newEntries, cancellationToken) + activeNewEntries + .Cast() + .ForEach(x => x.Stream.Seek(0, SeekOrigin.Begin)); + await SaveToAsync(stream, options, OldEntriesAsync, activeNewEntries, cancellationToken) .ConfigureAwait(false); } diff --git a/src/SharpCompress/Archives/AbstractWritableArchive.cs b/src/SharpCompress/Archives/AbstractWritableArchive.cs index 59d55771..42ff5d5d 100644 --- a/src/SharpCompress/Archives/AbstractWritableArchive.cs +++ b/src/SharpCompress/Archives/AbstractWritableArchive.cs @@ -32,7 +32,10 @@ public abstract partial class AbstractWritableArchive public void Dispose() { archive.pauseRebuilding = false; - archive.RebuildModifiedCollection(); + if (!archive.asyncRebuildPending) + { + archive.RebuildModifiedCollection(); + } } } @@ -42,6 +45,7 @@ public abstract partial class AbstractWritableArchive private readonly List modifiedEntries = new(); private bool hasModifications; private bool pauseRebuilding; + private bool asyncRebuildPending; internal AbstractWritableArchive(ArchiveType type) : base(type) { } @@ -53,6 +57,10 @@ public abstract partial class AbstractWritableArchive { get { + if (asyncRebuildPending && !pauseRebuilding) + { + RebuildModifiedCollection(); + } if (hasModifications) { return modifiedEntries; @@ -70,6 +78,7 @@ public abstract partial class AbstractWritableArchive return; } hasModifications = true; + asyncRebuildPending = false; newEntries.RemoveAll(v => removedEntries.Contains(v)); modifiedEntries.Clear(); modifiedEntries.AddRange(OldEntries.Concat(newEntries)); @@ -126,22 +135,44 @@ public abstract partial class AbstractWritableArchive private bool DoesKeyMatchExisting(string key) { - foreach (var path in Entries.Select(x => x.Key)) + foreach ( + var path in OldEntries + .Concat(newEntries) + .Where(x => !removedEntries.Contains(x)) + .Select(x => x.Key) + ) { - if (path is null) + if (path is not null && KeysMatch(path, key)) { - continue; + return true; } - var p = path.Replace('/', '\\'); - if (p.Length > 0 && p[0] == '\\') - { - p = p.Substring(1); - } - return string.Equals(p, key, StringComparison.OrdinalIgnoreCase); } return false; } + private bool KeysMatch(string left, string right) => + string.Equals(NormalizeKey(left), NormalizeKey(right), StringComparison.OrdinalIgnoreCase); + + private string NormalizeKey(string key) + { + key = key.Replace('\\', '/'); + if (Type == ArchiveType.Zip) + { + // ZipWriter removes drive prefixes and surrounding slashes when writing names. +#if LEGACY_DOTNET + var colon = key.IndexOf(':'); +#else + var colon = key.IndexOf(':', StringComparison.Ordinal); +#endif + if (colon >= 0) + { + key = key.Substring(colon + 1); + } + return key.Trim('/'); + } + return key.StartsWith('/') ? key.Substring(1) : key; + } + ValueTask IWritableAsyncArchive.RemoveEntryAsync(IArchiveEntry entry) => RemoveEntryAsync((TEntry)entry); @@ -180,9 +211,12 @@ public abstract partial class AbstractWritableArchive public void SaveTo(Stream stream, TOptions options) { + var activeNewEntries = newEntries.Where(x => !removedEntries.Contains(x)).ToList(); //reset streams of new entries - newEntries.Cast().ForEach(x => x.Stream.Seek(0, SeekOrigin.Begin)); - SaveTo(stream, options, OldEntries, newEntries); + activeNewEntries + .Cast() + .ForEach(x => x.Stream.Seek(0, SeekOrigin.Begin)); + SaveTo(stream, options, OldEntries, activeNewEntries); } protected TEntry CreateEntry( diff --git a/tests/SharpCompress.Test/Tar/TarArchiveDuplicateTests.cs b/tests/SharpCompress.Test/Tar/TarArchiveDuplicateTests.cs new file mode 100644 index 00000000..6d4aa083 --- /dev/null +++ b/tests/SharpCompress.Test/Tar/TarArchiveDuplicateTests.cs @@ -0,0 +1,42 @@ +using System.IO; +using System.Linq; +using System.Threading.Tasks; +using SharpCompress.Archives.Tar; +using SharpCompress.Common; +using Xunit; + +namespace SharpCompress.Test.Tar; + +public class TarArchiveDuplicateTests +{ + [Fact] + public void Tar_AddEntry_RejectsDuplicateAfterFirstEntry() + { + using var archive = TarArchive.CreateArchive(); + using var stream = new MemoryStream(); + archive.AddEntry("other", stream, false); + archive.AddEntry("dir/file", stream, false); + Assert.Throws(() => archive.AddEntry("DIR\\FILE", stream, false)); + var entry = archive.Entries.Single(x => x.Key == "dir/file"); + archive.RemoveEntry(entry); + archive.AddEntry("dir/file", stream, false); + } + + [Fact] + public async Task Tar_Async_ChecksPendingEntriesWhileRebuildingIsPaused() + { + await using var archive = await TarArchive.CreateAsyncArchive(); + using var stream = new MemoryStream(); + using (archive.PauseEntryRebuilding()) + { + await archive.AddEntryAsync("other", stream, false); + var entry = await archive.AddEntryAsync("dir/file", stream, false); + await Assert.ThrowsAsync(async () => + await archive.AddEntryAsync("DIR\\FILE", stream, false) + ); + await archive.RemoveEntryAsync(entry); + await archive.AddEntryAsync("dir/file", stream, false); + } + Assert.Equal(2, (await archive.EntriesAsync.ToListAsync()).Count); + } +} diff --git a/tests/SharpCompress.Test/Zip/ZipArchiveDuplicateTests.cs b/tests/SharpCompress.Test/Zip/ZipArchiveDuplicateTests.cs new file mode 100644 index 00000000..ea333644 --- /dev/null +++ b/tests/SharpCompress.Test/Zip/ZipArchiveDuplicateTests.cs @@ -0,0 +1,122 @@ +using System.IO; +using System.Linq; +using System.Threading.Tasks; +using SharpCompress.Archives.Zip; +using SharpCompress.Common; +using SharpCompress.Readers; +using SharpCompress.Writers.Zip; +using Xunit; + +namespace SharpCompress.Test.Zip; + +public class ZipArchiveDuplicateTests +{ + [Theory] + [InlineData("dir/file.txt", "DIR/FILE.TXT")] + [InlineData("dir/file.txt", "dir\\file.txt")] + [InlineData("file.txt", "C:/file.txt")] + [InlineData("file.txt", "//file.txt/")] + public void Zip_AddEntry_RejectsDuplicateAfterFirstEntry(string first, string duplicate) + { + using var archive = ZipArchive.CreateArchive(); + using var stream = new MemoryStream(); + archive.AddEntry("other", stream, false); + archive.AddEntry(first, stream, false); + Assert.Throws(() => archive.AddEntry(duplicate, stream, false)); + } + + [Fact] + public void Zip_PausedRebuilding_ChecksPendingAndRemovedEntries() + { + using var archive = ZipArchive.CreateArchive(); + using var stream = new MemoryStream(); + using (archive.PauseEntryRebuilding()) + { + var entry = archive.AddEntry("dir/file", stream, false); + Assert.Throws(() => archive.AddEntry("DIR\\FILE", stream, false)); + archive.RemoveEntry(entry); + archive.AddEntry("dir/file", stream, false); + archive.AddDirectoryEntry("folder"); + Assert.Throws(() => archive.AddDirectoryEntry("FOLDER/")); + } + Assert.Equal(2, archive.Entries.Count()); + } + + [Fact] + public async Task Zip_Async_ChecksPendingEntriesAndExposesModifications() + { + await using var archive = await ZipArchive.CreateAsyncArchive(); + using var stream = new MemoryStream(); + await archive.AddEntryAsync("other", stream, false); + var entry = await archive.AddEntryAsync("dir/file", stream, false); + await Assert.ThrowsAsync(async () => + await archive.AddEntryAsync("DIR\\FILE", stream, false) + ); + Assert.Equal(2, (await archive.EntriesAsync.ToListAsync()).Count); + await archive.RemoveEntryAsync(entry); + Assert.Single(await archive.EntriesAsync.ToListAsync()); + await archive.AddEntryAsync("dir/file", stream, false); + using (archive.PauseEntryRebuilding()) + { + await archive.AddDirectoryEntryAsync("folder"); + await Assert.ThrowsAsync(async () => + await archive.AddDirectoryEntryAsync("FOLDER/") + ); + } + Assert.Equal(3, (await archive.EntriesAsync.ToListAsync()).Count); + } + + [Fact] + public async Task Zip_ExistingEntries_AreCheckedSynchronouslyAndAsynchronously() + { + using var data = new MemoryStream(); + using var source = new MemoryStream(); + using (var archive = ZipArchive.CreateArchive()) + { + archive.AddEntry("other", source, false); + archive.AddEntry("dir/file", source, false); + archive.SaveTo( + data, + new ZipWriterOptions(CompressionType.Deflate) { LeaveStreamOpen = true } + ); + } + data.Position = 0; + using ( + var archive = ZipArchive.OpenArchive(data, new ReaderOptions { LeaveStreamOpen = true }) + ) + { + Assert.Throws(() => archive.AddEntry("DIR\\FILE", source, false)); + } + data.Position = 0; + await using ( + var archive = await ZipArchive.OpenAsyncArchive( + data, + new ReaderOptions { LeaveStreamOpen = true } + ) + ) + { + await Assert.ThrowsAsync(async () => + await archive.AddEntryAsync("DIR\\FILE", source, false) + ); + var entry = await archive.EntriesAsync.SingleAsync(x => x.Key == "dir/file"); + using (archive.PauseEntryRebuilding()) + { + await archive.RemoveEntryAsync(entry); + var temporary = await archive.AddEntryAsync("temporary", source, false); + await archive.RemoveEntryAsync(temporary); + await archive.AddEntryAsync("dir/file", source, false); + Assert.Equal(2, (await archive.EntriesAsync.ToListAsync()).Count); + } + Assert.Equal(2, (await archive.EntriesAsync.ToListAsync()).Count); + using var output = new MemoryStream(); + await archive.SaveToAsync( + output, + new ZipWriterOptions(CompressionType.Deflate) { LeaveStreamOpen = true } + ); + output.Position = 0; + using var saved = ZipArchive.OpenArchive(output); + Assert.Equal(2, saved.Entries.Count()); + Assert.DoesNotContain(saved.Entries, x => x.Key == "temporary"); + } + } +}