Skip to content
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

Add Request Decompression middleware #40279

Merged
merged 48 commits into from
Jun 3, 2022

Conversation

david-acker
Copy link
Member

@david-acker david-acker commented Feb 17, 2022

Adds middleware for decompressing HTTP requests.

Problem

In the past, the developer of the server would need to manually handle requests with compressed content by checking if a request was compressed and then decompressing the request body stream as appropriate.

Solution

This middleware uses the Content-Encoding header automatically identify and decompress requests with compressed content, so that the developer of the server does not need to handle this themselves.

The request decompression middleware is added using the UseRequestDecompression extension method for IApplicationBuilder and the AddRequestDecompression extension method for IServiceCollection.

Supported Content Encodings

Support for Brotli (br), Deflate (deflate), and GZip (gzip) are included out of the box.

Support for other content encodings can be added by registering a custom decompression provider class, which implements the IDecompressionProvider interface, along with a Content-Encoding header value in RequestDecompressionOptions.

Request Size Limits

The request body size limit enforced by the server (MaxRequestBodySize) and/or the endpoint (RequestSizeLimitAttribute) will also be enforced on the decompressed request body.

If the amount of data read from the decompressed request body exceeds the size limit, an InvalidOperationException is thrown to prevent any more data from being read. This protects against the threat of uncontrolled resource consumption from zip bombs

Edge Cases

  • If a request uses an unsupported Content-Encoding or has multiple encodings, an appropriate message will be logged, and the request will be passed on without being decompressed.
  • If the compressed content is invalid, an exception will be thrown when attempting to read the (decompressed) request body stream.

Fixes #40080

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Feb 17, 2022
@david-acker david-acker marked this pull request as draft February 17, 2022 02:17
@sebastienros
Copy link
Member

I wish my next middleware PR is as polished as this one.

@pranavkm
Copy link
Contributor

Random thought: what if we designed the feature like so: https://gist.github.com/pranavkm/8932a862a701061f43b6e64cc0b18931. It removes some of the newly introduced types at the expense of slightly more code in each of the compression providers.

@sebastienros / @Tratcher?

@david-acker
Copy link
Member Author

Trimming needs to be enabled on this new project. I suggest creating an issue to follow up and do it later. See #41704 as an example.

I've created a follow up issue (#41721) for this.

@david-acker david-acker requested a review from sebastienros May 18, 2022 22:17
@halter73
Copy link
Member

Thanks @david-acker! I should have paid more attention to the actual code on my last review.

@david-acker
Copy link
Member Author

@halter73 No worries! Thanks for the help with the IRequestSizeLimitMetadata-related changes!

@davidfowl
Copy link
Member

Can we make sure we have basic benchmarks setup so we can measure the overhead here?

@david-acker
Copy link
Member Author

david-acker commented May 20, 2022

I've added some benchmarks which cover both compressed and uncompressed requests as well as the scenarios where the IRequestSizeLimitMetadata is/isn't present.

Copy link
Member

@Tratcher Tratcher left a comment

Choose a reason for hiding this comment

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

This whole thing looks really clean, nice job.

@sebastienros sebastienros enabled auto-merge (squash) June 3, 2022 20:54
@sebastienros sebastienros merged commit d5a539f into dotnet:main Jun 3, 2022
@ghost ghost added this to the 7.0-preview6 milestone Jun 3, 2022
@adityamandaleeka
Copy link
Member

Congrats @david-acker! Happy to see this merged.

@david-acker david-acker deleted the request-decompression branch June 12, 2022 17:02
captainsafia pushed a commit to captainsafia/aspnetcore that referenced this pull request Jun 13, 2022
Co-authored-by: Pranav K <prkrishn@hotmail.com>
Co-authored-by: Sébastien Ros <sebastienros@gmail.com>
@sebastienros sebastienros added the blog-candidate Consider mentioning this in the release blog post label Jun 17, 2022
@adityamandaleeka adityamandaleeka added blog-candidate Consider mentioning this in the release blog post and removed blog-candidate Consider mentioning this in the release blog post labels Jun 18, 2022
@ghost
Copy link

ghost commented Jun 18, 2022

@david-acker, this change will be considered for inclusion in the blog post for the release it'll ship in. Nice work!

Please ensure that the original comment in this thread contains a clear explanation of what the change does, why it's important (what problem does it solve?), and, if relevant, include things like code samples and/or performance numbers.

This content may not be exactly what goes into the blog post, but it will help the team putting together the announcement.

Thanks!

@amcasey amcasey added area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlesware and removed area-runtime labels Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlesware blog-candidate Consider mentioning this in the release blog post community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RequestDecompression middleware