-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Add DecompressionMethods.Zstandard for HTTP automatic decompression #123531
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
Co-authored-by: rzikm <[email protected]>
Co-authored-by: rzikm <[email protected]>
Co-authored-by: rzikm <[email protected]>
rzikm
left a comment
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.
@copilot Please review my comments
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/DecompressionHandler.cs
Show resolved
Hide resolved
|
Tagging subscribers to this area: @karelz, @dotnet/ncl |
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
Adds Zstandard (zstd) support to HttpClientHandler/SocketsHttpHandler automatic response decompression by extending DecompressionMethods and wiring the new encoding into the existing decompression pipeline (mirroring the Brotli pattern).
Changes:
- Extended
System.Net.DecompressionMethodswithZstandard = 0x8(and updated reference assembly). - Added
zstdhandling inDecompressionHandlerand enabled it inSocketsHttpHandler’s supported decompression mask. - Added
System.IO.Compression.Zstandarddependency and expanded decompression/Accept-Encoding functional tests to cover Zstandard.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/System.Net.Primitives/src/System/Net/DecompressionMethods.cs | Adds DecompressionMethods.Zstandard enum value. |
| src/libraries/System.Net.Primitives/ref/System.Net.Primitives.cs | Updates public ref API to include Zstandard. |
| src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs | Includes Zstandard in SupportedDecompressionMethods. |
| src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/DecompressionHandler.cs | Adds zstd Accept-Encoding emission and ZstandardStream-based decompression. |
| src/libraries/System.Net.Http/src/System.Net.Http.csproj | Adds project reference to System.IO.Compression.Zstandard. |
| src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Decompression.cs | Adds Zstandard test coverage for decompression and Accept-Encoding behaviors. |
| List<string?> methods = [GZipEnabled ? Gzip : null, DeflateEnabled ? Deflate : null, BrotliEnabled ? Brotli : null, ZstandardEnabled ? Zstd : null]; | ||
| methods.RemoveAll(item => item is null); | ||
| _acceptEncodingHeaderValue = string.Join(", ", methods); |
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.
Span<string?> methods = ["", "", "", ""];
int count = 0;
if (GZipEnabled) methods[count++] = Gzip;
if (DeflateEnabled) methods[count++] = Deflate;
if (BrotliEnabled) methods[count++] = Brotli;
if (ZstandardEnabled) methods[count++] = Zstandard;
_acceptEncodingHeaderValue = string.Join(", ", methods.Slice(0, count);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.
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 comment appears incomplete - could you clarify what change you'd like to see for lines 39-41? Are you suggesting to use Span<string> with stackalloc instead of the List pattern?
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.
@copilot, incomplete how? I'm suggesting the code should look like this:
Span<string?> methods = ["", "", "", ""];
int count = 0;
if (GZipEnabled) methods[count++] = Gzip;
if (DeflateEnabled) methods[count++] = Deflate;
if (BrotliEnabled) methods[count++] = Brotli;
if (ZstandardEnabled) methods[count++] = Zstandard;
_acceptEncodingHeaderValue = string.Join(", ", methods.Slice(0, count);It initializes a span with a collection expression, ensuring enough space for the maximum number of encodings. Then it fills in just the encodings that are enabled. And then it concatenates those strings into a single string.
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 code suggestion appears to be cut off after "Span" - could you provide the complete code you'd like to see for lines 39-41?
|
@jkotas raised a good point in a related issue about trimming. #123532 (comment) I believe the same argument would apply here. |
Yes and no. If you use AutomaticDecompression, then regardless of which compression algorithms you specify, you'll root them all. But if you don't set AutomaticDecompression, the whole compression support becomes trimmable. @eerhardt did that a few years ago in #78198. |
Description
Adds Zstandard compression support to
DecompressionMethodsfor automatic HTTP response decompression, matching the API proposal. Follows the established pattern from Brotli support.Changes
API Surface
Zstandard = 0x8toDecompressionMethodsenum in System.Net.PrimitivesImplementation
DecompressionHandler: Added "zstd" encoding support withZstandardEnabledproperty andZstandardDecompressedContentclass usingZstandardStreamSocketsHttpHandler: UpdatedSupportedDecompressionMethodsto include ZstandardSystem.IO.Compression.ZstandardTests
HttpClientHandlerTest.Decompression.cscovering decompression, empty bodies, Accept-Encoding headers, and quality weightingsUsage
Notes
System.IO.Compression.Zstandardwhich is already availableOriginal prompt
DecompressionMethods.Zstdfor web requests #112075💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.