RequestDecompression middleware

[sebastienros]

The same way we have a ResponseCompression middleware we should have a RequestDecompression one that automatically decompress the requests containing Content-Encoding with supported formats (gzip, deflate, brotli, ...). Based on the same extensible compression factories.

Scenario: a client sends files to a server using gzip compressed streams. The developer of the server application doesn't have to care about how to decrompress the body.

[Tratcher]

Prior ask: #30544

[ghost]

Thanks for contacting us.

We're moving this issue to the .NET 7 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

[david-acker]

I'd be interested in helping out with this. This should follow the same pattern that the ResponseCompression middleware uses, correct? (i.e., an IDecompressionProvider interface that's implemented by the -DecompressionProvider classes for each of the supported formats)

[sebastienros]

@david-acker I think it should do the same thing, yes. Go for it! Since nobody would do it for now. I would use it then instead of doing it my controllers: https://github.com/dotnet/crank/blob/main/src/Microsoft.Crank.Agent/Controllers/JobsController.cs#L389

[alexanderkozlenko]

I would be glad to offer my help as well if needed: I've been using my own middleware implementation for a few years:

• GitHub: https://github.com/alexanderkozlenko/aspnetcore-request-decompression
• NuGet: https://www.nuget.org/packages/Anemonis.AspNetCore.RequestDecompression

[david-acker]

@sebastienros From a practical perspective, does it make sense to support decompressing requests that have multiple Content-Encodings applied (e.g. compressed with Brotli and then GZip)? I'd be happy to add this if you think it's worthwhile, but I just wanted to confirm first.

I'll put a draft PR with the working implementation soon so I can get some feedback.

[Tratcher]

From a practical perspective, does it make sense to support decompressing requests that have multiple Content-Encodings applied (e.g. compressed with Brotli and then GZip)? I'd be happy to add this if you think it's worthwhile, but I just wanted to confirm first.

Is that something people actually do? They'd get minimal returns on the second pass.

We don't support this on response compression. I wouldn't worry about it for the first implementation.

[david-acker]

That was my thought as well. I saw this mentioned in RFC 2616 and a few other places online (not with any examples of actual implementations though), but I figured I'd ask just to be sure.

[pranavkm]

API Review:

namespace Microsoft.AspNetCore.Http.Metadata;
+ public interface IRequestSizeLimitMetadata
+ {
+     public long MaxRequestBodySize { get; }
+ }

+ namespace Microsoft.AspNetCore.RequestDecompression;
+ public interface IDecompressionProvider
+ {
+     Stream GetDecompressionStream(Stream stream);
+ }

+ public interface IRequestDecompressionProvider
+ {
+     IDecompressionProvider? GetDecompressionProvider(HttpContext context);
+ }

+ public sealed class RequestDecompressionOptions
+ {
+     public IDictionary<string, IDecompressionProvider> DecompressionProviders { get; }
+ }
 
namespace Microsoft.AspNetCore.Builder;

+ public static class RequestDecompressionBuilderExtensions
+ {
+     public static IApplicationBuilder UseRequestDecompression(this IApplicationBuilder builder);
+ }

namespace Microsoft.Extensions.DependencyInjection;

+ public static class RequestDecompressionServiceExtensions
+ {
+     public static IServiceCollection AddRequestDecompression(this IServiceCollection services);
+     public static IServiceCollection AddRequestDecompression(this IServiceCollection services, Action<RequestDecompressionOptions> configureOptions);
+ }

namespace Microsoft.AspNetCore.Mvc;

- public class RequestSizeLimitAttribute : Attribute, IFilterFactory, IOrderedFilter
+ public class RequestSizeLimitAttribute : Attribute, IFilterFactory, IOrderedFilter, IRequestSizeLimitMetadata

[ghost]

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

• The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
• The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
• Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

[pranavkm]

API review:

Approved. Update the IRequestDecompressionProvider to return the Stream instead of the provider:

+ public interface IRequestDecompressionProvider
+ {
+     Stream? GetDecompressionStream(HttpContext context);
+ }

[halter73]

We should also support the DisableRequestSizeLimitAttribute if we're going to support RequestSizeLimitAttribute. This requires updating IRequestSizeLimitMetadata.MaxRequestBodySize to be nullable. Here are my proposed updates to the already-approved API:

namespace Microsoft.AspNetCore.Http.Metadata;

public interface IRequestSizeLimitMetadata
{
-    public long MaxRequestBodySize { get; }
+    public long? MaxRequestBodySize { get; }
}

namespace Microsoft.AspNetCore.Mvc;

- public class DisableRequestSizeLimitAttribute : Attribute, IFilterFactory, IOrderedFilter
+ public class DisableRequestSizeLimitAttribute : Attribute, IFilterFactory, IOrderedFilter, IRequestSizeLimitMetadata

Context: #40279 (comment)

[ghost]

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

• The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
• The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
• Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

[halter73]

API Review:

We think this is filling a gap. People would be surprised if the RequestSizeLimitAttribute works but DisableRequestSizeLimitAttribute doesn't. Approved.

namespace Microsoft.AspNetCore.Http.Metadata;

public interface IRequestSizeLimitMetadata
{
-    public long MaxRequestBodySize { get; }
+    public long? MaxRequestBodySize { get; }
}

namespace Microsoft.AspNetCore.Mvc;

- public class DisableRequestSizeLimitAttribute : Attribute, IFilterFactory, IOrderedFilter
+ public class DisableRequestSizeLimitAttribute : Attribute, IFilterFactory, IOrderedFilter, IRequestSizeLimitMetadata

We should update our docs to make it clear that disabling request size limits can be a security concern, particularly if the request body is being buffered. @david-acker Do you mind updating the doc comments for DisableRequestSizeLimitAttribute with a remark to this effect in your PR when you add the IRequestSizeLimitMetadata implementation to DisableRequestSizeLimitAttribute?

[david-acker]

I've added the following as a remark to the doc comments for DisableRequestSizeLimitAttribute:

Disabling the request body size limit can be a security concern in regards to uncontrolled resource consumption, particularly if the request body is being buffered.

Let me know if there's anything that you'd like me to add or change.

[danyltsiv]

this would be extremely useful!