Fix silent iteration failure when input stream throws on Flush()#1156
Fix silent iteration failure when input stream throws on Flush()#1156adamhathcock merged 4 commits intomasterfrom
Conversation
…n Flush() - Modified ZlibBaseStream.Flush() and FlushAsync() to only flush the underlying stream when in Writer mode - Added ThrowOnFlushStream mock for testing - Added regression tests for Deflate and LZMA compressed archives - All tests pass successfully Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
|
@adamhathcock It might be relevant to review the |
|
@copilot make an Async version of these tests |
Was the root of your issue this Flush when disposing |
- Added Archive_Iteration_DoesNotBreak_WhenFlushThrows_Deflate_Async - Added Archive_Iteration_DoesNotBreak_WhenFlushThrows_LZMA_Async - Both async tests mirror the sync versions and pass successfully Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
I just retested my case on the I think they are no longer necessary and you can remove them, given that the call to The root issue therefore stemmed from calls to |
adamhathcock
left a comment
There was a problem hiding this comment.
LZMA doesn't have this problem but better safe than sorry
There was a problem hiding this comment.
Pull request overview
This PR fixes a regression where ZIP archive iteration could silently stop after the first entry when the underlying input stream throws NotSupportedException on Flush() / FlushAsync(), and adds regression tests to prevent it from recurring.
Changes:
- Update
ZlibBaseStream.Flush/FlushAsyncto only call the underlying stream’sFlushwhen operating in write mode, avoiding meaningless and harmful flushes on read-only input streams. - Simplify
EntryStream.Dispose/DisposeAsyncnow that decompressionFlush/FlushAsyncno longer propagates to unsupported input-stream flush operations. - Introduce
ThrowOnFlushStreamtest mock and add sync/async ZIP reader regression tests for Deflate and LZMA archives; bumpMicrosoft.NET.ILLink.Tasksversions inpackages.lock.json.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/SharpCompress/Compressors/Deflate/ZlibBaseStream.cs |
Guards underlying stream Flush/FlushAsync calls behind a write-mode check while preserving buffer rewind semantics, preventing flush-induced failures on read-only forward-only streams. |
src/SharpCompress/Common/EntryStream.cs |
Removes now-unnecessary NotSupportedException handling around Deflate/LZMA Flush/FlushAsync, relying on the updated zlib stream behavior while keeping existing over-read “knock back” behavior. |
tests/SharpCompress.Test/Mocks/ThrowOnFlushStream.cs |
Adds a mock Stream that throws on Flush/FlushAsync to simulate forward-only or non-flushable input streams for regression tests. |
tests/SharpCompress.Test/Zip/ZipReaderTests.cs |
Adds synchronous regression tests ensuring Deflate and LZMA ZIP iteration continues past the first entry when the input stream rejects flush operations. |
tests/SharpCompress.Test/Zip/ZipReaderAsyncTests.cs |
Adds asynchronous counterparts using AsyncOnlyStream plus ThrowOnFlushStream to validate async iteration is robust to input-stream flush failures. |
src/SharpCompress/packages.lock.json |
Bumps Microsoft.NET.ILLink.Tasks versions for net10.0 and net8.0 target frameworks to newer patch releases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…archive-iteration Fix silent iteration failure when input stream throws on Flush() # Conflicts: # src/SharpCompress/packages.lock.json
Merge pull request #1156 from adamhathcock/copilot/fix-sharpcompress-…
Since 0.41.0, archive iteration stops after the first entry when the input stream throws
NotSupportedExceptioninFlush(). No exception is raised; iteration simply ends prematurely.Root cause:
ZlibBaseStream.Flush()unconditionally flushes the underlying stream regardless of read/write mode. WhenEntryStream.Dispose()callsFlush()on decompression streams (Deflate/LZMA), the flush propagates to the input stream, which:MultipartReaderStream, network streams, custom pipelines)NotSupportedExceptionis caughtChanges:
_streamMode == StreamMode.WriterExample of affected code:
With this fix, iteration completes successfully for all entries regardless of whether the input stream supports flushing.
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.