Skip to content

Add ZLibStream polyfill - #142

Merged
Tyrrrz merged 6 commits into
primefrom
copilot/add-polyfills-for-zlibstream
Oct 2, 2026
Merged

Tyrrrz merged 6 commits into
primefrom
copilot/add-polyfills-for-zlibstream

Conversation

Copilot AI commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

System.IO.Compression.ZLibStream (added in .NET 6) had no polyfill for older target frameworks.

Implementation

  • Added ZLibStream as a type polyfill (PolyShim/Net60/ZLibStream.cs), enabled on .NET Framework 4.5+, .NET Standard 1.1+, and .NET Core/.NET below 6.0.
  • The real ZLibStream relies on an internal zlib window-bits constructor of DeflateStream that isn't publicly exposed, so it can't be wrapped directly. Instead, the polyfill wraps the public raw DeflateStream (headerless deflate) and manually frames the data as a standard zlib stream:
    • Writes the 2-byte zlib header (CMF/FLG) lazily on first write, computed from the requested CompressionLevel to match native zlib output exactly.
    • Tracks an Adler-32 checksum of the uncompressed data and appends it as the 4-byte trailer on dispose.
    • On decompression, reads and validates the 2-byte header before delegating the remaining bytes to a raw inflate DeflateStream.
  • Unsupported members (Seek, SetLength, Length, Position) throw NotSupportedException, matching the real type.
using System.IO;
using System.IO.Compression;

using var compressed = new MemoryStream();
using (var zlib = new ZLibStream(compressed, CompressionLevel.Optimal, leaveOpen: true))
    zlib.Write(data, 0, data.Length);
// compressed now holds a standard zlib stream (0x78 0x9C header + deflate payload + Adler-32 trailer)

Tests

  • Added PolyShim.Tests/Net60/ZLibStreamTests.cs covering compression/decompression round-trips, header bytes per compression level, interop with DeflateStream-produced payloads, invalid header detection, and leaveOpen behavior.

Co-authored-by: Tyrrrz <1935960+Tyrrrz@users.noreply.github.com>
@Tyrrrz Tyrrrz added the enhancement New feature or request label Oct 1, 2026
@Tyrrrz
Tyrrrz marked this pull request as ready for review October 1, 2026 19:25
Copilot AI balanced review requested due to automatic review settings October 1, 2026 19:25
@codecov

codecov Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 79.83871% with 50 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.84%. Comparing base (37a7ea6) to head (cb3bb0c).
⚠️ Report is 1 commits behind head on prime.

Files with missing lines Patch % Lines
PolyShim/Net60/ZLibStream.cs 69.32% 34 Missing and 16 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            prime     #142      +/-   ##
==========================================
- Coverage   88.08%   87.84%   -0.24%     
==========================================
  Files         336      338       +2     
  Lines        7199     7447     +248     
  Branches      513      541      +28     
==========================================
+ Hits         6341     6542     +201     
- Misses        706      738      +32     
- Partials      152      167      +15     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Framing, validation, checksum integrity, and disposal paths contain correctness issues.

Review effort: Balanced
Findings: 1 High severity · 4 Medium severity

Open (5)
What changed in this PR

Adds a ZLibStream polyfill for pre-.NET 6 targets using manually framed DeflateStream data.

Changes:

  • Implements zlib framing, Adler-32 checksums, and stream behavior.
  • Adds compression, decompression, and lifecycle tests.
  • Registers the new polyfilled type.
File Description
Signatures.md Registers ZLibStream.
PolyShim/​Net60/​ZLibStream.cs Implements the polyfill.
PolyShim.Tests/​Net60/​ZLibStreamTests.cs Adds behavioral tests.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread PolyShim/Net60/ZLibStream.cs Outdated
Comment thread PolyShim/Net60/ZLibStream.cs
Comment thread PolyShim/Net60/ZLibStream.cs
Comment thread PolyShim/Net60/ZLibStream.cs Outdated
Comment thread PolyShim/Net60/ZLibStream.cs Outdated
@Tyrrrz

Tyrrrz commented Oct 1, 2026

Copy link
Copy Markdown
Owner

@copilot please evaluate review comments

Co-authored-by: Tyrrrz <1935960+Tyrrrz@users.noreply.github.com>

Copilot AI commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please evaluate review comments

All five review comments were valid, and I fixed them in the latest commit.

  • Adler-32 trailer: decompression now checks it. A small wrapper holds back the last 4 bytes of the source, and they are compared with the checksum of the data returned. Corrupted or truncated input now throws InvalidDataException.
  • Read/write arguments: the buffer range is checked before any framing. Zero-length reads and writes are no-ops, so they no longer consume or emit a header.
  • Disposal: the owned base stream is now closed in a finally block, after the deflate stream is finalized and the trailer is written.
  • Header validation: an empty source reads as an empty stream. A truncated header throws. A header with CINFO > 7 or FDICT set throws too.

I checked the changes with a standalone harness against .NET 8's native ZLibStream: round-trip, native interop, empty source, a corrupted trailer and a truncated stream. The existing 13 ZLibStream tests pass only on net10, where the polyfill isn't compiled, so they don't exercise this code. I didn't add tests for the new cases to the repo.

Copilot AI requested a review from Tyrrrz October 1, 2026 19:34
@Tyrrrz
Tyrrrz requested a balanced review from Copilot October 1, 2026 20:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Decompression can block or consume trailing data, and the polyfill omits required API behavior.

Review effort: Balanced
Findings: 1 Low severity

Open (1)
Resolved since last review (5)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Expose the wrapped stream through BaseStream

PolyShim/​Net60/​ZLibStream.cs:75

The .NET 6 ZLibStream API includes a public read-only BaseStream property. Omitting it makes code that accesses the underlying stream compile against the real type but fail on polyfill targets. Mirror the framework property through the wrapped DeflateStream.

This issue also appears in the following locations of the same file:

  • line 308
  • line 364

Comment thread PolyShim/Net60/ZLibStream.cs

@Tyrrrz Tyrrrz left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot a few review comments

Comment thread PolyShim/Net60/ZLibStream.cs Outdated
Comment thread PolyShim/Net60/ZLibStream.cs Outdated
Comment thread PolyShim/Net60/ZLibStream.cs
Comment thread PolyShim/Net60/ZLibStream.cs Outdated
Comment thread PolyShim/Net60/ZLibStream.cs Outdated
Comment thread PolyShim/Net60/ZLibStream.cs Outdated
Comment thread PolyShim.Tests/Net60/ZLibStreamTests.cs Outdated
Comment thread PolyShim.Tests/Net60/ZLibStreamTests.cs Outdated
Co-authored-by: Tyrrrz <1935960+Tyrrrz@users.noreply.github.com>
Copilot AI requested a review from Tyrrrz October 2, 2026 09:46

@Tyrrrz Tyrrrz left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot review comments

Comment thread PolyShim.Tests/Net60/ZLibStreamTests.cs
Comment thread PolyShim.Tests/Net60/ZLibStreamTests.cs Outdated
Comment thread PolyShim.Tests/Net60/ZLibStreamTests.cs Outdated
Comment thread PolyShim.Tests/Net60/ZLibStreamTests.cs Outdated
Comment thread PolyShim.Tests/Net60/ZLibStreamTests.cs Outdated
Co-authored-by: Tyrrrz <1935960+Tyrrrz@users.noreply.github.com>
Co-authored-by: Tyrrrz <1935960+Tyrrrz@users.noreply.github.com>
@Tyrrrz

Tyrrrz commented Oct 2, 2026

Copy link
Copy Markdown
Owner

@copilot please regenerate Signatures.md to resolve conflicts

Copilot AI commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please regenerate Signatures.md to resolve conflicts

Ran ./GenerateSignatures.csx; it regenerated Signatures.md with 591 signatures, and the output matches the checked-in file, so there was no diff to commit.

@Tyrrrz

Tyrrrz commented Oct 2, 2026

Copy link
Copy Markdown
Owner

@copilot please also merge in latest prime. Also, tests are failing for some reason.

…s-for-zlibstream

# Conflicts:
#	Signatures.md

Co-authored-by: Tyrrrz <1935960+Tyrrrz@users.noreply.github.com>

Copilot AI commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please also merge in latest prime. Also, tests are failing for some reason.

Merged the latest prime in cb3bb0c, regenerated Signatures.md, and fixed the Windows compile failure by renaming the compression stream locals. Focused ZLibStream tests pass 7/7. The full suite had three unrelated network-dependent HttpClient failures in this environment.

@Tyrrrz
Tyrrrz merged commit da0cc9a into prime Oct 2, 2026
7 checks passed
@Tyrrrz
Tyrrrz deleted the copilot/add-polyfills-for-zlibstream branch October 2, 2026 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants