From 8759cf08ff38d6f8ee1644255fa5728f28367b7e Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Wed, 4 Feb 2026 08:20:05 +0000 Subject: [PATCH] tests all pass --- .../Zip/StreamingZipHeaderFactory.Async.cs | 45 +--- .../Common/Zip/StreamingZipHeaderFactory.cs | 37 +-- src/SharpCompress/IO/IStreamStack.cs | 22 +- .../IO/RewindableStream.Async.cs | 211 ++++++++++++++++ src/SharpCompress/IO/RewindableStream.cs | 230 +++++++++++++++++- 5 files changed, 461 insertions(+), 84 deletions(-) diff --git a/src/SharpCompress/Common/Zip/StreamingZipHeaderFactory.Async.cs b/src/SharpCompress/Common/Zip/StreamingZipHeaderFactory.Async.cs index dded9a20..71004b77 100644 --- a/src/SharpCompress/Common/Zip/StreamingZipHeaderFactory.Async.cs +++ b/src/SharpCompress/Common/Zip/StreamingZipHeaderFactory.Async.cs @@ -289,43 +289,14 @@ internal sealed partial class StreamingZipHeaderFactory else if (localHeader.Flags.HasFlag(HeaderFlags.UsePostDataDescriptor)) { // Peek ahead to check if next data is a header or file data. - // For SeekableRewindableStream, use direct position save/restore to avoid - // interfering with any recording state set by the caller (e.g., ReaderFactory). - // Plain RewindableStream can use StartRecording/Rewind safely since it was - // created fresh by EnsureSeekable and isn't shared with the caller. - if (_rewindableStream is SeekableRewindableStream) - { - var savedPosition = _rewindableStream.Position; - var nextHeaderBytes = await _reader - .ReadUInt32Async(_cancellationToken) - .ConfigureAwait(false); - _rewindableStream.Position = savedPosition; - header.HasData = !IsHeader(nextHeaderBytes); - } - else - { - // Only start recording if not already recording. - // The stream may already be recording if it was created by ReaderFactory. - if (!_rewindableStream.IsRecording) - { - _rewindableStream.StartRecording(); - var nextHeaderBytes = await _reader - .ReadUInt32Async(_cancellationToken) - .ConfigureAwait(false); - _rewindableStream.Rewind(true); - header.HasData = !IsHeader(nextHeaderBytes); - } - else - { - // If already recording, save position and restore after peek - var savedPosition = _rewindableStream.Position; - var nextHeaderBytes = await _reader - .ReadUInt32Async(_cancellationToken) - .ConfigureAwait(false); - _rewindableStream.Position = savedPosition; - header.HasData = !IsHeader(nextHeaderBytes); - } - } + // Use the IStreamStack.Rewind mechanism to give back the peeked bytes. + var nextHeaderBytes = await _reader + .ReadUInt32Async(_cancellationToken) + .ConfigureAwait(false); + ((IStreamStack)_rewindableStream).Rewind(sizeof(uint)); + + // Check if next data is PostDataDescriptor, streamed file with 0 length + header.HasData = !IsHeader(nextHeaderBytes); } else // We are not streaming and compressed size is 0, we have no data { diff --git a/src/SharpCompress/Common/Zip/StreamingZipHeaderFactory.cs b/src/SharpCompress/Common/Zip/StreamingZipHeaderFactory.cs index 13d15ef6..e7091d42 100644 --- a/src/SharpCompress/Common/Zip/StreamingZipHeaderFactory.cs +++ b/src/SharpCompress/Common/Zip/StreamingZipHeaderFactory.cs @@ -174,37 +174,12 @@ internal partial class StreamingZipHeaderFactory : ZipHeaderFactory else if (local_header.Flags.HasFlag(HeaderFlags.UsePostDataDescriptor)) { // Peek ahead to check if next data is a header or file data. - // For SeekableRewindableStream, use direct position save/restore to avoid - // interfering with any recording state set by the caller (e.g., ReaderFactory). - // Plain RewindableStream can use StartRecording/Rewind safely since it was - // created fresh by EnsureSeekable and isn't shared with the caller. - if (rewindableStream is SeekableRewindableStream) - { - var savedPosition = rewindableStream.Position; - var nextHeaderBytes = reader.ReadUInt32(); - rewindableStream.Position = savedPosition; - header.HasData = !IsHeader(nextHeaderBytes); - } - else - { - // Only start recording if not already recording. - // The stream may already be recording if it was created by ReaderFactory. - if (!rewindableStream.IsRecording) - { - rewindableStream.StartRecording(); - var nextHeaderBytes = reader.ReadUInt32(); - rewindableStream.Rewind(true); - header.HasData = !IsHeader(nextHeaderBytes); - } - else - { - // If already recording, save position and restore after peek - var savedPosition = rewindableStream.Position; - var nextHeaderBytes = reader.ReadUInt32(); - rewindableStream.Position = savedPosition; - header.HasData = !IsHeader(nextHeaderBytes); - } - } + // Use the IStreamStack.Rewind mechanism to give back the peeked bytes. + var nextHeaderBytes = reader.ReadUInt32(); + ((IStreamStack)rewindableStream).Rewind(sizeof(uint)); + + // Check if next data is PostDataDescriptor, streamed file with 0 length + header.HasData = !IsHeader(nextHeaderBytes); } else // We are not streaming and compressed size is 0, we have no data { diff --git a/src/SharpCompress/IO/IStreamStack.cs b/src/SharpCompress/IO/IStreamStack.cs index d708402a..df9156c4 100644 --- a/src/SharpCompress/IO/IStreamStack.cs +++ b/src/SharpCompress/IO/IStreamStack.cs @@ -52,19 +52,31 @@ namespace SharpCompress.IO internal static void Rewind(this IStreamStack stream, int count) { - IStreamStack? buffStream = null; IStreamStack? current = stream; - while (buffStream == null && current != null) + while (current != null) { if (current is RewindableStream rewindableStream) { - buffStream = current; - rewindableStream.Position -= Math.Min(rewindableStream.Position, count); + // Try to rewind within the buffer. If the position is outside the buffered + // region, silently ignore (matching release behavior where streams without + // buffering simply didn't rewind). + var targetPosition = rewindableStream.Position - count; + if (targetPosition >= 0) + { + try + { + rewindableStream.Position = targetPosition; + } + catch (NotSupportedException) + { + // Cannot seek outside buffered region - silently ignore + } + } + return; } current = current.BaseStream() as IStreamStack; } } - } } diff --git a/src/SharpCompress/IO/RewindableStream.Async.cs b/src/SharpCompress/IO/RewindableStream.Async.cs index 799754bd..b2c6dbae 100644 --- a/src/SharpCompress/IO/RewindableStream.Async.cs +++ b/src/SharpCompress/IO/RewindableStream.Async.cs @@ -19,6 +19,39 @@ internal partial class RewindableStream return 0; } + // If recording is active or we're reading from the recording buffer, use legacy behavior + if (IsRecording || (isRewound && bufferStream.Position != bufferStream.Length)) + { + return await ReadWithRecordingAsync(buffer, offset, count, cancellationToken) + .ConfigureAwait(false); + } + + // If rolling buffer is enabled (and not recording), use rolling buffer logic + if (_rollingBuffer is not null) + { + return await ReadWithRollingBufferAsync(buffer, offset, count, cancellationToken) + .ConfigureAwait(false); + } + + // No buffering - read directly from stream + int read = await stream + .ReadAsync(buffer, offset, count, cancellationToken) + .ConfigureAwait(false); + streamPosition += read; + _logicalPosition = streamPosition; + return read; + } + + /// + /// Async version of ReadWithRecording (legacy behavior for format detection). + /// + private async Task ReadWithRecordingAsync( + byte[] buffer, + int offset, + int count, + CancellationToken cancellationToken + ) + { int read; if (isRewound && bufferStream.Position != bufferStream.Length) { @@ -37,7 +70,14 @@ internal partial class RewindableStream .WriteAsync(buffer, offset + read, tempRead, cancellationToken) .ConfigureAwait(false); } + else if (_rollingBuffer is not null && tempRead > 0) + { + // When transitioning out of recording mode, add to rolling buffer + // so that future rewinds will work + AddToRollingBuffer(buffer, offset + read, tempRead); + } streamPosition += tempRead; + _logicalPosition = streamPosition; read += tempRead; } if (bufferStream.Position == bufferStream.Length) @@ -57,9 +97,75 @@ internal partial class RewindableStream .ConfigureAwait(false); } streamPosition += read; + _logicalPosition = streamPosition; return read; } + /// + /// Async version of ReadWithRollingBuffer. + /// + private async Task ReadWithRollingBufferAsync( + byte[] buffer, + int offset, + int count, + CancellationToken cancellationToken + ) + { + int totalRead = 0; + + // If logical position is behind stream position, read from rolling buffer first + while (count > 0 && _logicalPosition < streamPosition) + { + long bytesFromEnd = streamPosition - _logicalPosition; + if (bytesFromEnd > _rollingBufferLength) + { + throw new InvalidOperationException( + "Logical position is outside rolling buffer range." + ); + } + + int bufferIndex = (int)( + (_rollingBufferWritePos - bytesFromEnd + _rollingBufferSize) % _rollingBufferSize + ); + int availableFromBuffer = (int)Math.Min(bytesFromEnd, count); + + int firstPart = Math.Min(availableFromBuffer, _rollingBufferSize - bufferIndex); + Array.Copy(_rollingBuffer!, bufferIndex, buffer, offset, firstPart); + if (firstPart < availableFromBuffer) + { + Array.Copy( + _rollingBuffer!, + 0, + buffer, + offset + firstPart, + availableFromBuffer - firstPart + ); + } + + totalRead += availableFromBuffer; + offset += availableFromBuffer; + count -= availableFromBuffer; + _logicalPosition += availableFromBuffer; + } + + // If more data needed, read from underlying stream + if (count > 0) + { + int read = await stream + .ReadAsync(buffer, offset, count, cancellationToken) + .ConfigureAwait(false); + if (read > 0) + { + AddToRollingBuffer(buffer, offset, read); + streamPosition += read; + _logicalPosition += read; + totalRead += read; + } + } + + return totalRead; + } + #if !LEGACY_DOTNET public override async ValueTask ReadAsync( Memory buffer, @@ -71,6 +177,34 @@ internal partial class RewindableStream return 0; } + // If recording is active or we're reading from the recording buffer, use legacy behavior + if (IsRecording || (isRewound && bufferStream.Position != bufferStream.Length)) + { + return await ReadWithRecordingAsync(buffer, cancellationToken).ConfigureAwait(false); + } + + // If rolling buffer is enabled (and not recording), use rolling buffer logic + if (_rollingBuffer is not null) + { + return await ReadWithRollingBufferAsync(buffer, cancellationToken) + .ConfigureAwait(false); + } + + // No buffering - read directly from stream + int read = await stream.ReadAsync(buffer, cancellationToken).ConfigureAwait(false); + streamPosition += read; + _logicalPosition = streamPosition; + return read; + } + + /// + /// Async version of ReadWithRecording for Memory<byte> (legacy behavior for format detection). + /// + private async ValueTask ReadWithRecordingAsync( + Memory buffer, + CancellationToken cancellationToken + ) + { int read; if (isRewound && bufferStream.Position != bufferStream.Length) { @@ -90,7 +224,15 @@ internal partial class RewindableStream .WriteAsync(buffer.Slice(read, tempRead), cancellationToken) .ConfigureAwait(false); } + else if (_rollingBuffer is not null && tempRead > 0) + { + // When transitioning out of recording mode, add to rolling buffer + // so that future rewinds will work + var tempBuffer = buffer.Slice(read, tempRead).ToArray(); + AddToRollingBuffer(tempBuffer, 0, tempRead); + } streamPosition += tempRead; + _logicalPosition = streamPosition; read += tempRead; } if (bufferStream.Position == bufferStream.Length) @@ -108,8 +250,72 @@ internal partial class RewindableStream .ConfigureAwait(false); } streamPosition += read; + _logicalPosition = streamPosition; return read; } + + /// + /// Async version of ReadWithRollingBuffer for Memory<byte>. + /// + private async ValueTask ReadWithRollingBufferAsync( + Memory buffer, + CancellationToken cancellationToken + ) + { + int totalRead = 0; + int count = buffer.Length; + int offset = 0; + + // If logical position is behind stream position, read from rolling buffer first + while (count > 0 && _logicalPosition < streamPosition) + { + long bytesFromEnd = streamPosition - _logicalPosition; + if (bytesFromEnd > _rollingBufferLength) + { + throw new InvalidOperationException( + "Logical position is outside rolling buffer range." + ); + } + + int bufferIndex = (int)( + (_rollingBufferWritePos - bytesFromEnd + _rollingBufferSize) % _rollingBufferSize + ); + int availableFromBuffer = (int)Math.Min(bytesFromEnd, count); + + int firstPart = Math.Min(availableFromBuffer, _rollingBufferSize - bufferIndex); + _rollingBuffer.AsSpan(bufferIndex, firstPart).CopyTo(buffer.Span.Slice(offset)); + if (firstPart < availableFromBuffer) + { + _rollingBuffer + .AsSpan(0, availableFromBuffer - firstPart) + .CopyTo(buffer.Span.Slice(offset + firstPart)); + } + + totalRead += availableFromBuffer; + offset += availableFromBuffer; + count -= availableFromBuffer; + _logicalPosition += availableFromBuffer; + } + + // If more data needed, read from underlying stream + if (count > 0) + { + int read = await stream + .ReadAsync(buffer.Slice(offset, count), cancellationToken) + .ConfigureAwait(false); + if (read > 0) + { + // AddToRollingBuffer expects byte[], so we need to copy + var tempBuffer = buffer.Slice(offset, read).ToArray(); + AddToRollingBuffer(tempBuffer, 0, read); + streamPosition += read; + _logicalPosition += read; + totalRead += read; + } + } + + return totalRead; + } #endif public override Task WriteAsync( @@ -150,6 +356,11 @@ internal partial class RewindableStream { isDisposed = true; await stream.DisposeAsync(); + if (_rollingBuffer is not null) + { + System.Buffers.ArrayPool.Shared.Return(_rollingBuffer); + _rollingBuffer = null; + } } } #endif diff --git a/src/SharpCompress/IO/RewindableStream.cs b/src/SharpCompress/IO/RewindableStream.cs index b6c734cd..9f7229ee 100644 --- a/src/SharpCompress/IO/RewindableStream.cs +++ b/src/SharpCompress/IO/RewindableStream.cs @@ -1,4 +1,5 @@ using System; +using System.Buffers; using System.IO; namespace SharpCompress.IO; @@ -13,7 +14,41 @@ internal partial class RewindableStream : Stream, IStreamStack private bool isDisposed; private long streamPosition; - public RewindableStream(Stream stream) => this.stream = stream; + // Rolling buffer for limited backward seeking without unbounded memory growth. + // This is a circular buffer that keeps the last N bytes read from the stream. + private byte[]? _rollingBuffer; + private int _rollingBufferSize; + private int _rollingBufferWritePos; // Next write position in circular buffer + private int _rollingBufferLength; // Number of valid bytes in rolling buffer (0 to _rollingBufferSize) + private long _logicalPosition; // The current logical read position (can be behind streamPosition) + + /// + /// Default size for rolling buffer (same as .NET Stream.CopyTo default) + /// + public const int DefaultRollingBufferSize = 81920; + + public RewindableStream(Stream stream) + { + this.stream = stream; + _logicalPosition = 0; + } + + /// + /// Creates a RewindableStream with a rolling buffer that enables limited backward seeking. + /// + /// The underlying stream to wrap. + /// Size of the rolling buffer in bytes. + public RewindableStream(Stream stream, int rollingBufferSize) + : this(stream) + { + if (rollingBufferSize > 0) + { + _rollingBuffer = ArrayPool.Shared.Rent(rollingBufferSize); + _rollingBufferSize = rollingBufferSize; + _rollingBufferWritePos = 0; + _rollingBufferLength = 0; + } + } internal virtual bool IsRecording { get; private set; } @@ -28,6 +63,11 @@ internal partial class RewindableStream : Stream, IStreamStack if (disposing) { stream.Dispose(); + if (_rollingBuffer is not null) + { + ArrayPool.Shared.Return(_rollingBuffer); + _rollingBuffer = null; + } } } @@ -74,7 +114,10 @@ internal partial class RewindableStream : Stream, IStreamStack { return new SeekableRewindableStream(stream); } - return new RewindableStream(stream); + + // For non-seekable streams, create a RewindableStream with rolling buffer + // to allow limited backward seeking (required by decompressors that over-read) + return new RewindableStream(stream, DefaultRollingBufferSize); } public virtual void StartRecording() @@ -112,10 +155,16 @@ internal partial class RewindableStream : Stream, IStreamStack { get { - if (isRewound || bufferStream.Position < bufferStream.Length) + // If recording is active or rewound from recording, use recording buffer position + if (IsRecording || (isRewound && bufferStream.Position < bufferStream.Length)) { return streamPosition - bufferStream.Length + bufferStream.Position; } + // If rolling buffer is active (and not recording), use logical position + if (_rollingBuffer is not null) + { + return _logicalPosition; + } return streamPosition; } set => SeekToPosition(value); @@ -123,18 +172,38 @@ internal partial class RewindableStream : Stream, IStreamStack private void SeekToPosition(long targetPosition) { - long bufferStart = streamPosition - bufferStream.Length; - long bufferEnd = streamPosition; + // If recording is active, use recording buffer for seeking + if (IsRecording || isRewound) + { + long bufferStart = streamPosition - bufferStream.Length; + long bufferEnd = streamPosition; - if (targetPosition >= bufferStart && targetPosition <= bufferEnd) - { - isRewound = true; - bufferStream.Position = targetPosition - bufferStart; + if (targetPosition >= bufferStart && targetPosition <= bufferEnd) + { + isRewound = true; + bufferStream.Position = targetPosition - bufferStart; + return; + } + throw new NotSupportedException("Cannot seek outside recorded region."); } - else + + // If rolling buffer is enabled, check if we can seek within it + if (_rollingBuffer is not null) { - throw new NotSupportedException("Cannot seek outside buffered region."); + long rollingBufferStart = streamPosition - _rollingBufferLength; + if (targetPosition >= rollingBufferStart && targetPosition <= streamPosition) + { + _logicalPosition = targetPosition; + return; + } + // Can't seek outside rolling buffer range + throw new NotSupportedException( + $"Cannot seek to position {targetPosition}. Valid range with rolling buffer: [{rollingBufferStart}, {streamPosition}]" + ); } + + // No buffering available + throw new NotSupportedException("Cannot seek on non-buffered stream."); } public override int Read(byte[] buffer, int offset, int count) @@ -143,6 +212,32 @@ internal partial class RewindableStream : Stream, IStreamStack { return 0; } + + // If recording is active or we're reading from the recording buffer, use legacy behavior + // Recording takes precedence over rolling buffer for format detection + if (IsRecording || (isRewound && bufferStream.Position != bufferStream.Length)) + { + return ReadWithRecording(buffer, offset, count); + } + + // If rolling buffer is enabled (and not recording), use rolling buffer logic + if (_rollingBuffer is not null) + { + return ReadWithRollingBuffer(buffer, offset, count); + } + + // No buffering - read directly from stream + int read = stream.Read(buffer, offset, count); + streamPosition += read; + _logicalPosition = streamPosition; + return read; + } + + /// + /// Reads data using the recording buffer (legacy behavior for format detection). + /// + private int ReadWithRecording(byte[] buffer, int offset, int count) + { int read; if (isRewound && bufferStream.Position != bufferStream.Length) { @@ -155,7 +250,14 @@ internal partial class RewindableStream : Stream, IStreamStack { bufferStream.Write(buffer, offset + read, tempRead); } + else if (_rollingBuffer is not null && tempRead > 0) + { + // When transitioning out of recording mode, add to rolling buffer + // so that future rewinds will work + AddToRollingBuffer(buffer, offset + read, tempRead); + } streamPosition += tempRead; + _logicalPosition = streamPosition; read += tempRead; } if (bufferStream.Position == bufferStream.Length) @@ -171,9 +273,115 @@ internal partial class RewindableStream : Stream, IStreamStack bufferStream.Write(buffer, offset, read); } streamPosition += read; + _logicalPosition = streamPosition; return read; } + /// + /// Reads data using the rolling buffer. If logical position is behind stream position, + /// serves data from the rolling buffer first. + /// + private int ReadWithRollingBuffer(byte[] buffer, int offset, int count) + { + int totalRead = 0; + + // If logical position is behind stream position, read from rolling buffer first + while (count > 0 && _logicalPosition < streamPosition) + { + // Calculate offset in rolling buffer + long bytesFromEnd = streamPosition - _logicalPosition; + if (bytesFromEnd > _rollingBufferLength) + { + // This shouldn't happen if SeekToPosition validated correctly + throw new InvalidOperationException( + "Logical position is outside rolling buffer range." + ); + } + + // Find the index in the circular buffer + // _rollingBufferWritePos is where next byte would be written (one past last valid byte) + // So the byte at _logicalPosition is at: (_rollingBufferWritePos - bytesFromEnd + _rollingBufferSize) % _rollingBufferSize + int bufferIndex = (int)( + (_rollingBufferWritePos - bytesFromEnd + _rollingBufferSize) % _rollingBufferSize + ); + int availableFromBuffer = (int)Math.Min(bytesFromEnd, count); + + // Read from rolling buffer (may wrap around) + int firstPart = Math.Min(availableFromBuffer, _rollingBufferSize - bufferIndex); + Array.Copy(_rollingBuffer!, bufferIndex, buffer, offset, firstPart); + if (firstPart < availableFromBuffer) + { + // Wrap around + Array.Copy( + _rollingBuffer!, + 0, + buffer, + offset + firstPart, + availableFromBuffer - firstPart + ); + } + + totalRead += availableFromBuffer; + offset += availableFromBuffer; + count -= availableFromBuffer; + _logicalPosition += availableFromBuffer; + } + + // If more data needed, read from underlying stream + if (count > 0) + { + int read = stream.Read(buffer, offset, count); + if (read > 0) + { + // Add to rolling buffer + AddToRollingBuffer(buffer, offset, read); + streamPosition += read; + _logicalPosition += read; + totalRead += read; + } + } + + return totalRead; + } + + /// + /// Adds data to the rolling buffer (circular). + /// + private void AddToRollingBuffer(byte[] data, int offset, int count) + { + if (_rollingBuffer is null || count == 0) + { + return; + } + + // If data is larger than buffer, only keep the last _rollingBufferSize bytes + if (count >= _rollingBufferSize) + { + Array.Copy( + data, + offset + count - _rollingBufferSize, + _rollingBuffer, + 0, + _rollingBufferSize + ); + _rollingBufferWritePos = 0; + _rollingBufferLength = _rollingBufferSize; + return; + } + + // Write data to circular buffer + int firstPart = Math.Min(count, _rollingBufferSize - _rollingBufferWritePos); + Array.Copy(data, offset, _rollingBuffer, _rollingBufferWritePos, firstPart); + if (firstPart < count) + { + // Wrap around + Array.Copy(data, offset + firstPart, _rollingBuffer, 0, count - firstPart); + } + + _rollingBufferWritePos = (_rollingBufferWritePos + count) % _rollingBufferSize; + _rollingBufferLength = Math.Min(_rollingBufferLength + count, _rollingBufferSize); + } + public override long Seek(long offset, SeekOrigin origin) { long targetPosition = origin switch