Merge pull request #2182 from RealOrangeOne/cache-expires-header

Set `Expires` header when caching responses

3 files changed, 84 insertions, 40 deletions

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<Content<Vec<u8>>> {
         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<Route> {
 
 #[get("/")]
 fn web_index() -> Cached<Option<NamedFile>> {
-    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<Content<Json<Value>>> {
     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 <https://fidoalliance.org/specs/fido-v2.0-id-20180227/fido-appid-and-facets-v2.0-id-20180227.html#determining-the-facetid-of-a-calling-application>:
-                //
-                // "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 <https://fidoalliance.org/specs/fido-v2.0-id-20180227/fido-appid-and-facets-v2.0-id-20180227.html#determining-the-facetid-of-a-calling-application>:
+                    //
+                    // "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("/<p..>", rank = 10)] // Only match this if the other routes don't match
 fn web_files(p: PathBuf) -> Cached<Option<NamedFile>> {
-    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/<uuid>/<file_id>")]
diff --git a/src/util.rs b/src/util.rs
index 2e47077b..1a5e674b 100644
--- a/src/util.rs
+++ b/src/util.rs
@@ -11,6 +11,9 @@ use rocket::{
     Data, Request, Response, Rocket,
 };
 
+use std::thread::sleep;
+use std::time::Duration;
+
 use crate::CONFIG;
 
 pub struct AppHeaders();
@@ -99,29 +102,53 @@ impl Fairing for Cors {
     }
 }
 
-pub struct Cached<R>(R, String);
+pub struct Cached<R> {
+    response: R,
+    is_immutable: bool,
+    ttl: u64,
+}
 
 impl<R> Cached<R> {
-    pub fn long(r: R) -> Cached<R> {
-        // 7 days
-        Self::ttl(r, 604800)
+    pub fn long(response: R, is_immutable: bool) -> Cached<R> {
+        Self {
+            response,
+            is_immutable,
+            ttl: 604800, // 7 days
+        }
     }
 
-    pub fn short(r: R) -> Cached<R> {
-        // 10 minutes
-        Self(r, String::from("public, max-age=600"))
+    pub fn short(response: R, is_immutable: bool) -> Cached<R> {
+        Self {
+            response,
+            is_immutable,
+            ttl: 600, // 10 minutes
+        }
     }
 
-    pub fn ttl(r: R, ttl: u64) -> Cached<R> {
-        Self(r, format!("public, immutable, max-age={}", ttl))
+    pub fn ttl(response: R, ttl: u64, is_immutable: bool) -> Cached<R> {
+        Self {
+            response,
+            is_immutable,
+            ttl,
+        }
     }
 }
 
 impl<'r, R: Responder<'r>> Responder<'r> for Cached<R> {
     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 = chrono::Local::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);
+                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,
@@ -409,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<Local>` as required for HTTP
+///
+/// https://httpwg.org/specs/rfc7231.html#http.date
+pub fn format_datetime_http(dt: &DateTime<Local>) -> String {
+    let expiry_time: chrono::DateTime<chrono::Utc> = 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)
+    expiry_time.to_rfc2822().replace("+0000", "GMT")
+}
+
 //
 // Deployment environment methods
 //
@@ -551,8 +589,6 @@ where
     }
 }
 
-use std::{thread::sleep, time::Duration};
-
 pub fn retry_db<F, T, E>(func: F, max_tries: u32) -> Result<T, E>
 where
     F: Fn() -> Result<T, E>,