diff --git a/src/SharpCompress/IO/SharpCompressStream.cs b/src/SharpCompress/IO/SharpCompressStream.cs index 101f7b163..e054be97d 100644 --- a/src/SharpCompress/IO/SharpCompressStream.cs +++ b/src/SharpCompress/IO/SharpCompressStream.cs @@ -206,11 +206,11 @@ public override int Read(byte[] buffer, int offset, int count) { 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 override int Read(byte[] buffer, int offset, int count) 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 override int Read(byte[] buffer, int offset, int count) } } + /// + /// 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. + /// + /// Buffer to fill + /// Offset in buffer (always 0 in current usage) + /// Number of bytes to read + /// Total number of bytes read (may be less than count if EOF is reached) + 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 @@ CancellationToken cancellationToken { 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 @@ CancellationToken cancellationToken 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 @@ CancellationToken cancellationToken } } + /// + /// Async version of FillBuffer. Implements the ReadFullyAsync pattern. + /// Reads in a loop until buffer is full or EOF is reached. + /// + private async Task 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 override async ValueTask ReadAsync( { 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 override async ValueTask ReadAsync( 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 override async ValueTask ReadAsync( } } + /// + /// Async version of FillBuffer for Memory{byte}. Implements the ReadFullyAsync pattern. + /// Reads in a loop until buffer is full or EOF is reached. + /// + private async ValueTask FillBufferMemoryAsync( + Memory 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 buffer, CancellationToken cancellationToken = default diff --git a/tests/SharpCompress.Test/Zip/ZipShortReadTests.cs b/tests/SharpCompress.Test/Zip/ZipShortReadTests.cs new file mode 100644 index 000000000..d61a27c70 --- /dev/null +++ b/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; + +/// +/// Tests for ZIP reading with streams that return short reads. +/// Reproduces the regression where ZIP parsing fails depending on Stream.Read chunking patterns. +/// +public class ZipShortReadTests : ReaderTests +{ + /// + /// A non-seekable stream that returns controlled short reads. + /// Simulates real-world network/multipart streams that legally return fewer bytes than requested. + /// + 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(); + } + + /// + /// Test that ZIP reading works correctly with short reads on non-seekable streams. + /// Uses a test archive and different chunking patterns. + /// + [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 ReadEntriesFromStream(Stream stream) + { + var names = new List(); + 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; + } +} diff --git a/tests/SharpCompress.Test/packages.lock.json b/tests/SharpCompress.Test/packages.lock.json index d36081cd8..7f87d400f 100644 --- a/tests/SharpCompress.Test/packages.lock.json +++ b/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, )",