From 970934a40b7e6d56184356c8412853966f7f30f3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 27 Jan 2026 15:51:50 +0000 Subject: [PATCH 1/5] Initial plan From a706a9d725e613c5ca79acf0fdaf1b07eec4b7c5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 27 Jan 2026 16:00:44 +0000 Subject: [PATCH 2/5] Fix ZIP parsing regression with short reads on non-seekable streams Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com> --- src/SharpCompress/IO/SharpCompressStream.cs | 113 +++++++++++++---- .../Zip/ZipShortReadTests.cs | 117 ++++++++++++++++++ tests/SharpCompress.Test/packages.lock.json | 12 ++ 3 files changed, 216 insertions(+), 26 deletions(-) create mode 100644 tests/SharpCompress.Test/Zip/ZipShortReadTests.cs diff --git a/src/SharpCompress/IO/SharpCompressStream.cs b/src/SharpCompress/IO/SharpCompressStream.cs index 101f7b16..59a3c52d 100644 --- a/src/SharpCompress/IO/SharpCompressStream.cs +++ b/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,30 @@ public class SharpCompressStream : Stream, IStreamStack } } + /// + /// Fills the buffer by reading from the underlying stream, handling short reads. + /// Continues reading until the buffer is full or EOF is reached. + /// This ensures that buffering properly handles non-seekable streams that return short reads. + /// + /// Buffer to fill + /// Offset in buffer + /// Number of bytes to read + /// Total number of bytes read + private int FillBuffer(byte[] buffer, int offset, int count) + { + int totalRead = 0; + while (totalRead < count) + { + int read = Stream.Read(buffer, offset + totalRead, count - totalRead); + if (read == 0) + { + break; // EOF reached + } + totalRead += read; + } + return totalRead; + } + public override long Seek(long offset, SeekOrigin origin) { if (_bufferingEnabled) @@ -324,13 +345,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 +362,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 +385,32 @@ public class SharpCompressStream : Stream, IStreamStack } } + /// + /// Async version of FillBuffer. Fills the buffer by reading from the underlying stream, handling short reads. + /// Continues reading until the buffer is full or EOF is reached. + /// + private async Task FillBufferAsync( + byte[] buffer, + int offset, + int count, + CancellationToken cancellationToken + ) + { + int totalRead = 0; + while (totalRead < count) + { + int read = await Stream + .ReadAsync(buffer, offset + totalRead, count - totalRead, cancellationToken) + .ConfigureAwait(false); + if (read == 0) + { + break; // EOF reached + } + totalRead += read; + } + return totalRead; + } + public override async Task WriteAsync( byte[] buffer, int offset, @@ -399,13 +441,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.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 +458,9 @@ 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 +479,30 @@ public class SharpCompressStream : Stream, IStreamStack } } + /// + /// Async version of FillBuffer for Memory{byte}. Fills the buffer by reading from the underlying stream, handling short reads. + /// Continues reading until the buffer is full or EOF is reached. + /// + private async ValueTask FillBufferMemoryAsync( + Memory buffer, + CancellationToken cancellationToken + ) + { + int totalRead = 0; + while (totalRead < buffer.Length) + { + int read = await Stream + .ReadAsync(buffer.Slice(totalRead), cancellationToken) + .ConfigureAwait(false); + if (read == 0) + { + break; // EOF reached + } + totalRead += read; + } + return totalRead; + } + 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 00000000..c3904331 --- /dev/null +++ b/tests/SharpCompress.Test/Zip/ZipShortReadTests.cs @@ -0,0 +1,117 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using SharpCompress.Common; +using SharpCompress.IO; +using SharpCompress.Readers; +using SharpCompress.Readers.Zip; +using SharpCompress.Test.Mocks; +using SharpCompress.Writers; +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 d36081cd..7f87d400 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, )", From 71655e04c474d3d93678a02401954e5d4e3e5d9e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 27 Jan 2026 16:02:26 +0000 Subject: [PATCH 3/5] Apply code formatting with CSharpier Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com> --- src/SharpCompress/IO/SharpCompressStream.cs | 10 ++++++++-- .../SharpCompress.Test/Zip/ZipShortReadTests.cs | 16 +++++++++++----- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/SharpCompress/IO/SharpCompressStream.cs b/src/SharpCompress/IO/SharpCompressStream.cs index 59a3c52d..e90a1b88 100644 --- a/src/SharpCompress/IO/SharpCompressStream.cs +++ b/src/SharpCompress/IO/SharpCompressStream.cs @@ -445,7 +445,10 @@ public class SharpCompressStream : Stream, IStreamStack if (_bufferedLength == 0) { _bufferPosition = 0; - _bufferedLength = await FillBufferMemoryAsync(_buffer.AsMemory(0, _bufferSize), cancellationToken) + _bufferedLength = await FillBufferMemoryAsync( + _buffer.AsMemory(0, _bufferSize), + cancellationToken + ) .ConfigureAwait(false); } int available = _bufferedLength - _bufferPosition; @@ -459,7 +462,10 @@ public class SharpCompressStream : Stream, IStreamStack } // If buffer exhausted, refill _bufferPosition = 0; - _bufferedLength = await FillBufferMemoryAsync(_buffer.AsMemory(0, _bufferSize), cancellationToken) + _bufferedLength = await FillBufferMemoryAsync( + _buffer.AsMemory(0, _bufferSize), + cancellationToken + ) .ConfigureAwait(false); if (_bufferedLength == 0) { diff --git a/tests/SharpCompress.Test/Zip/ZipShortReadTests.cs b/tests/SharpCompress.Test/Zip/ZipShortReadTests.cs index c3904331..1faa97a2 100644 --- a/tests/SharpCompress.Test/Zip/ZipShortReadTests.cs +++ b/tests/SharpCompress.Test/Zip/ZipShortReadTests.cs @@ -54,10 +54,16 @@ public class ZipShortReadTests : ReaderTests 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 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(); + + public override void Write(byte[] buffer, int offset, int count) => + throw new NotSupportedException(); } /// @@ -69,11 +75,11 @@ public class ZipShortReadTests : ReaderTests [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.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 + [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 @@ -82,7 +88,7 @@ public class ZipShortReadTests : ReaderTests { return; // Skip if file doesn't exist } - + var bytes = File.ReadAllBytes(zipPath); // Baseline with MemoryStream (seekable, no short reads) From d59e4c2a0dac46892189bee9f19f54e5b6998bea Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 27 Jan 2026 16:25:24 +0000 Subject: [PATCH 4/5] Refactor FillBuffer to use ReadFully pattern Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com> --- src/SharpCompress/IO/SharpCompressStream.cs | 78 ++++++++++++--------- 1 file changed, 45 insertions(+), 33 deletions(-) diff --git a/src/SharpCompress/IO/SharpCompressStream.cs b/src/SharpCompress/IO/SharpCompressStream.cs index e90a1b88..e054be97 100644 --- a/src/SharpCompress/IO/SharpCompressStream.cs +++ b/src/SharpCompress/IO/SharpCompressStream.cs @@ -249,26 +249,27 @@ public class SharpCompressStream : Stream, IStreamStack /// /// Fills the buffer by reading from the underlying stream, handling short reads. - /// Continues reading until the buffer is full or EOF is reached. - /// This ensures that buffering properly handles non-seekable streams that return short reads. + /// Implements the ReadFully pattern: reads in a loop until buffer is full or EOF is reached. /// /// Buffer to fill - /// Offset in buffer + /// Offset in buffer (always 0 in current usage) /// Number of bytes to read - /// Total number of bytes read + /// Total number of bytes read (may be less than count if EOF is reached) private int FillBuffer(byte[] buffer, int offset, int count) { - int totalRead = 0; - while (totalRead < 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) { - int read = Stream.Read(buffer, offset + totalRead, count - totalRead); - if (read == 0) + total += read; + if (total >= count) { - break; // EOF reached + return total; } - totalRead += read; } - return totalRead; + return total; } public override long Seek(long offset, SeekOrigin origin) @@ -386,8 +387,8 @@ public class SharpCompressStream : Stream, IStreamStack } /// - /// Async version of FillBuffer. Fills the buffer by reading from the underlying stream, handling short reads. - /// Continues reading until the buffer is full or EOF is reached. + /// 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, @@ -396,19 +397,25 @@ public class SharpCompressStream : Stream, IStreamStack CancellationToken cancellationToken ) { - int totalRead = 0; - while (totalRead < count) + // 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 + ) { - int read = await Stream - .ReadAsync(buffer, offset + totalRead, count - totalRead, cancellationToken) - .ConfigureAwait(false); - if (read == 0) + total += read; + if (total >= count) { - break; // EOF reached + return total; } - totalRead += read; } - return totalRead; + return total; } public override async Task WriteAsync( @@ -486,27 +493,32 @@ public class SharpCompressStream : Stream, IStreamStack } /// - /// Async version of FillBuffer for Memory{byte}. Fills the buffer by reading from the underlying stream, handling short reads. - /// Continues reading until the buffer is full or EOF is reached. + /// 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 ) { - int totalRead = 0; - while (totalRead < buffer.Length) + // 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 + ) { - int read = await Stream - .ReadAsync(buffer.Slice(totalRead), cancellationToken) - .ConfigureAwait(false); - if (read == 0) + total += read; + if (total >= buffer.Length) { - break; // EOF reached + return total; } - totalRead += read; } - return totalRead; + return total; } public override async ValueTask WriteAsync( From 8a3be35d676c7b181a3594d6ca17565f5d65e121 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Tue, 27 Jan 2026 16:43:13 +0000 Subject: [PATCH 5/5] Update tests/SharpCompress.Test/Zip/ZipShortReadTests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- tests/SharpCompress.Test/Zip/ZipShortReadTests.cs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tests/SharpCompress.Test/Zip/ZipShortReadTests.cs b/tests/SharpCompress.Test/Zip/ZipShortReadTests.cs index 1faa97a2..d61a27c7 100644 --- a/tests/SharpCompress.Test/Zip/ZipShortReadTests.cs +++ b/tests/SharpCompress.Test/Zip/ZipShortReadTests.cs @@ -1,13 +1,7 @@ using System; using System.Collections.Generic; using System.IO; -using System.Linq; -using SharpCompress.Common; -using SharpCompress.IO; using SharpCompress.Readers; -using SharpCompress.Readers.Zip; -using SharpCompress.Test.Mocks; -using SharpCompress.Writers; using Xunit; namespace SharpCompress.Test.Zip;