Merge pull request #1156 from adamhathcock/copilot/fix-sharpcompress-archive-iteration

Fix silent iteration failure when input stream throws on Flush()
# Conflicts:
#	src/SharpCompress/packages.lock.json

src/SharpCompress/Common/EntryStream.cs

@@ -79,25 +79,11 @@ public class EntryStream : Stream, IStreamStack
         {
             if (ss.BaseStream() is SharpCompress.Compressors.Deflate.DeflateStream deflateStream)
             {
-                try
-                {
-                    deflateStream.Flush(); //Deflate over reads. Knock it back
-                }
-                catch (NotSupportedException)
-                {
-                    // Ignore: underlying stream does not support required operations for Flush
-                }
+                deflateStream.Flush(); //Deflate over reads. Knock it back
             }
             else if (ss.BaseStream() is SharpCompress.Compressors.LZMA.LzmaStream lzmaStream)
             {
-                try
-                {
-                    lzmaStream.Flush(); //Lzma over reads. Knock it back
-                }
-                catch (NotSupportedException)
-                {
-                    // Ignore: underlying stream does not support required operations for Flush
-                }
+                lzmaStream.Flush(); //Lzma over reads. Knock it back
             }
         }
 #if DEBUG_STREAMS
@@ -125,25 +111,11 @@ public class EntryStream : Stream, IStreamStack
         {
             if (ss.BaseStream() is SharpCompress.Compressors.Deflate.DeflateStream deflateStream)
             {
-                try
-                {
-                    await deflateStream.FlushAsync().ConfigureAwait(false);
-                }
-                catch (NotSupportedException)
-                {
-                    // Ignore: underlying stream does not support required operations for Flush
-                }
+                await deflateStream.FlushAsync().ConfigureAwait(false);
             }
             else if (ss.BaseStream() is SharpCompress.Compressors.LZMA.LzmaStream lzmaStream)
             {
-                try
-                {
-                    await lzmaStream.FlushAsync().ConfigureAwait(false);
-                }
-                catch (NotSupportedException)
-                {
-                    // Ignore: underlying stream does not support required operations for Flush
-                }
+                await lzmaStream.FlushAsync().ConfigureAwait(false);
             }
         }
 #if DEBUG_STREAMS

src/SharpCompress/Compressors/Deflate/ZlibBaseStream.cs

@@ -586,7 +586,13 @@ internal class ZlibBaseStream : Stream, IStreamStack
 
     public override void Flush()
     {
-        _stream.Flush();
+        // Only flush the underlying stream when in write mode
+        // Flushing input streams during read operations is not meaningful
+        // and can cause issues with forward-only/non-seekable streams
+        if (_streamMode == StreamMode.Writer)
+        {
+            _stream.Flush();
+        }
         //rewind the buffer
         ((IStreamStack)this).Rewind(z.AvailableBytesIn); //unused
         z.AvailableBytesIn = 0;
@@ -594,7 +600,13 @@ internal class ZlibBaseStream : Stream, IStreamStack
 
     public override async Task FlushAsync(CancellationToken cancellationToken)
     {
-        await _stream.FlushAsync(cancellationToken).ConfigureAwait(false);
+        // Only flush the underlying stream when in write mode
+        // Flushing input streams during read operations is not meaningful
+        // and can cause issues with forward-only/non-seekable streams
+        if (_streamMode == StreamMode.Writer)
+        {
+            await _stream.FlushAsync(cancellationToken).ConfigureAwait(false);
+        }
         //rewind the buffer
         ((IStreamStack)this).Rewind(z.AvailableBytesIn); //unused
         z.AvailableBytesIn = 0;

tests/SharpCompress.Test/Mocks/ThrowOnFlushStream.cs

@@ -0,0 +1,73 @@
+using System;
+using System.IO;
+using System.Threading;
+using System.Threading.Tasks;
+
+namespace SharpCompress.Test.Mocks;
+
+/// <summary>
+/// A stream wrapper that throws NotSupportedException on Flush() calls.
+/// This is used to test that archive iteration handles streams that don't support flushing.
+/// </summary>
+public class ThrowOnFlushStream : Stream
+{
+    private readonly Stream inner;
+
+    public ThrowOnFlushStream(Stream inner)
+    {
+        this.inner = inner;
+    }
+
+    public override bool CanRead => inner.CanRead;
+
+    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("Flush not supported");
+
+    public override Task FlushAsync(CancellationToken cancellationToken) =>
+        throw new NotSupportedException("FlushAsync not supported");
+
+    public override int Read(byte[] buffer, int offset, int count) =>
+        inner.Read(buffer, offset, count);
+
+    public override Task<int> ReadAsync(
+        byte[] buffer,
+        int offset,
+        int count,
+        CancellationToken cancellationToken
+    ) => inner.ReadAsync(buffer, offset, count, cancellationToken);
+
+#if !NETFRAMEWORK && !NETSTANDARD2_0
+    public override ValueTask<int> ReadAsync(
+        Memory<byte> buffer,
+        CancellationToken cancellationToken = default
+    ) => inner.ReadAsync(buffer, cancellationToken);
+#endif
+
+    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);
+    }
+}

tests/SharpCompress.Test/Zip/ZipReaderAsyncTests.cs

@@ -305,4 +305,52 @@ public class ZipReaderAsyncTests : ReaderTests
             }
         }
     }
+
+    [Fact]
+    public async ValueTask Archive_Iteration_DoesNotBreak_WhenFlushThrows_Deflate_Async()
+    {
+        // Regression test: since 0.41.0, archive iteration would silently break
+        // when the input stream throws NotSupportedException in Flush().
+        // Only the first entry would be returned, then iteration would stop without exception.
+        var path = Path.Combine(TEST_ARCHIVES_PATH, "Zip.deflate.dd.zip");
+        using var fileStream = File.OpenRead(path);
+        using Stream stream = new ThrowOnFlushStream(fileStream);
+        await using var reader = ReaderFactory.OpenAsyncReader(new AsyncOnlyStream(stream));
+
+        var count = 0;
+        while (await reader.MoveToNextEntryAsync())
+        {
+            if (!reader.Entry.IsDirectory)
+            {
+                count++;
+            }
+        }
+
+        // Should iterate through all entries, not just the first one
+        Assert.True(count > 1, $"Expected more than 1 entry, but got {count}");
+    }
+
+    [Fact]
+    public async ValueTask Archive_Iteration_DoesNotBreak_WhenFlushThrows_LZMA_Async()
+    {
+        // Regression test: since 0.41.0, archive iteration would silently break
+        // when the input stream throws NotSupportedException in Flush().
+        // Only the first entry would be returned, then iteration would stop without exception.
+        var path = Path.Combine(TEST_ARCHIVES_PATH, "Zip.lzma.dd.zip");
+        using var fileStream = File.OpenRead(path);
+        using Stream stream = new ThrowOnFlushStream(fileStream);
+        await using var reader = ReaderFactory.OpenAsyncReader(new AsyncOnlyStream(stream));
+
+        var count = 0;
+        while (await reader.MoveToNextEntryAsync())
+        {
+            if (!reader.Entry.IsDirectory)
+            {
+                count++;
+            }
+        }
+
+        // Should iterate through all entries, not just the first one
+        Assert.True(count > 1, $"Expected more than 1 entry, but got {count}");
+    }
 }

tests/SharpCompress.Test/Zip/ZipReaderTests.cs

@@ -482,4 +482,52 @@ public class ZipReaderTests : ReaderTests
             }
         }
     }
+
+    [Fact]
+    public void Archive_Iteration_DoesNotBreak_WhenFlushThrows_Deflate()
+    {
+        // Regression test: since 0.41.0, archive iteration would silently break
+        // when the input stream throws NotSupportedException in Flush().
+        // Only the first entry would be returned, then iteration would stop without exception.
+        var path = Path.Combine(TEST_ARCHIVES_PATH, "Zip.deflate.dd.zip");
+        using var fileStream = File.OpenRead(path);
+        using Stream stream = new ThrowOnFlushStream(fileStream);
+        using var reader = ReaderFactory.OpenReader(stream);
+
+        var count = 0;
+        while (reader.MoveToNextEntry())
+        {
+            if (!reader.Entry.IsDirectory)
+            {
+                count++;
+            }
+        }
+
+        // Should iterate through all entries, not just the first one
+        Assert.True(count > 1, $"Expected more than 1 entry, but got {count}");
+    }
+
+    [Fact]
+    public void Archive_Iteration_DoesNotBreak_WhenFlushThrows_LZMA()
+    {
+        // Regression test: since 0.41.0, archive iteration would silently break
+        // when the input stream throws NotSupportedException in Flush().
+        // Only the first entry would be returned, then iteration would stop without exception.
+        var path = Path.Combine(TEST_ARCHIVES_PATH, "Zip.lzma.dd.zip");
+        using var fileStream = File.OpenRead(path);
+        using Stream stream = new ThrowOnFlushStream(fileStream);
+        using var reader = ReaderFactory.OpenReader(stream);
+
+        var count = 0;
+        while (reader.MoveToNextEntry())
+        {
+            if (!reader.Entry.IsDirectory)
+            {
+                count++;
+            }
+        }
+
+        // Should iterate through all entries, not just the first one
+        Assert.True(count > 1, $"Expected more than 1 entry, but got {count}");
+    }
 }