Fix enumerating zip with deleted entries (#53)

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.

Co-authored-by: Jonathan Pryor <jonpryor@vt.edu>

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

@@ -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);
 		}
 
 		/// <summary>
@@ -692,11 +692,40 @@ public ZipEntry ReadEntry (string entryName, bool caseSensitive = false)
 		}
 
 		public ZipEntry ReadEntry (ulong index)
+		{
+			return ReadEntry (index, throwIfDeleted: true);
+		}
+
+		/// <summary>
+		/// Read a zip entry, given an index.
+		/// 
+		/// 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.
+		/// </summary>
+		/// <param name="index">index to read</param>
+		/// <param name="throwIfDeleted">whether to return null or throw an exception for deleted entries</param>
+		/// <returns></returns>
+		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 (ret < 0) {
+				if (! throwIfDeleted) {
+					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 ();

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, throwIfDeleted:false);
 			return current;
 		}
 	}