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

[PR #1156] [MERGED] Fix silent iteration failure when input stream throws on Flush() #1596

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

📋 Pull Request Information

Original PR: https://github.com/adamhathcock/sharpcompress/pull/1156
Author: @Copilot
Created: 1/23/2026
Status: ✅ Merged
Merged: 1/24/2026
Merged by: @adamhathcock

Base: master ← Head: copilot/fix-sharpcompress-archive-iteration

📝 Commits (4)

📊 Changes

6 files changed (+193 additions, -40 deletions)

View changed files

📝 src/SharpCompress/Common/EntryStream.cs (+4 -32)
📝 src/SharpCompress/Compressors/Deflate/ZlibBaseStream.cs (+14 -2)
📝 src/SharpCompress/packages.lock.json (+6 -6)
➕ tests/SharpCompress.Test/Mocks/ThrowOnFlushStream.cs (+73 -0)
📝 tests/SharpCompress.Test/Zip/ZipReaderAsyncTests.cs (+48 -0)
📝 tests/SharpCompress.Test/Zip/ZipReaderTests.cs (+48 -0)

📄 Description

Since 0.41.0, archive iteration stops after the first entry when the input stream throws NotSupportedException in Flush(). No exception is raised; iteration simply ends prematurely.

Root cause: ZlibBaseStream.Flush() unconditionally flushes the underlying stream regardless of read/write mode. When EntryStream.Dispose() calls Flush() on decompression streams (Deflate/LZMA), the flush propagates to the input stream, which:

  • Has no semantic meaning for read-only operations
  • Breaks forward-only streams (ASP.NET Core MultipartReaderStream, network streams, custom pipelines)
  • Leaves internal state inconsistent even when NotSupportedException is caught

Changes:

  • ZlibBaseStream.Flush/FlushAsync: Only flush underlying stream when _streamMode == StreamMode.Writer
  • ThrowOnFlushStream: Test mock simulating streams that reject flush operations
  • ZipReaderTests: Regression tests for Deflate and LZMA compressed archives (sync)
  • ZipReaderAsyncTests: Regression tests for Deflate and LZMA compressed archives (async)

Example of affected code:

// Any stream wrapper that throws on Flush() would fail
public class ForwardOnlyStream : Stream {
    public override void Flush() => throw new NotSupportedException();
    // ...
}

using var stream = new ForwardOnlyStream(File.OpenRead("archive.zip"));
using var reader = ReaderFactory.OpenReader(stream);

while (reader.MoveToNextEntry()) { /* Stops after first entry */ }

With this fix, iteration completes successfully for all entries regardless of whether the input stream supports flushing.

Original prompt

This section details on the original issue you should resolve

<issue_title>Regression in SharpCompress 0.41.0: archive iteration breaks if input stream throws in Flush()</issue_title>
<issue_description>### Summary

Since SharpCompress 0.41.0, archive iteration silently breaks when the input stream throws in Flush().

Only the first entry is returned, then iteration stops without exception.

This can be reproduced using a simple wrapper stream around a FileStream.

Minimal reproduction

public sealed class ForwardOnlyThrowOnFlushStream : Stream
{
	private readonly Stream inner;

	public ForwardOnlyThrowOnFlushStream(Stream inner)
	{
		this.inner = inner;
	}

	public override bool CanRead => true;

	public override bool CanSeek => false;

	public override bool CanWrite => false;

	public override long Length => throw new NotSupportedException();

	public override long Position
	{
		get => throw new NotSupportedException();
		set => throw new NotSupportedException();
	}

	public override void Flush() => throw new NotSupportedException();

	public override int Read(byte[] buffer, int offset, int count) => inner.Read(buffer, offset, count);

	public override ValueTask<int> ReadAsync(Memory<byte> buffer, CancellationToken cancellationToken = default) => inner.ReadAsync(buffer, cancellationToken);

	public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException();

	public override void SetLength(long value) => throw new NotSupportedException();

	public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException();

	protected override void Dispose(bool disposing)
	{
		if (disposing)
		{
			inner.Dispose();
		}

		base.Dispose(disposing);
	}
}

Test program:

using var fs = File.OpenRead("multi-entry.zip");
using var stream = new ForwardOnlyThrowOnFlushStream(fs);

var reader = ReaderFactory.OpenAsyncReader(stream);

int count = 0;

while (await reader.MoveToNextEntryAsync())
{
	if (reader.Entry.IsDirectory)
	{
		continue;
	}

	count++;
}

Console.WriteLine(count);

Expected behavior

If the archive contains N entries, count == N.

Actual behavior (0.44.1)

count == 1

Iteration stops after the first entry.
No exception is thrown.

Workaround

If Flush() is implemented as a no-op:

public override void Flush() { }

Then all entries are iterated correctly.

Why this is a problem

Flush() has no semantic meaning for read-only input streams and should not be required for reading archives.

Any library stream that:

  • is read-only
  • forward-only
  • or explicitly does not support flushing

can silently break SharpCompress iteration.

This includes:

  • ASP.NET Core MultipartReaderStream
  • custom streaming pipelines
  • network streams

Regression info

  • Works with: SharpCompress 0.40.0 (or previous)
  • Fails with: SharpCompress 0.41.0 (or higher)

Suspected cause

SharpCompress 0.44.1 introduced a call to Stream.Flush() somewhere in the read / entry iteration pipeline.
If Flush() throws, the internal state becomes inconsistent and iteration stops early.

Related issue

This looks closely related to adamhathcock/sharpcompress#1150.

In adamhathcock/sharpcompress#1150, EntryStream.Dispose() started calling Flush() on internal decompression streams (Deflate/LZMA), which breaks valid streaming scenarios (non-seekable / forward-only streams). In my case, beyond the exception scenario discussed there, I now also observe a silent regression: iteration stops after the first entry.

Both issues share the same underlying theme: Flush() (or its side-effects) should not be required nor relied upon in a read-only streaming pipeline, especially for forward-only/non-seekable streams.</issue_description>

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.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/adamhathcock/sharpcompress/pull/1156 **Author:** [@Copilot](https://github.com/apps/copilot-swe-agent) **Created:** 1/23/2026 **Status:** ✅ Merged **Merged:** 1/24/2026 **Merged by:** [@adamhathcock](https://github.com/adamhathcock) **Base:** `master` ← **Head:** `copilot/fix-sharpcompress-archive-iteration` --- ### 📝 Commits (4) - [`d0f4483`](https://github.com/adamhathcock/sharpcompress/commit/d0f44839ffb1bc42ff61b298ff14c9465b865cd8) Initial plan - [`41e0c15`](https://github.com/adamhathcock/sharpcompress/commit/41e0c151de5701abf176c73868af6c0075c4fa3b) Fix regression: archive iteration breaking when input stream throws in Flush() - [`f2bb81d`](https://github.com/adamhathcock/sharpcompress/commit/f2bb81d6113d066d44ac2078320836d076fc9d36) Add async versions of archive iteration regression tests - [`f1102dc`](https://github.com/adamhathcock/sharpcompress/commit/f1102dc98086b74be9e6ce0a781ab6b7839575c2) Undoing https://github.com/adamhathcock/sharpcompress/pull/1151 ### 📊 Changes **6 files changed** (+193 additions, -40 deletions) <details> <summary>View changed files</summary> 📝 `src/SharpCompress/Common/EntryStream.cs` (+4 -32) 📝 `src/SharpCompress/Compressors/Deflate/ZlibBaseStream.cs` (+14 -2) 📝 `src/SharpCompress/packages.lock.json` (+6 -6) ➕ `tests/SharpCompress.Test/Mocks/ThrowOnFlushStream.cs` (+73 -0) 📝 `tests/SharpCompress.Test/Zip/ZipReaderAsyncTests.cs` (+48 -0) 📝 `tests/SharpCompress.Test/Zip/ZipReaderTests.cs` (+48 -0) </details> ### 📄 Description Since 0.41.0, archive iteration stops after the first entry when the input stream throws `NotSupportedException` in `Flush()`. No exception is raised; iteration simply ends prematurely. **Root cause:** `ZlibBaseStream.Flush()` unconditionally flushes the underlying stream regardless of read/write mode. When `EntryStream.Dispose()` calls `Flush()` on decompression streams (Deflate/LZMA), the flush propagates to the input stream, which: - Has no semantic meaning for read-only operations - Breaks forward-only streams (ASP.NET Core `MultipartReaderStream`, network streams, custom pipelines) - Leaves internal state inconsistent even when `NotSupportedException` is caught **Changes:** - **ZlibBaseStream.Flush/FlushAsync**: Only flush underlying stream when `_streamMode == StreamMode.Writer` - **ThrowOnFlushStream**: Test mock simulating streams that reject flush operations - **ZipReaderTests**: Regression tests for Deflate and LZMA compressed archives (sync) - **ZipReaderAsyncTests**: Regression tests for Deflate and LZMA compressed archives (async) **Example of affected code:** ```csharp // Any stream wrapper that throws on Flush() would fail public class ForwardOnlyStream : Stream { public override void Flush() => throw new NotSupportedException(); // ... } using var stream = new ForwardOnlyStream(File.OpenRead("archive.zip")); using var reader = ReaderFactory.OpenReader(stream); while (reader.MoveToNextEntry()) { /* Stops after first entry */ } ``` With this fix, iteration completes successfully for all entries regardless of whether the input stream supports flushing. <!-- START COPILOT ORIGINAL PROMPT --> <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>Regression in SharpCompress 0.41.0: archive iteration breaks if input stream throws in Flush()</issue_title> > <issue_description>### Summary > > Since SharpCompress **0.41.0**, archive iteration silently breaks when the input stream throws in `Flush()`. > > Only the **first entry** is returned, then iteration stops without exception. > > This can be reproduced using a simple wrapper stream around a `FileStream`. > > --- > > ### Minimal reproduction > > ```cs > public sealed class ForwardOnlyThrowOnFlushStream : Stream > { > private readonly Stream inner; > > public ForwardOnlyThrowOnFlushStream(Stream inner) > { > this.inner = inner; > } > > public override bool CanRead => true; > > public override bool CanSeek => false; > > public override bool CanWrite => false; > > public override long Length => throw new NotSupportedException(); > > public override long Position > { > get => throw new NotSupportedException(); > set => throw new NotSupportedException(); > } > > public override void Flush() => throw new NotSupportedException(); > > public override int Read(byte[] buffer, int offset, int count) => inner.Read(buffer, offset, count); > > public override ValueTask<int> ReadAsync(Memory<byte> buffer, CancellationToken cancellationToken = default) => inner.ReadAsync(buffer, cancellationToken); > > public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); > > public override void SetLength(long value) => throw new NotSupportedException(); > > public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(); > > protected override void Dispose(bool disposing) > { > if (disposing) > { > inner.Dispose(); > } > > base.Dispose(disposing); > } > } > ``` > > Test program: > > ```cs > using var fs = File.OpenRead("multi-entry.zip"); > using var stream = new ForwardOnlyThrowOnFlushStream(fs); > > var reader = ReaderFactory.OpenAsyncReader(stream); > > int count = 0; > > while (await reader.MoveToNextEntryAsync()) > { > if (reader.Entry.IsDirectory) > { > continue; > } > > count++; > } > > Console.WriteLine(count); > ``` > > --- > > ### Expected behavior > > If the archive contains N entries, `count == N`. > > --- > > ### Actual behavior (0.44.1) > > `count == 1` > > Iteration stops after the first entry. > No exception is thrown. > > --- > > ### Workaround > > If `Flush()` is implemented as a no-op: > > ```cs > public override void Flush() { } > ``` > > Then all entries are iterated correctly. > > --- > > ### Why this is a problem > > `Flush()` **has no semantic meaning for read-only input streams** and should not be required for reading archives. > > Any library stream that: > > - is read-only > - forward-only > - or explicitly does not support flushing > > can silently break SharpCompress iteration. > > This includes: > > - ASP.NET Core `MultipartReaderStream` > - custom streaming pipelines > - network streams > > --- > > ### Regression info > > - Works with: SharpCompress 0.40.0 (or previous) > - Fails with: SharpCompress 0.41.0 (or higher) > > --- > > ### Suspected cause > > SharpCompress 0.44.1 introduced a call to `Stream.Flush()` somewhere in the read / entry iteration pipeline. > If `Flush()` throws, the internal state becomes inconsistent and iteration stops early. > > --- > > ### Related issue > > This looks closely related to adamhathcock/sharpcompress#1150. > > In adamhathcock/sharpcompress#1150, `EntryStream.Dispose()` started calling `Flush()` on internal decompression streams (Deflate/LZMA), which breaks valid streaming scenarios (non-seekable / forward-only streams). In my case, beyond the exception scenario discussed there, I now also observe a **silent regression**: iteration stops after the first entry. > > Both issues share the same underlying theme: `Flush()` (or its side-effects) should not be required nor relied upon in a read-only streaming pipeline, especially for forward-only/non-seekable streams.</issue_description> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > </comments> > </details> <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes adamhathcock/sharpcompress#1155 <!-- 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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
claunia added the pull-request label 2026-01-29 22:21:18 +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#1596
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?