diff --git a/src/SharpCompress/Compressors/LZMA/DecoderStream.cs b/src/SharpCompress/Compressors/LZMA/DecoderStream.cs index 611561d4..d40eaa7b 100644 --- a/src/SharpCompress/Compressors/LZMA/DecoderStream.cs +++ b/src/SharpCompress/Compressors/LZMA/DecoderStream.cs @@ -1,4 +1,6 @@ using System; +using System.Buffers; +using System.Collections.Generic; using System.IO; using System.Threading; using System.Threading.Tasks; @@ -281,7 +283,8 @@ internal static class DecoderStreamHelper long startPos, long[] packSizes, CFolder folderInfo, - IPasswordProvider pass + IPasswordProvider pass, + ArrayPool? bufferPool = null ) { if (!folderInfo.CheckStructure()) @@ -290,22 +293,25 @@ internal static class DecoderStreamHelper } var inStreams = new Stream[folderInfo._packStreams.Count]; - for (var j = 0; j < folderInfo._packStreams.Count; j++) - { - inStreams[j] = new BufferedSubStream(inStream, startPos, packSizes[j]); - startPos += packSizes[j]; - } - var outStreams = new Stream[folderInfo._unpackSizes.Count]; - - FindPrimaryOutStreamIndex( - folderInfo, - out var primaryCoderIndex, - out var primaryOutStreamIndex - ); var encrypted = folderInfo.IsEncrypted(); try { + for (var j = 0; j < folderInfo._packStreams.Count; j++) + { + inStreams[j] = new BufferedSubStream( + inStream, + startPos, + packSizes[j], + bufferPool ?? ArrayPool.Shared + ); + startPos += packSizes[j]; + } + FindPrimaryOutStreamIndex( + folderInfo, + out var primaryCoderIndex, + out var primaryOutStreamIndex + ); var decoder = CreateDecoderStream( inStreams, packSizes, @@ -318,8 +324,14 @@ internal static class DecoderStreamHelper } catch (DataErrorException ex) when (encrypted) { + DisposeFailedStreams(outStreams, inStreams); throw EncryptedDecoderStream.DecodingException(ex); } + catch + { + DisposeFailedStreams(outStreams, inStreams); + throw; + } } internal static async ValueTask CreateDecoderStreamAsync( @@ -328,7 +340,8 @@ internal static class DecoderStreamHelper long[] packSizes, CFolder folderInfo, IPasswordProvider pass, - CancellationToken cancellationToken + CancellationToken cancellationToken, + ArrayPool? bufferPool = null ) { if (!folderInfo.CheckStructure()) @@ -337,22 +350,25 @@ internal static class DecoderStreamHelper } var inStreams = new Stream[folderInfo._packStreams.Count]; - for (var j = 0; j < folderInfo._packStreams.Count; j++) - { - inStreams[j] = new BufferedSubStream(inStream, startPos, packSizes[j]); - startPos += packSizes[j]; - } - var outStreams = new Stream[folderInfo._unpackSizes.Count]; - - FindPrimaryOutStreamIndex( - folderInfo, - out var primaryCoderIndex, - out var primaryOutStreamIndex - ); var encrypted = folderInfo.IsEncrypted(); try { + for (var j = 0; j < folderInfo._packStreams.Count; j++) + { + inStreams[j] = new BufferedSubStream( + inStream, + startPos, + packSizes[j], + bufferPool ?? ArrayPool.Shared + ); + startPos += packSizes[j]; + } + FindPrimaryOutStreamIndex( + folderInfo, + out var primaryCoderIndex, + out var primaryOutStreamIndex + ); var decoder = await CreateDecoderStreamAsync( inStreams, packSizes, @@ -367,7 +383,68 @@ internal static class DecoderStreamHelper } catch (DataErrorException ex) when (encrypted) { + await DisposeFailedStreamsAsync(outStreams, inStreams).ConfigureAwait(false); throw EncryptedDecoderStream.DecodingException(ex); } + catch + { + await DisposeFailedStreamsAsync(outStreams, inStreams).ConfigureAwait(false); + throw; + } + } + + private static void DisposeFailedStreams(Stream[] outputs, Stream[] inputs) + { + var seen = new HashSet(); + foreach (var streams in new[] { outputs, inputs }) + { + foreach (var stream in streams) + { + if (stream is null || !seen.Add(stream)) + { + continue; + } + try + { + stream.Dispose(); + } + catch (Exception) + { + // Preserve the construction exception and still release the other streams. + } + } + } + } + + private static async ValueTask DisposeFailedStreamsAsync(Stream[] outputs, Stream[] inputs) + { + var seen = new HashSet(); + foreach (var streams in new[] { outputs, inputs }) + { + foreach (var stream in streams) + { + if (stream is null || !seen.Add(stream)) + { + continue; + } + try + { + if (stream is IAsyncDisposable asyncDisposable) + { + await asyncDisposable.DisposeAsync().ConfigureAwait(false); + } + else + { +#pragma warning disable VSTHRD103 // Legacy streams expose only synchronous disposal. + stream.Dispose(); +#pragma warning restore VSTHRD103 + } + } + catch (Exception) + { + // Preserve the construction exception and still release the other streams. + } + } + } } } diff --git a/src/SharpCompress/IO/BufferedSubStream.cs b/src/SharpCompress/IO/BufferedSubStream.cs index e177fab5..5a5f7be7 100755 --- a/src/SharpCompress/IO/BufferedSubStream.cs +++ b/src/SharpCompress/IO/BufferedSubStream.cs @@ -13,8 +13,13 @@ internal partial class BufferedSubStream : Stream, IStreamStack private readonly Stream _stream; public BufferedSubStream(Stream stream, long origin, long bytesToRead) + : this(stream, origin, bytesToRead, ArrayPool.Shared) { } + + internal BufferedSubStream(Stream stream, long origin, long bytesToRead, ArrayPool pool) { _stream = stream ?? throw new ArgumentNullException(nameof(stream)); + _pool = pool; + _cache = pool.Rent(81920); this.origin = origin; this.BytesLeftToRead = bytesToRead; } @@ -29,7 +34,7 @@ internal partial class BufferedSubStream : Stream, IStreamStack if (disposing && _cache is not null) { - ArrayPool.Shared.Return(_cache); + _pool.Return(_cache); _cache = null; } base.Dispose(disposing); @@ -37,7 +42,8 @@ internal partial class BufferedSubStream : Stream, IStreamStack private int _cacheOffset; private int _cacheLength; - private byte[]? _cache = ArrayPool.Shared.Rent(81920); + private readonly ArrayPool _pool; + private byte[]? _cache; private long origin; private bool _isDisposed; diff --git a/tests/SharpCompress.Test/Streams/DecoderConstructionLifetimeTests.cs b/tests/SharpCompress.Test/Streams/DecoderConstructionLifetimeTests.cs new file mode 100644 index 00000000..a38fba9b --- /dev/null +++ b/tests/SharpCompress.Test/Streams/DecoderConstructionLifetimeTests.cs @@ -0,0 +1,77 @@ +using System; +using System.Buffers; +using System.IO; +using System.Threading; +using System.Threading.Tasks; +using SharpCompress.Common.SevenZip; +using SharpCompress.Compressors.LZMA; +using SharpCompress.Compressors.LZMA.Utilities; +using Xunit; + +namespace SharpCompress.Test.Streams; + +public class DecoderConstructionLifetimeTests +{ + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task UnsupportedDecoderReturnsPackCache(bool asyncOpen) + { + var folder = new CFolder(); + folder._coders.Add( + new CCoderInfo + { + _methodId = new CMethodId(ulong.MaxValue), + _numInStreams = 1, + _numOutStreams = 1, + } + ); + folder._packStreams.Add(0); + folder._unpackSizes.Add(1); + using var input = new MemoryStream(new byte[1]); + var pool = new CountingPool(); + if (asyncOpen) + { + await Assert.ThrowsAsync(async () => + await DecoderStreamHelper.CreateDecoderStreamAsync( + input, + 0, + new long[] { 1 }, + folder, + new NoPassword(), + CancellationToken.None, + pool + ) + ); + } + else + { + Assert.Throws(() => + DecoderStreamHelper.CreateDecoderStream( + input, + 0, + new long[] { 1 }, + folder, + new NoPassword(), + pool + ) + ); + } + Assert.Equal(1, pool.ReturnCount); + Assert.True(input.CanRead); + } + + private sealed class CountingPool : ArrayPool + { + public int ReturnCount { get; private set; } + + public override byte[] Rent(int minimumLength) => new byte[minimumLength]; + + public override void Return(byte[] array, bool clearArray = false) => ReturnCount++; + } + + private sealed class NoPassword : IPasswordProvider + { + public string? CryptoGetTextPassword() => null; + } +}