Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Runtime.InteropServices;
using System.Threading;
using System.Threading.Tasks;
using Xunit;

Expand Down Expand Up @@ -51,5 +52,118 @@ public static async Task RoundTrips_UnixFilePermissions(int expectedAttr)
}
}
}

[Fact]
public static async Task AsyncOnlyStream_NoSynchronousCalls()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is already a test for this, but it fails to validate that DisposeAsync has no sync calls due to a test bug

see #121624 (comment)

@rzikm rzikm Nov 24, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems the test does not assert the async-only execution because:

s.IsRestrictionEnabled is left to the default (false), same for NoAsync* version

public static async Task NoAsyncCallsWhenUsingSync()
{
using MemoryStream ms = new();
using NoAsyncCallsStream s = new(ms); // Only allows sync calls
// Create mode

And on top of it, it explicitly ignores violations if called from DeflateStream?!

// 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}");
}

I think we can safely remove that filtering.

Fixing those two places makes the new test redundant.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot Can you address my comment?

{
// This test verifies that async Zip methods don't make synchronous calls
// which would fail with async-only streams (e.g., Kestrel response streams)
var innerStream = new MemoryStream();
var asyncOnlyStream = new AsyncOnlyStream(innerStream);
byte[] testData = new byte[1024];
Random.Shared.NextBytes(testData);

await using (var zipArchive = new ZipArchive(asyncOnlyStream, ZipArchiveMode.Create, leaveOpen: true))
{
var entry = zipArchive.CreateEntry("TestEntry");
await using (var entryStream = await entry.OpenAsync())
{
await entryStream.WriteAsync(testData);
await entryStream.FlushAsync();
}
}

// Verify the archive was created successfully by reading from the inner stream
innerStream.Position = 0;
using (var zipArchive = new ZipArchive(innerStream, ZipArchiveMode.Read))
{
Assert.Single(zipArchive.Entries);
var entry = zipArchive.Entries[0];
Assert.Equal("TestEntry", entry.Name);
Assert.Equal(testData.Length, entry.Length);

using (var entryStream = entry.Open())
{
byte[] readData = new byte[testData.Length];
int bytesRead = entryStream.Read(readData);
Assert.Equal(testData.Length, bytesRead);
Assert.Equal(testData, readData);
}
}
}

private sealed class AsyncOnlyStream : Stream
{
private readonly MemoryStream _innerStream;

public AsyncOnlyStream(MemoryStream innerStream)
{
_innerStream = innerStream;
}

public override void Flush()
{
throw new NotSupportedException("Synchronous operations not supported");
}

public override int Read(byte[] buffer, int offset, int count)
{
throw new NotSupportedException("Synchronous operations not supported");
}

public override long Seek(long offset, SeekOrigin origin)
{
return _innerStream.Seek(offset, origin);
}

public override void SetLength(long value)
{
_innerStream.SetLength(value);
}

public override void Write(byte[] buffer, int offset, int count)
{
throw new NotSupportedException("Synchronous operations not supported");
}

public override Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken)
{
return _innerStream.CopyToAsync(destination, bufferSize, cancellationToken);
}

public override Task FlushAsync(CancellationToken cancellationToken)
{
return _innerStream.FlushAsync(cancellationToken);
}

public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken)
{
return _innerStream.WriteAsync(buffer, offset, count, cancellationToken);
}

public override ValueTask WriteAsync(ReadOnlyMemory<byte> buffer, CancellationToken cancellationToken = default)
{
return _innerStream.WriteAsync(buffer, cancellationToken);
}

public override ValueTask DisposeAsync()
{
return _innerStream.DisposeAsync();
}

public override bool CanRead => _innerStream.CanRead;

public override bool CanSeek => _innerStream.CanSeek;

public override bool CanWrite => _innerStream.CanWrite;

public override long Length => _innerStream.Length;

public override long Position
{
get => _innerStream.Position;
set => _innerStream.Position = value;
}
}
}
}
Loading