fix(zip): handle iterating updated entries in ZipInputStream (#642)

Author: piksel

src/ICSharpCode.SharpZipLib/Zip/ZipInputStream.cs

@@ -3,6 +3,7 @@
 using ICSharpCode.SharpZipLib.Zip.Compression;
 using ICSharpCode.SharpZipLib.Zip.Compression.Streams;
 using System;
+using System.Diagnostics;
 using System.IO;
 
 namespace ICSharpCode.SharpZipLib.Zip
@@ -181,31 +182,12 @@ public ZipEntry GetNextEntry()
 				CloseEntry();
 			}
 
-			int header = inputBuffer.ReadLeInt();
-
-			if (header == ZipConstants.CentralHeaderSignature ||
-				header == ZipConstants.EndOfCentralDirectorySignature ||
-				header == ZipConstants.CentralHeaderDigitalSignature ||
-				header == ZipConstants.ArchiveExtraDataSignature ||
-				header == ZipConstants.Zip64CentralFileHeaderSignature)
+			if (!SkipUntilNextEntry())
 			{
-				// No more individual entries exist
 				Dispose();
 				return null;
 			}
 
-			// -jr- 07-Dec-2003 Ignore spanning temporary signatures if found
-			// Spanning signature is same as descriptor signature and is untested as yet.
-			if ((header == ZipConstants.SpanningTempSignature) || (header == ZipConstants.SpanningSignature))
-			{
-				header = inputBuffer.ReadLeInt();
-			}
-
-			if (header != ZipConstants.LocalHeaderSignature)
-			{
-				throw new ZipException("Wrong Local header signature: 0x" + String.Format("{0:X}", header));
-			}
-
 			var versionRequiredToExtract = (short)inputBuffer.ReadLeShort();
 
 			flags = inputBuffer.ReadLeShort();
@@ -303,6 +285,54 @@ public ZipEntry GetNextEntry()
 			return entry;
 		}
 
+		/// <summary>
+		/// Reads bytes from the input stream until either a local file header signature, or another signature
+		/// indicating that no more entries should be present, is found.
+		/// </summary>
+		/// <exception cref="ZipException">Thrown if the end of the input stream is reached without any signatures found</exception>
+		/// <returns>Returns whether the found signature is for a local entry header</returns>
+		private bool SkipUntilNextEntry()
+		{
+			// First let's skip all null bytes since it's the sane padding to add when updating an entry with smaller size
+			var paddingSkipped = 0;
+			while(inputBuffer.ReadLeByte() == 0) {
+				paddingSkipped++;
+			}
+			
+			// Last byte read was not actually consumed, restore the offset
+			inputBuffer.Available += 1;
+			if(paddingSkipped > 0) {
+				Debug.WriteLine("Skipped {0} null byte(s) before reading signature", paddingSkipped);
+			}
+			
+			var offset = 0;
+			// Read initial header quad directly after the last entry
+			var header = (uint)inputBuffer.ReadLeInt();
+			do
+			{
+				switch (header)
+				{
+					case ZipConstants.CentralHeaderSignature:
+					case ZipConstants.EndOfCentralDirectorySignature:
+					case ZipConstants.CentralHeaderDigitalSignature:
+					case ZipConstants.ArchiveExtraDataSignature:
+					case ZipConstants.Zip64CentralFileHeaderSignature:
+						Debug.WriteLine("Non-entry signature found at offset {0,2}: 0x{1:x8}", offset, header);
+						// No more individual entries exist
+						return false;
+
+					case ZipConstants.LocalHeaderSignature:
+						Debug.WriteLine("Entry local header signature found at offset {0,2}: 0x{1:x8}", offset, header);
+						return true;
+					default:
+						// Current header quad did not match any signature, shift in another byte
+						header = (uint) (inputBuffer.ReadLeByte() << 24) | (header >> 8);
+						offset++;
+						break;
+				}
+			} while (true); // Loop until we either get an EOF exception or we find the next signature
+		}
+
 		/// <summary>
 		/// Read data descriptor at the end of compressed data.
 		/// </summary>
@@ -400,6 +430,7 @@ public void CloseEntry()
 
 			if ((inputBuffer.Available > csize) && (csize >= 0))
 			{
+				// Buffer can contain entire entry data. Internally offsetting position inside buffer
 				inputBuffer.Available = (int)((long)inputBuffer.Available - csize);
 			}
 			else

test/ICSharpCode.SharpZipLib.Tests/TestSupport/Utils.cs

@@ -1,6 +1,9 @@
 using NUnit.Framework;
 using System;
+using System.Diagnostics;
 using System.IO;
+using System.Text;
+using ICSharpCode.SharpZipLib.Tests.Zip;
 using System.Linq;
 using System.Threading.Tasks;
 
@@ -11,7 +14,10 @@ namespace ICSharpCode.SharpZipLib.Tests.TestSupport
 	/// </summary>
 	public static class Utils
 	{
+		public static int DummyContentLength = 16;
+
 		internal const int DefaultSeed = 5;
+		private static Random random = new Random(DefaultSeed);
 
 		/// <summary>
 		/// Returns the system root for the current platform (usually c:\ for windows and / for others)
@@ -115,6 +121,30 @@ public static string GetDummyFileName()
 		/// </summary>
 		/// <returns></returns>
 		public static TempFile GetTempFile() => new TempFile();
+
+		public static void PatchFirstEntrySize(Stream stream, int newSize)
+		{
+			using(stream)
+			{
+				var sizeBytes = BitConverter.GetBytes(newSize);
+
+				stream.Seek(18, SeekOrigin.Begin);
+				stream.Write(sizeBytes, 0, 4);
+				stream.Write(sizeBytes, 0, 4);
+			}
+		}
+	}
+	
+	public class TestTraceListener : TraceListener
+	{
+		private readonly TextWriter _writer;
+		public TestTraceListener(TextWriter writer)
+		{
+			_writer = writer;
+		}
+
+		public override void WriteLine(string message) => _writer.WriteLine(message);
+		public override void Write(string message) => _writer.Write(message);
 	}
 
 	public class TempFile : FileSystemInfo, IDisposable
@@ -137,6 +167,8 @@ public override void Delete()
 			_fileInfo.Delete();
 	    }
 
+		public FileStream Open(FileMode mode, FileAccess access) => _fileInfo.Open(mode, access);
+		public FileStream Open(FileMode mode) => _fileInfo.Open(mode);
 		public FileStream Create() => _fileInfo.Create();
 
 	    public static TempFile WithDummyData(int size, string dirPath = null, string filename = null, int seed = Utils.DefaultSeed)
@@ -182,9 +214,10 @@ public void Dispose()
     	}
 
     	#endregion IDisposable Support
-
-        
 	}
+  
+  
+  
     public class TempDir : FileSystemInfo, IDisposable
     {
 	    public override string Name => Path.GetFileName(FullName);

test/ICSharpCode.SharpZipLib.Tests/Zip/StreamHandling.cs

@@ -3,7 +3,10 @@
 using ICSharpCode.SharpZipLib.Zip;
 using NUnit.Framework;
 using System;
+using System.Diagnostics;
 using System.IO;
+using System.Linq;
+using System.Text;
 using Does = ICSharpCode.SharpZipLib.Tests.TestSupport.Does;
 
 namespace ICSharpCode.SharpZipLib.Tests.Zip
@@ -14,6 +17,12 @@ namespace ICSharpCode.SharpZipLib.Tests.Zip
 	[TestFixture]
 	public class StreamHandling : ZipBase
 	{
+		private TestTraceListener Listener;
+		[SetUp]
+		public void Init() => Trace.Listeners.Add(Listener = new TestTraceListener(TestContext.Out));
+		[TearDown]
+		public void Deinit() => Trace.Listeners.Remove(Listener);
+
 		private void MustFailRead(Stream s, byte[] buffer, int offset, int count)
 		{
 			bool exception = false;
@@ -540,5 +549,42 @@ public void ShouldThrowDescriptiveExceptionOnUncompressedDescriptorEntry()
 				});
 			}
 		}
+		
+		[Test]
+		[Category("Zip")]
+		public void IteratingOverEntriesInDirectUpdatedArchive([Values(0x0, 0x80)] byte padding)
+		{
+			using (var tempFile = new TempFile())
+			{
+				using (var zf = ZipFile.Create(tempFile))
+				{
+					zf.BeginUpdate();
+					// Add a "large" file, where the bottom 1023 bytes will become padding
+					var contentsAndPadding = Enumerable.Repeat(padding, count: 1024).ToArray();
+					zf.Add(new MemoryDataSource(contentsAndPadding), "FirstFile", CompressionMethod.Stored);
+					// Add a second file after the first one
+					zf.Add(new StringMemoryDataSource("fileContents"), "SecondFile", CompressionMethod.Stored);
+					zf.CommitUpdate();
+				}
+
+				// Since ZipFile doesn't support UpdateCommand.Modify yet we'll have to simulate it by patching the header
+				Utils.PatchFirstEntrySize(tempFile.Open(FileMode.Open), 1);
+
+				// Iterate updated entries
+				using (var fs = File.OpenRead(tempFile))
+				using (var zis = new ZipInputStream(fs))
+				{
+					var firstEntry = zis.GetNextEntry();
+					Assert.NotNull(firstEntry);
+					Assert.AreEqual(1, firstEntry.CompressedSize);
+					Assert.AreEqual(1, firstEntry.Size);
+					
+					var secondEntry = zis.GetNextEntry();
+					Assert.NotNull(secondEntry, "Zip entry following padding not found");
+					var contents = new StreamReader(zis, Encoding.UTF8, false, 128, true).ReadToEnd();
+					Assert.AreEqual("fileContents", contents);
+				}
+			}
+		}
 	}
 }