-
-
Notifications
You must be signed in to change notification settings - Fork 70
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
ETag / 304 unmodified support? #173
Comments
Yes, definitely a nice to have feature. |
I'm not a rustacean, but I may give it a go for fun next weekend; this source code looks pretty clean & self explanatory; if you have any thoughts or advice on direction/libs to utilize for high-perf file hashing (to generate an etag) or an algorithm, maybe post it here. |
Sure, the code in general is easy to jump-in, so don't hesitate to give it a try.
I found interesting this Actix file content, so you could have it a look at https://github.com/actix/actix-web/blob/master/actix-files/src/named.rs#L378 Just let me know here if you need any other assistance on your SWS journey. |
I looked into it, I think it would honestly be very little effort to put in defaults just for the common use case of "bundled app with static assets that have cache-busting names", but actually putting in a real etag (not weak etag - with the W/ prefix) could be a lot of work. The thing that lead me to rethink my decision to plop in a quick-and-easy new setting, was I think adding etag on top of what you have today would just add technical debt on top of the hardcoded cache preferences that are already in the program. Probably prior to etag support comes, the program will need more robust cache configuration options |
What exactly could be a lot of work in your opinion? Do you mean an algo to generate/validate strong etags efficiently?
The current
I think that the If you have ideas about how to improve the cache mechanism of SWS, please feel free to share them. |
Just to let know here that I started to work on a module to compute/check strong Etags via the |
Turned out that it got more tricky than I expected. You can find more context and progress on this thread https://users.rust-lang.org/t/how-to-compute-a-byte-for-byte-etag-using-tokio/92481 But besides, until exploring So I will leave it like this for now until I find time again to probably come back to the topic. |
It is probably possible to generate a weak ETag without having to read the whole body of a file. It can be done, for example, this way (just adding a patch on top of 8c6ab53 as it's not worth a PR): diff --git a/src/static_files.rs b/src/static_files.rs
index 6165696..df726e3 100644
--- a/src/static_files.rs
+++ b/src/static_files.rs
@@ -597,9 +597,10 @@ async fn response_body(
conditionals: Conditionals,
) -> Result<Response<Body>, StatusCode> {
let mut len = meta.len();
- let modified = meta.modified().ok().map(LastModified::from);
+ let modified = meta.modified().ok();
+ let last_modified_header = modified.map(LastModified::from);
- match conditionals.check(modified) {
+ match conditionals.check(last_modified_header) {
Cond::NoBody(resp) => Ok(resp),
Cond::WithBody(range) => {
let buf_size = optimal_buf_size(meta);
@@ -645,9 +646,12 @@ async fn response_body(
resp.headers_mut().typed_insert(ContentType::from(mime));
resp.headers_mut().typed_insert(AcceptRanges::bytes());
- if let Some(last_modified) = modified {
+ if let Some(last_modified) = last_modified_header {
resp.headers_mut().typed_insert(last_modified);
}
+ if let Some(last_modified) = modified {
+ insert_etag(&mut resp, last_modified);
+ }
Ok(resp)
})
@@ -663,6 +667,19 @@ async fn response_body(
}
}
+/// Inserts the ETag header based on the provided modified time. The value adeed is a weak ETag
+/// that also contains a fixed prefix `LMT` (Last Modified Time) to allow for future changes of the
+/// format of the ETag.
+///
+/// An example value might look like: `W/"LMT1705668081589469821"`.
+fn insert_etag(resp: &mut Response<Body>, modified: SystemTime) {
+ if let Ok(elapsed) = modified.duration_since(std::time::UNIX_EPOCH) {
+ let etag = format!(r#"W/"LMT{}""#, elapsed.as_nanos());
+ resp.headers_mut()
+ .typed_insert(headers::ETag::from_str(&etag).unwrap());
+ }
+}
+
struct BadRange;
fn bytes_range(range: Option<Range>, max_len: u64) -> Result<(u64, u64), BadRange> {
diff --git a/tests/compression_static.rs b/tests/compression_static.rs
index f614c6c..e8b43a0 100644
--- a/tests/compression_static.rs
+++ b/tests/compression_static.rs
@@ -66,6 +66,14 @@ async fn compression_static_file_exists() {
assert_eq!(headers["content-encoding"], "gzip");
assert_eq!(headers["accept-ranges"], "bytes");
assert!(!headers["last-modified"].is_empty());
+ {
+ let etag = headers["etag"].to_str().unwrap_or("");
+ assert!(
+ etag.starts_with(r#"W/MMS""#),
+ "should contain weak etag in {}",
+ etag
+ );
+ }
assert_eq!(
&headers["content-type"], "text/html",
"content-type is not html"
We would still need to extend the But the biggest problem with the ETag is that the re-validation will never be requested for at least the So if the objective is to be able to get a fresh resource when it changes, then just making use of an I'd suggest that this complexity might be hard to justify.
|
@dnagir as I said before. We can adopt a weak Etag using a combination of modified time and file size similar to your patch. However, I should be also in favor of adjusting the PR welcome. |
Note that a weak Etag has performance advantages which is likely why nginx uses it ( |
It seems like basically everything right now every html page seems to come back with cache control max age of 86400, if you deploy a new version of your app, most bundlers will hash asset names to cachebust other than your HTML files.
Describe the solution you'd like
Support etags, so people can configure HTML files and other non-hashed files to send not modified responses.
Describe alternatives you've considered
Using a different static web server
The text was updated successfully, but these errors were encountered: