Fix EntryStream.Dispose() throwing NotSupportedException on non-seekable streams#1151
Conversation
…eekable streams Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
|
@copilot format this |
Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
|
@copilot add async tests to go along with the sync tests added |
1 similar comment
|
@copilot add async tests to go along with the sync tests added |
Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
Added async tests
Added async tests |
There was a problem hiding this comment.
Pull request overview
This PR fixes a regression introduced in version 0.41.0 where EntryStream.Dispose() throws NotSupportedException when working with non-seekable streams. The issue occurs because Flush() calls on decompression streams can fail on non-seekable underlying streams (e.g., ASP.NET Core HttpRequest.Body). The fix wraps these Flush() calls in try-catch blocks to gracefully handle the exception while maintaining the over-read correction behavior for seekable streams.
Changes:
- Added exception handling around
Flush()andFlushAsync()calls inEntryStreamdisposal methods - Added comprehensive sync and async tests covering both Deflate and LZMA compression formats
- Updated project configuration to support conditional compilation for legacy .NET Framework
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/SharpCompress/Common/EntryStream.cs | Wrapped Flush/FlushAsync calls in try-catch blocks to ignore NotSupportedException |
| tests/SharpCompress.Test/Zip/ZipReaderTests.cs | Added sync tests for non-seekable stream disposal with Deflate and LZMA |
| tests/SharpCompress.Test/Zip/ZipReaderAsyncTests.cs | Added async tests for non-seekable stream disposal with Deflate and LZMA |
| tests/SharpCompress.Test/SharpCompress.Test.csproj | Added LEGACY_DOTNET define for net48 and fixed Linux-specific Mono.Posix.NETStandard dependency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ush-issue Fix EntryStream.Dispose() throwing NotSupportedException on non-seekable streams # Conflicts: # tests/SharpCompress.Test/Zip/ZipReaderTests.cs
Fix EntryStream.Dispose() NotSupportedException on non-seekable streams
Problem
Since version 0.41.0,
EntryStream.Dispose()callsFlush()on DeflateStream and LzmaStream to handle over-read issues. However, this causesNotSupportedExceptionin legitimate streaming scenarios with non-seekable streams (e.g., ASP.NET Core HttpRequest.Body).Solution
Wrapped the
Flush()andFlushAsync()calls in try-catch blocks that safely ignoreNotSupportedException. This maintains the over-read correction behavior for streams that support it, while gracefully degrading for non-seekable streams.Plan
Changes Made
EntryStream.Dispose()to wrapFlush()calls in try-catch blocksEntryStream.DisposeAsync()to wrapFlushAsync()calls in try-catch blocksNotSupportedExceptionfrom Flush operationsEntryStream_Dispose_DoesNotThrow_OnNonSeekableStream_DeflateandEntryStream_Dispose_DoesNotThrow_OnNonSeekableStream_LZMAEntryStream_Dispose_DoesNotThrow_OnNonSeekableStream_Deflate_AsyncandEntryStream_Dispose_DoesNotThrow_OnNonSeekableStream_LZMA_AsyncTest Results
Security Summary
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.