Add ZipArchiveEntry.Open(FileAccess) overload #122032

iremyux · 2025-11-28T10:58:04Z

This PR adds a new overload to ZipArchiveEntry.Open() that accepts a FileAccess parameter, allowing users to specify the desired access mode when opening an entry stream.

When a ZipArchive is opened in ZipArchiveMode.Update, calling ZipArchiveEntry.Open() always returns a read-write stream by invoking OpenInUpdateMode(). This causes the entire entry to be decompressed into memory, even when the caller only intends to read the entry's contents.

Update Mode Details:

FileAccess.Read: Opens a read-only stream over the entry's compressed data without loading it into memory. Useful for streaming reads.
FileAccess.Write: Opens an empty writable stream, discarding any existing entry data. Semantically equivalent to "replace the entry content entirely" (like FileMode.Create. The stream is backed by a MemoryStream and content is written to the archive when disposed.
FileAccess.ReadWrite: Same as parameterless Open() - loads existing data into memory and returns a read/write/seekable stream.

Fixes #101243

…net-runtime into 101243-open-overload

dotnet-policy-service · 2025-11-28T10:59:13Z

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

Copilot

Pull request overview

This PR adds a new Open(FileAccess access) overload to ZipArchiveEntry that allows callers to explicitly specify the desired file access mode when opening an entry. This is particularly valuable in ZipArchiveMode.Update, where the existing parameterless Open() always returns a read-write stream by decompressing the entire entry into memory. The new overload allows specifying FileAccess.Read to avoid this memory overhead when only reading is needed.

Key Changes:

New Open(FileAccess access) method validates that the requested access is compatible with the archive's mode and calls the appropriate internal open method (OpenInReadMode, OpenInWriteMode, or OpenInUpdateMode)
Three new error message resources for incompatible access modes
Comprehensive test coverage for all combinations of archive modes and access types

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
`src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs`	Implements the new `Open(FileAccess access)` overload with validation logic for different archive modes
`src/libraries/System.IO.Compression/ref/System.IO.Compression.cs`	Adds the new public API surface to the reference assembly
`src/libraries/System.IO.Compression/src/Resources/Strings.resx`	Adds three new error message resources for invalid access scenarios
`src/libraries/System.IO.Compression/tests/ZipArchive/zip_InvalidParametersAndStrangeFiles.cs`	Adds 11 new test methods covering all valid and invalid access combinations for each archive mode

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

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

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

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

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

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

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

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

The comment states that ReadWrite "opens in write mode" but based on the implementation (line 392 in ZipArchiveEntry.cs), it actually calls OpenInWriteMode() which may return a stream with different capabilities than implied. The comment should clarify what capabilities the returned stream actually has. Consider: "// ReadWrite is allowed in Create mode and calls OpenInWriteMode()"

            // ReadWrite should be allowed in Create mode (it opens in write mode)

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

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

Copilot

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Copilot · 2025-12-10T22:45:42Z

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

+        /// <summary>
+        /// Opens the entry with the specified access mode. This allows for more granular control over the returned stream's capabilities.
+        /// </summary>


The XML documentation summary doesn't adequately describe what each FileAccess mode does in different ZipArchiveModes, particularly the important behavioral differences in Update mode. Consider adding a remarks section that explains:

In Read mode: only FileAccess.Read is allowed

In Create mode: FileAccess.Write and FileAccess.ReadWrite are allowed (both behave as write-only)

In Update mode:

FileAccess.Read opens a read-only stream without loading into memory

FileAccess.Write provides an empty writable stream, discarding existing data

FileAccess.ReadWrite loads existing data into memory for modification (same as parameterless Open())

This is especially important because the behavior in Update mode with FileAccess.Write (discarding existing data) is semantically different from intuitive expectations.

Right, we should include the explanation in the remarks section.

Copilot · 2025-12-10T22:45:43Z

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

+                        FileAccess.ReadWrite => OpenInUpdateMode(),
+                        _ => throw new UnreachableException()


The UnreachableException in the switch expression default case is unnecessary. Since the access parameter is validated at lines 394-395 to be one of Read, Write, or ReadWrite, and all three cases are explicitly handled in the switch expression (lines 415-417), the default case at line 418 can never be reached. This makes the code slightly harder to reason about. Consider removing the default case entirely, as the switch expression will be exhaustive without it.

Suggested change

FileAccess.ReadWrite => OpenInUpdateMode(),

_ => throw new UnreachableException()

FileAccess.ReadWrite => OpenInUpdateMode()

rzikm · 2025-12-16T09:57:18Z

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

+        public System.IO.Stream Open(FileAccess access) { throw null; }
        public System.Threading.Tasks.Task<System.IO.Stream> OpenAsync(System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }


I think we have overlooked something again during API review, we should also have an Async version of the new overload.

I think letting API review board over an email should be enough and we don't need to wait for the response.

rzikm · 2025-12-16T09:58:03Z

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

+        /// <summary>
+        /// Opens the entry with the specified access mode. This allows for more granular control over the returned stream's capabilities.
+        /// </summary>


Right, we should include the explanation in the remarks section.

rzikm · 2025-12-16T10:01:42Z

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

+            _currentlyOpenForWrite = true;
+
+            // Create a fresh empty MemoryStream, discarding any previously loaded data
+            _storedUncompressedData = new MemoryStream();


if this is overwriting an existing stream here, we want to dispose it first.

rzikm · 2025-12-16T10:05:49Z

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

+            return OpenInWriteModeCore();
+        }
+
+        private WrappedStream OpenInWriteModeForUpdate()


This looks similar to OpenInUpdateMode, Would it be still readable if we added a parameter there that would control whether existing contents should be returned or not?

rzikm · 2025-12-16T10:07:25Z

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

+
+        [Theory]
+        [MemberData(nameof(Get_Booleans_Data))]
+        public static async Task OpenWithFileAccess_CreateMode_ReadWriteAccess_Succeeds(bool async)


This looks the same as the test above (only difference is the FileAccess), maybe take the FileAccess as another parameter?

rzikm · 2025-12-16T10:13:31Z

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

+
+        [Theory]
+        [MemberData(nameof(Get_Booleans_Data))]
+        public static async Task OpenWithFileAccess_UpdateMode_InvalidAccess_Throws(bool async)


This and the two above are almost the same. I think having only one of them is enough as the argument check happens before the logic branches on the ZipArchiveMode, but if you want to keep tests for all combinations, make it into a single parameterized test.

rzikm · 2025-12-16T10:15:30Z

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

+            ThrowIfInvalidArchive();
+
+            if (access is not FileAccess.Read and not FileAccess.Write and not FileAccess.ReadWrite)
+                throw new ArgumentException(SR.InvalidFileAccess, nameof(access));


I think ArgumentOutOfRangeException is better fit here

rzikm · 2025-12-16T10:17:40Z

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

+                    if (access != FileAccess.Read)
+                        throw new ArgumentException(SR.CannotBeWrittenInReadMode, nameof(access));
+                    return OpenInReadMode(checkOpenable: true);
+
+                case ZipArchiveMode.Create:
+                    if (access == FileAccess.Read)
+                        throw new ArgumentException(SR.CannotBeReadInCreateMode, nameof(access));
+                    return OpenInWriteMode();


Instead of ArgumentException, I think InvalidOperationException is more appropriate. The argument itself is in valid range, but the operation it tries to perform is not valid in the current state.

iremyux added 4 commits September 30, 2025 13:24

Add new Open(FileAccess access) method

f56ccb6

Add new Open(FileAccess access) method

b977ca7

Merge branch '101243-open-overload' of https://github.com/iremyux/dot…

04ddf87

…net-runtime into 101243-open-overload

Add tests

b35de54

iremyux self-assigned this Nov 28, 2025

github-actions bot added the area-System.IO.Compression label Nov 28, 2025

iremyux marked this pull request as ready for review November 28, 2025 11:26

Copilot AI review requested due to automatic review settings November 28, 2025 11:26

Copilot started reviewing on behalf of iremyux November 28, 2025 11:27 View session

Copilot finished reviewing on behalf of iremyux November 28, 2025 11:30

Copilot AI reviewed Nov 28, 2025

View reviewed changes

build-analysis bot mentioned this pull request Nov 28, 2025

[Long Running Test] 'System.Composition.UnitTests.ConcurrencyTests.SharedInstancesAreNotVisibleUntilActivationCompletes' #113686

Open

iremyux added 4 commits November 28, 2025 16:15

Fix test

24f7ebe

Add more tests

31f1476

Use default case for ZipArchiveMode.Update in Open(FileAccess access)

0f2e2ce

Add upfront validation for invalid FileAccess values

a046dfa

iremyux requested a review from Copilot November 28, 2025 16:02

Copilot started reviewing on behalf of iremyux November 28, 2025 16:03 View session

Copilot finished reviewing on behalf of iremyux November 28, 2025 16:06

Copilot AI reviewed Nov 28, 2025

View reviewed changes

src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs Outdated Show resolved Hide resolved

The switch expression needs a default case

a3cf84b

build-analysis bot mentioned this pull request Nov 28, 2025

Unable to read data from the transport connection: An existing connection was forcibly closed by the remote host. dotnet/dnceng#5922

Open

3 tasks

Change default switch case to UnreachableException

9878931

build-analysis bot mentioned this pull request Nov 28, 2025

[android][arm64] System.Net.Sockets.Tests.SendTo_SyncForceNonBlocking.Datagram_UDP_ShouldImplicitlyBindLocalEndpoint fails with NetworkUnreachable #120526

Open

Use OpenInUpdateMode for FileAccess.Write in Update mode

760a5aa

iremyux requested a review from Copilot November 28, 2025 19:42

Copilot started reviewing on behalf of iremyux November 28, 2025 19:43 View session

Copilot finished reviewing on behalf of iremyux November 28, 2025 19:45

Copilot AI reviewed Nov 28, 2025

View reviewed changes

build-analysis bot mentioned this pull request Nov 28, 2025

Cose.Tests.TestKeyRing.CreateECDsa fails with "OSStatus error -50" #121902

Open

iremyux requested review from a team and removed request for a team December 1, 2025 12:42

iremyux marked this pull request as draft December 1, 2025 14:46

iremyux added 3 commits December 8, 2025 11:18

Introduce OpenInWriteModeForUpdate method

2fda98c

Update comment

1320d08

Update check in one test

1aa78ae

build-analysis bot mentioned this pull request Dec 8, 2025

System.Security.Cryptography.Tests .IsSupported_AgreesWithPlatform failures #122228

Closed

iremyux and others added 2 commits December 10, 2025 23:28

Fix what happens on Update mode + Write access and its tests

a376e3a

Merge branch 'dotnet:main' into 101243-open-overload

f63c42a

iremyux marked this pull request as ready for review December 10, 2025 22:35

iremyux requested a review from Copilot December 10, 2025 22:35

Copilot started reviewing on behalf of iremyux December 10, 2025 22:36 View session

Copilot AI reviewed Dec 10, 2025

View reviewed changes

iremyux requested a review from a team December 11, 2025 09:28

rzikm added this to the 11.0.0 milestone Dec 11, 2025

rzikm reviewed Dec 16, 2025

View reviewed changes

		FileAccess.ReadWrite => OpenInUpdateMode(),
		_ => throw new UnreachableException()

		public System.IO.Stream Open(FileAccess access) { throw null; }
		public System.Threading.Tasks.Task<System.IO.Stream> OpenAsync(System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }

Add ZipArchiveEntry.Open(FileAccess) overload #122032

Are you sure you want to change the base?

Add ZipArchiveEntry.Open(FileAccess) overload #122032

Uh oh!

Conversation

iremyux commented Nov 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dotnet-policy-service bot commented Nov 28, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

iremyux commented Nov 28, 2025 •

edited

Loading