-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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 support for Last-Modified and If-Modified-Since headers #1376
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
use std::io; | ||
use std::{fs, io}; | ||
use std::path::{Path, PathBuf}; | ||
use std::ops::{Deref, DerefMut}; | ||
|
||
|
@@ -7,6 +7,17 @@ use tokio::fs::File; | |
use crate::request::Request; | ||
use crate::response::{self, Responder}; | ||
use crate::http::ContentType; | ||
use std::time::SystemTime; | ||
use time::{PrimitiveDateTime, OffsetDateTime}; | ||
|
||
// The spec for this header/format is described here | ||
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Last-Modified#Syntax | ||
// Last-Modified: <day-name>, <day> <month> <year> <hour>:<minute>:<second> GMT | ||
// Example: Last-Modified: Sat, 03 Oct 2015 21:28:00 GMT | ||
|
||
// Format codes can be found here. They are not compatible with libc strftime. | ||
// https://docs.rs/time/0.2.16/src/time/lib.rs.html#94-127 | ||
const TIME_FORMAT: &'static str = "%a, %d %b %Y %H:%M:%S GMT"; | ||
|
||
/// A file with an associated name; responds with the Content-Type based on the | ||
/// file extension. | ||
|
@@ -84,16 +95,42 @@ impl NamedFile { | |
/// the response according to the file's extension if the extension is | ||
/// recognized. See [`ContentType::from_extension()`] for more information. If | ||
/// you would like to stream a file with a different Content-Type than that | ||
/// implied by its extension, use a [`File`] directly. | ||
/// implied by its extension, use a [`File`] directly. On supported platforms, this | ||
/// will set or override the Last-Modified and header in the response according to the | ||
/// file's modified date on disk and respond with 304 Not Modified to conditional | ||
/// requests. See [`fs::Metadata::modified`] for more information on supported | ||
/// platforms. | ||
impl<'r> Responder<'r, 'static> for NamedFile { | ||
fn respond_to(self, req: &'r Request<'_>) -> response::Result<'static> { | ||
// using the file handle `self.1` is too much trouble here, tokio's `File::metadata` method | ||
// is async, so is into_std, and try_into_std gives you a `io::Result<std::fs::File>` | ||
let file_mod_time = fs::metadata(self.0.as_path()).ok() | ||
.and_then(|metadata| metadata.modified().ok()) | ||
.and_then(|modified| modified.duration_since(SystemTime::UNIX_EPOCH).ok()) | ||
.map(|duration| OffsetDateTime::from_unix_timestamp(duration.as_secs() as i64)); | ||
|
||
// if file_mod_time is < this date, we don't need to re-send the file. | ||
let cutoff = req.headers().get_one("If-Modified-Since") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should ensure the method was |
||
.and_then(|header| PrimitiveDateTime::parse(header, TIME_FORMAT).ok()) | ||
.map(|dt| dt.assume_utc()); | ||
|
||
if let Some((date, cutoff)) = file_mod_time.zip(cutoff) { | ||
if date <= cutoff { | ||
return response::status::NotModified.respond_to(req); | ||
} | ||
} | ||
|
||
let mut response = self.1.respond_to(req)?; | ||
if let Some(ext) = self.0.extension() { | ||
if let Some(ct) = ContentType::from_extension(&ext.to_string_lossy()) { | ||
response.set_header(ct); | ||
} | ||
} | ||
|
||
if let Some(time) = file_mod_time { | ||
response.set_raw_header("Last-Modified", time.format(TIME_FORMAT)); | ||
} | ||
|
||
Ok(response) | ||
} | ||
} | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
We can't safely use
std::fs::metadata
even in this function, since it's blocking. The problem ofrespond_to
beingasync
or non-async
came up a bit, and we had resolved it as "async
work should happen in anasync
constructor" (in this caseNamedFile::open
).This does mean that we would need to query the modification time even if there is no
If-Modified-Since
header to compare to; I'm not sure how big of a problem that is in terms of performance.