starred/sharpcompress
1
0
Fork 0
mirror of https://github.com/adamhathcock/sharpcompress.git synced 2026-02-03 21:23:38 +00:00
Code Issues 975 Packages Projects Releases 20 Wiki Activity

[PR #1151] Fix EntryStream.Dispose() throwing NotSupportedException on non-seekable streams #1591

New Issue
Open
opened 2026-01-29 22:21:16 +00:00 by claunia · 0 comments
claunia commented 2026-01-29 22:21:16 +00:00
Owner
Copy Link

Original Pull Request: https://github.com/adamhathcock/sharpcompress/pull/1151

State: closed
Merged: Yes

Fix EntryStream.Dispose() NotSupportedException on non-seekable streams

Problem

Since version 0.41.0, EntryStream.Dispose() calls Flush() on DeflateStream and LzmaStream to handle over-read issues. However, this causes NotSupportedException in legitimate streaming scenarios with non-seekable streams (e.g., ASP.NET Core HttpRequest.Body).

Solution

Wrapped the Flush() and FlushAsync() calls in try-catch blocks that safely ignore NotSupportedException. This maintains the over-read correction behavior for streams that support it, while gracefully degrading for non-seekable streams.

Plan

  • Explore repository structure and understand the issue
  • Examine EntryStream.Dispose() implementation
  • Create focused test to reproduce the issue
  • Implement fix: wrap Flush() calls in try-catch to swallow NotSupportedException
  • Verify fix works with test
  • Run broader tests to ensure no regressions
  • Code review and address feedback
  • Security scan
  • Format code with CSharpier
  • Add async tests

Changes Made

  • Modified EntryStream.Dispose() to wrap Flush() calls in try-catch blocks
  • Modified EntryStream.DisposeAsync() to wrap FlushAsync() calls in try-catch blocks
  • Both sync and async versions now safely ignore NotSupportedException from Flush operations
  • Added sync tests EntryStream_Dispose_DoesNotThrow_OnNonSeekableStream_Deflate and EntryStream_Dispose_DoesNotThrow_OnNonSeekableStream_LZMA
  • Added async tests EntryStream_Dispose_DoesNotThrow_OnNonSeekableStream_Deflate_Async and EntryStream_Dispose_DoesNotThrow_OnNonSeekableStream_LZMA_Async
  • Updated test comments to include version context (0.41.0)
  • Formatted code with CSharpier

Test Results

  • ✅ New sync tests pass (2/2)
  • ✅ New async tests pass (2/2)
  • ✅ ZipReaderTests pass (35/35)
  • ✅ ZipReaderAsyncTests pass (26/26)
  • ✅ TarReaderTests pass (18/18)
  • ✅ GZipReaderTests pass (2/2)
  • ✅ DisposalTests pass (13/13)

Security Summary

  • ✅ CodeQL scan completed with 0 alerts
  • No security vulnerabilities introduced
Original prompt

This section details on the original issue you should resolve

<issue_title>EntryStream.Dispose() calls Flush() on Deflate/LZMA streams causing NotSupportedException on non-seekable streams</issue_title>
<issue_description>Hi,

Since SharpCompress 0.41.0, EntryStream.Dispose() calls Flush() on some internal decompression streams:

//Need a safe standard approach to this - it's okay for compression to overreads. Handling needs to be standardised
if (_stream is IStreamStack ss)
{
    if (ss.BaseStream() is SharpCompress.Compressors.Deflate.DeflateStream deflateStream)
    {
        deflateStream.Flush(); //Deflate over reads. Knock it back
    }
    else if (ss.BaseStream() is SharpCompress.Compressors.LZMA.LzmaStream lzmaStream)
    {
        lzmaStream.Flush(); //Lzma over reads. Knock it back
    }
}

This causes a NotSupportedException in some legitimate streaming scenarios.

Context / real-world scenario

I'm using SharpCompress in a pure streaming pipeline in ASP.NET Core:

  • Source stream: HttpRequest.Body

  • Read via MultipartReader (multipart/form-data)

  • Archive entries are processed sequentially using ReaderFactory.Open(...).MoveToNextEntry()

  • Entry streams are non-seekable by design

In this setup, Flush() on DeflateStream / LzmaStream may internally try to access Position / Seek on the underlying stream stack, which is not supported and throws NotSupportedException.

This happens during EntryStream.Dispose(), which breaks the iteration and prevents moving to the next entry.

Why this is problematic

From a consumer point of view:

  • Dispose() is expected to be safe and non-throwing

  • Especially in streaming scenarios, Dispose() is required to advance to the next entry

  • Throwing NotSupportedException during Dispose() makes SharpCompress unusable in valid non-seekable streaming pipelines

Expected behavior / suggestion

At minimum, EntryStream.Dispose() should:

  • Not throw if Flush() is not supported

  • Swallow or ignore NotSupportedException coming from Flush()

Example defensive pattern:

try
{
    deflateStream.Flush();
}
catch (NotSupportedException)
{
    // ignore: underlying stream does not support required operations
}

Or more generally: Dispose() should never fail due to optional stream realignment logic.

Workaround on consumer side

Currently I have to wrap Dispose() with a try/catch and manually dispose the base stream via IStreamStack, which works but feels like something the library should handle.

Summary

  • The new Flush() in EntryStream.Dispose() breaks valid non-seekable streaming scenarios.

  • The over-read problem is real, but the current solution is unsafe.

  • Dispose() should not throw in this case.</issue_description>

<agent_instructions>try out the suggestion</agent_instructions>

Comments on the Issue (you are @copilot in this section)

💡 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.

**Original Pull Request:** https://github.com/adamhathcock/sharpcompress/pull/1151 **State:** closed **Merged:** Yes --- ## Fix EntryStream.Dispose() NotSupportedException on non-seekable streams ### Problem Since version 0.41.0, `EntryStream.Dispose()` calls `Flush()` on DeflateStream and LzmaStream to handle over-read issues. However, this causes `NotSupportedException` in legitimate streaming scenarios with non-seekable streams (e.g., ASP.NET Core HttpRequest.Body). ### Solution Wrapped the `Flush()` and `FlushAsync()` calls in try-catch blocks that safely ignore `NotSupportedException`. This maintains the over-read correction behavior for streams that support it, while gracefully degrading for non-seekable streams. ### Plan - [x] Explore repository structure and understand the issue - [x] Examine EntryStream.Dispose() implementation - [x] Create focused test to reproduce the issue - [x] Implement fix: wrap Flush() calls in try-catch to swallow NotSupportedException - [x] Verify fix works with test - [x] Run broader tests to ensure no regressions - [x] Code review and address feedback - [x] Security scan - [x] Format code with CSharpier - [x] Add async tests ### Changes Made - Modified `EntryStream.Dispose()` to wrap `Flush()` calls in try-catch blocks - Modified `EntryStream.DisposeAsync()` to wrap `FlushAsync()` calls in try-catch blocks - Both sync and async versions now safely ignore `NotSupportedException` from Flush operations - Added sync tests `EntryStream_Dispose_DoesNotThrow_OnNonSeekableStream_Deflate` and `EntryStream_Dispose_DoesNotThrow_OnNonSeekableStream_LZMA` - Added async tests `EntryStream_Dispose_DoesNotThrow_OnNonSeekableStream_Deflate_Async` and `EntryStream_Dispose_DoesNotThrow_OnNonSeekableStream_LZMA_Async` - Updated test comments to include version context (0.41.0) - Formatted code with CSharpier ### Test Results - ✅ New sync tests pass (2/2) - ✅ New async tests pass (2/2) - ✅ ZipReaderTests pass (35/35) - ✅ ZipReaderAsyncTests pass (26/26) - ✅ TarReaderTests pass (18/18) - ✅ GZipReaderTests pass (2/2) - ✅ DisposalTests pass (13/13) ### Security Summary - ✅ CodeQL scan completed with 0 alerts - No security vulnerabilities introduced <!-- START COPILOT ORIGINAL PROMPT --> <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>EntryStream.Dispose() calls Flush() on Deflate/LZMA streams causing NotSupportedException on non-seekable streams</issue_title> > <issue_description>Hi, > > Since SharpCompress **0.41.0**, `EntryStream.Dispose()` calls `Flush()` on some internal decompression streams: > > ```cs > //Need a safe standard approach to this - it's okay for compression to overreads. Handling needs to be standardised > if (_stream is IStreamStack ss) > { > if (ss.BaseStream() is SharpCompress.Compressors.Deflate.DeflateStream deflateStream) > { > deflateStream.Flush(); //Deflate over reads. Knock it back > } > else if (ss.BaseStream() is SharpCompress.Compressors.LZMA.LzmaStream lzmaStream) > { > lzmaStream.Flush(); //Lzma over reads. Knock it back > } > } > ``` > > This causes a `NotSupportedException` in some legitimate streaming scenarios. > > --- > > ### Context / real-world scenario > > I'm using SharpCompress in a pure streaming pipeline in ASP.NET Core: > > - Source stream: `HttpRequest.Body` > > - Read via MultipartReader (multipart/form-data) > > - Archive entries are processed sequentially using `ReaderFactory.Open(...).MoveToNextEntry()` > > - Entry streams are **non-seekable by design** > > In this setup, `Flush()` on `DeflateStream` / `LzmaStream `may internally try to access `Position` / `Seek` on the underlying stream stack, which is **not supported** and throws `NotSupportedException`. > > This happens during `EntryStream.Dispose()`, which breaks the iteration and prevents moving to the next entry. > > --- > > ### Why this is problematic > > From a consumer point of view: > > - `Dispose()` is expected to be **safe and non-throwing** > > - Especially in streaming scenarios, `Dispose()` is required to advance to the next entry > > - Throwing `NotSupportedException` during `Dispose()` makes SharpCompress unusable in valid non-seekable streaming pipelines > > --- > > ### Expected behavior / suggestion > > At minimum, `EntryStream.Dispose()` should: > > - Not throw if `Flush()` is not supported > > - Swallow or ignore `NotSupportedException` coming from `Flush()` > > Example defensive pattern: > > ```cs > try > { > deflateStream.Flush(); > } > catch (NotSupportedException) > { > // ignore: underlying stream does not support required operations > } > ``` > > Or more generally: `Dispose()` should never fail due to optional stream realignment logic. > > --- > > ### Workaround on consumer side > > Currently I have to wrap `Dispose()` with a try/catch and manually dispose the base stream via `IStreamStack`, which works but feels like something the library should handle. > > --- > > ### Summary > > - The new `Flush()` in `EntryStream.Dispose()` breaks valid non-seekable streaming scenarios. > > - The over-read problem is real, but the current solution is unsafe. > > - `Dispose()` should not throw in this case.</issue_description> > > <agent_instructions>try out the suggestion</agent_instructions> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > </comments> > </details> <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes adamhathcock/sharpcompress#1150 <!-- START COPILOT CODING AGENT TIPS --> --- 💡 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](https://gh.io/copilot-coding-agent-tips) in the docs.
claunia added the pull-request label 2026-01-29 22:21:16 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
bug
duplicate
enhancement
feedback requested
not bug
pull-request
Mirrored from GitHub Pull Request
 question
up for grabs
wontfix
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
claunia
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/sharpcompress#1591
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?