GZipFilter: do not compress if Cache-Control: no-transform by tsawada · Pull Request #12980 · playframework/playframework
Pull Request Checklist
- Have you read How to write the perfect pull request?
- Have you read through the contributor guidelines?
- Have you referenced any issues you're fixing using commit message keywords?
- Have you added copyright headers to new files?
- Have you checked that both Scala and Java APIs are updated?
- Have you updated the documentation for both Scala and Java sections?
- Have you added tests for any changed functionality?
Purpose
I believe this improves adherence to the HTTP standards.
Ref #12981
I believe this improves adherence to the HTTP standards.
I don't agree here. The RFCs talk about Cache-Control: no-transform only in regards of intermediaries (such as proxies or CDNs), that they must not modify the response body when no-transform is present (The link in your issue #12981 is about Google's CDN). The RFCs say nothing about the origin (here Play server). So my understanding is, only proxies and CDNs are bound to the no-transform value.
My understanding is, that a proxy/cdn is basically allowed to transform the content if no-transform is not set, no matter if the content is originally e.g. gzipped or not (e.g. a proxy could decide to change the compression of an image, meaning an image already was sent compressed to the proxy, but the proxy decides to compress it even more).
I think it's totally fine, or at least the RFC does not say it's not allowed, that an origin server sends a gzipped response that also has the no-transform header. no-transform just tells a proxy that processes the reponse to not transform the content.
Following RFCs are relevant:
https://datatracker.ietf.org/doc/html/rfc7234#section-5.2.2.4
5.2.2.4. no-transform
The "no-transform" response directive indicates that an intermediary
(regardless of whether it implements a cache) MUST NOT transform the
payload, as defined in Section 5.7.2 of [RFC7230].
and
https://datatracker.ietf.org/doc/html/rfc9110#section-7.7
A proxy MUST NOT transform the content (Section 6.4) of a response message that contains a no-transform cache directive (Section 5.2.2.6 of [CACHING]). Note that this does not apply to message transformations that do not change the content, such as the addition or removal of transfer codings (Section 7 of [HTTP/1.1]). A proxy MAY transform the content of a message that does not contain a no-transform cache directive. A proxy that transforms the content of a 200 (OK) response can inform downstream recipients that a transformation has been applied by changing the response status code to 203 (Non-Authoritative Information) (Section 15.3.4).
Given this is just about proxies, I am pretty sure we should not merge this PR as it just makes the assumption that it is not allowed to gzip a response when the no-transform header is present. But that is just an assumption not a fact by the RFCs.
@tsawada If you have a use case where you do not want to gzip a response that has the no-transform header set, you can just use the shouldGzip method described in the gzipfilter docs and check if the response has the no-transform header set or not.
mkurz
mentioned this pull request
Thanks for reviewing the PR and for explaining how to customize GZipFilter behavior!
I agree that these standards are written for CDNs and proxies rather than for origin servers. From the client's perspective, the entire Play server functions as an origin server, and in that context, the GZipFilter can be considered part of the origin server.
However, as a backend developer using Play as a library to write Actions, I view GZipFilter (and other filters provided by Play) as being quite similar to a CDN or proxy.
In my opinion, when an Action returns a Result with Cache-Control: no-transform, it is likely that the author intended for the result's content to be delivered to the client without alteration. Whether the component in question is a composite Action, a filter, a CDN, or a proxy, every element between the origin Action and the client is an intermediary and should respect the directives specified in the Cache-Control header.
@tsawada I fully get your point from a dev's perspective. So in the end this boils down on what we think devs most likely want to express by adding the Cache-Control: no-transform to a response, how Play filters should behave then.
(btw, currently it seems only the gzip filter is modiying the content of a response anyway: https://github.com/playframework/playframework/tree/3.1.0-M1/web/play-filters-helpers/src/main/scala/play/filters)
So, I was thinking about this... how about we solve this a bit more general and make it configurable in the gzip config here?
play.filters {
gzip {
# todo: document (e.g. if a response header has one of following values, the gzip filter will not be applied)
skipFilterForResponseHeaders {
Cache-Control = ["no-transform"]
#Some-other-header = ["foo", "bar"]
}
}
}
So the implementation would look for all the keys of skipFilterForResponseHeaders.* and if any of the header->value pair is present in the actual response header the gzip filter would be skipped.
For the above example
...
Cache-Control = ["no-transform"] # Skip if response has header `Cache-Control: no-transform`
Some-other-header = ["foo", "bar"] # Skip if response has header `Some-other-header: foo` OR if response has header `Some-other-header: bar`
...
Of course we should document this in https://www.playframework.com/documentation/3.0.x/GzipEncoding
This would be bit more future proof and more customizable for devs IMHO
What do you think? Or maybe you have a better idea - let me know 👍
My idea would also cover if you want to add Vary: Accept-Encoding to that config like you said in #12981
Thanks!
I personally don't need more flexibility than just ignoring Cache-Control with no-transform and maybe Vary with Accept-Encoding, but I think what you proposed sound simple and is flexible enough to cover all my use cases 👍
Please let me know if you want me to modify this PR in that approach, or if it is easier for you to just leave it to you.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters