-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Add span-based Deflate, ZLib and GZip encoder/decoder APIs #123145
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?
Changes from all commits
e402958
95ac930
88acc57
6f1ab80
8926759
bb2d89c
a21a800
4b87dcd
aabc3a6
cfb3d7e
f031eed
1d18eec
0cf5b04
01fba5a
03fbcbc
665ae10
ee0a437
238aa58
aeebd78
ab9ee4a
cdb1cb4
46c5a8f
e35f082
fb4b1c8
f82ddc4
89ba215
6f05f53
c6384e4
f3dd1c4
91b2890
e52c80e
802d228
8177c6a
46cf8b4
5c226ac
0c5eb13
38202d3
2b5ad84
5bf24f0
e56c631
382b7c2
ba72b26
82038d8
ae7c2a9
a1658e7
d294740
913b797
c4948b8
fdfad6b
3486f09
c5d2412
15f0fd6
d6d47bf
b1fa69c
0ce58db
0ffe1ee
c7fca14
8a31cc5
f2a26ab
93f42ce
2ab5044
9cb4ff5
e5f6990
f2ed2e5
b8cd755
dbf1903
b963848
254cf71
23aab9a
6274e44
274ad66
da347ae
2a0b947
fd5206f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -269,13 +269,14 @@ public void TryDecompress_RandomData_ReturnsFalse(bool useDictionary) | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (useDictionary && !SupportsDictionaries) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
269
to
270
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test does not use dictionaries, so we can change this to Fact only, also, the name uses "RandomData", but this PR changes the implementation to use 0xFF.... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // 0xFF is an invalid first byte for all three formats: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The comment relates to GZip, Deflate and Zlib, but the test suite is shared for Brotli and Zstandard as well. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // - GZip requires magic bytes 0x1F 0x8B | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // - ZLib requires a valid CMF byte (0x78 for deflate with window size) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // - ZLib requires a valid CMF byte (0x78 for deflate with window size) | |
| // - ZLib requires the first byte (CMF) to indicate deflate with a supported window size; 0xFF is not a valid CMF value |
Copilot
AI
Mar 11, 2026
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.
maxLength is a long, but array lengths require an int. new byte[maxLength] won’t compile. Cast with checked((int)maxLength) (and/or assert the bound fits in int.MaxValue) before allocating.
| byte[] destination = new byte[maxLength]; | |
| using EncoderAdapter encoder = CreateEncoder(); | |
| OperationStatus status = encoder.Compress(input, destination, out _, out int written, isFinalBlock: true); | |
| Assert.Equal(OperationStatus.Done, status); | |
| Assert.True(written <= maxLength); | |
| Assert.InRange(maxLength, 0, int.MaxValue); | |
| int maxLengthInt = checked((int)maxLength); | |
| byte[] destination = new byte[maxLengthInt]; | |
| using EncoderAdapter encoder = CreateEncoder(); | |
| OperationStatus status = encoder.Compress(input, destination, out _, out int written, isFinalBlock: true); | |
| Assert.Equal(OperationStatus.Done, status); | |
| Assert.True(written <= maxLengthInt); |
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.
Most of the tests here use some variation of byte[] destination = new byte[GetMaxCompressedLength(input.Length)] so having an extra tests feels redundant.
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.
This should use MinQuality and MaxQuality, since this runs for Brotli and Zstandard as well (and they have different range for quality).
The destination arrays for both compressed and decompressed data can be allocated outside of the loop.
I think this test is redundant with RoundTrip_SuccessfullyCompressesAndDecompresses
Copilot
AI
Feb 25, 2026
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.
RoundTrip_AllCompressionLevels doesn't assert the OperationStatus (or bytesConsumed/bytesWritten) from Compress/Decompress. If these calls start returning DestinationTooSmall/NeedMoreData/InvalidData, the test failure will be less actionable (or could miss partial-progress scenarios). Consider asserting status == Done, consumed == input.Length, and written == input.Length for each iteration.
| encoder.Compress(input, compressed, out _, out int compressedSize, isFinalBlock: true); | |
| byte[] decompressed = new byte[input.Length]; | |
| using var decoder = CreateDecoder(); | |
| decoder.Decompress(compressed.AsSpan(0, compressedSize), decompressed, out _, out _); | |
| OperationStatus compressStatus = encoder.Compress(input, compressed, out int bytesConsumed, out int compressedSize, isFinalBlock: true); | |
| Assert.Equal(OperationStatus.Done, compressStatus); | |
| Assert.Equal(input.Length, bytesConsumed); | |
| byte[] decompressed = new byte[input.Length]; | |
| using var decoder = CreateDecoder(); | |
| OperationStatus decompressStatus = decoder.Decompress(compressed.AsSpan(0, compressedSize), decompressed, out int decompressedBytesConsumed, out int decompressedBytesWritten); | |
| Assert.Equal(OperationStatus.Done, decompressStatus); | |
| Assert.Equal(compressedSize, decompressedBytesConsumed); | |
| Assert.Equal(input.Length, decompressedBytesWritten); |
Copilot
AI
Mar 11, 2026
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 new RoundTrip_AllCompressionLevels test ignores the OperationStatus / bytesConsumed / bytesWritten results from Compress and Decompress. This can make the test pass even if the codec returns NeedMoreData or doesn’t consume all input. Capture and assert the expected statuses (e.g., Done) and verify full consumption/writes.
| encoder.Compress(input, compressed, out _, out int compressedSize, isFinalBlock: true); | |
| byte[] decompressed = new byte[input.Length]; | |
| using var decoder = CreateDecoder(); | |
| decoder.Decompress(compressed.AsSpan(0, compressedSize), decompressed, out _, out _); | |
| OperationStatus compressStatus = encoder.Compress(input, compressed, out int inputBytesConsumed, out int compressedSize, isFinalBlock: true); | |
| Assert.Equal(OperationStatus.Done, compressStatus); | |
| Assert.Equal(input.Length, inputBytesConsumed); | |
| byte[] decompressed = new byte[input.Length]; | |
| using var decoder = CreateDecoder(); | |
| OperationStatus decompressStatus = decoder.Decompress(compressed.AsSpan(0, compressedSize), decompressed, out int decompressBytesConsumed, out int decompressBytesWritten); | |
| Assert.Equal(OperationStatus.Done, decompressStatus); | |
| Assert.Equal(compressedSize, decompressBytesConsumed); | |
| Assert.Equal(decompressed.Length, decompressBytesWritten); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| namespace System.IO.Compression | ||
| { | ||
| /// <summary> | ||
| /// Specifies the compression format for zlib-based encoders. | ||
| /// </summary> | ||
| internal enum CompressionFormat | ||
| { | ||
| /// <summary>Raw deflate format (no header/trailer).</summary> | ||
| Deflate, | ||
| /// <summary>ZLib format (zlib header/trailer).</summary> | ||
| ZLib, | ||
| /// <summary>GZip format (gzip header/trailer).</summary> | ||
| GZip | ||
| } | ||
| } |
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.
Interop signature for compressBound uses uint for both input and return. zlib-ng's compressBound takes/returns z_uintmax_t (potentially 64-bit), so this P/Invoke can truncate the bound on 64-bit platforms. Consider switching to nuint/ulong (matching ZSTD_compressBound patterns) together with a native export that returns a pointer-sized/64-bit value.