From 7fb33de13e939bac9159f54b411268ed6feddfee Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Fri, 29 May 2026 12:21:20 +0200 Subject: [PATCH 1/4] Share SubReadStream across System.IO.Compression and System.Formats.Tar Both ZipArchive and Tar used their own internal SubReadStream classes that exposed a bounded view over a 'super' stream. Tar additionally had a SeekableSubReadStream subclass to support seeking. The two implementations have converged in functionality, so consolidate them into a single sealed class at src/libraries/Common/src/System/IO/SubReadStream.cs that supports both seekable and unseekable super streams. The unified class also keeps Tar's AdvanceToEnd/AdvanceToEndAsync APIs (no-ops for Zip since Zip never calls them) so TarReader can continue to discard unread data segments. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Common/src/System/IO/SubReadStream.cs | 284 ++++++++++++++++++ .../src/System.Formats.Tar.csproj | 3 +- .../Formats/Tar/SeekableSubReadStream.cs | 113 ------- .../src/System/Formats/Tar/SubReadStream.cs | 188 ------------ .../src/System/Formats/Tar/TarHeader.Read.cs | 49 +-- .../src/System/Formats/Tar/TarReader.cs | 4 +- .../src/Resources/Strings.resx | 9 + .../src/System.IO.Compression.csproj | 1 + .../System/IO/Compression/ZipCustomStreams.cs | 217 ------------- 9 files changed, 323 insertions(+), 545 deletions(-) create mode 100644 src/libraries/Common/src/System/IO/SubReadStream.cs delete mode 100644 src/libraries/System.Formats.Tar/src/System/Formats/Tar/SeekableSubReadStream.cs delete mode 100644 src/libraries/System.Formats.Tar/src/System/Formats/Tar/SubReadStream.cs diff --git a/src/libraries/Common/src/System/IO/SubReadStream.cs b/src/libraries/Common/src/System/IO/SubReadStream.cs new file mode 100644 index 00000000000000..f4e6996567236e --- /dev/null +++ b/src/libraries/Common/src/System/IO/SubReadStream.cs @@ -0,0 +1,284 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Buffers; +using System.Diagnostics; +using System.Threading; +using System.Threading.Tasks; + +namespace System.IO +{ + // Stream that wraps a super stream and exposes a window [startPosition, startPosition + maxLength). + // Supports both seekable and unseekable super streams. When the super stream is seekable, this stream + // is seekable too and will reposition the super stream before each read if needed. When the super + // stream is unseekable, reads must be sequential and Seek/Position-set throw. + // Does not support writing. + internal sealed class SubReadStream : Stream + { + private const int MaxAdvanceBufferLength = 4096; + + private bool _hasReachedEnd; + private readonly long _startInSuperStream; + private long _positionInSuperStream; + private readonly long _endInSuperStream; + private readonly Stream _superStream; + private bool _isDisposed; + + public SubReadStream(Stream superStream, long startPosition, long maxLength) + { + ArgumentNullException.ThrowIfNull(superStream); + if (!superStream.CanRead) + { + throw new ArgumentException(SR.IO_NotSupported_UnreadableStream, nameof(superStream)); + } + ArgumentOutOfRangeException.ThrowIfNegative(startPosition); + ArgumentOutOfRangeException.ThrowIfNegative(maxLength); + ArgumentOutOfRangeException.ThrowIfGreaterThan(startPosition, long.MaxValue - maxLength, nameof(startPosition)); + + _startInSuperStream = startPosition; + _positionInSuperStream = startPosition; + _endInSuperStream = startPosition + maxLength; + _superStream = superStream; + } + + public override long Length + { + get + { + ThrowIfDisposed(); + return _endInSuperStream - _startInSuperStream; + } + } + + public override long Position + { + get + { + ThrowIfDisposed(); + return _positionInSuperStream - _startInSuperStream; + } + set + { + ThrowIfDisposed(); + if (!CanSeek) + { + throw new NotSupportedException(SR.IO_NotSupported_UnseekableStream); + } + ArgumentOutOfRangeException.ThrowIfNegative(value); + + long newPositionInSuperStream = _startInSuperStream + value; + _superStream.Position = newPositionInSuperStream; + _positionInSuperStream = newPositionInSuperStream; + } + } + + public override bool CanRead => !_isDisposed && _superStream.CanRead; + + public override bool CanSeek => !_isDisposed && _superStream.CanSeek; + + public override bool CanWrite => false; + + private long Remaining => _endInSuperStream - _positionInSuperStream; + + private int LimitByRemaining(int bufferSize) => (int)Math.Max(0, Math.Min(Remaining, bufferSize)); + + // Positions the super stream past the end of this stream's window. After calling this method, + // subsequent reads on this stream throw . + internal void AdvanceToEnd() + { + _hasReachedEnd = true; + + long remaining = Remaining; + _positionInSuperStream = _endInSuperStream; + AdvanceSuperStream(remaining); + } + + internal ValueTask AdvanceToEndAsync(CancellationToken cancellationToken) + { + _hasReachedEnd = true; + + long remaining = Remaining; + _positionInSuperStream = _endInSuperStream; + return AdvanceSuperStreamAsync(remaining, cancellationToken); + } + + private void AdvanceSuperStream(long bytesToDiscard) + { + if (_superStream.CanSeek) + { + _superStream.Position += bytesToDiscard; + } + else if (bytesToDiscard > 0) + { + byte[] buffer = ArrayPool.Shared.Rent((int)Math.Min(MaxAdvanceBufferLength, bytesToDiscard)); + while (bytesToDiscard > 0) + { + int currentLengthToRead = (int)Math.Min(MaxAdvanceBufferLength, bytesToDiscard); + _superStream.ReadExactly(buffer.AsSpan(0, currentLengthToRead)); + bytesToDiscard -= currentLengthToRead; + } + ArrayPool.Shared.Return(buffer); + } + } + + private async ValueTask AdvanceSuperStreamAsync(long bytesToDiscard, CancellationToken cancellationToken) + { + cancellationToken.ThrowIfCancellationRequested(); + + if (_superStream.CanSeek) + { + _superStream.Position += bytesToDiscard; + } + else if (bytesToDiscard > 0) + { + byte[] buffer = ArrayPool.Shared.Rent((int)Math.Min(MaxAdvanceBufferLength, bytesToDiscard)); + while (bytesToDiscard > 0) + { + int currentLengthToRead = (int)Math.Min(MaxAdvanceBufferLength, bytesToDiscard); + await _superStream.ReadExactlyAsync(buffer, 0, currentLengthToRead, cancellationToken).ConfigureAwait(false); + bytesToDiscard -= currentLengthToRead; + } + ArrayPool.Shared.Return(buffer); + } + } + + private void ThrowIfDisposed() + { + ObjectDisposedException.ThrowIf(_isDisposed, this); + } + + private void ThrowIfCantRead() + { + if (!_superStream.CanRead) + { + throw new NotSupportedException(SR.IO_NotSupported_UnreadableStream); + } + } + + private void ThrowIfBeyondEndOfStream() + { + if (_hasReachedEnd) + { + throw new EndOfStreamException(); + } + } + + public override int Read(byte[] buffer, int offset, int count) + { + ValidateBufferArguments(buffer, offset, count); + return Read(buffer.AsSpan(offset, count)); + } + + public override int Read(Span destination) + { + ThrowIfDisposed(); + ThrowIfCantRead(); + ThrowIfBeyondEndOfStream(); + + if (_superStream.CanSeek && _superStream.Position != _positionInSuperStream) + { + _superStream.Seek(_positionInSuperStream, SeekOrigin.Begin); + } + + destination = destination[..LimitByRemaining(destination.Length)]; + + int ret = _superStream.Read(destination); + + _positionInSuperStream += ret; + return ret; + } + + public override int ReadByte() + { + byte b = default; + return Read(new Span(ref b)) == 1 ? b : -1; + } + + public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) + { + if (cancellationToken.IsCancellationRequested) + { + return Task.FromCanceled(cancellationToken); + } + ValidateBufferArguments(buffer, offset, count); + return ReadAsync(new Memory(buffer, offset, count), cancellationToken).AsTask(); + } + + public override ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken = default) + { + if (cancellationToken.IsCancellationRequested) + { + return ValueTask.FromCanceled(cancellationToken); + } + ThrowIfDisposed(); + ThrowIfCantRead(); + ThrowIfBeyondEndOfStream(); + return ReadAsyncCore(buffer, cancellationToken); + } + + private async ValueTask ReadAsyncCore(Memory buffer, CancellationToken cancellationToken) + { + Debug.Assert(!_hasReachedEnd); + + cancellationToken.ThrowIfCancellationRequested(); + + if (_superStream.CanSeek && _superStream.Position != _positionInSuperStream) + { + _superStream.Seek(_positionInSuperStream, SeekOrigin.Begin); + } + + buffer = buffer[..LimitByRemaining(buffer.Length)]; + + int ret = await _superStream.ReadAsync(buffer, cancellationToken).ConfigureAwait(false); + + _positionInSuperStream += ret; + return ret; + } + + public override long Seek(long offset, SeekOrigin origin) + { + ThrowIfDisposed(); + + if (!CanSeek) + { + throw new NotSupportedException(SR.IO_NotSupported_UnseekableStream); + } + + long newPositionInSuperStream = origin switch + { + SeekOrigin.Begin => _startInSuperStream + offset, + SeekOrigin.Current => _positionInSuperStream + offset, + SeekOrigin.End => _endInSuperStream + offset, + _ => throw new ArgumentOutOfRangeException(nameof(origin)), + }; + + if (newPositionInSuperStream < _startInSuperStream) + { + throw new IOException(SR.IO_SeekBeforeBegin); + } + + long actualPositionInSuperStream = _superStream.Seek(newPositionInSuperStream, SeekOrigin.Begin); + _positionInSuperStream = actualPositionInSuperStream; + + return _positionInSuperStream - _startInSuperStream; + } + + public override void SetLength(long value) => throw new NotSupportedException(SR.IO_NotSupported_UnwritableStream); + + public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(SR.IO_NotSupported_UnwritableStream); + + public override void Flush() { } + + public override Task FlushAsync(CancellationToken cancellationToken) => + cancellationToken.IsCancellationRequested ? Task.FromCanceled(cancellationToken) : + Task.CompletedTask; + + // Close the stream for reading. Note that this does NOT close the super stream (since + // this stream is just a 'chunk' of the super stream). + protected override void Dispose(bool disposing) + { + _isDisposed = true; + base.Dispose(disposing); + } + } +} diff --git a/src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj b/src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj index e8ae5d512d488a..65eb95b9caf1e9 100644 --- a/src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj +++ b/src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj @@ -34,10 +34,9 @@ - - + diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/SeekableSubReadStream.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/SeekableSubReadStream.cs deleted file mode 100644 index 4fc690443531f8..00000000000000 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/SeekableSubReadStream.cs +++ /dev/null @@ -1,113 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Diagnostics; -using System.IO; -using System.Threading; -using System.Threading.Tasks; - -namespace System.Formats.Tar -{ - // Stream that allows wrapping a super stream and specify the lower and upper limits that can be read from it. - // It is meant to be used when the super stream is seekable. - // Does not support writing. - internal sealed class SeekableSubReadStream : SubReadStream - { - public SeekableSubReadStream(Stream superStream, long startPosition, long maxLength) - : base(superStream, startPosition, maxLength) - { - if (!superStream.CanSeek) - { - throw new ArgumentException(SR.IO_NotSupported_UnseekableStream, nameof(superStream)); - } - } - - public override bool CanSeek => !_isDisposed; - - public override long Position - { - get - { - ThrowIfDisposed(); - return _positionInSuperStream - _startInSuperStream; - } - set - { - ThrowIfDisposed(); - ArgumentOutOfRangeException.ThrowIfNegative(value); - ArgumentOutOfRangeException.ThrowIfGreaterThanOrEqual(value, _endInSuperStream); - _positionInSuperStream = _startInSuperStream + value; - } - } - - public override int Read(Span destination) - { - ThrowIfDisposed(); - VerifyPositionInSuperStream(); - - // parameter validation sent to _superStream.Read - int origCount = destination.Length; - int count = destination.Length; - - if ((ulong)(_positionInSuperStream + count) > (ulong)_endInSuperStream) - { - count = Math.Max(0, (int)(_endInSuperStream - _positionInSuperStream)); - } - - Debug.Assert(count >= 0); - Debug.Assert(count <= origCount); - - if (count > 0) - { - int bytesRead = _superStream.Read(destination.Slice(0, count)); - _positionInSuperStream += bytesRead; - return bytesRead; - } - - return 0; - } - - public override ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken = default) - { - if (cancellationToken.IsCancellationRequested) - { - return ValueTask.FromCanceled(cancellationToken); - } - ThrowIfDisposed(); - VerifyPositionInSuperStream(); - return ReadAsyncCore(buffer, cancellationToken); - } - - public override long Seek(long offset, SeekOrigin origin) - { - ThrowIfDisposed(); - - long newPositionInSuperStream = origin switch - { - SeekOrigin.Begin => _startInSuperStream + offset, - SeekOrigin.Current => _positionInSuperStream + offset, - SeekOrigin.End => _endInSuperStream + offset, - _ => throw new ArgumentOutOfRangeException(nameof(origin)), - }; - - if (newPositionInSuperStream < _startInSuperStream) - { - throw new IOException(SR.IO_SeekBeforeBegin); - } - - _positionInSuperStream = newPositionInSuperStream; - - return _positionInSuperStream - _startInSuperStream; - } - - private void VerifyPositionInSuperStream() - { - if (_positionInSuperStream != _superStream.Position) - { - // Since we can seek, if the stream had its position pointer moved externally, - // we must bring it back to the last read location on this stream - _superStream.Seek(_positionInSuperStream, SeekOrigin.Begin); - } - } - } -} diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/SubReadStream.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/SubReadStream.cs deleted file mode 100644 index a884565193fc1d..00000000000000 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/SubReadStream.cs +++ /dev/null @@ -1,188 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Diagnostics; -using System.IO; -using System.Runtime.InteropServices; -using System.Threading; -using System.Threading.Tasks; - -namespace System.Formats.Tar -{ - // Stream that allows wrapping a super stream and specify the lower and upper limits that can be read from it. - // It is meant to be used when the super stream is unseekable. - // Does not support writing. - internal class SubReadStream : Stream - { - protected bool _hasReachedEnd; - protected readonly long _startInSuperStream; - protected long _positionInSuperStream; - protected readonly long _endInSuperStream; - protected readonly Stream _superStream; - protected bool _isDisposed; - - public SubReadStream(Stream superStream, long startPosition, long maxLength) - { - if (!superStream.CanRead) - { - throw new ArgumentException(SR.IO_NotSupported_UnreadableStream, nameof(superStream)); - } - if (startPosition > long.MaxValue - maxLength) - { - throw new InvalidDataException(SR.TarInvalidNumber); - } - _startInSuperStream = startPosition; - _positionInSuperStream = startPosition; - _endInSuperStream = startPosition + maxLength; - _superStream = superStream; - _isDisposed = false; - _hasReachedEnd = false; - } - - public override long Length - { - get - { - ThrowIfDisposed(); - return _endInSuperStream - _startInSuperStream; - } - } - - public override long Position - { - get - { - ThrowIfDisposed(); - return _positionInSuperStream - _startInSuperStream; - } - set - { - ThrowIfDisposed(); - throw new InvalidOperationException(SR.IO_NotSupported_UnseekableStream); - } - } - - public override bool CanRead => !_isDisposed; - - public override bool CanSeek => false; - - public override bool CanWrite => false; - - private long Remaining => _endInSuperStream - _positionInSuperStream; - - private int LimitByRemaining(int bufferSize) => (int)Math.Min(Remaining, bufferSize); - - internal ValueTask AdvanceToEndAsync(CancellationToken cancellationToken) - { - _hasReachedEnd = true; - - long remaining = Remaining; - _positionInSuperStream = _endInSuperStream; - return TarHelpers.AdvanceStreamAsync(_superStream, remaining, cancellationToken); - } - - internal void AdvanceToEnd() - { - _hasReachedEnd = true; - - long remaining = Remaining; - _positionInSuperStream = _endInSuperStream; - TarHelpers.AdvanceStream(_superStream, remaining); - } - - protected void ThrowIfDisposed() - { - ObjectDisposedException.ThrowIf(_isDisposed, this); - } - - private void ThrowIfBeyondEndOfStream() - { - if (_hasReachedEnd) - { - throw new EndOfStreamException(); - } - } - - public override int Read(byte[] buffer, int offset, int count) - { - ValidateBufferArguments(buffer, offset, count); - return Read(buffer.AsSpan(offset, count)); - } - - public override int Read(Span destination) - { - ThrowIfDisposed(); - ThrowIfBeyondEndOfStream(); - - destination = destination[..LimitByRemaining(destination.Length)]; - - int ret = _superStream.Read(destination); - - _positionInSuperStream += ret; - - return ret; - } - - public override int ReadByte() - { - byte b = default; - return Read(new Span(ref b)) == 1 ? b : -1; - } - - public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) - { - if (cancellationToken.IsCancellationRequested) - { - return Task.FromCanceled(cancellationToken); - } - ValidateBufferArguments(buffer, offset, count); - return ReadAsync(new Memory(buffer, offset, count), cancellationToken).AsTask(); - } - - public override ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken = default) - { - if (cancellationToken.IsCancellationRequested) - { - return ValueTask.FromCanceled(cancellationToken); - } - ThrowIfDisposed(); - ThrowIfBeyondEndOfStream(); - return ReadAsyncCore(buffer, cancellationToken); - } - - protected async ValueTask ReadAsyncCore(Memory buffer, CancellationToken cancellationToken) - { - Debug.Assert(!_hasReachedEnd); - - cancellationToken.ThrowIfCancellationRequested(); - - buffer = buffer[..LimitByRemaining(buffer.Length)]; - - int ret = await _superStream.ReadAsync(buffer, cancellationToken).ConfigureAwait(false); - - _positionInSuperStream += ret; - - return ret; - } - - public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(SR.IO_NotSupported_UnseekableStream); - - public override void SetLength(long value) => throw new NotSupportedException(SR.IO_NotSupported_UnseekableStream); - - public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(SR.IO_NotSupported_UnwritableStream); - - public override void Flush() { } - - public override Task FlushAsync(CancellationToken cancellationToken) => - cancellationToken.IsCancellationRequested ? Task.FromCanceled(cancellationToken) : - Task.CompletedTask; - - // Close the stream for reading. Note that this does NOT close the superStream (since - // the substream is just 'a chunk' of the super-stream - protected override void Dispose(bool disposing) - { - _isDisposed = true; - base.Dispose(disposing); - } - } -} diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Read.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Read.cs index d5de558c294e22..30e0111e7a4f30 100644 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Read.cs +++ b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Read.cs @@ -230,7 +230,8 @@ internal void ReplaceNormalAttributesWithExtended(IEnumerable An attempt was made to move the position before the beginning of the stream. + + Stream does not support reading. + + + Stream does not support seeking. + + + Stream does not support writing. + The CRC32 checksum of the extracted data does not match the expected value from the archive. diff --git a/src/libraries/System.IO.Compression/src/System.IO.Compression.csproj b/src/libraries/System.IO.Compression/src/System.IO.Compression.csproj index 926452322b590d..65497953e0af72 100644 --- a/src/libraries/System.IO.Compression/src/System.IO.Compression.csproj +++ b/src/libraries/System.IO.Compression/src/System.IO.Compression.csproj @@ -25,6 +25,7 @@ + diff --git a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipCustomStreams.cs b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipCustomStreams.cs index 9c03fe925060f4..90f5ea6c0f2a6c 100644 --- a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipCustomStreams.cs +++ b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipCustomStreams.cs @@ -251,223 +251,6 @@ public override async ValueTask DisposeAsync() } } - internal sealed class SubReadStream : Stream - { - private readonly long _startInSuperStream; - private long _positionInSuperStream; - private readonly long _endInSuperStream; - private readonly Stream _superStream; - private bool _canRead; - private bool _isDisposed; - - public SubReadStream(Stream superStream, long startPosition, long maxLength) - { - if (startPosition > long.MaxValue - maxLength) - { - throw new InvalidDataException(SR.LocalFileHeaderCorrupt); - } - _startInSuperStream = startPosition; - _positionInSuperStream = startPosition; - _endInSuperStream = startPosition + maxLength; - _superStream = superStream; - _canRead = true; - _isDisposed = false; - } - - public override long Length - { - get - { - ThrowIfDisposed(); - - return _endInSuperStream - _startInSuperStream; - } - } - - public override long Position - { - get - { - ThrowIfDisposed(); - - return _positionInSuperStream - _startInSuperStream; - } - set - { - ThrowIfDisposed(); - - if (!CanSeek) - { - throw new NotSupportedException(SR.SeekingNotSupported); - } - - ArgumentOutOfRangeException.ThrowIfNegative(value); - - long newPositionInSuperStream = _startInSuperStream + value; - _superStream.Position = newPositionInSuperStream; - _positionInSuperStream = newPositionInSuperStream; - } - } - - public override bool CanRead => _superStream.CanRead && _canRead; - - public override bool CanSeek => _superStream.CanSeek && !_isDisposed; - - public override bool CanWrite => false; - - private void ThrowIfDisposed() - { - if (_isDisposed) - throw new ObjectDisposedException(GetType().ToString(), SR.HiddenStreamName); - } - - private void ThrowIfCantRead() - { - if (!CanRead) - throw new NotSupportedException(SR.ReadingNotSupported); - } - - public override int Read(byte[] buffer, int offset, int count) - { - // parameter validation sent to _superStream.Read - int origCount = count; - - ThrowIfDisposed(); - ThrowIfCantRead(); - - if (_superStream.Position != _positionInSuperStream) - _superStream.Seek(_positionInSuperStream, SeekOrigin.Begin); - if (_positionInSuperStream + count > _endInSuperStream) - count = (int)Math.Max(0L, _endInSuperStream - _positionInSuperStream); - - Debug.Assert(count >= 0); - Debug.Assert(count <= origCount); - - int ret = _superStream.Read(buffer, offset, count); - - _positionInSuperStream += ret; - return ret; - } - - public override int Read(Span destination) - { - // parameter validation sent to _superStream.Read - int origCount = destination.Length; - int count = destination.Length; - - ThrowIfDisposed(); - ThrowIfCantRead(); - - if (_superStream.Position != _positionInSuperStream) - _superStream.Seek(_positionInSuperStream, SeekOrigin.Begin); - if (_positionInSuperStream + count > _endInSuperStream) - count = (int)Math.Max(0L, _endInSuperStream - _positionInSuperStream); - - Debug.Assert(count >= 0); - Debug.Assert(count <= origCount); - - int ret = _superStream.Read(destination.Slice(0, count)); - - _positionInSuperStream += ret; - return ret; - } - - public override int ReadByte() - { - byte b = default; - return Read(new Span(ref b)) == 1 ? b : -1; - } - - public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) - { - ValidateBufferArguments(buffer, offset, count); - return ReadAsync(new Memory(buffer, offset, count), cancellationToken).AsTask(); - } - - public override ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken = default) - { - ThrowIfDisposed(); - ThrowIfCantRead(); - return Core(buffer, cancellationToken); - - async ValueTask Core(Memory buffer, CancellationToken cancellationToken) - { - if (_superStream.Position != _positionInSuperStream) - { - _superStream.Seek(_positionInSuperStream, SeekOrigin.Begin); - } - - if (_positionInSuperStream > _endInSuperStream - buffer.Length) - { - buffer = buffer.Slice(0, (int)Math.Max(0L, _endInSuperStream - _positionInSuperStream)); - } - - int ret = await _superStream.ReadAsync(buffer, cancellationToken).ConfigureAwait(false); - - _positionInSuperStream += ret; - return ret; - } - } - - public override long Seek(long offset, SeekOrigin origin) - { - ThrowIfDisposed(); - - if (!CanSeek) - { - throw new NotSupportedException(SR.SeekingNotSupported); - } - - long newPositionInSuperStream = origin switch - { - SeekOrigin.Begin => _startInSuperStream + offset, - SeekOrigin.Current => _positionInSuperStream + offset, - SeekOrigin.End => _endInSuperStream + offset, - _ => throw new ArgumentOutOfRangeException(nameof(origin)), - }; - - if (newPositionInSuperStream < _startInSuperStream) - { - throw new IOException(SR.IO_SeekBeforeBegin); - } - - long actualPositionInSuperStream = _superStream.Seek(newPositionInSuperStream, SeekOrigin.Begin); - _positionInSuperStream = actualPositionInSuperStream; - - return _positionInSuperStream - _startInSuperStream; - } - - public override void SetLength(long value) - { - ThrowIfDisposed(); - throw new NotSupportedException(SR.SetLengthRequiresSeekingAndWriting); - } - - public override void Write(byte[] buffer, int offset, int count) - { - ThrowIfDisposed(); - throw new NotSupportedException(SR.WritingNotSupported); - } - - public override void Flush() - { - ThrowIfDisposed(); - throw new NotSupportedException(SR.WritingNotSupported); - } - - // Close the stream for reading. Note that this does NOT close the superStream (since - // the substream is just 'a chunk' of the super-stream - protected override void Dispose(bool disposing) - { - if (disposing && !_isDisposed) - { - _canRead = false; - _isDisposed = true; - } - base.Dispose(disposing); - } - } - internal sealed class CheckSumAndSizeWriteStream : Stream { private readonly Func _baseStreamFactory; From 4b1ee102d6fdaa583e5f346808e89de8da7a985f Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Mon, 1 Jun 2026 12:10:08 +0200 Subject: [PATCH 2/4] Address review feedback - Wrap ArrayPool rent/return in try/finally in both AdvanceSuperStream paths so the buffer is returned if the inner Read(Exactly) throws. - Drop the redundant cancellation pre-check from ReadAsync; the synchronous ThrowIfCancellationRequested in ReadAsyncCore already handles it. - Drop the redundant nameof argument from ArgumentOutOfRangeException.ThrowIfGreaterThan; the helper captures the argument expression by default. - Fix indentation of the new comment lines in TarHeader.Read.cs ProcessDataBlock header. - Correct the stale 'seekable streams' wording in TarReader's AdvanceDataStreamIfNeeded / Async to 'unseekable streams' (the branch only runs when the archive stream is not seekable). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Common/src/System/IO/SubReadStream.cs | 42 ++++++++++--------- .../src/System/Formats/Tar/TarHeader.Read.cs | 4 +- .../src/System/Formats/Tar/TarReader.cs | 4 +- 3 files changed, 27 insertions(+), 23 deletions(-) diff --git a/src/libraries/Common/src/System/IO/SubReadStream.cs b/src/libraries/Common/src/System/IO/SubReadStream.cs index f4e6996567236e..f5efbedf8066da 100644 --- a/src/libraries/Common/src/System/IO/SubReadStream.cs +++ b/src/libraries/Common/src/System/IO/SubReadStream.cs @@ -33,7 +33,7 @@ public SubReadStream(Stream superStream, long startPosition, long maxLength) } ArgumentOutOfRangeException.ThrowIfNegative(startPosition); ArgumentOutOfRangeException.ThrowIfNegative(maxLength); - ArgumentOutOfRangeException.ThrowIfGreaterThan(startPosition, long.MaxValue - maxLength, nameof(startPosition)); + ArgumentOutOfRangeException.ThrowIfGreaterThan(startPosition, long.MaxValue - maxLength); _startInSuperStream = startPosition; _positionInSuperStream = startPosition; @@ -111,13 +111,19 @@ private void AdvanceSuperStream(long bytesToDiscard) else if (bytesToDiscard > 0) { byte[] buffer = ArrayPool.Shared.Rent((int)Math.Min(MaxAdvanceBufferLength, bytesToDiscard)); - while (bytesToDiscard > 0) + try { - int currentLengthToRead = (int)Math.Min(MaxAdvanceBufferLength, bytesToDiscard); - _superStream.ReadExactly(buffer.AsSpan(0, currentLengthToRead)); - bytesToDiscard -= currentLengthToRead; + while (bytesToDiscard > 0) + { + int currentLengthToRead = (int)Math.Min(MaxAdvanceBufferLength, bytesToDiscard); + _superStream.ReadExactly(buffer.AsSpan(0, currentLengthToRead)); + bytesToDiscard -= currentLengthToRead; + } + } + finally + { + ArrayPool.Shared.Return(buffer); } - ArrayPool.Shared.Return(buffer); } } @@ -132,13 +138,19 @@ private async ValueTask AdvanceSuperStreamAsync(long bytesToDiscard, Cancellatio else if (bytesToDiscard > 0) { byte[] buffer = ArrayPool.Shared.Rent((int)Math.Min(MaxAdvanceBufferLength, bytesToDiscard)); - while (bytesToDiscard > 0) + try + { + while (bytesToDiscard > 0) + { + int currentLengthToRead = (int)Math.Min(MaxAdvanceBufferLength, bytesToDiscard); + await _superStream.ReadExactlyAsync(buffer, 0, currentLengthToRead, cancellationToken).ConfigureAwait(false); + bytesToDiscard -= currentLengthToRead; + } + } + finally { - int currentLengthToRead = (int)Math.Min(MaxAdvanceBufferLength, bytesToDiscard); - await _superStream.ReadExactlyAsync(buffer, 0, currentLengthToRead, cancellationToken).ConfigureAwait(false); - bytesToDiscard -= currentLengthToRead; + ArrayPool.Shared.Return(buffer); } - ArrayPool.Shared.Return(buffer); } } @@ -196,20 +208,12 @@ public override int ReadByte() public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { - if (cancellationToken.IsCancellationRequested) - { - return Task.FromCanceled(cancellationToken); - } ValidateBufferArguments(buffer, offset, count); return ReadAsync(new Memory(buffer, offset, count), cancellationToken).AsTask(); } public override ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken = default) { - if (cancellationToken.IsCancellationRequested) - { - return ValueTask.FromCanceled(cancellationToken); - } ThrowIfDisposed(); ThrowIfCantRead(); ThrowIfBeyondEndOfStream(); diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Read.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Read.cs index 30e0111e7a4f30..2673bd4e21aa16 100644 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Read.cs +++ b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Read.cs @@ -230,8 +230,8 @@ internal void ReplaceNormalAttributesWithExtended(IEnumerable 0) { - // When working with seekable streams, every time we return an entry, we avoid advancing the pointer beyond the data section + // When working with unseekable streams, every time we return an entry, we avoid advancing the pointer beyond the data section // This is so the user can read the data if desired. But if the data was not read by the user, we need to advance the pointer // here until it's located at the beginning of the next entry header. // This should only be done if the previous entry came from a TarReader and it still had its original SubReadStream. @@ -244,7 +244,7 @@ internal async ValueTask AdvanceDataStreamIfNeededAsync(CancellationToken cancel } else if (_previouslyReadEntry._header._size > 0) { - // When working with seekable streams, every time we return an entry, we avoid advancing the pointer beyond the data section + // When working with unseekable streams, every time we return an entry, we avoid advancing the pointer beyond the data section // This is so the user can read the data if desired. But if the data was not read by the user, we need to advance the pointer // here until it's located at the beginning of the next entry header. // This should only be done if the previous entry came from a TarReader and it still had its original SubReadStream. From bba5dc44c4f1f7253a10f8ea57ab99faf410caea Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Mon, 1 Jun 2026 17:43:06 +0200 Subject: [PATCH 3/4] Remove unused prefix in csproj --- .../src/System.IO.Compression.csproj | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/libraries/System.IO.Compression/src/System.IO.Compression.csproj b/src/libraries/System.IO.Compression/src/System.IO.Compression.csproj index 65497953e0af72..070415553fe72a 100644 --- a/src/libraries/System.IO.Compression/src/System.IO.Compression.csproj +++ b/src/libraries/System.IO.Compression/src/System.IO.Compression.csproj @@ -14,22 +14,19 @@ - - - - - - - - - - - - - - - - + + + + + + + + + + + + + @@ -42,12 +39,15 @@ + + From 788fd71bba5d7ba41e8654a3ed50aee490f8005c Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Mon, 1 Jun 2026 18:22:32 +0200 Subject: [PATCH 4/4] Make ZipArchiveEntry compressed-data bounds check overflow-safe Now that SubReadStream throws ArgumentOutOfRangeException for startPosition + maxLength overflow, a corrupt zip64 archive with offsetOfCompressedData/compressedSize values near long.MaxValue could slip past IsOpenableFinalVerifications (where 'offsetOfCompressedData + _compressedSize > Length' would wrap around) and leak an ArgumentOutOfRangeException out of the public Zip API instead of the expected InvalidDataException(LocalFileHeaderCorrupt). Rewrite the check using 'Length - offsetOfCompressedData' plus non-negative guards so the corruption is detected and reported as InvalidDataException. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/System/IO/Compression/ZipArchiveEntry.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs index e84666144c9379..3006ff5d2d4df3 100644 --- a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs +++ b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs @@ -981,7 +981,11 @@ private bool IsOpenableInitialVerifications(bool needToUncompress, out string? m private bool IsOpenableFinalVerifications(bool needToLoadIntoMemory, long offsetOfCompressedData, out string? message) { message = null; - if (offsetOfCompressedData + _compressedSize > _archive.ArchiveStream.Length) + // Use an overflow-safe form: corrupt zip64 archives can report compressed sizes / offsets + // near long.MaxValue, which would wrap around in 'offsetOfCompressedData + _compressedSize'. + if (offsetOfCompressedData < 0 || + _compressedSize < 0 || + _compressedSize > _archive.ArchiveStream.Length - offsetOfCompressedData) { message = SR.LocalFileHeaderCorrupt; return false;