From 82617b56d3543f40360932c1e1e045f6c996e304 Mon Sep 17 00:00:00 2001 From: Bret Johnson Date: Thu, 14 May 2020 19:42:43 -0400 Subject: [PATCH] Fix enumerating zip with deleted entries Previously, if a zip had deleted entries then enumerating the entries in the zip would throw an exception - the ReadEntry call on a deleted entry would thrown an exception. With this change, deleted entries are instead skipped in the enumeration. --- LibZipSharp.UnitTest/ZipTests.cs | 60 ++++++++++++++++++++++++++++++++ ZipArchive.cs | 31 +++++++++++++++-- ZipEntryEnumerator.cs | 12 +++++-- build_native | 0 build_windows | 0 5 files changed, 98 insertions(+), 5 deletions(-) mode change 100755 => 100644 build_native mode change 100755 => 100644 build_windows diff --git a/LibZipSharp.UnitTest/ZipTests.cs b/LibZipSharp.UnitTest/ZipTests.cs index 3dcd72d..4c6e69c 100644 --- a/LibZipSharp.UnitTest/ZipTests.cs +++ b/LibZipSharp.UnitTest/ZipTests.cs @@ -1,5 +1,6 @@ using NUnit.Framework; using System; +using System.Collections.Generic; using System.IO; using System.Text; using System.Threading; @@ -157,5 +158,64 @@ public void SmallTextFile () } } } + + [TestCase(false)] + [TestCase(true)] + public void EnumerateSkipDeletedEntries(bool deleteFromExistingFile) + { + var ms = new MemoryStream (Encoding.UTF8.GetBytes(TEXT)); + File.WriteAllText ("file1.txt", "1111"); + string filePath = Path.GetFullPath("file1.txt"); + if (File.Exists ("test-archive-write.zip")) + File.Delete ("test-archive-write.zip"); + + ZipArchive zip = null; + try { + zip = ZipArchive.Open ("test-archive-write.zip", FileMode.CreateNew); + + ZipEntry e; + e = zip.AddFile (filePath, "/path/ZipTestCopy1.exe"); + e = zip.AddFile (filePath, "/path/ZipTestCopy2.exe"); + var text = "Hello World"; + e = zip.AddEntry ("/data/foo1.txt", text, Encoding.UTF8, CompressionMethod.Store); + e = zip.AddEntry ("/data/foo2.txt", File.OpenRead(filePath), CompressionMethod.Store); + + if (deleteFromExistingFile) { + zip.Close (); + zip = ZipArchive.Open ("test-archive-write.zip", FileMode.Open); + } + + ValidateEnumeratedEntries (zip, "path/ZipTestCopy1.exe", "path/ZipTestCopy2.exe", "data/foo1.txt", "data/foo2.txt"); + + // Delete first + zip.DeleteEntry ("path/ZipTestCopy1.exe"); + ValidateEnumeratedEntries (zip, "path/ZipTestCopy2.exe", "data/foo1.txt", "data/foo2.txt"); + + // Delete last + zip.DeleteEntry ("data/foo2.txt"); + ValidateEnumeratedEntries (zip, "path/ZipTestCopy2.exe", "data/foo1.txt"); + + // Delete middle + zip.DeleteEntry ("path/ZipTestCopy2.exe"); + ValidateEnumeratedEntries (zip, "data/foo1.txt"); + + // Delete all + zip.DeleteEntry ("data/foo1.txt"); + ValidateEnumeratedEntries (zip); + } + finally { + zip?.Dispose(); + } + } + + void ValidateEnumeratedEntries (ZipArchive zip, params string[] expectedEntries) + { + var actualEntries = new List(); + foreach (var entry in zip) { + actualEntries.Add(entry.FullName); + } + + Assert.AreEqual(expectedEntries, actualEntries.ToArray()); + } } } \ No newline at end of file diff --git a/ZipArchive.cs b/ZipArchive.cs index a076d3a..dc27ec2 100644 --- a/ZipArchive.cs +++ b/ZipArchive.cs @@ -691,12 +691,36 @@ public ZipEntry ReadEntry (string entryName, bool caseSensitive = false) return ReadEntry ((ulong)LookupEntry (entryName, caseSensitive)); } - public ZipEntry ReadEntry (ulong index) + /// + /// Read a zip entry, given an index. + /// + /// By default, if the entry is deleted then an exception is thrown (the error will be ErrorCode.Deleted or + /// ErrorCode.Inval, depending on whether the deleted entry previously existed in the zip or was newly + /// added - that's just how libzip handles that). If returnNullIfDeleted is true, then null is returned + /// for deleted entries and an exception is just thrown for other errors. + /// + /// index to read + /// whether to return null or throw an exception for deleted entries + /// + public ZipEntry ReadEntry (ulong index, bool returnNullIfDeleted = false) { Native.zip_stat_t stat; int ret = Native.zip_stat_index (archive, index, OperationFlags.None, out stat); - if (ret < 0) - throw GetErrorException (); + if (ret < 0) { + if (returnNullIfDeleted) { + IntPtr error = Native.zip_get_error(archive); + + if (error != IntPtr.Zero) { + int zip_error = Native.zip_error_code_zip(error); + // Deleted is returned when the deleted entry existed when the zip was opened + // Inval is returned when the deleted entry was newly added to the zip, then deleted + if (zip_error == (int) ErrorCode.Deleted || zip_error == (int)ErrorCode.Inval) + return null; + } + } + + throw GetErrorException(); + } var ze = ZipEntry.Create (this, stat); ze.Init (); @@ -848,6 +872,7 @@ public void Close () archive = IntPtr.Zero; } + /// /// Closes the native ZIP archive handle and frees associated resources, if called from the finalizer. /// diff --git a/ZipEntryEnumerator.cs b/ZipEntryEnumerator.cs index ce3df7d..e91689f 100644 --- a/ZipEntryEnumerator.cs +++ b/ZipEntryEnumerator.cs @@ -62,7 +62,15 @@ public bool MoveNext () // Calling it each time because the archive can change in the meantime long nentries = archive.EntryCount; - if (nentries < 0 || index >= (ulong)nentries) + if (nentries < 0) + return false; + + // Skip past any deleted entires + while (index < (ulong)nentries && ReadEntry(index) == null) { + ++index; + } + + if (index >= (ulong)nentries) return false; return true; } @@ -87,7 +95,7 @@ ZipEntry ReadEntry (ulong index) if (current != null && current.Index == index) return current; - current = archive.ReadEntry (index); + current = archive.ReadEntry (index, returnNullIfDeleted:true); return current; } } diff --git a/build_native b/build_native old mode 100755 new mode 100644 diff --git a/build_windows b/build_windows old mode 100755 new mode 100644