-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Add ZipArchiveEntry.Open(FileAccess) overload #122032
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Tagging subscribers to this area: @dotnet/area-system-io-compression |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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, orOpenInUpdateMode) - 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
Show resolved
Hide resolved
src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.IO.Compression/tests/ZipArchive/zip_InvalidParametersAndStrangeFiles.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
| /// <summary> | ||
| /// Opens the entry with the specified access mode. This allows for more granular control over the returned stream's capabilities. | ||
| /// </summary> | ||
| /// <param name="access">The desired file access mode for the returned stream.</param> |
Copilot
AI
Nov 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The <param> description should be a noun phrase that begins with an introductory article, per the documentation guidelines. Consider: "The file access mode for the returned stream."
| /// Opens the entry with the specified access mode. This allows for more granular control over the returned stream's capabilities. | ||
| /// </summary> | ||
| /// <param name="access">The desired file access mode for the returned stream.</param> | ||
| /// <returns>A Stream that represents the contents of the entry with the specified access capabilities.</returns> |
Copilot
AI
Nov 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The <returns> description should be a noun phrase that begins with an introductory article. Consider: "A stream that represents the contents of the entry with the specified access capabilities."
| /// <returns>A Stream that represents the contents of the entry with the specified access capabilities.</returns> | |
| /// <returns>A <see cref="Stream"/> that represents the contents of the entry with the specified access capabilities.</returns> |
| using Stream stream = entry.Open(FileAccess.ReadWrite); | ||
| Assert.True(stream.CanWrite); |
Copilot
AI
Nov 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Create mode with FileAccess.ReadWrite, the test verifies stream.CanWrite but doesn't verify stream.CanRead like the corresponding Update mode test does (line 2324-2326). For consistency and completeness, consider adding an assertion for stream.CanRead to match the behavior documented in the comment on line 2256.
When a
ZipArchiveis opened inZipArchiveMode.Update, callingZipArchiveEntry.Open()always returns a read-write stream by invokingOpenInUpdateMode(). This causes the entire entry to be decompressed into memory, even when the caller only intends to read the entry's contents.This PR adds a new
Open(FileAccess access)overload toZipArchiveEntrythat allows callers to explicitly specify the desired access mode:FileAccess.Read: Opens the entry in read modeOpenInReadMode, avoiding the memory overhead of decompression into aMemoryStream. This is useful when you only need to read the entry's contents inUpdatemode.FileAccess.Write: Opens the entry in write modeOpenInWriteMode.FileAccess.ReadWrite: Opens the entry in update modeOpenInUpdateMode, matching the current parameterlessOpen()behavior.The method validates that the requested access is compatible with the archive's mode:
In
Readmode: OnlyFileAccess.Readis allowedIn
Createmode: OnlyFileAccess.WriteorFileAccess.ReadWriteis allowedIn
Updatemode: All access modes are allowed.FileAccess.Readreturns a read-only stream, whileFileAccess.WriteandFileAccess.ReadWriteboth return a read-write-seekable stream. (FileAccess.WriteandFileAccess.ReadWritebehave identically)Fixes #101243