Skip to content

Commit 9847f9f

Browse files
committed
fix(io-compression): the three streams enforce their own mode (#2152)
DeflateStream, GZipStream and ZLibStream now reject a Read on a Compress-mode stream and a Write on a Decompress-mode stream with InvalidOperationException, matching .NET's EnsureDecompressionMode/EnsureCompressionMode (DeflateStream.cs:387-403) and carrying SR.CannotReadFromDeflateStream / SR.CannotWriteToDeflateStream verbatim (Strings.resx:122,125). THE READ ROW IS THE ONE THAT MATTERED. It used to return 0 -- indistinguishable from end-of-stream -- so a read loop over a Compress-mode stream terminated normally having produced nothing, with no diagnostic anywhere. The Write row already threw, but named zlib's internal Z_STREAM_ERROR ("deflate error -2") for what is a caller mistake. A caller that checks CanRead/CanWrite is unaffected; those properties already reported the right answer, which is exactly why the defect was invisible. THE ORDER IS TRANSCRIBED, NOT CHOSEN. .NET runs ValidateBufferArguments in Read(byte[],int,int) and then ReadCore opens with EnsureDecompressionMode() followed by EnsureNotDisposed() (DeflateStream.cs:284-309; Write is the same shape at :531-570). So arguments, then mode, then disposed -- and a stream that is BOTH disposed and in the wrong mode reports the MODE. One existing test asserted the opposite for a Compress-mode Read and is updated rather than worked around. A MUTATION PROVED THE EXCEPTION TYPE ALONE CANNOT EXPRESS THAT ORDERING. ObjectDisposedException derives from InvalidOperationException here as it does in .NET, so EXPECT_THROW(..., InvalidOperationException) is satisfied by BOTH orders and the swapped-order mutation went uncaught. The message is the only discriminator, so the message is what the test asserts. Recorded rather than quietly fixed. FLUSH DELIBERATELY GETS NO GUARD. .NET's Flush() opens with EnsureNotDisposed() alone and then no-ops for a Decompress-mode stream (DeflateStream.cs:210-215). Adding one would be a plausible-looking symmetry the reference does not have, so a test pins its absence. A CORRECTION MADE ON THE WAY PAST. ThrowInvalidCompressionMode's doc-comment recorded that #2148 chose the base ArgumentException from the audit's managed probe and that "/rv is absent here to narrow it further". /rv is present now and confirms the choice exactly: DeflateStream.cs:99 is `throw new ArgumentException(SR.ArgumentOutOfRange_Enum, nameof(mode))`, and ArgumentOutOfRange_Enum is "Enum value was out of legal range." -- same type, same message, same parameter name. The caveat is replaced by the measurement. Two pins inverted, four cases added (IO_Compression 101 -> 103). Five mutations, all caught -- one only after the ordering test stopped asserting a type and started asserting a message. Downstream, measured per SA-2 condition 5: neither cna nor mobile-eggbert references any of the three stream types -- zero sites in both. Neither modified. Gate: 17,304 run, 17,304 passed, 0 failed, 0 skipped across 38 executables, GREEN. docs/Migration-CompressionStreamModeGuards.md
1 parent ddbb4c9 commit 9847f9f

9 files changed

Lines changed: 314 additions & 23 deletions

File tree

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
<!-- SPDX-License-Identifier: MIT -->
2+
<!-- Copyright (c) Robert Vokac and contributors -->
3+
4+
# Migration — the compression streams enforce their own mode (ticket #2152)
5+
6+
*2026-08-18.* `DeflateStream`, `GZipStream` and `ZLibStream` now reject a `Read` on a
7+
Compress-mode stream and a `Write` on a Decompress-mode stream with
8+
`InvalidOperationException`, matching .NET.
9+
10+
Landed under `docs/StandingApprovals.md` SA-5. No signature, layout or `noexcept` change.
11+
12+
---
13+
14+
## 1. What changed
15+
16+
| Call | Was | Is |
17+
|---|---|---|
18+
| `Read` on an open **Compress**-mode stream | returned **0** | `InvalidOperationException("Reading from the compression stream is not supported.")` |
19+
| `Write` on an open **Decompress**-mode stream | `IOException("DeflateStream: deflate error -2")` | `InvalidOperationException("Writing to the compression stream is not supported.")` |
20+
| `Read` on a **closed Compress**-mode stream | `ObjectDisposedException` | the **mode** error — see §3 |
21+
| `Write` on a **closed Decompress**-mode stream | `ObjectDisposedException` | the **mode** error |
22+
| `Flush()` in either mode | no mode guard | **no mode guard** (unchanged, deliberately — §4) |
23+
| every call in the mode that allows it || **unchanged** |
24+
25+
**The `Read` row is the one that mattered.** Returning 0 is indistinguishable from end-of-stream,
26+
so a read loop over a Compress-mode stream terminated normally having produced nothing, with no
27+
diagnostic anywhere. The `Write` row already threw; it just named zlib's internal `Z_STREAM_ERROR`
28+
for what is a caller mistake.
29+
30+
A caller that checks `CanRead`/`CanWrite` first is unaffected — those properties already reported
31+
the right answer, which is exactly why the defect was invisible.
32+
33+
## 2. The reference
34+
35+
```csharp
36+
private void EnsureDecompressionMode()
37+
{
38+
if (_mode != CompressionMode.Decompress) ThrowCannotReadFromDeflateStreamException();
39+
static void ThrowCannotReadFromDeflateStreamException() =>
40+
throw new InvalidOperationException(SR.CannotReadFromDeflateStream);
41+
}
42+
```
43+
*(`DeflateStream.cs:387-395`; `EnsureCompressionMode` is the mirror at `:396-403`.)*
44+
45+
The two messages are `Strings.resx:122,125` verbatim. They name *"the compression stream"* rather
46+
than a concrete type, so all three wrappers share one string — which is what .NET does too, since
47+
`GZipStream` and `ZLibStream` delegate to `DeflateStream`.
48+
49+
## 3. The order is transcribed, not chosen
50+
51+
```csharp
52+
public override int Read(byte[] buffer, int offset, int count)
53+
{
54+
ValidateBufferArguments(buffer, offset, count); // 1. arguments
55+
return ReadCore(new Span<byte>(buffer, offset, count));
56+
}
57+
58+
internal int ReadCore(Span<byte> buffer)
59+
{
60+
EnsureDecompressionMode(); // 2. mode
61+
EnsureNotDisposed(); // 3. disposed
62+
```
63+
*(`DeflateStream.cs:284-309`; `Write`/`WriteCore` is the same shape at `:531-570`.)*
64+
65+
So a stream that is **both** disposed and in the wrong mode reports the **mode**. That is a
66+
behaviour change for one existing test, which is updated rather than worked around.
67+
68+
**A mutation proved the type alone cannot express this.** `ObjectDisposedException` derives from
69+
`InvalidOperationException` — here as in .NETso `EXPECT_THROW(…, InvalidOperationException)` is
70+
satisfied by *both* orders and the swapped-order mutation went uncaught. The message is the only
71+
discriminator, so the message is what the test asserts.
72+
73+
## 4. `Flush()` deliberately has no guard
74+
75+
.NET's `Flush()` opens with `EnsureNotDisposed()` alone and then no-ops for a Decompress-mode
76+
stream (`DeflateStream.cs:210-215`). Adding a mode guard there would be a plausible-looking
77+
symmetry the reference does not have, so a test pins its absence.
78+
79+
## 5. A correction made on the way past
80+
81+
`ThrowInvalidCompressionMode`'s doc-comment recorded that #2148 chose the **base**
82+
`ArgumentException` from the audit's managed probe and that *"`/rv` is absent here to narrow it
83+
further"*. `/rv` is present now and confirms the choice exactly: `DeflateStream.cs:99` is
84+
`throw new ArgumentException(SR.ArgumentOutOfRange_Enum, nameof(mode))`, and
85+
`ArgumentOutOfRange_Enum` is *"Enum value was out of legal range."*same type, same message,
86+
same parameter name. The caveat is replaced with the measurement.
87+
88+
## 6. To migrate
89+
90+
```cpp
91+
// A stream opened to compress can only be written; one opened to decompress can only be read.
92+
if (stream.getCanReadProperty()) { /* … Read … */ }
93+
```
94+
95+
`CanRead` and `CanWrite` have always reported this correctly. Code that branched on them needs no
96+
change; code that did not now gets a diagnostic instead of silence.
97+
98+
## 7. Evidence
99+
100+
| Mutation | Caught |
101+
|---|---|
102+
| Drop the read-mode guard (one stream type) ||
103+
| Drop the write-mode guard (one stream type) ||
104+
| Run the mode check **after** the disposed check | ✅ — **only after** the test asserted the message; the type alone could not see it |
105+
| Add a mode guard to `Flush()` ||
106+
| Swap the two messages | ✅ (2 tests) |
107+
108+
## 8. Downstream, measured
109+
110+
Per SA-2 condition 5: neither `cna` nor `mobile-eggbert` references `DeflateStream`, `GZipStream`
111+
or `ZLibStream` — **zero sites in both**. Neither repository was modified.

modules/io-compression/include/System/IO/Compression/CompressionArgumentValidation.hpp

Lines changed: 64 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -92,18 +92,76 @@ namespace System::IO::Compression::Detail {
9292
/**
9393
* @brief Throws `ArgumentException("Enum value was out of legal range.", paramName)`.
9494
*
95-
* The **base** `ArgumentException`, not the derived `ArgumentOutOfRangeException`, because the
96-
* audit's own managed probe for SR-AUD-258 recorded the category .NET reports for this exact
97-
* call as `ArgumentException` (`audit/.../DeflateStream.cpp.audit.md`, "current .NET prints
98-
* `invalidMode=ArgumentException`"), and `/rv` is absent here to narrow it further. Same
99-
* choice, for the same reason, as `System::Threading`'s `EventWaitHandle` (#1954) and
100-
* `System::Uri`'s `Uri(string, UriKind)` (#1992).
95+
* The **base** `ArgumentException`, not the derived `ArgumentOutOfRangeException`. #2148 chose
96+
* this from the audit's own managed probe and recorded that `/rv` was absent to narrow it
97+
* further. **It is present now, and it confirms the choice exactly**: `DeflateStream.cs:99` is
98+
* `throw new ArgumentException(SR.ArgumentOutOfRange_Enum, nameof(mode))`, and
99+
* `ArgumentOutOfRange_Enum` is *"Enum value was out of legal range."* — the same type, the same
100+
* message and the same parameter name. Verified 2026-08-18 by ticket #2152.
101101
*/
102102
[[noreturn]] void ThrowInvalidCompressionMode(const char* paramName);
103103

104104
/** @brief Throws `ObjectDisposedException(typeName, "Cannot access a closed Stream.")`. */
105105
[[noreturn]] void ThrowStreamClosed(const char* typeName);
106106

107+
/**
108+
* @brief Throws `InvalidOperationException("Reading from the compression stream is not supported.")`.
109+
*
110+
* .NET's `SR.CannotReadFromDeflateStream`, transcribed
111+
* (`System.IO.Compression/src/Resources/Strings.resx:122`). The message names *the compression
112+
* stream* rather than the concrete type, so all three wrappers share one string, exactly as
113+
* .NET's three do — `GZipStream` and `ZLibStream` delegate to `DeflateStream` there.
114+
*/
115+
[[noreturn]] void ThrowCannotReadFromCompressionStream();
116+
117+
/**
118+
* @brief Throws `InvalidOperationException("Writing to the compression stream is not supported.")`.
119+
*
120+
* .NET's `SR.CannotWriteToDeflateStream` (`Strings.resx:125`).
121+
*/
122+
[[noreturn]] void ThrowCannotWriteToCompressionStream();
123+
124+
/**
125+
* @brief Rejects a read on a stream that was opened to **compress**.
126+
*
127+
* Ticket #2152 (SR-AUD post-audit, measured under #2148 and deliberately left then). Before it,
128+
* a `Read` on an open Compress-mode stream returned **0** — a silent wrong answer a caller
129+
* cannot distinguish from end-of-stream — and a `Write` on an open Decompress-mode stream
130+
* surfaced zlib's `Z_STREAM_ERROR` as `IOException("DeflateStream: deflate error -2")`, naming
131+
* an internal error code for a caller mistake. .NET guards both
132+
* (`DeflateStream.cs:387-403`).
133+
*
134+
* **The position matters and is transcribed rather than chosen.** .NET runs
135+
* `ValidateBufferArguments` in `Read(byte[], int, int)`, then `ReadCore` opens with
136+
* `EnsureDecompressionMode(); EnsureNotDisposed();` — in that order (`DeflateStream.cs:284-309`).
137+
* So the mode check sits **between** the argument validation and the disposed check, and a
138+
* *disposed* Compress-mode stream reports the **mode** error, not `ObjectDisposedException`.
139+
*
140+
* `Flush()` deliberately gets **no** guard: .NET's opens with `EnsureNotDisposed()` alone and
141+
* then no-ops for a Decompress-mode stream (`DeflateStream.cs:210-215`).
142+
*
143+
* @throws System::InvalidOperationException when @p mode is not `Decompress`.
144+
*/
145+
inline void EnsureDecompressionMode(CompressionMode mode) {
146+
if (mode != CompressionMode::Decompress) {
147+
ThrowCannotReadFromCompressionStream();
148+
}
149+
}
150+
151+
/**
152+
* @brief Rejects a write on a stream that was opened to **decompress**.
153+
*
154+
* The mirror of EnsureDecompressionMode; see that function for the ordering rule and for what
155+
* each door did before ticket #2152.
156+
*
157+
* @throws System::InvalidOperationException when @p mode is not `Compress`.
158+
*/
159+
inline void EnsureCompressionMode(CompressionMode mode) {
160+
if (mode != CompressionMode::Compress) {
161+
ThrowCannotWriteToCompressionStream();
162+
}
163+
}
164+
107165
/**
108166
* @brief Rejects a `CompressionMode` outside the two members the enum declares.
109167
*

modules/io-compression/src/System/IO/Compression/CompressionArgumentValidation.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include "System/ArgumentNullException.hpp"
77
#include "System/ArgumentOutOfRangeException.hpp"
88
#include "System/ObjectDisposedException.hpp"
9+
#include "System/InvalidOperationException.hpp"
910

1011
#include <zlib.h>
1112

@@ -37,6 +38,18 @@ namespace System::IO::Compression::Detail {
3738
throw System::ObjectDisposedException(typeName, "Cannot access a closed Stream.");
3839
}
3940

41+
// Ticket #2152. Both messages are SR.CannotReadFromDeflateStream / SR.CannotWriteToDeflateStream
42+
// transcribed from System.IO.Compression/src/Resources/Strings.resx:122,125. They name "the
43+
// compression stream" rather than a concrete type, so all three wrappers share one string --
44+
// which is what .NET does too, since GZipStream and ZLibStream delegate here.
45+
void ThrowCannotReadFromCompressionStream() {
46+
throw System::InvalidOperationException("Reading from the compression stream is not supported.");
47+
}
48+
49+
void ThrowCannotWriteToCompressionStream() {
50+
throw System::InvalidOperationException("Writing to the compression stream is not supported.");
51+
}
52+
4053
intcs ResolveZLibStrategy(ZLibCompressionStrategy strategy) {
4154
switch (strategy) {
4255
case ZLibCompressionStrategy::Default: return Z_DEFAULT_STRATEGY;

modules/io-compression/src/System/IO/Compression/DeflateStream.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,11 @@ SharpRuntime::intcs DeflateStream::Read(SharpRuntime::bytecs* buffer,
146146
// Ticket #2148: a closed stream used to answer 0 here, indistinguishable from "the compressed
147147
// stream is exhausted". The buffer arguments are validated FIRST, matching .NET, whose
148148
// Read(byte[],int,int) runs ValidateBufferArguments before ReadCore's EnsureNotDisposed.
149+
// Ticket #2152: .NET runs EnsureDecompressionMode BEFORE EnsureNotDisposed
150+
// (DeflateStream.cs:305-308), so a disposed Compress-mode stream reports the MODE
151+
// error rather than ObjectDisposedException. This used to return 0 -- a silent wrong
152+
// answer a caller cannot tell from end-of-stream.
153+
Detail::EnsureDecompressionMode(mode_);
149154
Detail::ThrowIfStreamClosed(state_ && state_->initialized, "DeflateStream");
150155
if (state_->finished || count == 0) return 0;
151156

@@ -186,6 +191,11 @@ void DeflateStream::Write(const SharpRuntime::bytecs* buffer,
186191
if (count < 0) throw System::ArgumentOutOfRangeException("count", "Non-negative number required.");
187192
// Ticket #2148: this is the door the finding names. A write to a closed stream used to return
188193
// silently, so the caller's bytes vanished with no diagnostic at all.
194+
// Ticket #2152, the mirror of Read's guard and in the same position
195+
// (DeflateStream.cs:569). This used to reach deflate() and surface zlib's
196+
// Z_STREAM_ERROR as IOException("DeflateStream: deflate error -2"), naming an
197+
// internal error code for what is a caller mistake.
198+
Detail::EnsureCompressionMode(mode_);
189199
Detail::ThrowIfStreamClosed(state_ && state_->initialized, "DeflateStream");
190200
if (count == 0) return;
191201

modules/io-compression/src/System/IO/Compression/GZipStream.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,11 @@ SharpRuntime::intcs GZipStream::Read(SharpRuntime::bytecs* buffer,
137137
if (count < 0) throw System::ArgumentOutOfRangeException("count", "Non-negative number required.");
138138
// Ticket #2148: a closed stream used to answer 0 here, indistinguishable from "the
139139
// compressed stream is exhausted". Buffer arguments are validated first, as .NET does.
140+
// Ticket #2152: .NET runs EnsureDecompressionMode BEFORE EnsureNotDisposed
141+
// (DeflateStream.cs:305-308), so a disposed Compress-mode stream reports the MODE
142+
// error rather than ObjectDisposedException. This used to return 0 -- a silent wrong
143+
// answer a caller cannot tell from end-of-stream.
144+
Detail::EnsureDecompressionMode(mode_);
140145
Detail::ThrowIfStreamClosed(state_ && state_->initialized, "GZipStream");
141146
if (state_->finished || count == 0) return 0;
142147

@@ -177,6 +182,11 @@ void GZipStream::Write(const SharpRuntime::bytecs* buffer,
177182
if (count < 0) throw System::ArgumentOutOfRangeException("count", "Non-negative number required.");
178183
// Ticket #2148: this is the door the finding names. A write to a closed stream used to
179184
// return silently, so the caller's bytes vanished with no diagnostic at all.
185+
// Ticket #2152, the mirror of Read's guard and in the same position
186+
// (DeflateStream.cs:569). This used to reach deflate() and surface zlib's
187+
// Z_STREAM_ERROR as IOException("GZipStream: deflate error -2"), naming an
188+
// internal error code for what is a caller mistake.
189+
Detail::EnsureCompressionMode(mode_);
180190
Detail::ThrowIfStreamClosed(state_ && state_->initialized, "GZipStream");
181191
if (count == 0) return;
182192

modules/io-compression/src/System/IO/Compression/ZLibStream.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,11 @@ SharpRuntime::intcs ZLibStream::Read(SharpRuntime::bytecs* buffer,
136136
if (count < 0) throw System::ArgumentOutOfRangeException("count", "Non-negative number required.");
137137
// Ticket #2148: a closed stream used to answer 0 here, indistinguishable from "the
138138
// compressed stream is exhausted". Buffer arguments are validated first, as .NET does.
139+
// Ticket #2152: .NET runs EnsureDecompressionMode BEFORE EnsureNotDisposed
140+
// (DeflateStream.cs:305-308), so a disposed Compress-mode stream reports the MODE
141+
// error rather than ObjectDisposedException. This used to return 0 -- a silent wrong
142+
// answer a caller cannot tell from end-of-stream.
143+
Detail::EnsureDecompressionMode(mode_);
139144
Detail::ThrowIfStreamClosed(state_ && state_->initialized, "ZLibStream");
140145
if (state_->finished || count == 0) return 0;
141146

@@ -176,6 +181,11 @@ void ZLibStream::Write(const SharpRuntime::bytecs* buffer,
176181
if (count < 0) throw System::ArgumentOutOfRangeException("count", "Non-negative number required.");
177182
// Ticket #2148: this is the door the finding names. A write to a closed stream used to
178183
// return silently, so the caller's bytes vanished with no diagnostic at all.
184+
// Ticket #2152, the mirror of Read's guard and in the same position
185+
// (DeflateStream.cs:569). This used to reach deflate() and surface zlib's
186+
// Z_STREAM_ERROR as IOException("ZLibStream: deflate error -2"), naming an
187+
// internal error code for what is a caller mistake.
188+
Detail::EnsureCompressionMode(mode_);
179189
Detail::ThrowIfStreamClosed(state_ && state_->initialized, "ZLibStream");
180190
if (count == 0) return;
181191

0 commit comments

Comments
 (0)