diff --git a/src/SharpCompress/Common/SevenZip/SevenZipFilePart.cs b/src/SharpCompress/Common/SevenZip/SevenZipFilePart.cs index 156d3e711..5c0f1d062 100644 --- a/src/SharpCompress/Common/SevenZip/SevenZipFilePart.cs +++ b/src/SharpCompress/Common/SevenZip/SevenZipFilePart.cs @@ -55,7 +55,7 @@ internal override Stream GetCompressedStream() { folderStream.Skip(skipSize); } - return new ReadOnlySubStream(folderStream, Header.Size); + return new ReadOnlySubStream(folderStream, Header.Size, leaveOpen: false); } public CompressionType CompressionType diff --git a/src/SharpCompress/Compressors/LZMA/LZipStream.cs b/src/SharpCompress/Compressors/LZMA/LZipStream.cs index e7059003e..e409e39e6 100644 --- a/src/SharpCompress/Compressors/LZMA/LZipStream.cs +++ b/src/SharpCompress/Compressors/LZMA/LZipStream.cs @@ -45,10 +45,12 @@ void IStreamStack.SetPosition(long position) { } private bool _finished; private long _writeCount; + private readonly Stream? _originalStream; public LZipStream(Stream stream, CompressionMode mode) { Mode = mode; + _originalStream = stream; if (mode == CompressionMode.Decompress) { @@ -125,6 +127,10 @@ protected override void Dispose(bool disposing) { Finish(); _stream.Dispose(); + if (Mode == CompressionMode.Compress) + { + _originalStream?.Dispose(); + } } } diff --git a/src/SharpCompress/IO/ReadOnlySubStream.cs b/src/SharpCompress/IO/ReadOnlySubStream.cs index 5e317c257..86ef16459 100644 --- a/src/SharpCompress/IO/ReadOnlySubStream.cs +++ b/src/SharpCompress/IO/ReadOnlySubStream.cs @@ -16,11 +16,11 @@ internal class ReadOnlySubStream : SharpCompressStream, IStreamStack private long _position; - public ReadOnlySubStream(Stream stream, long bytesToRead) - : this(stream, null, bytesToRead) { } + public ReadOnlySubStream(Stream stream, long bytesToRead, bool leaveOpen = true) + : this(stream, null, bytesToRead, leaveOpen) { } - public ReadOnlySubStream(Stream stream, long? origin, long bytesToRead) - : base(stream, leaveOpen: true, throwOnDispose: false) + public ReadOnlySubStream(Stream stream, long? origin, long bytesToRead, bool leaveOpen = true) + : base(stream, leaveOpen, throwOnDispose: false) { if (origin != null && stream.Position != origin.Value) { diff --git a/src/SharpCompress/IO/SharpCompressStream.cs b/src/SharpCompress/IO/SharpCompressStream.cs index 185a87668..8261505f7 100644 --- a/src/SharpCompress/IO/SharpCompressStream.cs +++ b/src/SharpCompress/IO/SharpCompressStream.cs @@ -138,8 +138,6 @@ public SharpCompressStream( #endif } - internal bool IsRecording { get; private set; } - protected override void Dispose(bool disposing) { #if DEBUG_STREAMS diff --git a/src/SharpCompress/Utility.cs b/src/SharpCompress/Utility.cs index 6c6276026..8f2d6788a 100644 --- a/src/SharpCompress/Utility.cs +++ b/src/SharpCompress/Utility.cs @@ -71,48 +71,16 @@ public static void Skip(this Stream source, long advanceAmount) return; } - using var buffer = MemoryPool.Shared.Rent(TEMP_BUFFER_SIZE); - while (advanceAmount > 0) - { - var toRead = (int)Math.Min(buffer.Memory.Length, advanceAmount); - var read = source.Read(buffer.Memory.Slice(0, toRead).Span); - if (read <= 0) - { - break; - } - advanceAmount -= read; - } + using var readOnlySubStream = new IO.ReadOnlySubStream(source, advanceAmount); + readOnlySubStream.CopyTo(Stream.Null); } - public static void Skip(this Stream source) - { - using var buffer = MemoryPool.Shared.Rent(TEMP_BUFFER_SIZE); - while (source.Read(buffer.Memory.Span) > 0) { } - } + public static void Skip(this Stream source) => source.CopyTo(Stream.Null); - public static async Task SkipAsync( - this Stream source, - CancellationToken cancellationToken = default - ) + public static Task SkipAsync(this Stream source, CancellationToken cancellationToken = default) { - var array = ArrayPool.Shared.Rent(TEMP_BUFFER_SIZE); - try - { - while (true) - { - var read = await source - .ReadAsync(array, 0, array.Length, cancellationToken) - .ConfigureAwait(false); - if (read <= 0) - { - break; - } - } - } - finally - { - ArrayPool.Shared.Return(array); - } + cancellationToken.ThrowIfCancellationRequested(); + return source.CopyToAsync(Stream.Null); } public static DateTime DosDateToDateTime(ushort iDate, ushort iTime) diff --git a/tests/SharpCompress.Test/Streams/DisposalTests.cs b/tests/SharpCompress.Test/Streams/DisposalTests.cs new file mode 100644 index 000000000..9d30ab76f --- /dev/null +++ b/tests/SharpCompress.Test/Streams/DisposalTests.cs @@ -0,0 +1,189 @@ +using System; +using System.IO; +using SharpCompress.Common; +using SharpCompress.Compressors; +using SharpCompress.Compressors.Deflate; +using SharpCompress.Compressors.LZMA; +using SharpCompress.Compressors.Lzw; +using SharpCompress.Compressors.PPMd; +using SharpCompress.Compressors.Reduce; +using SharpCompress.Compressors.ZStandard; +using SharpCompress.IO; +using SharpCompress.Readers; +using SharpCompress.Test.Mocks; +using Xunit; + +namespace SharpCompress.Test.Streams; + +public class DisposalTests +{ + private void VerifyStreamDisposal( + Func createStream, + bool supportsLeaveOpen = true + ) + { + // 1. Test Dispose behavior (should dispose inner stream) + { + using var innerStream = new TestStream(new MemoryStream()); + // createStream(stream, leaveOpen: false) + var stream = createStream(innerStream, false); + stream.Dispose(); + + // Some streams might not support disposal of inner stream (e.g. PpmdStream apparently) + // But for those that satisfy the pattern, we assert true. + Assert.True( + innerStream.IsDisposed, + "Stream should have been disposed when leaveOpen=false" + ); + } + + // 2. Test LeaveOpen behavior (should NOT dispose inner stream) + if (supportsLeaveOpen) + { + using var innerStream = new TestStream(new MemoryStream()); + // createStream(stream, leaveOpen: true) + var stream = createStream(innerStream, true); + stream.Dispose(); + Assert.False( + innerStream.IsDisposed, + "Stream should NOT have been disposed when leaveOpen=true" + ); + } + } + + private void VerifyAlwaysDispose(Func createStream) + { + using var innerStream = new TestStream(new MemoryStream()); + var stream = createStream(innerStream); + stream.Dispose(); + Assert.True(innerStream.IsDisposed, "Stream should have been disposed (AlwaysDispose)"); + } + + private void VerifyNeverDispose(Func createStream) + { + using var innerStream = new TestStream(new MemoryStream()); + var stream = createStream(innerStream); + stream.Dispose(); + Assert.False(innerStream.IsDisposed, "Stream should NOT have been disposed (NeverDispose)"); + } + + [Fact] + public void SourceStream_Disposal() + { + VerifyStreamDisposal( + (stream, leaveOpen) => + new SourceStream( + stream, + i => null, + new ReaderOptions { LeaveStreamOpen = leaveOpen } + ) + ); + } + + [Fact] + public void ProgressReportingStream_Disposal() + { + VerifyStreamDisposal( + (stream, leaveOpen) => + new ProgressReportingStream( + stream, + new Progress(), + "", + 0, + leaveOpen: leaveOpen + ) + ); + } + + [Fact] + public void DataDescriptorStream_Disposal() + { + // DataDescriptorStream DOES dispose inner stream + VerifyAlwaysDispose(stream => new DataDescriptorStream(stream)); + } + + [Fact] + public void DeflateStream_Disposal() + { + // DeflateStream in SharpCompress always disposes inner stream + VerifyAlwaysDispose(stream => new DeflateStream(stream, CompressionMode.Compress)); + } + + [Fact] + public void GZipStream_Disposal() + { + // GZipStream in SharpCompress always disposes inner stream + VerifyAlwaysDispose(stream => new GZipStream(stream, CompressionMode.Compress)); + } + + [Fact] + public void LzwStream_Disposal() + { + VerifyStreamDisposal( + (stream, leaveOpen) => + { + var lzw = new LzwStream(stream); + lzw.IsStreamOwner = !leaveOpen; + return lzw; + } + ); + } + + [Fact] + public void PpmdStream_Disposal() + { + // PpmdStream seems to not dispose inner stream based on code analysis + // It takes PpmdProperties which we need to mock or create. + var props = new PpmdProperties(); + VerifyNeverDispose(stream => new PpmdStream(props, stream, false)); + } + + [Fact] + public void LzmaStream_Disposal() + { + // LzmaStream always disposes inner stream + // Need to provide valid properties to avoid crash in constructor (invalid window size) + // 5 bytes: 1 byte properties + 4 bytes dictionary size (little endian) + // Dictionary size = 1024 (0x400) -> 00 04 00 00 + var lzmaProps = new byte[] { 0, 0, 4, 0, 0 }; + VerifyAlwaysDispose(stream => new LzmaStream(lzmaProps, stream)); + } + + [Fact] + public void LZipStream_Disposal() + { + // LZipStream always disposes inner stream + // Use Compress mode to avoid need for valid input header + VerifyAlwaysDispose(stream => new LZipStream(stream, CompressionMode.Compress)); + } + + [Fact] + public void ReduceStream_Disposal() + { + // ReduceStream does not dispose inner stream + VerifyNeverDispose(stream => new ReduceStream(stream, 0, 0, 1)); + } + + [Fact] + public void ZStandard_CompressionStream_Disposal() + { + VerifyStreamDisposal( + (stream, leaveOpen) => + new CompressionStream(stream, level: 0, bufferSize: 0, leaveOpen: leaveOpen) + ); + } + + [Fact] + public void ZStandard_DecompressionStream_Disposal() + { + VerifyStreamDisposal( + (stream, leaveOpen) => + new DecompressionStream( + stream, + bufferSize: 0, + checkEndOfStream: false, + leaveOpen: leaveOpen + ) + ); + } +}