Fix DeflateStream retaining stale buffer reference after write failure mid-operation - #132407
Fix DeflateStream retaining stale buffer reference after write failure mid-operation#132407alinpahontu2912 wants to merge 2 commits into
Conversation
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @karelz, @dotnet/area-system-io-compression |
There was a problem hiding this comment.
Pull request overview
This PR hardens System.IO.Compression’s deflate implementation against mid-operation failures by ensuring the zlib deflater/inflater won’t retain (and later read from) stale caller-provided input buffers after an exception propagates (e.g., write failure/cancellation), including during subsequent Dispose/flush paths.
Changes:
- Add
UnsetInput()helpers toDeflaterandInflaterto discard any unconsumed input and release pinned references. - Ensure
DeflateStreamclears engine input on exceptions in deflate-output write loops and inCopyTodecompression forwarding. - Add regression tests intended to validate that stale input isn’t retained after failure paths.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/libraries/System.IO.Compression/src/System/IO/Compression/DeflateZLib/Inflater.cs | Adds UnsetInput() to drop unconsumed pinned input for inflate paths. |
| src/libraries/System.IO.Compression/src/System/IO/Compression/DeflateZLib/DeflateStream.cs | Clears stale input on exceptional paths during compress output writes and CopyTo decompression forwarding. |
| src/libraries/System.IO.Compression/src/System/IO/Compression/DeflateZLib/Deflater.cs | Adds UnsetInput() to drop unconsumed pinned input for deflate paths. |
| src/libraries/Common/tests/System/IO/Compression/CompressionStreamUnitTestBase.cs | Adds regression tests exercising exception mid-operation behavior for compression and decompression. |
iremyux
left a comment
There was a problem hiding this comment.
The copilot comment looks legit, otherwise LGTM
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
src/libraries/System.IO.Compression/src/System/IO/Compression/DeflateZLib/Inflater.cs:287
- The XML doc comment has a grammatical typo: 'may since have' should be 'may have since' for correct wording.
/// the inflater doesn't retain a dangling reference to a buffer the caller may since have reused or freed.
src/libraries/Common/tests/System/IO/Compression/CompressionStreamUnitTestBase.cs:172
- This reflection lookup restricts to
BindingFlags.Public, butNeedsInputon the internal engine types may be non-public. If so,GetMethod(...)returns null and the null-forgiving operator will cause a test failure. UseBindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance(or otherwise handle a nullMethodInfo) to make the test resilient to visibility.
MethodInfo needsInputMethod = engine.GetType().GetMethod("NeedsInput", BindingFlags.Public | BindingFlags.Instance)!;
return (bool)needsInputMethod.Invoke(engine, null)!;
| /// Must be called if an in-progress operation is abandoned (e.g. due to an exception or cancellation) so | ||
| /// the deflater doesn't retain a dangling reference to a buffer the caller may since have reused or freed. | ||
| /// </summary> | ||
| internal void UnsetInput() => DeallocateInputBufferHandle(resetStreamHandle: true); |
Fixes #132393