Merge pull request #1161 from adamhathcock/copilot/sub-pr-1160

Fix ArrayPool corruption from double-disposal in BufferedSubStream

src/SharpCompress/IO/BufferedSubStream.cs

@@ -29,17 +29,25 @@ internal class BufferedSubStream : SharpCompressStream, IStreamStack
 #if DEBUG_STREAMS
         this.DebugDispose(typeof(BufferedSubStream));
 #endif
-        if (disposing)
+        if (_isDisposed)
+        {
+            return;
+        }
+        _isDisposed = true;
+
+        if (disposing && _cache is not null)
         {
             ArrayPool<byte>.Shared.Return(_cache);
+            _cache = null;
         }
         base.Dispose(disposing);
     }
 
     private int _cacheOffset;
     private int _cacheLength;
-    private readonly byte[] _cache = ArrayPool<byte>.Shared.Rent(81920);
+    private byte[]? _cache = ArrayPool<byte>.Shared.Rent(81920);
     private long origin;
+    private bool _isDisposed;
 
     private long BytesLeftToRead { get; set; }
 
@@ -61,7 +69,12 @@ internal class BufferedSubStream : SharpCompressStream, IStreamStack
 
     private void RefillCache()
     {
-        var count = (int)Math.Min(BytesLeftToRead, _cache.Length);
+        if (_isDisposed)
+        {
+            throw new ObjectDisposedException(nameof(BufferedSubStream));
+        }
+
+        var count = (int)Math.Min(BytesLeftToRead, _cache!.Length);
         _cacheOffset = 0;
         if (count == 0)
         {
@@ -83,7 +96,12 @@ internal class BufferedSubStream : SharpCompressStream, IStreamStack
 
     private async ValueTask RefillCacheAsync(CancellationToken cancellationToken)
     {
-        var count = (int)Math.Min(BytesLeftToRead, _cache.Length);
+        if (_isDisposed)
+        {
+            throw new ObjectDisposedException(nameof(BufferedSubStream));
+        }
+
+        var count = (int)Math.Min(BytesLeftToRead, _cache!.Length);
         _cacheOffset = 0;
         if (count == 0)
         {
@@ -118,7 +136,7 @@ internal class BufferedSubStream : SharpCompressStream, IStreamStack
             }
 
             count = Math.Min(count, _cacheLength - _cacheOffset);
-            Buffer.BlockCopy(_cache, _cacheOffset, buffer, offset, count);
+            Buffer.BlockCopy(_cache!, _cacheOffset, buffer, offset, count);
             _cacheOffset += count;
         }
 
@@ -136,7 +154,7 @@ internal class BufferedSubStream : SharpCompressStream, IStreamStack
             }
         }
 
-        return _cache[_cacheOffset++];
+        return _cache![_cacheOffset++];
     }
 
     public override async Task<int> ReadAsync(
@@ -159,7 +177,7 @@ internal class BufferedSubStream : SharpCompressStream, IStreamStack
             }
 
             count = Math.Min(count, _cacheLength - _cacheOffset);
-            Buffer.BlockCopy(_cache, _cacheOffset, buffer, offset, count);
+            Buffer.BlockCopy(_cache!, _cacheOffset, buffer, offset, count);
             _cacheOffset += count;
         }
 
@@ -186,7 +204,7 @@ internal class BufferedSubStream : SharpCompressStream, IStreamStack
             }
 
             count = Math.Min(count, _cacheLength - _cacheOffset);
-            _cache.AsSpan(_cacheOffset, count).CopyTo(buffer.Span);
+            _cache!.AsSpan(_cacheOffset, count).CopyTo(buffer.Span);
             _cacheOffset += count;
         }
 

tests/SharpCompress.Test/Streams/SharpCompressStreamTest.cs

@@ -97,4 +97,25 @@ public class SharpCompressStreamTests
             }
         }
     }
+
+    [Fact]
+    public void BufferedSubStream_DoubleDispose_DoesNotCorruptArrayPool()
+    {
+        // This test verifies that calling Dispose multiple times on BufferedSubStream
+        // doesn't return the same array to the pool twice, which would cause pool corruption
+        byte[] data = new byte[0x10000];
+        using (MemoryStream ms = new MemoryStream(data))
+        {
+            var stream = new BufferedSubStream(ms, 0, data.Length);
+
+            // First disposal
+            stream.Dispose();
+
+            // Second disposal should not throw or corrupt the pool
+            stream.Dispose();
+        }
+
+        // If we got here without an exception, the test passed
+        Assert.True(true);
+    }
 }