ZipArchiveEntry.Open expectedly throws an ArgumentOutOfRangeException

[rjacobw]

Describe the bug

ZipArchiveEntry.cs incorrectly assumes that constructing a MemoryStream with a negative length is "okay" (see ZipArchiveEntry.GetCompressedData) The reality is that the MemoryStream constructor now throws an ArgumentOutOfRangeException (here) when an unchecked cast causes an uint to wrap around.

This appears to be new to netcore (versus netframework), and does cause unexpected failures where none had previously existed.

To Reproduce

Call ZipArchiveEntry.Open on a very large entry (> int.MaxValue).

Exceptions (if any)

ArgumentOutOfRangeException is thrown from the MemoryStream constructor.

Further technical details

This is no longer true -

<ZipArchiveEntry.cs>

// if _uncompressedSize > int.MaxValue, it's still okay, because MemoryStream will just
// grow as data is copied into it
_storedUncompressedData = new MemoryStream((int)_uncompressedSize);

<MemoryStream.cs>

public MemoryStream(int capacity)
{
    ArgumentOutOfRangeException.ThrowIfNegative(capacity);
    ArgumentOutOfRangeException.ThrowIfGreaterThan(capacity, MemStreamMaxLength);

[dotnet-policy-service]

Tagging subscribers to this area: @karelz, @dotnet/area-system-io-compression
See info in area-owners.md if you want to be subscribed.

[KalleOlaviNiemitalo]

Should it be casting (uint)_uncompressedSize instead?

… no, the MemoryStream constructor takes int capacity rather than long capacity.

runtime/src/libraries/System.Private.CoreLib/src/System/IO/MemoryStream.cs

Line 37 in 055e04b

private static int MemStreamMaxLength => Array.MaxLength;

runtime/src/libraries/System.Private.CoreLib/src/System/IO/MemoryStream.cs

Lines 44 to 49 in 055e04b

	public MemoryStream(int capacity)
	{
	ArgumentOutOfRangeException.ThrowIfNegative(capacity);
	ArgumentOutOfRangeException.ThrowIfGreaterThan(capacity, MemStreamMaxLength);
	_buffer = capacity != 0 ? new byte[capacity] : [];

So MemoryStream just doesn't support streams larger than what byte[] can hold. If the uncompressed size of the zip entry actually is larger than that, then I suppose some exception needs to be thrown, but perhaps not ArgumentOutOfRangeException.

[rjacobw]

Could you not use a segmented stream approach so as not to break the Zip implementation for archives containing large entries? It seems like ZipArchiveEntry already allows for large entries (here), but now that MemoryStream throws an exception from the constructor, ZipArchiveEntry can no longer use the multiple buffer approach mentioned in IsOpenableFinalVerifications.

[KalleOlaviNiemitalo]

Multiple buffers were implemented in dotnet/corefx@0920c09 but AFAICT only for compressed data, not uncompressed. The commit message mentions:

There are other limitations working with large files in archives that are not addressed, including opening a ZipArchiveEntry with a greater than 2GB uncompressed size.

so I'm not sure whether such huge entries can ever have been opened in update mode.

[alinpahontu2912]

Hey @rjacobw, how are you using the archive? If you are opening the archive in update mode, it fails because we don't support updating large entries in memory. We are relying on memory stream, which can only grow up to array.maxlength, not int.maxvalue.

[rjacobw]

@alinpahontu2912 we do open archives in update mode, and have been since NetFramework. We've recently gone through a very large push to build and ship on top of net10 (moving off net48), and we're now seeing an error where we had never seen any such errors before.

After digging a bit, I'm not sure how it ever worked (but I'm sure it did). Even looking back at MemoryStream on ReferenceSource would suggest that the comment in ZipArchiveEntry indicating that MemoryStream can handle negative capacities was wrong - though I'd presume that comment was written at time when perhaps MemoryStream did happy accept negative capacities.

Given that MemoryStream used to limit capacities to int.MaxValue (here), but now uses Array.MaxLength (a smaller value), I wonder if perhaps we're seeing entries fail to open in update mode simply because the sizes are in that gap between those two values.

[alinpahontu2912]

@rjacobw you're right that the gap between Array.MaxLength (0x7FFFFFC7) and int.MaxValue is real, but it's only 56 bytes. Even in .NET Framework, allocating a byte[] larger than Array.MaxLength would fail with OutOfMemoryException. So entries in that narrow gap would not have succeeded before either. The failure mode just changed from OutOfMemoryException to ArgumentOutOfRangeException.

The more likely scenario for entries > int.MaxValue is the unchecked (int) cast wrapping to a negative value. We'll look into addressing this.