-
Notifications
You must be signed in to change notification settings - Fork 28
Add new behaviours to improve Etag compatibility #67
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 new behaviours to improve Etag compatibility #67
Conversation
4b08f0e
to
243191c
Compare
@alexprengere here is a PR to try to help with conditional requests and compression with flask-compress. I did include some small tests to validate basic behaviour work as expected, I would appreciate any feedback on this. I hope this little contribution can help a few guys that were raising issues at least in two approaches while not breaking the default behaviour of the package. |
ae6d185
to
7616d6f
Compare
Thanks a lot! I will take some time to review all this, I will also ask the original authors of the etag-related issues their opinion on this. |
I tried my best trying to make it retro-compatible and being the less intrusive possible but maybe there are possible improvements on top of it! Looking forward for the feedback! |
README.md
Outdated
| `COMPRESS_REGISTER` | Specifies if compression should be automatically registered. | `True` | | ||
| `COMPRESS_ALGORITHM` | Supported compression algorithms. | `['zstd', 'br', 'gzip', 'deflate']` | | ||
| `COMPRESS_STREAMS` | Compress content streams. | `True` | | ||
| `COMPRESS_MUTATE_WEAK_ETAGS` | Compress mutates weak etags. | `True` | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be removed 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch @alexprengere I did remove the reference at the README.md, thanks for your review! Looking forward to start testing this in real world!
0b834bc
to
c1f9ae5
Compare
c1f9ae5
to
51e56b8
Compare
Flask-Compress always mutates the ETag when it's present and compresses a response. That’s reasonable, but it can lead to surprising behavior (see #65): conditional requests (If-None-Match, ranges) are evaluated against a different representation than the one actually sent, because compression and ETag mutation happen after the app or other middleware ran make_conditional.
This PR add one new default behavioir and an opt-in behavior to make things predictable while preserving backwards compatibility.
What’s new
Preserve weak ETags across encodings
The response already has a weak ETag (W/"..."), Flask-Compress will not append :{algorithm}.
Why: per RFC 7232, weak ETags are semantic validators and can be reused across transfer encodings. With Vary: Accept-Encoding in place, caches and clients behave correctly without per-encoding tag mutations.
Evaluate conditionals on the final, compressed representation
Config:
COMPRESS_EVALUATE_CONDITIONAL_REQUEST
(default: False)When True, the extension calls
response.make_conditional(request)
after it sets the compressed body, Content-Encoding, Content-Length, and (if applicable) the ETag.Why: this ensures 304 Not Modified and 206 Partial Content are computed against the bytes actually sent on the wire. It fixes the “mutated-after-the-fact” mismatch described in #65 without forcing apps to add their own “finisher” hook.
Both switches are off by default (or “legacy behaviour” by default), so existing apps won’t break.
Notes
Vary: Accept-Encoding is still enforced.
We use
response.get_etag()
/response.set_etag()
(Werkzeug) instead of string slicing, so weak/strong handling is correct and future-proof.make_conditional()
is gated to safe methods and runs only after the final body and headers are set.Tests
Added unit tests showing:
COMPRESS_EVALUATE_CONDITIONAL_REQUEST=True
, conditional GETs are evaluated on the compressed representation (304 with empty body, no Content-Encoding on 304, ETag echoed).