fix(fetch): revalidate disk cache for request control headers

moli-fetch/src/blocking/cache/vary.rs

@@ -1,6 +1,9 @@
 use std::str;
 
-use moli_http_cache::{HttpCacheVaryHeader, response_vary_header_names};
+use moli_http_cache::{
+    HttpCacheVaryHeader, request_cache_control_forbids_storage, request_header_requires_validation,
+    response_vary_header_names,
+};
 use url::Url;
 
 use crate::{FetchConfig, Request};
@@ -62,8 +65,13 @@ pub(super) fn vary_headers_match(
 }
 
 pub(super) fn request_headers_allow_http_cache(headers: &[(String, String)]) -> bool {
-    headers.iter().enumerate().all(|(index, (name, _))| {
-        supported_cache_request_header(name)
+    headers.iter().enumerate().all(|(index, (name, value))| {
+        // Revalidation controls select the existing conditional-cache path;
+        // they must not turn a fresh entry into an unconditional cache miss.
+        // Other unsupported controls keep the conservative bypass policy.
+        (supported_cache_request_header(name) || request_header_requires_validation(name, value))
+            && !(name.eq_ignore_ascii_case("cache-control")
+                && request_cache_control_forbids_storage(value))
             && !headers[..index]
                 .iter()
                 .any(|(previous, _)| previous.eq_ignore_ascii_case(name))

moli-fetch/src/tests/cache_request_controls.rs

@@ -0,0 +1,91 @@
+use super::*;
+
+#[test]
+fn request_cache_control_headers_revalidate_fresh_cached_responses() {
+    for (name, value) in [
+        ("Cache-Control", "max-age=0"),
+        ("cache-control", "no-cache"),
+        ("Pragma", "no-cache"),
+    ] {
+        let cache_dir = unique_test_cache_dir();
+        let server = ScriptedHttpServer::spawn(vec![
+            ScriptedResponse::ok("cached body")
+                .with_header("Cache-Control", "max-age=60")
+                .with_header("ETag", "\"version-1\""),
+            ScriptedResponse::status(304, "Not Modified")
+                .with_header("Cache-Control", "max-age=600")
+                .with_header("ETag", "\"version-1\""),
+        ]);
+        let mut config = FetchConfig::default();
+        config.set_http_cache_dir(Some(cache_dir.display().to_string()));
+        let first =
+            fetch_with_config_for_test(&config, Request::get(&server.url()).unwrap()).unwrap();
+        assert_eq!(first.body_text(), "cached body");
+        let cached =
+            fetch_with_config_for_test(&config, Request::get(&server.url()).unwrap()).unwrap();
+        assert!(cached.from_cache);
+        assert_eq!(server.hits(), 1);
+
+        let mut request = Request::get(&server.url()).unwrap();
+        request.request_headers = vec![(name.to_owned(), value.to_owned())].into();
+        let validated = fetch_with_config_for_test(&config, request).unwrap();
+        assert_eq!(
+            validated.status, 200,
+            "{name}: {value} must merge the cached 200 response with its 304 validation"
+        );
+        assert_eq!(validated.body_text(), "cached body");
+        assert!(validated.from_cache);
+        let requests = server.requests();
+        assert_eq!(requests.len(), 2);
+        assert!(
+            requests[1]
+                .to_ascii_lowercase()
+                .contains("if-none-match: \"version-1\""),
+            "cached validators missing from {}",
+            requests[1]
+        );
+        assert!(requests[1].to_ascii_lowercase().contains(&format!(
+            "{}: {}",
+            name.to_ascii_lowercase(),
+            value
+        )));
+
+        let cached =
+            fetch_with_config_for_test(&config, Request::get(&server.url()).unwrap()).unwrap();
+        assert_eq!(cached.body_text(), "cached body");
+        assert!(cached.from_cache);
+        assert_eq!(server.hits(), 2);
+        server.shutdown();
+        fs::remove_dir_all(cache_dir).unwrap();
+    }
+}
+
+#[test]
+fn request_no_store_does_not_reuse_or_replace_an_existing_cached_response() {
+    let cache_dir = unique_test_cache_dir();
+    let server = ScriptedHttpServer::spawn(vec![
+        ScriptedResponse::ok("original").with_header("Cache-Control", "max-age=600"),
+        ScriptedResponse::ok("private one").with_header("Cache-Control", "max-age=600"),
+        ScriptedResponse::ok("private two").with_header("Cache-Control", "max-age=600"),
+    ]);
+    let mut config = FetchConfig::default();
+    config.set_http_cache_dir(Some(cache_dir.display().to_string()));
+    let original =
+        fetch_with_config_for_test(&config, Request::get(&server.url()).unwrap()).unwrap();
+    assert_eq!(original.body_text(), "original");
+    for body in ["private one", "private two"] {
+        let mut request = Request::get(&server.url()).unwrap();
+        request.request_headers =
+            vec![("Cache-Control".to_owned(), "max-age=0, No-Store".to_owned())].into();
+        let response = fetch_with_config_for_test(&config, request).unwrap();
+        assert_eq!(response.body_text(), body);
+        assert!(!response.from_cache);
+        let cached =
+            fetch_with_config_for_test(&config, Request::get(&server.url()).unwrap()).unwrap();
+        assert_eq!(cached.body_text(), "original");
+        assert!(cached.from_cache);
+    }
+    assert_eq!(server.hits(), 3);
+    server.shutdown();
+    fs::remove_dir_all(cache_dir).unwrap();
+}

moli-fetch/src/tests/mod.rs

@@ -1,3 +1,4 @@
+mod cache_request_controls;
 mod cookie_context;
 mod cookie_store;
 mod header_bytes;

moli-http-cache/src/lib.rs

@@ -26,8 +26,9 @@ pub use self::{
     policy::{
         HttpCacheResponsePolicy, cache_expires_at_unix_ms, cacheable_response_parts_policy,
         cached_response_is_fresh_immutable, cached_response_is_stale,
-        request_cache_control_requires_validation, request_header_requires_validation,
-        request_pragma_requires_validation, response_cache_policy, unix_now_ms,
+        request_cache_control_forbids_storage, request_cache_control_requires_validation,
+        request_header_requires_validation, request_pragma_requires_validation,
+        response_cache_policy, unix_now_ms,
     },
     revalidation::{merge_not_modified_headers, validation_headers_from_headers},
     store::{HttpCacheEntryInfo, HttpCacheStats, HttpCacheStore, HttpCachedEntryReader},

moli-http-cache/src/policy.rs

@@ -170,6 +170,21 @@ pub fn request_header_requires_validation(name: &str, value: &str) -> bool {
     }
 }
 
+/// Request `no-store` cannot be overridden by a revalidation directive in the
+/// same field. Quoted extension arguments are not independent directives.
+pub fn request_cache_control_forbids_storage(value: &str) -> bool {
+    split_outside_quoted_strings(value, ',')
+        .into_iter()
+        .map(str::trim)
+        .any(|directive| {
+            directive
+                .split_once('=')
+                .map(|(name, _)| name.trim())
+                .unwrap_or(directive)
+                .eq_ignore_ascii_case("no-store")
+        })
+}
+
 pub fn request_cache_control_requires_validation(value: &str) -> bool {
     split_outside_quoted_strings(value, ',')
         .into_iter()
@@ -222,6 +237,27 @@ pub fn unix_now_ms() -> u64 {
 mod tests {
     use super::*;
 
+    #[test]
+    fn request_no_store_is_not_hidden_by_validation_or_forged_by_quoted_extensions() {
+        for value in [
+            "no-store",
+            "NO-STORE",
+            "max-age=0, no-store",
+            "no-store, no-cache",
+        ] {
+            assert!(request_cache_control_forbids_storage(value), "{value}");
+        }
+        for value in [
+            "",
+            "max-age=0",
+            "no-cache",
+            "other=\"no-store\"",
+            "other=\"x, no-store, y\"",
+        ] {
+            assert!(!request_cache_control_forbids_storage(value), "{value}");
+        }
+    }
+
     #[test]
     fn freshness_requires_expiration_to_be_strictly_after_now() {
         let immutable_headers: Vec<(String, Vec<u8>)> = vec![(