Support returning Not Modified responses in FileResponse #2955
Conversation
paulo-raca
force-pushed
the
FileResponseNotModified
branch
from
d6d6c2b to
63b8950
Compare
paulo-raca
force-pushed
the
FileResponseNotModified
branch
from
63b8950 to
8b9e306
Compare
|
@abersheeran I know you've played a lot with this. Does this PR make sense? |
| "Not Modified" response could be returned instead. | ||
| """ | ||
| if http_if_none_match is not None: | ||
| match = [tag.strip(" W/") for tag in http_if_none_match.split(",")] |
There was a problem hiding this comment.
There are some issues here; the strong and weak semantics of etag should not be handled this way.
There was a problem hiding this comment.
W/ (case-sensitive) indicates that a weak validator is used. Weak ETags are easy to generate, but are far less useful for comparisons. Strong validators are ideal for comparisons but can be very difficult to generate efficiently. Weak ETag values of two representations of the same resources might be semantically equivalent, but not byte-for-byte identical. This means weak ETags prevent caching when byte range requests are used, but strong ETags mean range requests can still be cached.
https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/ETag
When requesting a range, different handling is required. However, this difference has been ignored here.
|
Hello, is this related to #2891 ? |
4 participants
Summary
Looking at the souce for StaticFiles, it verifies the timestamp/etag and returns
HTTP 304 - Not Modifiedwhen appropriate.IMHO it would be simpler / more reusable if this was part of FileResponse.
(I brought this up a few hours ago in #2954, and decided to give it a try)
I haven't written new tests: There already is a ton of them and there is good coverage.
Checklist