feat: implements remaining http cache conditionals #407
Conversation
| @@ -1648,11 +1646,11 @@ async fn zip_dir<W: AsyncWrite + Unpin>( | |||
| Ok(()) | |||
| } | |||
|
|
|||
| fn extract_cache_headers(meta: &Metadata) -> Option<(Option<ETag>, LastModified)> { | |||
| fn extract_cache_headers(meta: &Metadata) -> Option<(ETag, LastModified)> { | |||
There was a problem hiding this comment.
ETag and Last-Modified can be used separately.
We should refactor the extract_cache_headers function as follows:
fn extract_cache_headers(meta: &Metadata) -> (Option<ETag>, Option<LastModified>)
There was a problem hiding this comment.
In the second commit yesterday, I removed this code change again. There is actually no reason to separate Etag and LastModified as those both are computed by the existing file meta data information.
ETag can fail, if the file's timestamp isn't present. In this case, LastModified fails too.
Etag can also fail, if the .parse method fails when the string is ill-formated [1]. But the string is almost static except when there is no valid timestamp, in this case we fail before. (size doesn't seem to have a failure mode at seems to be guaranteed)
That was my reasoning to revert the change on this function, as it seemed unnecessary.
But it might be safer to have separate checks.
I am unsure which path to take. Going with the rolled back changes or do you want the 2 options and checks if etag could be parsed?
[1] headers crate etag parse: https://doc.rust-lang.org/nightly/core/str/trait.FromStr.html#tymethod.from_str
There was a problem hiding this comment.
fn extract_cache_headers(meta: &Metadata) -> Option<(ETag, LastModified)> is ok.
sigoden
changed the title
Hi,
this PR fixes/implements some of the HTTP conditionals like "if-modified":
This PR implements/fixes those conditionals and also adds some testcases for all 4.
I hope you like it.
For reference, MDN Documentation: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/If-Unmodified-Since
edit: clarified "if-modified-since". I misread the code.