Compression: Add additional checks in ShouldCompress

aspnet / BasicMiddleware

[Archived] Basic middleware components for ASP.NET Core. Project moved to https://github.com/aspnet/AspNetCore

Apache License 2.0

169 stars 84 forks source link

Compression: Add additional checks in ShouldCompress #186

Closed spboyer closed 7 years ago

spboyer commented 7 years ago

Needed additional checks in https://github.com/aspnet/BasicMiddleware/blob/dev/src/Microsoft.AspNetCore.ResponseCompression/ResponseCompressionProvider.cs#L107

When 'Cache-Control: no-transform' is present, content should not be gzipped and 'Vary' Response header should not be present

Tratcher commented 7 years ago

Spec reference?

spboyer commented 7 years ago

https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.9.5

JunTaoLuo commented 7 years ago

@spboyer How are you using this feature? Also the cache directive is meant to be used by proxies and caches so it's not clear that it applies to the response compression middleware here.

spboyer commented 7 years ago

Looking at the tests for should compress, even though Kestrel is not a proxy it may receive no-transform and should not modify the resource being requested.

JunTaoLuo commented 7 years ago

@spboyer which tests are you referring to? I still don't see why the no-transform directive, which only applies to proxies and caches, would be significant here. The compression is a middleware on the server and shouldn't be looking for and changing its behaviour based on cache related headers. If the client wants a un-transformed resource, it should specify Accept-Encoding: identity to obtain the untransformed version.

spboyer commented 7 years ago

The "tests" i was referring to is the ShouldCompressResponse and GetCompressionProvider.

Noted the Accept-Encoding: identity should be passed for a non-Encoded. Closing issue.