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.

Author: Bret Johnson

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<string>();
+			foreach (var entry in zip) {
+				actualEntries.Add(entry.FullName);
+			}
+
+			Assert.AreEqual(expectedEntries, actualEntries.ToArray());
+		}
 	}
 }

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)
+		/// <summary>
+		/// 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.
+		/// </summary>
+		/// <param name="index">index to read</param>
+		/// <param name="returnNullIfDeleted">whether to return null or throw an exception for deleted entries</param>
+		/// <returns></returns>
+		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;
 		}
 
+
 		/// <summary>
 		/// Closes the native ZIP archive handle and frees associated resources, if called from the finalizer.
 		/// </summary>

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;
 		}
 	}