OpenAsyncReader, OpenAsyncArchive and others must be async for Tar detection#1210
OpenAsyncReader, OpenAsyncArchive and others must be async for Tar detection#1210adamhathcock merged 6 commits intomasterfrom
Conversation
Code Review SummaryStatus: No Issues Found | Recommendation: Merge OverviewThis PR introduces significant improvements to the async API consistency across SharpCompress:
Files Reviewed (30+ files)Archive Factory Changes:
Archive Type-Specific Changes:
Reader Factory Changes:
Factory Implementations:
Documentation:
Test Updates:
Quality Observations
|
There was a problem hiding this comment.
Pull request overview
This pull request addresses issue #1206 where compressed TAR formats were broken when using ArchiveFactory.OpenArchive(). The root cause was that TAR detection within compressed streams (e.g., tar.gz, tar.bz2) was performed synchronously in async code paths, which doesn't work for streams that only support async reads (like AsyncOnlyStream).
The fix modernizes the async API surface by changing OpenAsyncArchive and OpenAsyncReader methods to return ValueTask<T> instead of directly returning the archive/reader instance. This allows the methods to perform actual async I/O during TAR format detection. Additionally, CancellationToken support is added throughout the async APIs.
Changes:
- Modified all
OpenAsyncArchiveandOpenAsyncReadermethods to returnValueTask<T>and acceptCancellationTokenparameters, enabling truly asynchronous TAR detection - Updated
TarArchiveto track compression type internally, allowing it to properly handle compressed TAR files by decompressing the underlying stream before reading entries - Added
TarFactory.GetCompressionType/Asynchelper methods to detect TAR compression types synchronously and asynchronously - Updated documentation to use
await usingpattern for async archive operations
Reviewed changes
Copilot reviewed 43 out of 43 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/SharpCompress/Archives/Tar/TarArchive.cs |
Added _compressionType field and GetStream() method to decompress TAR streams based on detected compression type |
src/SharpCompress/Archives/Tar/TarArchive.Factory.cs |
Updated OpenAsyncArchive overloads to use GetCompressionTypeAsync for TAR detection; changed return types to ValueTask<T> |
src/SharpCompress/Factories/TarFactory.cs |
Added GetCompressionType/Async methods to detect compression type in TAR files; updated factory methods to properly await async archive creation |
src/SharpCompress/Readers/Tar/TarReader.Factory.cs |
Moved TAR detection logic from TarReader.cs and made OpenAsyncReader truly async with TAR wrapper detection |
src/SharpCompress/Archives/IArchiveFactory.cs |
Changed OpenAsyncArchive methods to return ValueTask<IAsyncArchive> and accept CancellationToken |
src/SharpCompress/Archives/IArchiveOpenable.cs |
Updated interface to require ValueTask<TASync> return types with CancellationToken support |
src/SharpCompress/Archives/IMultiArchiveOpenable.cs |
Updated multi-volume archive async methods to return ValueTask<TASync> |
src/SharpCompress/Archives/IWritableArchiveOpenable.cs |
Changed CreateAsyncArchive to return ValueTask<T> |
src/SharpCompress/Readers/IReaderOpenable.cs |
Updated reader interface async methods to return ValueTask<IAsyncReader> with cancellation support |
src/SharpCompress/Factories/*.cs |
Updated all factory implementations (GZip, Rar, SevenZip, Zip, Lzw) to properly return ValueTask<T> and check cancellation tokens |
src/SharpCompress/Readers/*/Reader.Factory.cs |
Updated all reader factory implementations to return ValueTask<IAsyncReader> with cancellation support |
tests/SharpCompress.Test/**/*AsyncTests.cs |
Fixed all async test cases to properly await OpenAsyncArchive and OpenAsyncReader calls |
tests/SharpCompress.Test/Tar/TarArchiveTests.cs |
Added tests for compressed TAR detection via ArchiveFactory.OpenArchive |
docs/API.md |
Updated documentation examples to use await using pattern for async archives |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public static async ValueTask<CompressionType> GetCompressionTypeAsync( | ||
| Stream stream, | ||
| CancellationToken cancellationToken = default | ||
| ) | ||
| { | ||
| stream.Seek(0, SeekOrigin.Begin); | ||
| foreach (var wrapper in TarWrapper.Wrappers) | ||
| { | ||
| stream.Seek(0, SeekOrigin.Begin); | ||
| if (wrapper.IsMatch(stream)) | ||
| { | ||
| stream.Seek(0, SeekOrigin.Begin); | ||
| var decompressedStream = wrapper.CreateStream(stream); | ||
| if ( | ||
| await TarArchive | ||
| .IsTarFileAsync(decompressedStream, cancellationToken) | ||
| .ConfigureAwait(false) | ||
| ) | ||
| { | ||
| return wrapper.CompressionType; | ||
| } | ||
| } | ||
| } | ||
| throw new InvalidFormatException("Not a tar file."); | ||
| } |
There was a problem hiding this comment.
The GetCompressionTypeAsync method has the same issue as the synchronous version - it doesn't restore the stream position if no wrapper matches or if a wrapper matches but the decompressed stream is not a tar file. The stream should be reset to the beginning after checking all wrappers without finding a match, and should be reset before continuing to the next wrapper if IsTarFileAsync returns false.
| public static CompressionType GetCompressionType(Stream stream) | ||
| { | ||
| stream.Seek(0, SeekOrigin.Begin); | ||
| foreach (var wrapper in TarWrapper.Wrappers) | ||
| { | ||
| stream.Seek(0, SeekOrigin.Begin); | ||
| if (wrapper.IsMatch(stream)) | ||
| { | ||
| stream.Seek(0, SeekOrigin.Begin); | ||
| var decompressedStream = wrapper.CreateStream(stream); | ||
| if (TarArchive.IsTarFile(decompressedStream)) | ||
| { | ||
| return wrapper.CompressionType; | ||
| } | ||
| } | ||
| } | ||
| throw new InvalidFormatException("Not a tar file."); | ||
| } |
There was a problem hiding this comment.
The GetCompressionType method doesn't restore the stream position if no wrapper matches or if a wrapper matches but the decompressed stream is not a tar file. After checking all wrappers without finding a match, the stream position should be reset to the beginning. Similarly, if a wrapper matches but IsTarFile returns false, the stream should be reset before continuing to the next wrapper.
addresses #1206
Tar detection was done wrong and not in all code paths. This means that Open/Create for asynchronous paths must be asynchronous which breaks the API unfortunately
This pull request refactors the async archive opening APIs across the codebase to use
ValueTask<T>instead of returning the archive type directly, and adds support for cancellation tokens. It also updates related interfaces and documentation to reflect these changes, and ensures that async APIs can be properly awaited and canceled. Additionally, the documentation and sample usages are updated to useawait usingfor asynchronous disposables.API Modernization and Async Improvements
OpenAsyncArchivemethods in archive factories (such asGZipArchive,RarArchive,SevenZipArchive) to returnValueTask<T>and accept aCancellationToken, enabling more efficient async usage and cancellation support. (src/SharpCompress/Archives/GZip/GZipArchive.Factory.cs[1] [2];src/SharpCompress/Archives/Rar/RarArchive.Factory.cs[3] [4];src/SharpCompress/Archives/SevenZip/SevenZipArchive.Factory.cs[5] [6]IArchiveFactory,IArchiveOpenable,IMultiArchiveOpenable,IWritableArchiveOpenable) to reflect the new async signatures usingValueTask<T>and to acceptCancellationToken. (src/SharpCompress/Archives/IArchiveFactory.cs[1] [2];src/SharpCompress/Archives/IArchiveOpenable.cs[3];src/SharpCompress/Archives/IMultiArchiveOpenable.cs[4];src/SharpCompress/Archives/IWritableArchiveOpenable.cs[5]Documentation and Sample Usage Updates
docs/API.mdto useawait usingfor asynchronous disposables, reflecting the new async API usage patterns. [1] [2] [3]Code Cleanup and Consistency
usingdirectives and made minor cleanups in several files for consistency and clarity. (src/SharpCompress/Archives/ArchiveFactory.cs[1]src/SharpCompress/Archives/Tar/TarArchive.Factory.cs[2]TarArchive.Factoryto call the multi-file overload for consistency. (src/SharpCompress/Archives/Tar/TarArchive.Factory.cssrc/SharpCompress/Archives/Tar/TarArchive.Factory.csL40-L45)These changes modernize the async API surface, improve cancellation support, and align documentation and usage with current C# best practices.