-
Notifications
You must be signed in to change notification settings - Fork 509
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
NotSupportedExceptionduringDispose()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
NotSupportedExceptioncoming fromFlush()
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()inEntryStream.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.