Fix async ZipArchive calling non-async Stream methods

src/libraries/Common/tests/System/IO/Compression/NoAsyncCallsStream.cs

@@ -13,9 +13,6 @@ internal sealed class NoAsyncCallsStream : Stream
 
     public NoAsyncCallsStream(Stream stream) => _s = stream;
 
-    // Allows temporarily disabling the current stream's sync API usage restriction.
-    public bool IsRestrictionEnabled { get; set; }
-
     public override bool CanRead => _s.CanRead;
     public override bool CanSeek => _s.CanSeek;
     public override bool CanTimeout => _s.CanTimeout;
@@ -47,18 +44,11 @@ internal sealed class NoAsyncCallsStream : Stream
     public override void WriteByte(byte value) => _s.WriteByte(value);
 
     // Async
-    public override Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken) =>
-        IsRestrictionEnabled ? throw new InvalidOperationException() : _s.CopyToAsync(destination, bufferSize, cancellationToken);
-    public override ValueTask DisposeAsync() =>
-        IsRestrictionEnabled ? throw new InvalidOperationException() : _s.DisposeAsync();
-    public override Task FlushAsync(CancellationToken cancellationToken) =>
-        IsRestrictionEnabled ? throw new InvalidOperationException() : _s.FlushAsync();
-    public override Task<int> ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) =>
-        IsRestrictionEnabled ? throw new InvalidOperationException() : _s.ReadAsync(buffer, offset, count, cancellationToken);
-    public override ValueTask<int> ReadAsync(Memory<byte> buffer, CancellationToken cancellationToken = default) =>
-        IsRestrictionEnabled ? throw new InvalidOperationException() : _s.ReadAsync(buffer, cancellationToken);
-    public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) =>
-        IsRestrictionEnabled ? throw new InvalidOperationException() : _s.WriteAsync(buffer, offset, count, cancellationToken);
-    public override ValueTask WriteAsync(ReadOnlyMemory<byte> buffer, CancellationToken cancellationToken = default) =>
-        IsRestrictionEnabled ? throw new InvalidOperationException() : _s.WriteAsync(buffer, cancellationToken);
+    public override Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken) => throw new InvalidOperationException();
+    public override ValueTask DisposeAsync() => throw new InvalidOperationException();
+    public override Task FlushAsync(CancellationToken cancellationToken) => throw new InvalidOperationException();
+    public override Task<int> ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) => throw new InvalidOperationException();
+    public override ValueTask<int> ReadAsync(Memory<byte> buffer, CancellationToken cancellationToken = default) => throw new InvalidOperationException();
+    public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) => throw new InvalidOperationException();
+    public override ValueTask WriteAsync(ReadOnlyMemory<byte> buffer, CancellationToken cancellationToken = default) => throw new InvalidOperationException();
 }

src/libraries/Common/tests/System/IO/Compression/NoSyncCallsStream.cs

@@ -13,9 +13,6 @@ internal sealed class NoSyncCallsStream : Stream
 
     public NoSyncCallsStream(Stream stream) => _s = stream;
 
-    // Allows temporarily disabling the current stream's sync API usage restriction.
-    public bool IsRestrictionEnabled { get; set; }
-
     public override bool CanRead => _s.CanRead;
     public override bool CanSeek => _s.CanSeek;
     public override bool CanTimeout => _s.CanTimeout;
@@ -33,105 +30,18 @@ internal sealed class NoSyncCallsStream : Stream
     public override string? ToString() => _s.ToString();
 
     // Sync
-    public override IAsyncResult BeginRead(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) =>
-        IsRestrictionEnabled ? throw new InvalidOperationException() : _s.BeginRead(buffer, offset, count, callback, state);
-    public override IAsyncResult BeginWrite(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) =>
-        IsRestrictionEnabled ? throw new InvalidOperationException() : _s.BeginRead(buffer, offset, count, callback, state);
-    public override void CopyTo(Stream destination, int bufferSize)
-    {
-        if (IsRestrictionEnabled)
-        {
-            throw new InvalidOperationException();
-        }
-        else
-        {
-            _s.CopyTo(destination, bufferSize);
-        }
-    }
-    protected override void Dispose(bool disposing)
-    {
-        // _disposing = true;
-        if (IsRestrictionEnabled)
-        {
-            throw new InvalidOperationException();
-        }
-        else
-        {
-            _s.Dispose();
-        }
-    }
-    public override int EndRead(IAsyncResult asyncResult) => IsRestrictionEnabled ? throw new InvalidOperationException() : _s.EndRead(asyncResult);
-    public override void EndWrite(IAsyncResult asyncResult)
-    {
-        if (IsRestrictionEnabled)
-        {
-            throw new InvalidOperationException();
-        }
-        else
-        {
-            _s.EndWrite(asyncResult);
-        }
-    }
-    public override void Flush()
-    {
-        if (IsRestrictionEnabled)
-        {
-            throw new InvalidOperationException();
-        }
-        else
-        {
-            _s.Flush();
-        }
-    }
-    public override int Read(byte[] buffer, int offset, int count) =>
-        IsRestrictionEnabled ? throw new InvalidOperationException() : _s.Read(buffer, offset, count);
-    public override int Read(Span<byte> buffer) =>
-        IsRestrictionEnabled ? throw new InvalidOperationException() : _s.Read(buffer);
-    public override void Write(byte[] buffer, int offset, int count)
-    {
-        bool isDeflateStream = false;
-
-        // Get the stack trace to determine the calling method
-        var stackTrace = new System.Diagnostics.StackTrace();
-        var callingMethod = stackTrace.GetFrame(1)?.GetMethod();
-
-        // Check if the calling method belongs to the DeflateStream class
-        if (callingMethod?.DeclaringType == typeof(System.IO.Compression.DeflateStream))
-        {
-            isDeflateStream = true;
-        }
-
-        if (!isDeflateStream && IsRestrictionEnabled)
-        {
-            throw new InvalidOperationException($"Parent class is {callingMethod.DeclaringType}");
-        }
-        else
-        {
-            _s.Write(buffer, offset, count);
-        }
-    }
-    public override void Write(ReadOnlySpan<byte> buffer)
-    {
-        if (IsRestrictionEnabled)
-        {
-            throw new InvalidOperationException();
-        }
-        else
-        {
-            _s.Write(buffer);
-        }
-    }
-    public override void WriteByte(byte value)
-    {
-        if (IsRestrictionEnabled)
-        {
-            throw new InvalidOperationException();
-        }
-        else
-        {
-            _s.WriteByte(value);
-        }
-    }
+    public override IAsyncResult BeginRead(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) => throw new InvalidOperationException();
+    public override IAsyncResult BeginWrite(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) => throw new InvalidOperationException();
+    public override void CopyTo(Stream destination, int bufferSize) => throw new InvalidOperationException();
+    protected override void Dispose(bool disposing) => throw new InvalidOperationException();
+    public override int EndRead(IAsyncResult asyncResult) => throw new InvalidOperationException();
+    public override void EndWrite(IAsyncResult asyncResult) => throw new InvalidOperationException();
+    public override void Flush() => throw new InvalidOperationException();
+    public override int Read(byte[] buffer, int offset, int count) => throw new InvalidOperationException();
+    public override int Read(Span<byte> buffer) => throw new InvalidOperationException();
+    public override void Write(byte[] buffer, int offset, int count) => throw new InvalidOperationException();
+    public override void Write(ReadOnlySpan<byte> buffer) => throw new InvalidOperationException();
+    public override void WriteByte(byte value) => throw new InvalidOperationException();
 
     // Async
     public override Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken) => _s.CopyToAsync(destination, bufferSize, cancellationToken);

src/libraries/System.IO.Compression.ZipFile/tests/ZipFile.Open.cs

@@ -110,20 +110,13 @@ public async Task InvalidFilesAsync()
             await Assert.ThrowsAsync<InvalidDataException>(() => ZipFile.OpenAsync(testArchive.Path, ZipArchiveMode.Update, default));
         }
 
-        await using (ZipArchive archive = await ZipFile.OpenReadAsync(bad("CDoffsetInBoundsWrong.zip"), default))
-        {
-            Assert.Throws<InvalidDataException>(() => { var x = archive.Entries; });
-        }
-
+        await Assert.ThrowsAsync<InvalidDataException>(() => ZipFile.OpenReadAsync(bad("CDoffsetInBoundsWrong.zip"), default));
         using (TempFile testArchive = CreateTempCopyFile(bad("CDoffsetInBoundsWrong.zip"), GetTestFilePath()))
         {
             await Assert.ThrowsAsync<InvalidDataException>(() => ZipFile.OpenAsync(testArchive.Path, ZipArchiveMode.Update, default));
         }
 
-        await using (ZipArchive archive = await ZipFile.OpenReadAsync(bad("numberOfEntriesDifferent.zip"), default))
-        {
-            Assert.Throws<InvalidDataException>(() => { var x = archive.Entries; });
-        }
+        await Assert.ThrowsAsync<InvalidDataException>(() => ZipFile.OpenReadAsync(bad("numberOfEntriesDifferent.zip"), default));
         using (TempFile testArchive = CreateTempCopyFile(bad("numberOfEntriesDifferent.zip"), GetTestFilePath()))
         {
             await Assert.ThrowsAsync<InvalidDataException>(() => ZipFile.OpenAsync(testArchive.Path, ZipArchiveMode.Update, default));

src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchive.Async.cs

@@ -92,6 +92,10 @@ public static async Task<ZipArchive> CreateAsync(Stream stream, ZipArchiveMode m
                     break;
                 case ZipArchiveMode.Read:
                     await zipArchive.ReadEndOfCentralDirectoryAsync(cancellationToken).ConfigureAwait(false);
+
+                    // As there is no API for accessing .Entries asynchronously, we are expected to read the central
+                    // directory up-front
+                    await zipArchive.EnsureCentralDirectoryReadAsync(cancellationToken).ConfigureAwait(false);
                     break;
                 case ZipArchiveMode.Update:
                 default:

src/libraries/System.IO.Compression/src/System/IO/Compression/ZipCustomStreams.cs

@@ -216,6 +216,20 @@ protected override void Dispose(bool disposing)
             }
             base.Dispose(disposing);
         }
+
+        public override async ValueTask DisposeAsync()
+        {
+            if (!_isDisposed)
+            {
+                _onClosed?.Invoke(_zipArchiveEntry);
+
+                if (_closeBaseStream)
+                    await _baseStream.DisposeAsync().ConfigureAwait(false);
+
+                _isDisposed = true;
+            }
+            await base.DisposeAsync().ConfigureAwait(false);
+        }
     }
 
     internal sealed class SubReadStream : Stream

src/libraries/System.IO.Compression/tests/ZipArchive/zip_InvalidParametersAndStrangeFiles.cs

@@ -644,9 +644,20 @@ public static async Task ZipArchive_InvalidStream(string zipname, bool async)
         public static async Task ZipArchive_InvalidEntryTable(string zipname, bool async)
         {
             string filename = bad(zipname);
-            await using (ZipArchive archive = await CreateZipArchive(async, await StreamHelpers.CreateTempCopyStream(filename), ZipArchiveMode.Read))
+            using (var stream = await StreamHelpers.CreateTempCopyStream(filename))
             {
-                Assert.Throws<InvalidDataException>(() => archive.Entries[0]);
+                if (async)
+                {
+                    // as CreateAsync reads the entry table immediately, it throws immediately instead of on accessing .Entries
+                    await Assert.ThrowsAsync<InvalidDataException>(() => CreateZipArchive(async, stream, ZipArchiveMode.Read));
+                }
+                else
+                {
+                    await using (ZipArchive archive = await CreateZipArchive(async, stream, ZipArchiveMode.Read))
+                    {
+                        Assert.Throws<InvalidDataException>(() => archive.Entries[0]);
+                    }
+                }
             }
         }
 
@@ -1484,24 +1495,24 @@ public static async Task NoAsyncCallsWhenUsingSync()
             // Create mode
             using (ZipArchive archive = new ZipArchive(s, ZipArchiveMode.Create, leaveOpen: true, entryNameEncoding: Encoding.UTF8))
             {
-                using MemoryStream normalZipStream = await StreamHelpers.CreateTempCopyStream(zfile("normal.zip"));
-                normalZipStream.Position = 0;
+                using MemoryStream asyncZipStream = await StreamHelpers.CreateTempCopyStream(zfile("normal.zip"));
+                asyncZipStream.Position = 0;
 
                 // Note this is not using NoAsyncCallsStream, so it can be opened in async mode
-                await using (ZipArchive normalZipArchive = await ZipArchive.CreateAsync(normalZipStream, ZipArchiveMode.Read, leaveOpen: false, entryNameEncoding: null))
+                await using (ZipArchive asyncZipArchive = await ZipArchive.CreateAsync(asyncZipStream, ZipArchiveMode.Read, leaveOpen: false, entryNameEncoding: null))
                 {
-                    var normalZipEntries = normalZipArchive.Entries;
+                    var asyncZipEntries = asyncZipArchive.Entries;
 
-                    foreach (ZipArchiveEntry normalEntry in normalZipEntries)
+                    foreach (ZipArchiveEntry asyncEntry in asyncZipEntries)
                     {
-                        ZipArchiveEntry newEntry = archive.CreateEntry(normalEntry.FullName);
+                        ZipArchiveEntry newEntry = archive.CreateEntry(asyncEntry.FullName);
                         using (Stream newEntryStream = newEntry.Open())
                         {
                             // Note the parent archive is not using NoAsyncCallsStream, so it can be opened in async mode
-                            await using (Stream normalEntryStream = await normalEntry.OpenAsync())
+                            await using (Stream asyncEntryStream = await asyncEntry.OpenAsync())
                             {
-                                // Note the parent archive is not using NoAsyncCallsStream, so it can be copied in async mode
-                                await normalEntryStream.CopyToAsync(newEntryStream);
+                                // The entry needs to be copied in sync mode as the destination archive does not support async writes
+                                asyncEntryStream.CopyTo(newEntryStream);
                             }
                         }
                     }
@@ -1515,11 +1526,7 @@ public static async Task NoAsyncCallsWhenUsingSync()
             {
                 _ = archive.Comment;
 
-                // Entries is sync only
-                s.IsRestrictionEnabled = false;
                 var entries = archive.Entries;
-                s.IsRestrictionEnabled = true;
-
                 foreach (var entry in entries)
                 {
                     _ = archive.GetEntry(entry.Name);
@@ -1550,11 +1557,7 @@ public static async Task NoAsyncCallsWhenUsingSync()
             // Update mode
             using (ZipArchive archive = new ZipArchive(s, ZipArchiveMode.Update, leaveOpen: false, entryNameEncoding: Encoding.UTF8))
             {
-                // Entries is sync only
-                s.IsRestrictionEnabled = false;
                 ZipArchiveEntry entryToDelete = archive.Entries[0];
-                s.IsRestrictionEnabled = true;
-
                 entryToDelete.Delete();
 
                 ZipArchiveEntry entry = archive.CreateEntry("mynewentry.txt");
@@ -1593,8 +1596,8 @@ public static async Task NoSyncCallsWhenUsingAsync()
                             // Note the parent archive is not using NoSyncCallsStream, so it can be opened in sync mode
                             using (Stream normalEntryStream = normalEntry.Open())
                             {
-                                // Note the parent archive is not using NoSyncCallsStream, so it can be copied in sync mode
-                                normalEntryStream.CopyTo(newEntryStream);
+                                // The entry needs to be copied in async mode as the destination archive does not support sync writes
+                                await normalEntryStream.CopyToAsync(newEntryStream);
                             }
                         }
                     }
@@ -1608,11 +1611,7 @@ public static async Task NoSyncCallsWhenUsingAsync()
             {
                 _ = archive.Comment;
 
-                // Entries is sync only
-                s.IsRestrictionEnabled = false;
                 var entries = archive.Entries;
-                s.IsRestrictionEnabled = true;
-
                 foreach (var entry in entries)
                 {
                     _ = archive.GetEntry(entry.Name);
@@ -1642,11 +1641,7 @@ public static async Task NoSyncCallsWhenUsingAsync()
 
             await using (ZipArchive archive = await ZipArchive.CreateAsync(s, ZipArchiveMode.Update, leaveOpen: false, entryNameEncoding: Encoding.UTF8))
             {
-                // Entries is sync only
-                s.IsRestrictionEnabled = false;
                 ZipArchiveEntry entryToDelete = archive.Entries[0];
-                s.IsRestrictionEnabled = true;
-
                 entryToDelete.Delete(); // Delete is async only
 
                 ZipArchiveEntry entry = archive.CreateEntry("mynewentry.txt");