From 80b0a8f8e34154a706f80d3aa27cadc02d0baef7 Mon Sep 17 00:00:00 2001 From: Bret Johnson Date: Thu, 14 May 2020 19:42:43 -0400 Subject: [PATCH 1/6] 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 | 30 ++++++++++++++-- ZipEntryEnumerator.cs | 12 +++++-- build_native | 0 build_windows | 0 5 files changed, 97 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 3dcd72dc..4c6e69c4 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 a076d3a5..b0d4acac 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 (); diff --git a/ZipEntryEnumerator.cs b/ZipEntryEnumerator.cs index ce3df7da..e91689f8 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 From 65fa52a25f5b6ba72145f11359d569aa0ac7e475 Mon Sep 17 00:00:00 2001 From: Bret Johnson Date: Fri, 15 May 2020 12:21:10 -0400 Subject: [PATCH 2/6] Changed parameter to throwIfDeleted, per PR suggestion --- ZipArchive.cs | 23 ++++++++++++++--------- ZipEntryEnumerator.cs | 2 +- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/ZipArchive.cs b/ZipArchive.cs index b0d4acac..42b4dda4 100644 --- a/ZipArchive.cs +++ b/ZipArchive.cs @@ -368,8 +368,8 @@ public ZipEntry AddFileToDirectory (string sourcePath, string archiveDirectory = string destDir = NormalizeArchivePath (true, archiveDirectory); string destFile = useFileDirectory ? GetRootlessPath (sourcePath) : Path.GetFileName (sourcePath); return AddFile (sourcePath, - String.IsNullOrEmpty (destDir) ? null : destDir + "/" + destFile, - permissions, compressionMethod, overwriteExisting); + String.IsNullOrEmpty (destDir) ? null : destDir + "/" + destFile, + permissions, compressionMethod, overwriteExisting); } /// @@ -691,23 +691,28 @@ public ZipEntry ReadEntry (string entryName, bool caseSensitive = false) return ReadEntry ((ulong)LookupEntry (entryName, caseSensitive)); } + public ZipEntry ReadEntry(ulong index) + { + return ReadEntry (index, throwIfDeleted: true); + } + /// /// 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. + /// When throwIfDeleted is true, 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 throwIfDeleted is false 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 + /// whether to return null or throw an exception for deleted entries /// - public ZipEntry ReadEntry (ulong index, bool returnNullIfDeleted = false) + public ZipEntry ReadEntry (ulong index, bool throwIfDeleted) { Native.zip_stat_t stat; int ret = Native.zip_stat_index (archive, index, OperationFlags.None, out stat); if (ret < 0) { - if (returnNullIfDeleted) { + if (! throwIfDeleted) { IntPtr error = Native.zip_get_error(archive); if (error != IntPtr.Zero) { diff --git a/ZipEntryEnumerator.cs b/ZipEntryEnumerator.cs index e91689f8..6dd30efa 100644 --- a/ZipEntryEnumerator.cs +++ b/ZipEntryEnumerator.cs @@ -95,7 +95,7 @@ ZipEntry ReadEntry (ulong index) if (current != null && current.Index == index) return current; - current = archive.ReadEntry (index, returnNullIfDeleted:true); + current = archive.ReadEntry (index, throwIfDeleted:false); return current; } } From 19a3d03d86db4c71a58a83996e67d78896facb90 Mon Sep 17 00:00:00 2001 From: Bret Johnson Date: Fri, 15 May 2020 12:25:58 -0400 Subject: [PATCH 3/6] Undo unintended file attributes change --- build_native | 0 build_windows | 0 2 files changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 build_native mode change 100644 => 100755 build_windows diff --git a/build_native b/build_native old mode 100644 new mode 100755 diff --git a/build_windows b/build_windows old mode 100644 new mode 100755 From 64a604f5aac3d4ce258f0a568bb0ac86c7dac91a Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Fri, 15 May 2020 13:56:39 -0400 Subject: [PATCH 4/6] Fix code formatting --- LibZipSharp.UnitTest/ZipTests.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/LibZipSharp.UnitTest/ZipTests.cs b/LibZipSharp.UnitTest/ZipTests.cs index 4c6e69c4..ba95f72b 100644 --- a/LibZipSharp.UnitTest/ZipTests.cs +++ b/LibZipSharp.UnitTest/ZipTests.cs @@ -159,13 +159,13 @@ public void SmallTextFile () } } - [TestCase(false)] - [TestCase(true)] - public void EnumerateSkipDeletedEntries(bool deleteFromExistingFile) + [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"); + string filePath = Path.GetFullPath ("file1.txt"); if (File.Exists ("test-archive-write.zip")) File.Delete ("test-archive-write.zip"); @@ -204,7 +204,7 @@ public void EnumerateSkipDeletedEntries(bool deleteFromExistingFile) ValidateEnumeratedEntries (zip); } finally { - zip?.Dispose(); + zip?.Dispose (); } } @@ -212,10 +212,10 @@ void ValidateEnumeratedEntries (ZipArchive zip, params string[] expectedEntries) { var actualEntries = new List(); foreach (var entry in zip) { - actualEntries.Add(entry.FullName); + actualEntries.Add (entry.FullName); } - Assert.AreEqual(expectedEntries, actualEntries.ToArray()); + Assert.AreEqual (expectedEntries, actualEntries.ToArray ()); } } -} \ No newline at end of file +} From b539ce93628feef4ed05c4f42eafc9a59c8e17da Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Fri, 15 May 2020 13:58:26 -0400 Subject: [PATCH 5/6] Fix formatting --- ZipArchive.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/ZipArchive.cs b/ZipArchive.cs index 42b4dda4..917f7a55 100644 --- a/ZipArchive.cs +++ b/ZipArchive.cs @@ -368,8 +368,8 @@ public ZipEntry AddFileToDirectory (string sourcePath, string archiveDirectory = string destDir = NormalizeArchivePath (true, archiveDirectory); string destFile = useFileDirectory ? GetRootlessPath (sourcePath) : Path.GetFileName (sourcePath); return AddFile (sourcePath, - String.IsNullOrEmpty (destDir) ? null : destDir + "/" + destFile, - permissions, compressionMethod, overwriteExisting); + String.IsNullOrEmpty (destDir) ? null : destDir + "/" + destFile, + permissions, compressionMethod, overwriteExisting); } /// @@ -691,7 +691,7 @@ public ZipEntry ReadEntry (string entryName, bool caseSensitive = false) return ReadEntry ((ulong)LookupEntry (entryName, caseSensitive)); } - public ZipEntry ReadEntry(ulong index) + public ZipEntry ReadEntry (ulong index) { return ReadEntry (index, throwIfDeleted: true); } @@ -713,10 +713,10 @@ public ZipEntry ReadEntry (ulong index, bool throwIfDeleted) int ret = Native.zip_stat_index (archive, index, OperationFlags.None, out stat); if (ret < 0) { if (! throwIfDeleted) { - IntPtr error = Native.zip_get_error(archive); + IntPtr error = Native.zip_get_error (archive); if (error != IntPtr.Zero) { - int zip_error = Native.zip_error_code_zip(error); + 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) @@ -724,7 +724,7 @@ public ZipEntry ReadEntry (ulong index, bool throwIfDeleted) } } - throw GetErrorException(); + throw GetErrorException (); } var ze = ZipEntry.Create (this, stat); From 1bce09645efa1bfe6264a6a7fd42602e725c2bbc Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Fri, 15 May 2020 13:59:00 -0400 Subject: [PATCH 6/6] Fix formatting --- ZipEntryEnumerator.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ZipEntryEnumerator.cs b/ZipEntryEnumerator.cs index 6dd30efa..a9a429ce 100644 --- a/ZipEntryEnumerator.cs +++ b/ZipEntryEnumerator.cs @@ -66,7 +66,7 @@ public bool MoveNext () return false; // Skip past any deleted entires - while (index < (ulong)nentries && ReadEntry(index) == null) { + while (index < (ulong)nentries && ReadEntry (index) == null) { ++index; }