From 4584cfe3c18820d8235628cdf629655abd601bc7 Mon Sep 17 00:00:00 2001 From: Jake Howard Date: Tue, 28 Dec 2021 16:24:42 +0000 Subject: [PATCH 1/4] Additionally set expires header when caching responses Browsers are rather smart, but also dumb. This uses the `Expires` header alongside `cache-control` to better prompt the browser to actually cache. Unfortunately, firefox still tries to "race" its own cache, in an attempt to respond to requests faster, so still ends up making a bunch of requests which could have been cached. Doesn't appear there's any way around this. --- Cargo.lock | 1 + Cargo.toml | 1 + src/api/icons.rs | 9 ++++++-- src/api/web.rs | 53 +++++++++++++++++++++++++----------------------- src/util.rs | 51 ++++++++++++++++++++++++++++++++++------------ 5 files changed, 75 insertions(+), 40 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8af4f0ea..1a048db5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3295,6 +3295,7 @@ dependencies = [ "governor", "handlebars", "html5ever", + "httpdate", "idna 0.2.3", "job_scheduler", "jsonwebtoken", diff --git a/Cargo.toml b/Cargo.toml index f6ff7f68..82c96386 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -80,6 +80,7 @@ uuid = { version = "0.8.2", features = ["v4"] } chrono = { version = "0.4.19", features = ["serde"] } chrono-tz = "0.6.1" time = "0.2.27" +httpdate = "1.0" # Job scheduler job_scheduler = "1.2.1" diff --git a/src/api/icons.rs b/src/api/icons.rs index 8d87b10a..3d1de094 100644 --- a/src/api/icons.rs +++ b/src/api/icons.rs @@ -103,14 +103,19 @@ fn icon_internal(domain: String) -> Cached>> { return Cached::ttl( Content(ContentType::new("image", "png"), FALLBACK_ICON.to_vec()), CONFIG.icon_cache_negttl(), + true, ); } match get_icon(&domain) { Some((icon, icon_type)) => { - Cached::ttl(Content(ContentType::new("image", icon_type), icon), CONFIG.icon_cache_ttl()) + Cached::ttl(Content(ContentType::new("image", icon_type), icon), CONFIG.icon_cache_ttl(), true) } - _ => Cached::ttl(Content(ContentType::new("image", "png"), FALLBACK_ICON.to_vec()), CONFIG.icon_cache_negttl()), + _ => Cached::ttl( + Content(ContentType::new("image", "png"), FALLBACK_ICON.to_vec()), + CONFIG.icon_cache_negttl(), + true, + ), } } diff --git a/src/api/web.rs b/src/api/web.rs index 9c960c27..154dc2cf 100644 --- a/src/api/web.rs +++ b/src/api/web.rs @@ -22,41 +22,44 @@ pub fn routes() -> Vec { #[get("/")] fn web_index() -> Cached> { - Cached::short(NamedFile::open(Path::new(&CONFIG.web_vault_folder()).join("index.html")).ok()) + Cached::short(NamedFile::open(Path::new(&CONFIG.web_vault_folder()).join("index.html")).ok(), false) } #[get("/app-id.json")] fn app_id() -> Cached>> { let content_type = ContentType::new("application", "fido.trusted-apps+json"); - Cached::long(Content( - content_type, - Json(json!({ - "trustedFacets": [ - { - "version": { "major": 1, "minor": 0 }, - "ids": [ - // Per : - // - // "In the Web case, the FacetID MUST be the Web Origin [RFC6454] - // of the web page triggering the FIDO operation, written as - // a URI with an empty path. Default ports are omitted and any - // path component is ignored." - // - // This leaves it unclear as to whether the path must be empty, - // or whether it can be non-empty and will be ignored. To be on - // the safe side, use a proper web origin (with empty path). - &CONFIG.domain_origin(), - "ios:bundle-id:com.8bit.bitwarden", - "android:apk-key-hash:dUGFzUzf3lmHSLBDBIv+WaFyZMI" ] - }] - })), - )) + Cached::long( + Content( + content_type, + Json(json!({ + "trustedFacets": [ + { + "version": { "major": 1, "minor": 0 }, + "ids": [ + // Per : + // + // "In the Web case, the FacetID MUST be the Web Origin [RFC6454] + // of the web page triggering the FIDO operation, written as + // a URI with an empty path. Default ports are omitted and any + // path component is ignored." + // + // This leaves it unclear as to whether the path must be empty, + // or whether it can be non-empty and will be ignored. To be on + // the safe side, use a proper web origin (with empty path). + &CONFIG.domain_origin(), + "ios:bundle-id:com.8bit.bitwarden", + "android:apk-key-hash:dUGFzUzf3lmHSLBDBIv+WaFyZMI" ] + }] + })), + ), + true, + ) } #[get("/", rank = 10)] // Only match this if the other routes don't match fn web_files(p: PathBuf) -> Cached> { - Cached::long(NamedFile::open(Path::new(&CONFIG.web_vault_folder()).join(p)).ok()) + Cached::long(NamedFile::open(Path::new(&CONFIG.web_vault_folder()).join(p)).ok(), true) } #[get("/attachments//")] diff --git a/src/util.rs b/src/util.rs index 2e47077b..aacdd868 100644 --- a/src/util.rs +++ b/src/util.rs @@ -11,6 +11,10 @@ use rocket::{ Data, Request, Response, Rocket, }; +use httpdate::HttpDate; +use std::thread::sleep; +use std::time::{Duration, SystemTime}; + use crate::CONFIG; pub struct AppHeaders(); @@ -99,29 +103,52 @@ impl Fairing for Cors { } } -pub struct Cached(R, String); +pub struct Cached { + response: R, + is_immutable: bool, + ttl: u64, +} impl Cached { - pub fn long(r: R) -> Cached { - // 7 days - Self::ttl(r, 604800) + pub fn long(response: R, is_immutable: bool) -> Cached { + Self { + response, + is_immutable, + ttl: 604800, // 7 days + } } - pub fn short(r: R) -> Cached { - // 10 minutes - Self(r, String::from("public, max-age=600")) + pub fn short(response: R, is_immutable: bool) -> Cached { + Self { + response, + is_immutable, + ttl: 600, // 10 minutes + } } - pub fn ttl(r: R, ttl: u64) -> Cached { - Self(r, format!("public, immutable, max-age={}", ttl)) + pub fn ttl(response: R, ttl: u64, is_immutable: bool) -> Cached { + Self { + response, + is_immutable, + ttl: ttl, + } } } impl<'r, R: Responder<'r>> Responder<'r> for Cached { fn respond_to(self, req: &Request) -> response::Result<'r> { - match self.0.respond_to(req) { + let cache_control_header = if self.is_immutable { + format!("public, immutable, max-age={}", self.ttl) + } else { + format!("public, max-age={}", self.ttl) + }; + + let time_now = SystemTime::now(); + + match self.response.respond_to(req) { Ok(mut res) => { - res.set_raw_header("Cache-Control", self.1); + res.set_raw_header("Cache-Control", cache_control_header); + res.set_raw_header("Expires", HttpDate::from(time_now + Duration::from_secs(self.ttl)).to_string()); Ok(res) } e @ Err(_) => e, @@ -551,8 +578,6 @@ where } } -use std::{thread::sleep, time::Duration}; - pub fn retry_db(func: F, max_tries: u32) -> Result where F: Fn() -> Result, From 248e7dabc2e389ac67e77c1c6078259c29b7f997 Mon Sep 17 00:00:00 2001 From: Jake Howard Date: Tue, 28 Dec 2021 21:54:09 +0000 Subject: [PATCH 2/4] Collapse field name definition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Daniel García --- src/util.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util.rs b/src/util.rs index aacdd868..1d70e097 100644 --- a/src/util.rs +++ b/src/util.rs @@ -130,7 +130,7 @@ impl Cached { Self { response, is_immutable, - ttl: ttl, + ttl, } } } From 690d0ed1bb3c9d3b22e3b50aebbcd4bb3b3764d0 Mon Sep 17 00:00:00 2001 From: Jake Howard Date: Wed, 29 Dec 2021 16:17:38 +0000 Subject: [PATCH 3/4] Add our own HTTP date formatter --- Cargo.lock | 1 - Cargo.toml | 1 - src/util.rs | 19 +++++++++++++++---- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1a048db5..8af4f0ea 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3295,7 +3295,6 @@ dependencies = [ "governor", "handlebars", "html5ever", - "httpdate", "idna 0.2.3", "job_scheduler", "jsonwebtoken", diff --git a/Cargo.toml b/Cargo.toml index 82c96386..f6ff7f68 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -80,7 +80,6 @@ uuid = { version = "0.8.2", features = ["v4"] } chrono = { version = "0.4.19", features = ["serde"] } chrono-tz = "0.6.1" time = "0.2.27" -httpdate = "1.0" # Job scheduler job_scheduler = "1.2.1" diff --git a/src/util.rs b/src/util.rs index 1d70e097..0b287ccf 100644 --- a/src/util.rs +++ b/src/util.rs @@ -11,9 +11,8 @@ use rocket::{ Data, Request, Response, Rocket, }; -use httpdate::HttpDate; use std::thread::sleep; -use std::time::{Duration, SystemTime}; +use std::time::Duration; use crate::CONFIG; @@ -143,12 +142,13 @@ impl<'r, R: Responder<'r>> Responder<'r> for Cached { format!("public, max-age={}", self.ttl) }; - let time_now = SystemTime::now(); + let time_now = chrono::Local::now(); match self.response.respond_to(req) { Ok(mut res) => { res.set_raw_header("Cache-Control", cache_control_header); - res.set_raw_header("Expires", HttpDate::from(time_now + Duration::from_secs(self.ttl)).to_string()); + let expiry_time = time_now + chrono::Duration::seconds(self.ttl.try_into().unwrap()); + res.set_raw_header("Expires", format_datetime_http(&expiry_time)); Ok(res) } e @ Err(_) => e, @@ -436,6 +436,17 @@ pub fn format_naive_datetime_local(dt: &NaiveDateTime, fmt: &str) -> String { format_datetime_local(&Local.from_utc_datetime(dt), fmt) } +/// Formats a `DateTime` as required for HTTP +/// +/// https://httpwg.org/specs/rfc7231.html#http.date +pub fn format_datetime_http(dt: &DateTime) -> String { + let expiry_time: chrono::DateTime = chrono::DateTime::from_utc(dt.naive_utc(), chrono::Utc); + + // HACK: HTTP expects the date to always be GMT (UTC) rather than giving an + // offset (which would always be 0 in UTC anyway) + return expiry_time.to_rfc2822().replace("+0000", "GMT"); +} + // // Deployment environment methods // From 6ddbe84bde04c7ec8b52e9641b490adedd8b22a2 Mon Sep 17 00:00:00 2001 From: Jake Howard Date: Wed, 29 Dec 2021 16:29:42 +0000 Subject: [PATCH 4/4] Remove unnecessary return --- src/util.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util.rs b/src/util.rs index 0b287ccf..1a5e674b 100644 --- a/src/util.rs +++ b/src/util.rs @@ -444,7 +444,7 @@ pub fn format_datetime_http(dt: &DateTime) -> String { // HACK: HTTP expects the date to always be GMT (UTC) rather than giving an // offset (which would always be 0 in UTC anyway) - return expiry_time.to_rfc2822().replace("+0000", "GMT"); + expiry_time.to_rfc2822().replace("+0000", "GMT") } //