comply with http caching validation with etag and if-modified-since#2891
comply with http caching validation with etag and if-modified-since#2891Smixi-syn wants to merge 6 commits intoKludex:mainfrom
Conversation
|
lol i just spent a day figuring this problem out as well. i did the same research and came to the same conclusion on how to fix it. i was about to prepare a PR but i found that @Smixi-syn already made an awesome one that even contains tests. thanks @Smixi-syn for the effort! |
starlette/staticfiles.py
Outdated
| try: | ||
| if "if-none-match" in request_headers: | ||
| if_none_match = request_headers["if-none-match"] | ||
| etag = response_headers["etag"] |
There was a problem hiding this comment.
Can the etag not be available here?
There was a problem hiding this comment.
Currently, FileResponse always compute the etag. To be future proof or just to ensure this behavior, I adjusted the logic, and considered that it could be removed (for reasons I don't know, maybe a user would not want to give etags ?) in case it is not present, the method will return False and the entire file will be sent.
starlette/staticfiles.py
Outdated
| else: | ||
| try: | ||
| if_modified_since = parsedate(request_headers["if-modified-since"]) | ||
| last_modified = parsedate(response_headers["last-modified"]) | ||
| if if_modified_since is not None and last_modified is not None and if_modified_since >= last_modified: | ||
| return True | ||
| except KeyError: | ||
| pass |
There was a problem hiding this comment.
I would prefer to avoid an unnecessary diff.
| else: | |
| try: | |
| if_modified_since = parsedate(request_headers["if-modified-since"]) | |
| last_modified = parsedate(response_headers["last-modified"]) | |
| if if_modified_since is not None and last_modified is not None and if_modified_since >= last_modified: | |
| return True | |
| except KeyError: | |
| pass | |
| try: | |
| if_modified_since = parsedate(request_headers["if-modified-since"]) | |
| last_modified = parsedate(response_headers["last-modified"]) | |
| if if_modified_since is not None and last_modified is not None and if_modified_since >= last_modified: | |
| return True | |
| except KeyError: | |
| pass |
There was a problem hiding this comment.
I adjusted the code so the when If-None-Match is present it returns early.
|
I also added a test func for the edge case of the missing etag header in response, and fixed linting rules. |
3 participants
Summary
Change the caching validation precedence to comply with HTTP RFC: https://datatracker.ietf.org/doc/html/rfc7232#section-6
The issue was that it must not check the "If-Modified-Since" header when the "If-None-Match" is set. It prioritize file content for caching instead of time based caching (I encountered this when timestamp on file are incorrect for example).
Checklist