Skip to content

Support conditional compression #6925

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

Closed
Tratcher opened this issue Jan 22, 2019 · 5 comments
Closed

Support conditional compression #6925

Tratcher opened this issue Jan 22, 2019 · 5 comments
Assignees
Labels
area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares Done This issue has been fixed enhancement This issue represents an ask for new feature or an enhancement to an existing one

Comments

@Tratcher
Copy link
Member

Split from #2458 (comment)

Background:
Response compression is disabled by default for HTTPS requests due to BEAST and CRIME attacks. These attacks are primarily a concern for dynamic content that the user can influence. Since this is not the case for static assets we should have some way for those assets to opt into compression. This is especially true for the StaticFiles middleware, and transitively Blazor apps.

Proposal:
@DamianEdwards: I think the eventual fix here would be to introduce a new request feature that the static file middleware can set when serving files that indicates the response is from static content, thus allowing the compression middleware to safely compress responses only when the feature indicates it was static in nature. Of course it's still possible for another middleware in-between to change the content and introduce issues again. If we care enough about that we could have the feature actually poison the response headers and body (e.g. via wrapping) such that they can't be modified. Also, technically it isn't about what's static but what contains user manipulable content, so the feature should likely be built around that premise instead, allowing any response (including dynamic responses from things like MVC actions) to declare that they're free from user manipulation and thus safe to compress over HTTPS.

Another approach we can take is to use endpoint metadata (if StaticFiles is going to be converted).

@Tratcher Tratcher added area-middleware enhancement This issue represents an ask for new feature or an enhancement to an existing one labels Jan 22, 2019
@Tratcher Tratcher changed the title Support conditional compression for static assets Support conditional compression Jan 22, 2019
@muratg
Copy link
Contributor

muratg commented Jan 22, 2019

cc @danroth27 @shirhatti

@danroth27
Copy link
Member

@muratg Can we get this one assigned and put in a milestone?

@muratg muratg added this to the 3.0.0-preview4 milestone Feb 6, 2019
@muratg
Copy link
Contributor

muratg commented Feb 6, 2019

@Tratcher what's the cost of this?

@Tratcher
Copy link
Member Author

Tratcher commented Feb 6, 2019

Small. There's a little bit of design to work out but the implementation is easy enough. E.g. how does the feature interact with the existing response compression settings like EnableForHttps, MimeTypes, and ExcludedMimeTypes?

I think the feature will have a single enum field with levels like:

  • No compression
  • Use the existing ResponseCompressionOptions
  • Override only EnableForHttps, not MimeTypes and ExcludedMimeTypes. This is what StaticFiles wants.
  • Override only MimeTypes and ExcludedMimeTypes, not EnableForHttps.
  • Override all ResponseCompressionOptions, but not the Accept-Encoding header.
  • Override everything and compress. What compression scheme would you pick for this? The first one?

Not sure we have a scenario for all of these yet (or good names for them), but using an enum gives us some future proofing. We need at least the first 3.

Static files would need a new option to opt-in/out of this behavior. Use the same enum? Make sure it can be overridden in the StaticFiles OnPrepareResponse event.

@shirhatti
Copy link
Contributor

cc @JunTaoLuo

Tratcher added a commit that referenced this issue Mar 6, 2019
Tratcher added a commit that referenced this issue Mar 7, 2019
@Tratcher Tratcher added the Done This issue has been fixed label Mar 7, 2019
@Tratcher Tratcher closed this as completed Mar 7, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Dec 3, 2019
@amcasey amcasey added area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares and removed area-runtime labels Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares Done This issue has been fixed enhancement This issue represents an ask for new feature or an enhancement to an existing one
Projects
None yet
Development

No branches or pull requests

6 participants