From 81879e9de4ff6ff45acb450717c0aba8710ed520 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Mon, 19 Oct 2026 14:10:30 +0100 Subject: [PATCH 1/2] Pool RAR Huffman scratch buffers and validate async checksums Reuse unpacker-owned pooled scratch arrays for async RAR5 and RAR2 Huffman tables, preserving alphabet bounds and zeroed history tails. Match async Archive CRC32/BLAKE2sp verification to the synchronous path and dispose entry streams when initialization fails. Add regression tests and profiling documentation showing approximately 3.13 MiB fewer managed allocations per 200 MiB extraction. Keep this change based directly on master without unrelated branch commits. Validation: 2,983 .NET 10 Release tests passed, all six library targets built, net48 tests compiled, and formatting checks passed. --- docs/API.md | 9 + docs/RAR_PROFILING.md | 153 ++++++++++++++ .../Archives/Rar/RarArchiveEntry.Async.cs | 42 ++-- .../Rar/UnpackV2017/Unpack.unpack20_async.cs | 10 +- .../Rar/UnpackV2017/Unpack.unpack50_async.cs | 10 +- .../Rar/UnpackV2017/Unpack.unpack_cpp.cs | 12 ++ .../Compressors/Rar/UnpackV2017/unpack_hpp.cs | 5 + .../Rar/RarCrcExtractionTests.cs | 57 +++++ .../Rar/RarUnpackScratchTests.cs | 197 ++++++++++++++++++ 9 files changed, 474 insertions(+), 21 deletions(-) create mode 100644 docs/RAR_PROFILING.md create mode 100644 tests/SharpCompress.Test/Rar/RarUnpackScratchTests.cs diff --git a/docs/API.md b/docs/API.md index 295feeda..80cc4053 100644 --- a/docs/API.md +++ b/docs/API.md @@ -101,6 +101,15 @@ The stream overloads of `DetectArchive` and `InspectArchive` preserve the suppli | `ZipDataDescriptorEntryCount` | `Zip.HasEntriesWithDeferredSizes` | | `SolidStreamCount` | `SolidStreamCount` | +### RAR Entry Integrity + +RAR Archive and Reader entry streams validate CRC32 (or RAR5 BLAKE2sp when +present) on both synchronous and asynchronous extraction paths. Drain an entry +stream to the end to perform validation; a mismatch throws +`InvalidFormatException`. Disposing a partially read stream does not validate +the unread payload. Encrypted entries retain the existing checksum-checking +limitations; this does not add authentication for encrypted RAR data. + ### RAR5 Archive Comments `SharpCompress.Common.Rar.RarVolume.Comment` exposes UTF-8 archive comments, including encrypted comments when `ReaderOptions.Password` is supplied. Comments are loaded while entries are enumerated; the property can be `null` before enumeration or when no comment is present. diff --git a/docs/RAR_PROFILING.md b/docs/RAR_PROFILING.md new file mode 100644 index 00000000..eac97a8b --- /dev/null +++ b/docs/RAR_PROFILING.md @@ -0,0 +1,153 @@ +# RAR pooling and async checksum evaluation + +## Branch and changes + +Evaluated on 2026-10-19 against `78006f94` on `adam/release-to-master`. +The comparison uses a clean build of that commit and a separate build containing +the working-tree RAR changes, not timings carried over from another branch. +The branch already contains the async unpacker-disposal fix. + +The RAR changes were subsequently ported to `perf/rar-profile-pooling`, based +directly on `master` at `0fbf36d6`, without the unrelated commits from +`adam/release-to-master`. Measurements below remain explicitly tied to the +profiled `78006f94` baseline; they are not a new profile of the master-based port. + +Changes: + +- Async RAR Archive entry streams now select CRC32 or BLAKE2sp verification + using the same rules as the synchronous path. Failed initialization disposes + the newly constructed stream before propagating the exception. +- Async RAR5 and RAR2 Huffman-table scratch buffers are lazily rented, reused + for the unpacker's lifetime, and returned on disposal. Sync stack buffers + are unchanged. +- RAR5 zero-run decoding is bounded by the alphabet size, not a rented array's + capacity. RAR2 table-history padding is cleared to preserve the previous + zero-initialization behavior. +- CRC32 lookup tables were already cached and shared. They are intentionally + not pooled: there is no per-block allocation to eliminate, and returning a + table still referenced by checksum streams would be unsafe. + +## Workload and measurement + +The existing synthetic RAR5 workload is reused: **102.50 MiB compressed** +(107,473,921 bytes), **200 MiB extracted**, four 50 MiB files alternating +64 KiB of seeded random data and 64 KiB of structured records. RAR 7.22 created +it with normal compression, an 8 MiB dictionary, CRC32, no solid compression, +no encryption, and one volume: + +```sh +rar a -cfg- -m3 -md8m -mt1 -s- -ep -htc -qo- -idq mixed-100mb.rar \ + input/mixed-0.bin input/mixed-1.bin input/mixed-2.bin input/mixed-3.bin +``` + +Archive SHA-256: +`f6dcb78eaffa527f9394f47fc869ee396a180f7cbe6e4eb1430a60fe8b925fdd`. + +Release `net10.0` runs use the same harness and archive for both builds. +Each scenario uses a fresh process, two warmups and seven measured extractions. +Version order is reversed between the Archive and Reader scenarios. Output is +discarded; Archive drains verify extracted byte counts and Reader drains use +the normal entry-writing path. Caller-owned streams set `LeaveStreamOpen = true`. + +Memory-backed runs preload compressed input outside the measured region. +File-backed runs use a 4 KiB FileStream buffer, `SequentialScan`, and +`FileOptions.Asynchronous`; their timings include warm filesystem-cache effects, +not cold-disk performance. No builds or test runs compete with profiling. + +Managed allocations are exact `GC.GetTotalAllocatedBytes(true)` deltas around +extraction and disposal. They exclude input preloading and between-iteration +logging but include a 64 KiB caller buffer, unchanged between versions and +unused in Reader scenarios. They are allocations, not live-memory measurements. +The decoder still needs its pooled 8 MiB dictionary and may rent a 4 MiB temporary +output buffer; low warm allocations do not mean those buffers disappeared. + +Separate traces perform ten measured extractions after warmup, using +`dotnet-trace` 10.0.750501 with `dotnet-sampled-thread-time` and GC allocation +ticks. The analyzer uses TraceEvent 3.1.21. CPU stack shares are sampled thread +time on macOS, not exact instruction CPU time. Inlining can attribute helper +work to the decoder loop; file-backed stacks can include waits/safepoints. + +Archive async now performs checksum work absent from its baseline. Its timing +is therefore not an equivalent-work comparison; Reader async already checked +checksums and better isolates the pooling change. This experiment does not +justify claims about all RAR files, dictionary sizes, encryption, solid archives, +or cold storage. + +The recorded runtime is .NET 10.0.13 on the same Apple M4/macOS ARM64 host used +for the earlier evaluation. + +## Fresh results + +Medians per extraction of the complete 200 MiB payload: + +| Input / async API | Baseline elapsed | Updated elapsed | Baseline allocations | Updated allocations | +| --- | ---: | ---: | ---: | ---: | +| Memory / Archive | 702 ms | 755 ms | 5.09 MiB | 1.96 MiB | +| Memory / Reader | 726 ms | 764 ms | 4.64 MiB | 1.51 MiB | +| File / Archive | 716 ms | 800 ms | 8.37 MiB | 5.25 MiB | +| File / Reader | 805 ms | 770 ms | 11.44 MiB | 4.34 MiB | + +The memory-backed results save approximately **3.13 MiB per extraction**, +reducing warm allocations by approximately **62% for Archive** and **67% for +Reader**. File-backed allocations include variable async suspension and pool +misses. In particular, the file Reader baseline is higher than in the previous +branch's run; do not attribute its entire allocation difference to scratch +pooling. The memory-backed comparison better isolates that change. + +This is not evidence of a CPU speedup. Archive now does additional checksum +work, and Reader memory-backed median process CPU was 734 ms before and 770 ms +after in this run. Timings vary with JIT code generation and machine state; +these short sequential measurements demonstrate allocation savings, not a +general throughput improvement. + +All six traces report zero lost events. Baseline allocation samples attribute +approximately 3.0–3.1 MiB per extraction to `ReadTablesAsync` arrays; the updated +measured phases contain **no sampled recurring allocations at those sites**. +The exact total-allocation deltas above are more precise than the sampled +per-type attribution. + +The updated memory-backed Archive trace attributes approximately 83.8% of leaf +samples to the RAR5 decoder loop, 10.2% to `Crc32Stream.CalculateCrc`, and 3.4% to +`MakeDecodeTables`. The baseline Archive trace has no bulk CRC hotspot, confirming +that checksum work is now present. Remaining memory-backed allocations are +primarily `Task` results; file-backed reads also allocate suspended async +state machines and task adapters. Those read-chain allocations are not changed +by scratch pooling. + +## Artifacts and reproduction + +Fresh artifacts are isolated from previous branch evaluations: + +```text +/private/var/folders/6c/j56r6kqn5yj7q1_c6vm5c0t80000gn/T/opencode/rar-100mb-profile/evaluation-78006f94/ +``` + +This contains the baseline source export, separate `head-harness/` and +`pooled-harness/` builds, library hashes, repository-state snapshot, RAR diff, +untraced logs, raw `.nettrace` files, `.speedscope.json` files, and analysis JSON. +Open the Speedscope files in . Filenames identify +version, API, and input source. The original archive and harness are in the +parent directory. + +From that parent directory, `prepare-evaluation.py` builds the current HEAD +baseline and working-tree harnesses without changing the repository, and +`evaluate-current.py evaluation-78006f94` runs the comparison. The preparation +step assumes the updated library has already been built in Release for net10.0. + +## Validation + +- All **544 targeted RAR tests** pass on .NET 10 Release, including CRC32 and + BLAKE2sp mismatch detection, both async read overloads, and entry-copy APIs. +- New scratch-buffer tests cover reuse with poisoned buffers, solid entries, + oversized rents, RAR2 history padding, cancellation/checksum-failure cleanup, + repeated disposal, and non-seekable input ownership. +- The complete .NET 10 Release run on the profiling branch reported **2,995 + passed, no failures**. Validation of the master-based port is recorded below. +- All six library targets build; net48 test compilation succeeds. net48 tests + were not executed on macOS. +- CSharpier formatting/check and `git diff --check` pass. The RAR source diff + matches the saved profiling snapshot. + +The master-based port also passes all 544 targeted RAR tests, all six library +builds, net48 test compilation, and the CSharpier formatting check. Its complete +.NET 10 Release run reports **2,983 passed, no failures**. diff --git a/src/SharpCompress/Archives/Rar/RarArchiveEntry.Async.cs b/src/SharpCompress/Archives/Rar/RarArchiveEntry.Async.cs index 471609aa..49662e74 100644 --- a/src/SharpCompress/Archives/Rar/RarArchiveEntry.Async.cs +++ b/src/SharpCompress/Archives/Rar/RarArchiveEntry.Async.cs @@ -15,29 +15,39 @@ public partial class RarArchiveEntry CancellationToken cancellationToken = default ) { + var readStream = await MultiVolumeReadOnlyAsyncStream + .Create(Parts.ToAsyncEnumerable().CastAsync()) + .ConfigureAwait(false); RarStream stream; if (IsRarV3) { - stream = new RarStream( - archive.UnpackV1.Value, - FileHeader, - await MultiVolumeReadOnlyAsyncStream - .Create(Parts.ToAsyncEnumerable().CastAsync()) - .ConfigureAwait(false) - ); + stream = await RarCrcStream + .CreateAsync(archive.UnpackV1.Value, FileHeader, readStream, cancellationToken) + .ConfigureAwait(false); + } + else if (FileHeader.FileCrc?.Length > 5) + { + stream = await RarBLAKE2spStream + .CreateAsync(archive.UnpackV2017.Value, FileHeader, readStream, cancellationToken) + .ConfigureAwait(false); } else { - stream = new RarStream( - archive.UnpackV2017.Value, - FileHeader, - await MultiVolumeReadOnlyAsyncStream - .Create(Parts.ToAsyncEnumerable().CastAsync()) - .ConfigureAwait(false) - ); + stream = await RarCrcStream + .CreateAsync(archive.UnpackV2017.Value, FileHeader, readStream, cancellationToken) + .ConfigureAwait(false); } - await stream.InitializeAsync(cancellationToken).ConfigureAwait(false); - return stream; + try + { + await stream.InitializeAsync(cancellationToken).ConfigureAwait(false); + return stream; + } + catch + { + // Initialization can fail before the caller can dispose the pooled stream buffers. + await stream.DisposeAsyncCompat().ConfigureAwait(false); + throw; + } } } diff --git a/src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack20_async.cs b/src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack20_async.cs index 84d43060..ee624b1d 100644 --- a/src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack20_async.cs +++ b/src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack20_async.cs @@ -199,8 +199,14 @@ internal partial class Unpack private async ValueTask ReadTables20Async(CancellationToken cancellationToken = default) { - byte[] BitLength = new byte[checked((int)BC20)]; - byte[] Table = new byte[checked((int)MC20 * 4)]; + var BitLength = tableReadBitLengths = EnsureCapacity( + tableReadBitLengths, + checked((int)BC20) + ); + var Table = tableReadLengths = EnsureCapacity(tableReadLengths, checked((int)MC20 * 4)); + // Only TableSize entries are decoded, but the full history table is copied + // below. Preserve the zero-filled tail provided by the former new array. + Array.Clear(Table, 0, checked((int)MC20 * 4)); if (Inp.InAddr > ReadTop - 25) { if (!await UnpReadBufAsync(cancellationToken).ConfigureAwait(false)) diff --git a/src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack50_async.cs b/src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack50_async.cs index fbb2d607..0812261c 100644 --- a/src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack50_async.cs +++ b/src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack50_async.cs @@ -602,7 +602,7 @@ internal partial class Unpack } } - var BitLength = new byte[checked((int)BC)]; + var BitLength = tableReadBitLengths = EnsureCapacity(tableReadBitLengths, checked((int)BC)); for (int I = 0; I < BC; I++) { uint Length = (byte)(Inp.fgetbits() >> 12); @@ -618,7 +618,8 @@ internal partial class Unpack else { ZeroCount += 2; - while (ZeroCount-- > 0 && I < BitLength.Length) + // A rented array can be larger than the RAR alphabet. + while (ZeroCount-- > 0 && I < BC) { BitLength[I++] = 0; } @@ -634,7 +635,10 @@ internal partial class Unpack MakeDecodeTables(BitLength, 0, BlockTables.BD, BC); - var Table = new byte[checked((int)HUFF_TABLE_SIZE)]; + var Table = tableReadLengths = EnsureCapacity( + tableReadLengths, + checked((int)HUFF_TABLE_SIZE) + ); const int TableSize = checked((int)HUFF_TABLE_SIZE); for (int I = 0; I < TableSize; ) { diff --git a/src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack_cpp.cs b/src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack_cpp.cs index 72c808f9..a4d78a58 100644 --- a/src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack_cpp.cs +++ b/src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack_cpp.cs @@ -73,6 +73,18 @@ internal sealed partial class Unpack : BitInput FilterDstMemory = Array.Empty(); } + if (tableReadBitLengths.Length != 0) + { + ArrayPool.Shared.Return(tableReadBitLengths); + tableReadBitLengths = Array.Empty(); + } + + if (tableReadLengths.Length != 0) + { + ArrayPool.Shared.Return(tableReadLengths); + tableReadLengths = Array.Empty(); + } + disposed = true; } diff --git a/src/SharpCompress/Compressors/Rar/UnpackV2017/unpack_hpp.cs b/src/SharpCompress/Compressors/Rar/UnpackV2017/unpack_hpp.cs index dd78b27c..61f89112 100644 --- a/src/SharpCompress/Compressors/Rar/UnpackV2017/unpack_hpp.cs +++ b/src/SharpCompress/Compressors/Rar/UnpackV2017/unpack_hpp.cs @@ -245,6 +245,11 @@ internal partial class Unpack private byte[] FilterSrcMemory = Array.Empty(); private byte[] FilterDstMemory = Array.Empty(); + // Async table reads cannot keep stack buffers across awaits. Reuse these for + // the unpacker's lifetime, just like its dictionary and filter buffers. + private byte[] tableReadBitLengths = Array.Empty(); + private byte[] tableReadLengths = Array.Empty(); + // Filters code, one entry per filter. private readonly List Filters = new(); private readonly List FilterPool = new(); diff --git a/tests/SharpCompress.Test/Rar/RarCrcExtractionTests.cs b/tests/SharpCompress.Test/Rar/RarCrcExtractionTests.cs index f2dc8f4f..7e2599ce 100644 --- a/tests/SharpCompress.Test/Rar/RarCrcExtractionTests.cs +++ b/tests/SharpCompress.Test/Rar/RarCrcExtractionTests.cs @@ -1,5 +1,8 @@ +using System; using System.IO; using System.Linq; +using System.Threading; +using System.Threading.Tasks; using SharpCompress.Archives; using SharpCompress.Archives.Rar; using SharpCompress.Common; @@ -21,6 +24,60 @@ public class RarCrcExtractionTests : ArchiveTests Assert.Throws(() => entry.WriteToFile(destination)); } + [Theory] + [InlineData("Rar.rar", false)] + [InlineData("Rar5.rar", false)] + [InlineData("Rar5.crc_blake2.rar", false)] +#if !LEGACY_DOTNET + [InlineData("Rar.rar", true)] + [InlineData("Rar5.rar", true)] + [InlineData("Rar5.crc_blake2.rar", true)] +#endif + public async Task Rar_Archive_ReadAsync_Throws_On_Crc_Mismatch( + string archiveName, + bool useMemory + ) + { + await using var archive = await RarArchive.OpenAsyncArchive( + Path.Combine(TEST_ARCHIVES_PATH, archiveName), + cancellationToken: CancellationToken.None + ); + var entry = await archive.EntriesAsync.Where(e => !e.IsDirectory).FirstAsync(); + CorruptCrc((RarArchiveEntry)entry); + using var input = await entry.OpenEntryStreamAsync(CancellationToken.None); + var buffer = new byte[4096]; + + await Assert.ThrowsAsync(async () => + { +#if !LEGACY_DOTNET + if (useMemory) + { + while (await input.ReadAsync(buffer.AsMemory(), CancellationToken.None) != 0) { } + return; + } +#endif + while (await input.ReadAsync(buffer, 0, buffer.Length, CancellationToken.None) != 0) { } + }); + } + + [Theory] + [InlineData("Rar.rar")] + [InlineData("Rar5.rar")] + [InlineData("Rar5.crc_blake2.rar")] + public async Task Rar_Archive_WriteToAsync_Throws_On_Crc_Mismatch(string archiveName) + { + await using var archive = await RarArchive.OpenAsyncArchive( + Path.Combine(TEST_ARCHIVES_PATH, archiveName), + cancellationToken: CancellationToken.None + ); + var entry = await archive.EntriesAsync.Where(e => !e.IsDirectory).FirstAsync(); + CorruptCrc((RarArchiveEntry)entry); + + await Assert.ThrowsAsync(async () => + await entry.WriteToAsync(Stream.Null, cancellationToken: CancellationToken.None) + ); + } + private static RarArchiveEntry CorruptFirstFileCrc(IArchive archive) { var entry = archive.Entries.OfType().First(e => !e.IsDirectory); diff --git a/tests/SharpCompress.Test/Rar/RarUnpackScratchTests.cs b/tests/SharpCompress.Test/Rar/RarUnpackScratchTests.cs new file mode 100644 index 00000000..fb2b49a4 --- /dev/null +++ b/tests/SharpCompress.Test/Rar/RarUnpackScratchTests.cs @@ -0,0 +1,197 @@ +using System; +using System.Buffers; +using System.IO; +using System.Linq; +using System.Reflection; +using System.Threading; +using System.Threading.Tasks; +using SharpCompress.Archives; +using SharpCompress.Archives.Rar; +using SharpCompress.Common; +using SharpCompress.Compressors.Rar; +using SharpCompress.Compressors.Rar.UnpackV2017; +using SharpCompress.Readers; +using SharpCompress.Readers.Rar; +using SharpCompress.Test.Mocks; +using Xunit; + +namespace SharpCompress.Test.Rar; + +public class RarUnpackScratchTests : TestBase +{ + [Theory] + [InlineData("Rar5.rar")] + [InlineData("Rar5.solid.rar")] + [InlineData("Rar5.crc_blake2.rar")] + public async Task Rar_Async_Reuses_Huffman_Scratch_Between_Entries(string archiveName) + { + using var source = File.OpenRead(Path.Combine(TEST_ARCHIVES_PATH, archiveName)); + await using var archive = await RarArchive.OpenAsyncArchive( + source, + ReaderOptions.ForExternalStream, + CancellationToken.None + ); + byte[]? bitLengths = null; + byte[]? lengths = null; + var entryCount = 0; + await foreach (var entry in archive.EntriesAsync) + { + if (entry.IsDirectory) + { + continue; + } + await entry.WriteToAsync(Stream.Null, cancellationToken: CancellationToken.None); + var unpack = ((RarArchive)archive).UnpackV2017.Value; + var currentBitLengths = GetScratch(unpack, "tableReadBitLengths"); + var currentLengths = GetScratch(unpack, "tableReadLengths"); + Assert.NotEmpty(currentBitLengths); + Assert.NotEmpty(currentLengths); + if (bitLengths is not null) + { + Assert.Same(bitLengths, currentBitLengths); + Assert.Same(lengths, currentLengths); + } + bitLengths = currentBitLengths; + lengths = currentLengths; + // Reused buffers must not depend on the zero initialization of new arrays. + currentBitLengths.AsSpan().Fill(0xCC); + currentLengths.AsSpan().Fill(0xCC); + entryCount++; + } + Assert.True(entryCount > 1); + await archive.DisposeAsync(); + Assert.True(source.CanRead); + Assert.Empty(GetScratch(((RarArchive)archive).UnpackV2017.Value, "tableReadBitLengths")); + Assert.Empty(GetScratch(((RarArchive)archive).UnpackV2017.Value, "tableReadLengths")); + } + + [Theory] + [InlineData(false)] +#if !LEGACY_DOTNET + [InlineData(true)] +#endif + public async Task Rar_Async_Releases_Huffman_Scratch_After_Failure(bool cancel) + { + using var source = File.OpenRead(Path.Combine(TEST_ARCHIVES_PATH, "Rar5.rar")); + var archive = await RarArchive.OpenAsyncArchive( + source, + ReaderOptions.ForExternalStream, + CancellationToken.None + ); + await using (archive) + { + var entry = await archive.EntriesAsync.Where(e => !e.IsDirectory).FirstAsync(); + using var input = await entry.OpenEntryStreamAsync(CancellationToken.None); + var buffer = new byte[1024]; + Assert.True( + await input.ReadAsync(buffer, 0, buffer.Length, CancellationToken.None) > 0 + ); + var unpack = ((RarArchive)archive).UnpackV2017.Value; + Assert.NotEmpty(GetScratch(unpack, "tableReadLengths")); + if (cancel) + { +#if !LEGACY_DOTNET + using var cancellation = new CancellationTokenSource(); + cancellation.Cancel(); + await Assert.ThrowsAnyAsync(async () => + await input.ReadAsync(buffer.AsMemory(), cancellation.Token) + ); +#endif + } + else + { + ((RarArchiveEntry)entry).FileHeader.FileCrc.NotNull()[0] ^= 0xFF; + await Assert.ThrowsAsync(async () => + await input.CopyToAsync(Stream.Null, 81920, CancellationToken.None) + ); + } + } + await archive.DisposeAsync(); + Assert.Empty(GetScratch(((RarArchive)archive).UnpackV2017.Value, "tableReadBitLengths")); + Assert.Empty(GetScratch(((RarArchive)archive).UnpackV2017.Value, "tableReadLengths")); + Assert.True(source.CanRead); + } + + [Fact] + public async Task Rar5_Async_Zero_Run_Uses_Alphabet_Size_Not_Rented_Length() + { + using var unpack = new Unpack(); + var bitLengths = ArrayPool.Shared.Rent((int)PackDef.BC); + bitLengths.AsSpan().Fill(0xCC); + GetField("tableReadBitLengths").SetValue(unpack, bitLengths); + GetField("BlockHeader").SetValue(unpack, new UnpackBlockHeader { TablePresent = true }); + using var input = new BitInput(false) { ExternalBuffer = true, InBuf = new byte[4096] }; + // Two runs of 17 zeros exceed the 20-symbol alphabet, but not the rented array. + input.InBuf[0] = 0xFF; + input.InBuf[1] = 0xFF; + var readTables = typeof(Unpack).GetMethod( + "ReadTablesAsync", + BindingFlags.Instance | BindingFlags.NonPublic + )!; + Assert.True( + await (ValueTask)readTables.Invoke(unpack, [input, CancellationToken.None])! + ); + for (var i = (int)PackDef.BC; i < bitLengths.Length; i++) + { + Assert.Equal(0xCC, bitLengths[i]); + } + } + + [Fact] + public async Task Rar2_Async_Huffman_History_Clears_Unused_Pooled_Tail() + { + using var unpack = new Unpack(); + var lengths = ArrayPool.Shared.Rent(checked((int)PackDef.MC20 * 4)); + lengths.AsSpan().Fill(0xCC); + GetField("tableReadLengths").SetValue(unpack, lengths); + // Zero code lengths decode a non-audio table, leaving the audio-history tail unused. + unpack.InBuf.AsSpan().Clear(); + GetField("ReadTop").SetValue(unpack, 4096); + var readTables = typeof(Unpack).GetMethod( + "ReadTables20Async", + BindingFlags.Instance | BindingFlags.NonPublic + )!; + Assert.True(await (ValueTask)readTables.Invoke(unpack, [CancellationToken.None])!); + Assert.All(GetScratch(unpack, "UnpOldTable20"), value => Assert.Equal(0, value)); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Rar_Reader_Async_Dispose_Releases_Huffman_Scratch(bool leaveStreamOpen) + { + using var source = new ForwardOnlyStream( + File.OpenRead(Path.Combine(TEST_ARCHIVES_PATH, "Rar5.solid.rar")) + ); + await using var reader = await ReaderFactory.OpenAsyncReader( + source, + new ReaderOptions { LeaveStreamOpen = leaveStreamOpen }, + CancellationToken.None + ); + while (await reader.MoveToNextEntryAsync(CancellationToken.None)) + { + if (!reader.Entry.IsDirectory) + { + await reader.WriteEntryToAsync(Stream.Null, CancellationToken.None); + } + } + var unpack = ( + (Lazy) + typeof(RarReader) + .GetProperty("UnpackV2017", BindingFlags.Instance | BindingFlags.NonPublic)! + .GetValue(reader)! + ).Value; + Assert.NotEmpty(GetScratch(unpack, "tableReadLengths")); + await reader.DisposeAsync(); + await reader.DisposeAsync(); + Assert.Empty(GetScratch(unpack, "tableReadBitLengths")); + Assert.Empty(GetScratch(unpack, "tableReadLengths")); + Assert.Equal(leaveStreamOpen, source.CanRead); + } + + private static FieldInfo GetField(string name) => + typeof(Unpack).GetField(name, BindingFlags.Instance | BindingFlags.NonPublic)!; + + private static byte[] GetScratch(object unpack, string name) => + (byte[])GetField(name).GetValue(unpack)!; +} From 714a27fd40631586f0df93c587c97e758b62c38c Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Tue, 20 Oct 2026 11:11:43 +0100 Subject: [PATCH 2/2] Fix RAR checksum handling for empty reads and checksum-free entries --- docs/API.md | 8 +- .../Rar/RarBLAKE2spStream.Async.cs | 12 +- .../Compressors/Rar/RarBLAKE2spStream.cs | 4 +- .../Compressors/Rar/RarCrcStream.Async.cs | 6 +- .../Compressors/Rar/RarCrcStream.cs | 3 +- .../Rar/RarCrcExtractionTests.cs | 135 ++++++++++++++++++ 6 files changed, 153 insertions(+), 15 deletions(-) diff --git a/docs/API.md b/docs/API.md index 9bd7cd06..97255d83 100644 --- a/docs/API.md +++ b/docs/API.md @@ -106,8 +106,12 @@ The stream overloads of `DetectArchive` and `InspectArchive` preserve the suppli RAR Archive and Reader entry streams validate CRC32 (or RAR5 BLAKE2sp when present) on both synchronous and asynchronous extraction paths. Drain an entry stream to the end to perform validation; a mismatch throws -`InvalidFormatException`. Disposing a partially read stream does not validate -the unread payload. Encrypted entries retain the existing checksum-checking +`InvalidFormatException`. Entries without a checksum are extracted without +checksum validation. Disposing a partially read Archive entry stream does not +validate the unread payload. Reader entry streams normally drain unread data +on disposal (synchronous or asynchronous) unless the reader is cancelled, so +disposal can validate the checksum and throw on a mismatch. +Encrypted entries retain the existing checksum-checking limitations; this does not add authentication for encrypted RAR data. ### RAR5 Archive Comments diff --git a/src/SharpCompress/Compressors/Rar/RarBLAKE2spStream.Async.cs b/src/SharpCompress/Compressors/Rar/RarBLAKE2spStream.Async.cs index 53560283..d112e34d 100644 --- a/src/SharpCompress/Compressors/Rar/RarBLAKE2spStream.Async.cs +++ b/src/SharpCompress/Compressors/Rar/RarBLAKE2spStream.Async.cs @@ -32,10 +32,10 @@ internal partial class RarBLAKE2spStream : RarStream { Update(_blake2sp!, new ReadOnlySpan(buffer, offset, result)); } - else + else if (count != 0) { EnsureHash(); - if (!disableCRCCheck && !GetCrc().SequenceEqual(readStream.CurrentCrc) && count != 0) + if (!disableCRCCheck && !GetCrc().SequenceEqual(readStream.CurrentCrc)) { // NOTE: we use the last FileHeader in a multipart volume to check CRC throw new InvalidFormatException("file crc mismatch"); @@ -56,14 +56,10 @@ internal partial class RarBLAKE2spStream : RarStream { Update(_blake2sp!, buffer.Span.Slice(0, result)); } - else + else if (!buffer.IsEmpty) { EnsureHash(); - if ( - !disableCRCCheck - && !GetCrc().SequenceEqual(readStream.CurrentCrc) - && buffer.Length != 0 - ) + if (!disableCRCCheck && !GetCrc().SequenceEqual(readStream.CurrentCrc)) { // NOTE: we use the last FileHeader in a multipart volume to check CRC throw new InvalidFormatException("file crc mismatch"); diff --git a/src/SharpCompress/Compressors/Rar/RarBLAKE2spStream.cs b/src/SharpCompress/Compressors/Rar/RarBLAKE2spStream.cs index 54ecda29..e9e80bb6 100644 --- a/src/SharpCompress/Compressors/Rar/RarBLAKE2spStream.cs +++ b/src/SharpCompress/Compressors/Rar/RarBLAKE2spStream.cs @@ -348,10 +348,10 @@ internal partial class RarBLAKE2spStream : RarStream { Update(this._blake2sp!, new ReadOnlySpan(buffer, offset, result)); } - else + else if (count != 0) { EnsureHash(); - if (!disableCRCCheck && !GetCrc().SequenceEqual(readStream.CurrentCrc) && count != 0) + if (!disableCRCCheck && !GetCrc().SequenceEqual(readStream.CurrentCrc)) { // NOTE: we use the last FileHeader in a multipart volume to check CRC throw new InvalidFormatException("file crc mismatch"); diff --git a/src/SharpCompress/Compressors/Rar/RarCrcStream.Async.cs b/src/SharpCompress/Compressors/Rar/RarCrcStream.Async.cs index 0042af7a..394c3baf 100644 --- a/src/SharpCompress/Compressors/Rar/RarCrcStream.Async.cs +++ b/src/SharpCompress/Compressors/Rar/RarCrcStream.Async.cs @@ -34,8 +34,9 @@ internal partial class RarCrcStream : RarStream } else if ( !_disableCrc - && GetCrc() != BitConverter.ToUInt32(_readStream.NotNull().CurrentCrc.NotNull(), 0) && count != 0 + && _readStream.CurrentCrc is { } crc + && GetCrc() != BitConverter.ToUInt32(crc, 0) ) { // NOTE: we use the last FileHeader in a multipart volume to check CRC @@ -59,8 +60,9 @@ internal partial class RarCrcStream : RarStream } else if ( !_disableCrc - && GetCrc() != BitConverter.ToUInt32(_readStream.NotNull().CurrentCrc.NotNull(), 0) && buffer.Length != 0 + && _readStream.CurrentCrc is { } crc + && GetCrc() != BitConverter.ToUInt32(crc, 0) ) { // NOTE: we use the last FileHeader in a multipart volume to check CRC diff --git a/src/SharpCompress/Compressors/Rar/RarCrcStream.cs b/src/SharpCompress/Compressors/Rar/RarCrcStream.cs index 233b05be..b9be981d 100644 --- a/src/SharpCompress/Compressors/Rar/RarCrcStream.cs +++ b/src/SharpCompress/Compressors/Rar/RarCrcStream.cs @@ -48,8 +48,9 @@ internal partial class RarCrcStream : RarStream } else if ( !_disableCrc - && GetCrc() != BitConverter.ToUInt32(_readStream.NotNull().CurrentCrc.NotNull(), 0) && count != 0 + && _readStream.CurrentCrc is { } crc + && GetCrc() != BitConverter.ToUInt32(crc, 0) ) { // NOTE: we use the last FileHeader in a multipart volume to check CRC diff --git a/tests/SharpCompress.Test/Rar/RarCrcExtractionTests.cs b/tests/SharpCompress.Test/Rar/RarCrcExtractionTests.cs index 7e2599ce..37755f83 100644 --- a/tests/SharpCompress.Test/Rar/RarCrcExtractionTests.cs +++ b/tests/SharpCompress.Test/Rar/RarCrcExtractionTests.cs @@ -1,17 +1,152 @@ using System; using System.IO; using System.Linq; +using System.Text; using System.Threading; using System.Threading.Tasks; using SharpCompress.Archives; using SharpCompress.Archives.Rar; using SharpCompress.Common; +using SharpCompress.Compressors.Rar; +using SharpCompress.Readers; +using SharpCompress.Readers.Rar; using Xunit; namespace SharpCompress.Test.Rar; public class RarCrcExtractionTests : ArchiveTests { + [Theory] + [InlineData(0)] + [InlineData(1)] +#if !LEGACY_DOTNET + [InlineData(2)] +#endif + public async Task Rar_Archive_Blake2_Empty_Read_Does_Not_Finalize_Hash(int readMode) + { + var path = Path.Combine(TEST_ARCHIVES_PATH, "Rar5.crc_blake2.rar"); + using var expectedArchive = RarArchive.OpenArchive(path); + var expectedEntry = expectedArchive.Entries.First(e => !e.IsDirectory); + using var expectedInput = expectedEntry.OpenEntryStream(); + using var expected = new MemoryStream(); + expectedInput.CopyTo(expected); + + await using var archive = await RarArchive.OpenAsyncArchive( + path, + cancellationToken: CancellationToken.None + ); + var entry = await archive.EntriesAsync.Where(e => !e.IsDirectory).FirstAsync(); + using var input = await entry.OpenEntryStreamAsync(CancellationToken.None); + var buffer = new byte[4096]; + using var output = new MemoryStream(); + int read; + do + { + // Empty requests before, during and after extraction must not finalize the hash. + Assert.Equal(0, await Read(input, buffer, 0, readMode)); + read = await Read(input, buffer, buffer.Length, readMode); + output.Write(buffer, 0, read); + } while (read != 0); + + Assert.Equal(0, await Read(input, buffer, 0, readMode)); + Assert.Equal(expected.ToArray(), output.ToArray()); + } + + [Theory] + [InlineData(0, false)] + [InlineData(0, true)] + [InlineData(1, false)] + [InlineData(1, true)] +#if !LEGACY_DOTNET + [InlineData(2, false)] + [InlineData(2, true)] +#endif + public async Task Rar5_Entry_Without_Checksum_Extracts(int readMode, bool useReader) + { + using var source = CreateChecksumFreeArchive(); + using var output = new MemoryStream(); + var buffer = new byte[32]; + if (useReader && readMode == 0) + { + using var reader = RarReader.OpenReader(source, ReaderOptions.ForExternalStream); + Assert.True(reader.MoveToNextEntry()); + using var input = reader.OpenEntryStream(); + await Drain(input); + } + else if (useReader) + { + await using var reader = await ReaderFactory.OpenAsyncReader( + source, + ReaderOptions.ForExternalStream, + CancellationToken.None + ); + Assert.True(await reader.MoveToNextEntryAsync(CancellationToken.None)); + await using var input = await reader.OpenEntryStreamAsync(CancellationToken.None); + await Drain(input); + } + else + { + await using var archive = await RarArchive.OpenAsyncArchive( + source, + ReaderOptions.ForExternalStream, + CancellationToken.None + ); + var entry = (RarArchiveEntry)await archive.EntriesAsync.FirstAsync(); + Assert.Null(entry.FileHeader.FileCrc); + using var input = await entry.OpenEntryStreamAsync(CancellationToken.None); + await Drain(input); + } + Assert.Equal("hello", Encoding.UTF8.GetString(output.ToArray())); + + async Task Drain(Stream input) + { + Assert.Equal(0, await Read(input, buffer, 0, readMode)); + int read; + while ((read = await Read(input, buffer, buffer.Length, readMode)) != 0) + { + output.Write(buffer, 0, read); + } + } + } + + private static async Task Read(Stream input, byte[] buffer, int count, int readMode) + { + if (readMode == 0) + { + return input.Read(buffer, 0, count); + } +#if !LEGACY_DOTNET + if (readMode == 2) + { + return await input.ReadAsync(buffer.AsMemory(0, count), CancellationToken.None); + } +#endif + return await input.ReadAsync(buffer, 0, count, CancellationToken.None); + } + + private static MemoryStream CreateChecksumFreeArchive() + { + var archive = new MemoryStream(); + using var writer = new BinaryWriter(archive, Encoding.UTF8, leaveOpen: true); + writer.Write(new byte[] { 0x52, 0x61, 0x72, 0x21, 0x1a, 0x07, 0x01, 0x00 }); + WriteHeader([1, 0, 0]); + // Stored RAR5 file, no CRC flag or hash extra record; all vints fit in one byte. + WriteHeader([2, 2, 5, 0, 5, 0, 0, 1, 1, (byte)'a']); + writer.Write(Encoding.UTF8.GetBytes("hello")); + WriteHeader([5, 0, 0]); + archive.Position = 0; + return archive; + + void WriteHeader(byte[] body) + { + var data = new byte[body.Length + 1]; + data[0] = (byte)body.Length; + body.CopyTo(data, 1); + writer.Write(~RarCRC.CheckCrc(uint.MaxValue, data, 0, data.Length)); + writer.Write(data); + } + } + [Theory] [InlineData("Rar.rar")] [InlineData("Rar5.rar")]