Merge pull request #1169 from adamhathcock/copilot/fix-zip-parsing-regression

Fix ZIP parsing failure on non-seekable streams with short reads

src/SharpCompress/IO/SharpCompressStream.cs

@@ -206,11 +206,11 @@ public class SharpCompressStream : Stream, IStreamStack
         {
             ValidateBufferState();
 
-            // Fill buffer if needed
+            // Fill buffer if needed, handling short reads from underlying stream
             if (_bufferedLength == 0)
             {
-                _bufferedLength = Stream.Read(_buffer!, 0, _bufferSize);
                 _bufferPosition = 0;
+                _bufferedLength = FillBuffer(_buffer!, 0, _bufferSize);
             }
             int available = _bufferedLength - _bufferPosition;
             int toRead = Math.Min(count, available);
@@ -222,11 +222,8 @@ public class SharpCompressStream : Stream, IStreamStack
                 return toRead;
             }
             // If buffer exhausted, refill
-            int r = Stream.Read(_buffer!, 0, _bufferSize);
-            if (r == 0)
-                return 0;
-            _bufferedLength = r;
             _bufferPosition = 0;
+            _bufferedLength = FillBuffer(_buffer!, 0, _bufferSize);
             if (_bufferedLength == 0)
             {
                 return 0;
@@ -250,6 +247,31 @@ public class SharpCompressStream : Stream, IStreamStack
         }
     }
 
+    /// <summary>
+    /// Fills the buffer by reading from the underlying stream, handling short reads.
+    /// Implements the ReadFully pattern: reads in a loop until buffer is full or EOF is reached.
+    /// </summary>
+    /// <param name="buffer">Buffer to fill</param>
+    /// <param name="offset">Offset in buffer (always 0 in current usage)</param>
+    /// <param name="count">Number of bytes to read</param>
+    /// <returns>Total number of bytes read (may be less than count if EOF is reached)</returns>
+    private int FillBuffer(byte[] buffer, int offset, int count)
+    {
+        // Implement ReadFully pattern but return the actual count read
+        // This is the same logic as Utility.ReadFully but returns count instead of bool
+        var total = 0;
+        int read;
+        while ((read = Stream.Read(buffer, offset + total, count - total)) > 0)
+        {
+            total += read;
+            if (total >= count)
+            {
+                return total;
+            }
+        }
+        return total;
+    }
+
     public override long Seek(long offset, SeekOrigin origin)
     {
         if (_bufferingEnabled)
@@ -324,13 +346,12 @@ public class SharpCompressStream : Stream, IStreamStack
         {
             ValidateBufferState();
 
-            // Fill buffer if needed
+            // Fill buffer if needed, handling short reads from underlying stream
             if (_bufferedLength == 0)
             {
-                _bufferedLength = await Stream
-                    .ReadAsync(_buffer!, 0, _bufferSize, cancellationToken)
-                    .ConfigureAwait(false);
                 _bufferPosition = 0;
+                _bufferedLength = await FillBufferAsync(_buffer!, 0, _bufferSize, cancellationToken)
+                    .ConfigureAwait(false);
             }
             int available = _bufferedLength - _bufferPosition;
             int toRead = Math.Min(count, available);
@@ -342,13 +363,9 @@ public class SharpCompressStream : Stream, IStreamStack
                 return toRead;
             }
             // If buffer exhausted, refill
-            int r = await Stream
-                .ReadAsync(_buffer!, 0, _bufferSize, cancellationToken)
-                .ConfigureAwait(false);
-            if (r == 0)
-                return 0;
-            _bufferedLength = r;
             _bufferPosition = 0;
+            _bufferedLength = await FillBufferAsync(_buffer!, 0, _bufferSize, cancellationToken)
+                .ConfigureAwait(false);
             if (_bufferedLength == 0)
             {
                 return 0;
@@ -369,6 +386,38 @@ public class SharpCompressStream : Stream, IStreamStack
         }
     }
 
+    /// <summary>
+    /// Async version of FillBuffer. Implements the ReadFullyAsync pattern.
+    /// Reads in a loop until buffer is full or EOF is reached.
+    /// </summary>
+    private async Task<int> FillBufferAsync(
+        byte[] buffer,
+        int offset,
+        int count,
+        CancellationToken cancellationToken
+    )
+    {
+        // Implement ReadFullyAsync pattern but return the actual count read
+        // This is the same logic as Utility.ReadFullyAsync but returns count instead of bool
+        var total = 0;
+        int read;
+        while (
+            (
+                read = await Stream
+                    .ReadAsync(buffer, offset + total, count - total, cancellationToken)
+                    .ConfigureAwait(false)
+            ) > 0
+        )
+        {
+            total += read;
+            if (total >= count)
+            {
+                return total;
+            }
+        }
+        return total;
+    }
+
     public override async Task WriteAsync(
         byte[] buffer,
         int offset,
@@ -399,13 +448,15 @@ public class SharpCompressStream : Stream, IStreamStack
         {
             ValidateBufferState();
 
-            // Fill buffer if needed
+            // Fill buffer if needed, handling short reads from underlying stream
             if (_bufferedLength == 0)
             {
-                _bufferedLength = await Stream
-                    .ReadAsync(_buffer.AsMemory(0, _bufferSize), cancellationToken)
-                    .ConfigureAwait(false);
                 _bufferPosition = 0;
+                _bufferedLength = await FillBufferMemoryAsync(
+                        _buffer.AsMemory(0, _bufferSize),
+                        cancellationToken
+                    )
+                    .ConfigureAwait(false);
             }
             int available = _bufferedLength - _bufferPosition;
             int toRead = Math.Min(buffer.Length, available);
@@ -417,13 +468,12 @@ public class SharpCompressStream : Stream, IStreamStack
                 return toRead;
             }
             // If buffer exhausted, refill
-            int r = await Stream
-                .ReadAsync(_buffer.AsMemory(0, _bufferSize), cancellationToken)
-                .ConfigureAwait(false);
-            if (r == 0)
-                return 0;
-            _bufferedLength = r;
             _bufferPosition = 0;
+            _bufferedLength = await FillBufferMemoryAsync(
+                    _buffer.AsMemory(0, _bufferSize),
+                    cancellationToken
+                )
+                .ConfigureAwait(false);
             if (_bufferedLength == 0)
             {
                 return 0;
@@ -442,6 +492,35 @@ public class SharpCompressStream : Stream, IStreamStack
         }
     }
 
+    /// <summary>
+    /// Async version of FillBuffer for Memory{byte}. Implements the ReadFullyAsync pattern.
+    /// Reads in a loop until buffer is full or EOF is reached.
+    /// </summary>
+    private async ValueTask<int> FillBufferMemoryAsync(
+        Memory<byte> buffer,
+        CancellationToken cancellationToken
+    )
+    {
+        // Implement ReadFullyAsync pattern but return the actual count read
+        var total = 0;
+        int read;
+        while (
+            (
+                read = await Stream
+                    .ReadAsync(buffer.Slice(total), cancellationToken)
+                    .ConfigureAwait(false)
+            ) > 0
+        )
+        {
+            total += read;
+            if (total >= buffer.Length)
+            {
+                return total;
+            }
+        }
+        return total;
+    }
+
     public override async ValueTask WriteAsync(
         ReadOnlyMemory<byte> buffer,
         CancellationToken cancellationToken = default

tests/SharpCompress.Test/Zip/ZipShortReadTests.cs

@@ -0,0 +1,117 @@
+using System;
+using System.Collections.Generic;
+using System.IO;
+using SharpCompress.Readers;
+using Xunit;
+
+namespace SharpCompress.Test.Zip;
+
+/// <summary>
+/// Tests for ZIP reading with streams that return short reads.
+/// Reproduces the regression where ZIP parsing fails depending on Stream.Read chunking patterns.
+/// </summary>
+public class ZipShortReadTests : ReaderTests
+{
+    /// <summary>
+    /// A non-seekable stream that returns controlled short reads.
+    /// Simulates real-world network/multipart streams that legally return fewer bytes than requested.
+    /// </summary>
+    private sealed class PatternReadStream : Stream
+    {
+        private readonly MemoryStream _inner;
+        private readonly int _firstReadSize;
+        private readonly int _chunkSize;
+        private bool _firstReadDone;
+
+        public PatternReadStream(byte[] bytes, int firstReadSize, int chunkSize)
+        {
+            _inner = new MemoryStream(bytes, writable: false);
+            _firstReadSize = firstReadSize;
+            _chunkSize = chunkSize;
+        }
+
+        public override int Read(byte[] buffer, int offset, int count)
+        {
+            int limit = !_firstReadDone ? _firstReadSize : _chunkSize;
+            _firstReadDone = true;
+
+            int toRead = Math.Min(count, limit);
+            return _inner.Read(buffer, offset, toRead);
+        }
+
+        public override bool CanRead => true;
+        public override bool CanSeek => false;
+        public override bool CanWrite => false;
+        public override long Length => throw new NotSupportedException();
+        public override long Position
+        {
+            get => throw new NotSupportedException();
+            set => throw new NotSupportedException();
+        }
+
+        public override void Flush() => throw new NotSupportedException();
+
+        public override long Seek(long offset, SeekOrigin origin) =>
+            throw new NotSupportedException();
+
+        public override void SetLength(long value) => throw new NotSupportedException();
+
+        public override void Write(byte[] buffer, int offset, int count) =>
+            throw new NotSupportedException();
+    }
+
+    /// <summary>
+    /// Test that ZIP reading works correctly with short reads on non-seekable streams.
+    /// Uses a test archive and different chunking patterns.
+    /// </summary>
+    [Theory]
+    [InlineData("Zip.deflate.zip", 1000, 4096)]
+    [InlineData("Zip.deflate.zip", 999, 4096)]
+    [InlineData("Zip.deflate.zip", 100, 4096)]
+    [InlineData("Zip.deflate.zip", 50, 512)]
+    [InlineData("Zip.deflate.zip", 1, 1)] // Extreme case: 1 byte at a time
+    [InlineData("Zip.deflate.dd.zip", 1000, 4096)]
+    [InlineData("Zip.deflate.dd.zip", 999, 4096)]
+    [InlineData("Zip.zip64.zip", 3816, 4096)]
+    [InlineData("Zip.zip64.zip", 3815, 4096)] // Similar to the issue pattern
+    public void Zip_Reader_Handles_Short_Reads(string zipFile, int firstReadSize, int chunkSize)
+    {
+        // Use an existing test ZIP file
+        var zipPath = Path.Combine(TEST_ARCHIVES_PATH, zipFile);
+        if (!File.Exists(zipPath))
+        {
+            return; // Skip if file doesn't exist
+        }
+
+        var bytes = File.ReadAllBytes(zipPath);
+
+        // Baseline with MemoryStream (seekable, no short reads)
+        var baseline = ReadEntriesFromStream(new MemoryStream(bytes, writable: false));
+        Assert.NotEmpty(baseline);
+
+        // Non-seekable stream with controlled short read pattern
+        var chunked = ReadEntriesFromStream(new PatternReadStream(bytes, firstReadSize, chunkSize));
+        Assert.Equal(baseline, chunked);
+    }
+
+    private List<string> ReadEntriesFromStream(Stream stream)
+    {
+        var names = new List<string>();
+        using var reader = ReaderFactory.Open(stream, new ReaderOptions { LeaveStreamOpen = true });
+
+        while (reader.MoveToNextEntry())
+        {
+            if (reader.Entry.IsDirectory)
+            {
+                continue;
+            }
+
+            names.Add(reader.Entry.Key!);
+
+            using var entryStream = reader.OpenEntryStream();
+            entryStream.CopyTo(Stream.Null);
+        }
+
+        return names;
+    }
+}

tests/SharpCompress.Test/packages.lock.json

@@ -29,6 +29,12 @@
           "Microsoft.NETFramework.ReferenceAssemblies.net48": "1.0.3"
         }
       },
+      "Mono.Posix.NETStandard": {
+        "type": "Direct",
+        "requested": "[1.0.0, )",
+        "resolved": "1.0.0",
+        "contentHash": "vSN/L1uaVwKsiLa95bYu2SGkF0iY3xMblTfxc8alSziPuVfJpj3geVqHGAA75J7cZkMuKpFVikz82Lo6y6LLdA=="
+      },
       "xunit": {
         "type": "Direct",
         "requested": "[2.9.3, )",
@@ -216,6 +222,12 @@
           "Microsoft.NETFramework.ReferenceAssemblies.net461": "1.0.3"
         }
       },
+      "Mono.Posix.NETStandard": {
+        "type": "Direct",
+        "requested": "[1.0.0, )",
+        "resolved": "1.0.0",
+        "contentHash": "vSN/L1uaVwKsiLa95bYu2SGkF0iY3xMblTfxc8alSziPuVfJpj3geVqHGAA75J7cZkMuKpFVikz82Lo6y6LLdA=="
+      },
       "xunit": {
         "type": "Direct",
         "requested": "[2.9.3, )",