-
Notifications
You must be signed in to change notification settings - Fork 27
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
flask.Response.make_conditional fails to return 304 due to Etag mismatch #24
Comments
Just came across this myself. I'm curious if altering the etag in flask-compress is even necessary, since it's adding edit: just saw #15 In the interim, I've added a
This would cause issues if you're using
Of course, this is just to get around #15 and restore conditional requests. I'm not clear if etags do need to be unique for different encodings and their ramifications-- potential range request disasters if the edit: https://datatracker.ietf.org/doc/html/rfc7232#section-2.3.3 So I suppose flask-compress may want to call
Basically, all conditional request scenarios need careful consideration. for reference: https://stackoverflow.com/questions/33947562/is-it-possible-to-send-http-response-using-gzip-and-byte-ranges-at-the-same-time |
@alexprengere I've done a little more research on this. I've seen etags with the encoding appended, like flask-compress is currently doing. As well, it's common to just weaken the etag (cloudflare does this). If we use this approach instead, flask/werkzeug's Something like:
As far as range requests, it looks like it's common to simply not support compression in this scenario. We could just check for a
I did see some interesting behavior testing range requests with cloudflare. CF will accept gzip encoding for upstream requests, and will then happily serve a gzipped range response (with an unweakened etag). But for any other encodings (no accept-encoding, or br or whatever else), it will fall back to identity and weaken the etag. This obviously can still lead to major issues, if say a HEAD request is made (with accept-encoding) to determine the content-length to subsequently make a range request.. But realistically, such responses should be excluded from compression by the user anyway (excluded mimetypes, etc), so it's mostly a non-issue. Either way, range requests are performed on the uncompressed data which can lead to all sorts of surprises. |
Thanks a lot for this. Would you be able to work on a PR I can review? |
I have the similar issue, I am using flask/werkzeug for serving static files. When browser requests some file (main.js for example) But when browser pass this value in Actually, in my opinion it is not quite P.S.: I also do not understand how weakeaning the validation can help with resolving this issue. |
Apologies for the late response. Since I'm not using It works because
https://datatracker.ietf.org/doc/html/rfc7232#section-2.3.2 Weakening etags when compressing is fairly common, at least for downstream servers. E.g. https://developers.cloudflare.com/cache/reference/etag-headers/#behavior-with-respect-strong-etags-disabled However, this approach does not work for As for the mid-air collision example, this would be implemented outside of flask's native If strong etags for compressed content is desired, flask-compress would need to hook into one of flask/werkzeug's functions: I think the simplest solution would be to:
Actually, in the last scenario, the etag can have the compression algorithm tacked on like it is currently. The user will just need to handle it accordingly when doing any custom conditional checks. |
This is more a need for technique than a library failure. Due to the necessary changes to modify Etags when returning compressed content, the Etags don't match when calling make_conditional
So, how would one get flask to acknowledge a matching etag on compressed content? Should I intercept and strip the trailing :gzip off the normally immutable request headers?
I'm at a loss. Any recommendations? Thanks in advance.
The text was updated successfully, but these errors were encountered: