Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 105 additions & 26 deletions src/SharpCompress/IO/SharpCompressStream.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
Expand All @@ -250,6 +247,31 @@ public override int Read(byte[] buffer, int offset, int count)
}
}

/// <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)
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand All @@ -369,6 +386,38 @@ CancellationToken cancellationToken
}
}

/// <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,
Expand Down Expand Up @@ -399,13 +448,15 @@ public override async ValueTask<int> 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);
Expand All @@ -417,13 +468,12 @@ public override async ValueTask<int> 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;
Expand All @@ -442,6 +492,35 @@ public override async ValueTask<int> ReadAsync(
}
}

/// <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
Expand Down
117 changes: 117 additions & 0 deletions tests/SharpCompress.Test/Zip/ZipShortReadTests.cs
Original file line number Diff line number Diff line change
@@ -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;
}
}
12 changes: 12 additions & 0 deletions tests/SharpCompress.Test/packages.lock.json
Original file line number Diff line number Diff line change
Expand Up @@ -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, )",
Expand Down Expand Up @@ -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, )",
Expand Down