diff --git a/moli-fetch/src/blocking/cache/key.rs b/moli-fetch/src/blocking/cache/key.rs index 68b7428b2b..7f4a61d8f5 100644 --- a/moli-fetch/src/blocking/cache/key.rs +++ b/moli-fetch/src/blocking/cache/key.rs @@ -6,6 +6,7 @@ use url::Url; use crate::{FetchConfig, Request}; +use super::super::literal_cookie_header_for_request; use super::vary::request_headers_allow_http_cache; pub(super) fn cache_store_and_key_for_request( @@ -23,8 +24,7 @@ pub(super) fn cache_store_and_key_for_request( || !config.default_request_headers().is_empty() || !subresource_validation_allows_http_cache(request) || cookie_header.is_some() - || request.additional_cookies().is_some() - || config.additional_cookies().is_some() + || literal_cookie_header_for_request(config, request, request_url).is_some() || !request_headers_allow_http_cache(&request.request_headers.to_byte_strings()) || !matches!(request_url.scheme(), "http" | "https") || !request_url.username().is_empty() diff --git a/moli-fetch/src/blocking/mod.rs b/moli-fetch/src/blocking/mod.rs index a1e49f467d..65fea24f50 100644 --- a/moli-fetch/src/blocking/mod.rs +++ b/moli-fetch/src/blocking/mod.rs @@ -171,15 +171,14 @@ pub(crate) fn outgoing_request_headers_for_url( .to_byte_strings() } -fn outgoing_request_header_bytes_for_url( - config: &FetchConfig, - request: &Request, +// Header emission and cache eligibility must select the same literal data, +// including host scope, credentials, empty values and explicit-header overrides. +fn literal_cookie_header_for_request<'a>( + config: &'a FetchConfig, + request: &'a Request, request_url: &Url, - cookie_header: Option<&str>, -) -> crate::RequestHeaders { - let mut outgoing = crate::RequestHeaders::default(); - - let additional = request +) -> Option<&'a str> { + request .additional_cookies() .or_else(|| config.additional_cookies_for_request(request, request_url)) .filter(|value| !value.is_empty()) @@ -193,7 +192,18 @@ fn outgoing_request_header_bytes_for_url( .default_request_headers() .iter() .any(|(name, _)| name.eq_ignore_ascii_case("cookie")) - }); + }) +} + +fn outgoing_request_header_bytes_for_url( + config: &FetchConfig, + request: &Request, + request_url: &Url, + cookie_header: Option<&str>, +) -> crate::RequestHeaders { + let mut outgoing = crate::RequestHeaders::default(); + + let additional = literal_cookie_header_for_request(config, request, request_url); let cookie_header = match (cookie_header, additional) { (Some(stored), Some(literal)) => { Some(std::borrow::Cow::Owned(format!("{stored}; {literal}"))) diff --git a/moli-fetch/src/tests/cache_request_controls.rs b/moli-fetch/src/tests/cache_request_controls.rs index 0be78b7cb4..de555053a8 100644 --- a/moli-fetch/src/tests/cache_request_controls.rs +++ b/moli-fetch/src/tests/cache_request_controls.rs @@ -1,5 +1,182 @@ use super::*; +#[derive(Clone, Copy, Debug)] +enum CacheTransport { + Html, + Raw, +} + +impl CacheTransport { + async fn fetch(self, config: &FetchConfig, request: Request) -> Result { + // Reopen the client on every request to exercise the shared disk cache. + let client = FetchClient::new(config, new_shared_browser_cookie_store()); + let response = match self { + Self::Html => { + client + .fetch_html_stream(request) + .await? + .into_materialized_text_response() + .await + } + Self::Raw => client.fetch(request).await, + }; + assert!(client.shutdown().is_clean()); + response + } +} + +fn cacheable_response(body: &str) -> ScriptedResponse { + ScriptedResponse::ok(body).with_header("Cache-Control", "max-age=600") +} + +#[tokio::test] +async fn literal_cookie_cache_ignores_unbound_and_out_of_scope_configuration() -> Result<()> { + for transport in [CacheTransport::Html, CacheTransport::Raw] { + for bound in [false, true] { + let cache_dir = unique_test_cache_dir(); + let server = ScriptedHttpServer::spawn(vec![cacheable_response("public"); 2]); + let mut config = FetchConfig::default(); + config.set_http_cache_dir(Some(cache_dir.display().to_string())); + config.set_additional_cookies(Some("session=literal".to_owned())); + if bound { + config.bind_additional_cookie_scope(&Request::get("https://app.test/start")?); + } + let request = Request::new("GET", &server.url(), None, Vec::new())? + .with_request_mode(RequestMode::NoCors) + .with_credentials_mode(RequestCredentialsMode::Include); + + for from_cache in [false, true] { + let response = transport.fetch(&config, request.clone()).await?; + assert_eq!( + response.body_text(), + "public", + "{transport:?}, bound={bound}" + ); + assert_eq!( + response.from_cache, from_cache, + "{transport:?}, bound={bound}" + ); + } + assert_eq!(server.hits(), 1, "{transport:?}, bound={bound}"); + assert!( + !server.requests()[0] + .to_ascii_lowercase() + .contains("\r\ncookie:") + ); + server.shutdown(); + fs::remove_dir_all(cache_dir)?; + } + } + Ok(()) +} + +#[tokio::test] +async fn literal_cookie_cache_obeys_browser_credentials() -> Result<()> { + for transport in [CacheTransport::Html, CacheTransport::Raw] { + for in_config in [false, true] { + for credentials in [ + RequestCredentialsMode::Omit, + RequestCredentialsMode::SameOrigin, + ] { + let cache_dir = unique_test_cache_dir(); + let server = ScriptedHttpServer::spawn(vec![cacheable_response("public"); 2]); + let mut config = FetchConfig::default(); + config.set_http_cache_dir(Some(cache_dir.display().to_string())); + let mut request = Request::new_browser( + "GET", + Url::parse(&server.url())?, + None, + Vec::new(), + moli_url::WebOrigin::from_url(&Url::parse("https://other.test/page")?), + ) + .with_request_mode(RequestMode::NoCors) + .with_credentials_mode(credentials); + if in_config { + config.set_additional_cookies(Some("session=literal".to_owned())); + config.bind_additional_cookie_scope(&Request::get(&server.url())?); + } else { + request = request.with_additional_cookies(Some("session=literal".to_owned())); + } + + for from_cache in [false, true] { + let response = transport.fetch(&config, request.clone()).await?; + assert_eq!(response.body_text(), "public"); + assert_eq!( + response.from_cache, from_cache, + "{transport:?}, in_config={in_config}, {credentials:?}" + ); + } + assert_eq!(server.hits(), 1); + assert!( + !server.requests()[0] + .to_ascii_lowercase() + .contains("\r\ncookie:") + ); + server.shutdown(); + fs::remove_dir_all(cache_dir)?; + } + } + } + Ok(()) +} + +#[tokio::test] +async fn literal_cookie_cache_keeps_credentialed_responses_private() -> Result<()> { + for transport in [CacheTransport::Html, CacheTransport::Raw] { + for in_config in [false, true] { + let cache_dir = unique_test_cache_dir(); + let server = ScriptedHttpServer::spawn(vec![ + cacheable_response("public"), + cacheable_response("private one"), + cacheable_response("private two"), + ]); + let mut config = FetchConfig::default(); + config.set_http_cache_dir(Some(cache_dir.display().to_string())); + let public = Request::new("GET", &server.url(), None, Vec::new())? + .with_request_mode(RequestMode::NoCors) + .with_credentials_mode(RequestCredentialsMode::Include); + let response = transport.fetch(&config, public.clone()).await?; + assert_eq!(response.body_text(), "public"); + assert!(!response.from_cache); + + let mut private = public.clone(); + if in_config { + config.set_additional_cookies(Some("session=literal".to_owned())); + config.bind_additional_cookie_scope(&Request::get(&server.url())?); + } else { + private = private.with_additional_cookies(Some("session=literal".to_owned())); + } + for body in ["private one", "private two"] { + let response = transport.fetch(&config, private.clone()).await?; + assert_eq!( + response.body_text(), + body, + "{transport:?}, in_config={in_config}" + ); + assert!(!response.from_cache); + } + + config.set_additional_cookies(None); + let response = transport.fetch(&config, public).await?; + assert_eq!(response.body_text(), "public"); + assert!(response.from_cache); + assert_eq!(server.hits(), 3); + for (index, request) in server.requests().iter().enumerate() { + assert_eq!( + request + .to_ascii_lowercase() + .contains("\r\ncookie: session=literal\r\n"), + index != 0, + "{transport:?}, in_config={in_config}: {request}" + ); + } + server.shutdown(); + fs::remove_dir_all(cache_dir)?; + } + } + Ok(()) +} + #[test] fn request_cache_control_headers_revalidate_fresh_cached_responses() { for (name, value) in [