-
Notifications
You must be signed in to change notification settings - Fork 499
Fix ArchiveFactory.Open double-wrapping causing "Cannot determine compressed stream type" on Linux #997
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
Use SharpCompressStream.Create instead of constructor to properly handle streams that are already wrapped. This prevents potential buffering issues when opening ZIP files, particularly on Linux systems. Added tests to verify both raw FileStream and pre-wrapped stream scenarios. Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes an issue where ZIP files fail to open on Linux by preventing double-wrapping of streams when ArchiveFactory.Open is called with a stream that's already wrapped in SharpCompressStream. The fix changes ArchiveFactory.Open to use the SharpCompressStream.Create static factory method instead of directly constructing a new instance, which checks if the stream is already wrapped and reuses it if appropriate.
- Changed
ArchiveFactory.Opento useSharpCompressStream.Createinstead of direct constructor call - Added two test cases to verify the fix handles both pre-wrapped and raw file streams correctly
- Updated package lock file resolving
Microsoft.NET.ILLink.Tasksfrom version 8.0.17 to 8.0.0
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/SharpCompress/Archives/ArchiveFactory.cs | Changed stream wrapping to use factory method that prevents double-wrapping |
| tests/SharpCompress.Test/ArchiveTests.cs | Added tests for both pre-wrapped and raw file stream scenarios |
| src/SharpCompress/packages.lock.json | Downgraded Microsoft.NET.ILLink.Tasks package version |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| // Test that ArchiveFactory.Open works correctly with a stream that's already wrapped | ||
| // This addresses the issue where ZIP files fail to open on Linux | ||
| var testArchive = Path.Combine(TEST_ARCHIVES_PATH, "Zip.bzip2.noEmptyDirs.zip"); |
Copilot
AI
Oct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Call to 'System.IO.Path.Combine'.
| { | ||
| // Test that ArchiveFactory.Open works correctly with a raw FileStream | ||
| // This is the common use case reported in the issue | ||
| var testArchive = Path.Combine(TEST_ARCHIVES_PATH, "Zip.bzip2.noEmptyDirs.zip"); |
Copilot
AI
Oct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Call to 'System.IO.Path.Combine'.
…ive-linux-issue # Conflicts: # src/SharpCompress/packages.lock.json
|
While testing the new release, it turned out the issue was simply a missing Git LFS installation in the CI test pipeline. However, the problem in this library is still the misleading error message when the stream is actually empty: Maybe we can add a check for an empty stream and throw a more specific error message in that case? |
ArchiveFactory.Open(stream)fails on Linux with "Cannot determine compressed stream type" when the stream is already wrapped or when buffering state needs careful management.Root Cause
ArchiveFactory.Openunconditionally creates a newSharpCompressStreamwrapper:This causes double-wrapping when the stream is already a
SharpCompressStream, leading to inconsistent buffering state that manifests differently across platforms.Changes
ArchiveFactory.cs: Use
SharpCompressStream.Createinstead of constructorTests: Added coverage for both pre-wrapped and raw FileStream scenarios
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.