diff --git a/src/cdn/fastly.rs b/src/cdn/fastly.rs index f78cde3b0..dceae4188 100644 --- a/src/cdn/fastly.rs +++ b/src/cdn/fastly.rs @@ -210,7 +210,7 @@ mod tests { let m = fastly_api .mock("POST", "/service/test-sid-1/purge") .match_header(FASTLY_KEY, "test-token") - .match_header(&SURROGATE_KEY, "crate-foo crate-bar") + .match_header(&SURROGATE_KEY, "crate-bar crate-foo") .with_status(200) .create_async() .await; @@ -246,7 +246,7 @@ mod tests { let m = fastly_api .mock("POST", "/service/test-sid-1/purge") .match_header(FASTLY_KEY, "test-token") - .match_header(&SURROGATE_KEY, "crate-foo crate-bar") + .match_header(&SURROGATE_KEY, "crate-bar crate-foo") .with_status(500) .create_async() .await; diff --git a/src/db/types/krate_name.rs b/src/db/types/krate_name.rs index 0b963426b..041db4add 100644 --- a/src/db/types/krate_name.rs +++ b/src/db/types/krate_name.rs @@ -32,6 +32,12 @@ use std::{io::Write, str::FromStr}; )] pub struct KrateName(String); +impl From<&KrateName> for KrateName { + fn from(krate_name: &KrateName) -> Self { + krate_name.clone() + } +} + impl FromStr for KrateName { type Err = anyhow::Error; diff --git a/src/lib.rs b/src/lib.rs index 2de789a13..eeed9c191 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,6 +1,10 @@ //! [Docs.rs](https://docs.rs) (formerly cratesfyi) is an open source project to host //! documentation of crates for the Rust Programming Language. -#![allow(clippy::cognitive_complexity)] +#![allow( + clippy::cognitive_complexity, + // TODO: `AxumNope::Redirect(EscapedURI, CachePolicy)` is too big. + clippy::result_large_err, +)] pub use self::build_queue::{ AsyncBuildQueue, BuildQueue, queue_rebuilds, queue_rebuilds_faulty_rustdoc, diff --git a/src/test/mod.rs b/src/test/mod.rs index 5b2130daa..4b08aa3d7 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -15,9 +15,8 @@ use crate::{ storage::{AsyncStorage, Storage, StorageKind}, test::test_metrics::CollectedMetrics, web::{ - build_axum_app, - cache::{self}, - headers::{IfNoneMatch, SURROGATE_CONTROL}, + build_axum_app, cache, + headers::{IfNoneMatch, SURROGATE_CONTROL, SurrogateKeys}, page::TemplateData, }, }; @@ -75,6 +74,11 @@ pub(crate) fn assert_cache_headers_eq( response.headers().get(&SURROGATE_CONTROL), "surrogate control header mismatch" ); + assert_eq!( + expected_headers.surrogate_keys.as_ref(), + response.headers().typed_get::().as_ref(), + "surrogate key header mismatch" + ); } pub(crate) trait AxumResponseTestExt { @@ -106,7 +110,7 @@ impl AxumResponseTestExt for axum::response::Response { assert!(config.cache_control_stale_while_revalidate.is_some()); // This method is only about asserting if the handler did set the right _policy_. - assert_cache_headers_eq(self, &cache_policy.render(config)); + assert_cache_headers_eq(self, &cache_policy.render(config).unwrap()); } fn error_for_status(self) -> Result diff --git a/src/web/build_details.rs b/src/web/build_details.rs index 25e81824d..bda5334dd 100644 --- a/src/web/build_details.rs +++ b/src/web/build_details.rs @@ -73,13 +73,14 @@ pub(crate) async fn build_details_handler( let version = match_version(&mut conn, params.name(), params.req_version()) .await? .assume_exact_name()? - .into_canonical_req_version_or_else(|version| { + .into_canonical_req_version_or_else(|confirmed_name, version| { + let params = params + .clone() + .with_confirmed_name(Some(confirmed_name)) + .with_req_version(version); AxumNope::Redirect( - params - .clone() - .with_req_version(version) - .build_details_url(id, build_params.filename.as_deref()), - CachePolicy::ForeverInCdn, + params.build_details_url(id, build_params.filename.as_deref()), + CachePolicy::ForeverInCdn(confirmed_name.into()), ) })? .into_version(); diff --git a/src/web/builds.rs b/src/web/builds.rs index b400aca3a..a88913bb4 100644 --- a/src/web/builds.rs +++ b/src/web/builds.rs @@ -67,10 +67,14 @@ pub(crate) async fn build_list_handler( let version = match_version(&mut conn, params.name(), params.req_version()) .await? .assume_exact_name()? - .into_canonical_req_version_or_else(|version| { + .into_canonical_req_version_or_else(|confirmed_name, version| { + let params = params + .clone() + .with_confirmed_name(Some(confirmed_name)) + .with_req_version(version); AxumNope::Redirect( - params.clone().with_req_version(version).builds_url(), - CachePolicy::ForeverInCdn, + params.builds_url(), + CachePolicy::ForeverInCdn(confirmed_name.into()), ) })? .into_version(); diff --git a/src/web/cache.rs b/src/web/cache.rs index 6900419b5..4ccffb286 100644 --- a/src/web/cache.rs +++ b/src/web/cache.rs @@ -1,15 +1,9 @@ use crate::{ config::Config, - db::types::krate_name::KrateName, - web::{ - extractors::Path, - headers::{SURROGATE_CONTROL, SURROGATE_KEY, SurrogateKeys}, - }, + web::headers::{SURROGATE_CONTROL, SURROGATE_KEY, SurrogateKey, SurrogateKeys}, }; use axum::{ - Extension, - extract::{MatchedPath, Request as AxumHttpRequest}, - middleware::Next, + Extension, extract::Request as AxumHttpRequest, middleware::Next, response::Response as AxumResponse, }; use axum_extra::headers::HeaderMapExt as _; @@ -17,24 +11,35 @@ use http::{ HeaderMap, HeaderValue, StatusCode, header::{CACHE_CONTROL, ETAG}, }; -use serde::Deserialize; use std::sync::Arc; use tracing::error; +/// a surrogate key that is attached to _all_ content. +/// This enables us to use the fastly "soft purge" for everything. +pub const SURROGATE_KEY_ALL: SurrogateKey = SurrogateKey::from_static("all"); + #[derive(Debug, Clone, PartialEq)] pub struct ResponseCacheHeaders { pub cache_control: Option, pub surrogate_control: Option, + pub surrogate_keys: Option, pub needs_cdn_invalidation: bool, + + // whether to add a global surrogate key to this response. + // Needed only when we actually cache something. + pub is_caching_something: bool, } impl ResponseCacheHeaders { - fn set_on_response(&self, headers: &mut HeaderMap) { - if let Some(ref cache_control) = self.cache_control { - headers.insert(CACHE_CONTROL, cache_control.clone()); + fn set_on_response(self, headers: &mut HeaderMap) { + if let Some(cache_control) = self.cache_control { + headers.insert(CACHE_CONTROL, cache_control); + } + if let Some(surrogate_control) = self.surrogate_control { + headers.insert(&SURROGATE_CONTROL, surrogate_control); } - if let Some(ref surrogate_control) = self.surrogate_control { - headers.insert(&SURROGATE_CONTROL, surrogate_control.clone()); + if let Some(surrogate_keys) = self.surrogate_keys { + headers.typed_insert(surrogate_keys); } } } @@ -48,27 +53,33 @@ impl ResponseCacheHeaders { pub static NO_CACHING: ResponseCacheHeaders = ResponseCacheHeaders { cache_control: Some(HeaderValue::from_static("max-age=0")), surrogate_control: None, + surrogate_keys: None, needs_cdn_invalidation: false, + is_caching_something: false, }; /// Cache for a short time in the browser & in the CDN. /// Helps protecting against traffic spikes. -pub static SHORT: ResponseCacheHeaders = ResponseCacheHeaders { +static SHORT: ResponseCacheHeaders = ResponseCacheHeaders { cache_control: Some(HeaderValue::from_static("public, max-age=60")), surrogate_control: None, + surrogate_keys: None, needs_cdn_invalidation: false, + is_caching_something: true, }; /// don't cache, don't even store. Never. Ever. -pub static NO_STORE_MUST_REVALIDATE: ResponseCacheHeaders = ResponseCacheHeaders { +static NO_STORE_MUST_REVALIDATE: ResponseCacheHeaders = ResponseCacheHeaders { cache_control: Some(HeaderValue::from_static( "no-cache, no-store, must-revalidate, max-age=0", )), surrogate_control: None, + surrogate_keys: None, needs_cdn_invalidation: false, + is_caching_something: false, }; -pub static FOREVER_IN_FASTLY_CDN: ResponseCacheHeaders = ResponseCacheHeaders { +static FOREVER_IN_FASTLY_CDN: ResponseCacheHeaders = ResponseCacheHeaders { // explicitly forbid browser caching, same as NO_CACHING above. cache_control: Some(HeaderValue::from_static("max-age=0")), @@ -79,8 +90,10 @@ pub static FOREVER_IN_FASTLY_CDN: ResponseCacheHeaders = ResponseCacheHeaders { // especially in combination with our fastly compute service. // https://www.fastly.com/documentation/guides/concepts/edge-state/cache/stale/ surrogate_control: Some(HeaderValue::from_static("max-age=31536000")), + surrogate_keys: None, needs_cdn_invalidation: true, + is_caching_something: true, }; /// cache forever in browser & CDN. @@ -88,17 +101,18 @@ pub static FOREVER_IN_FASTLY_CDN: ResponseCacheHeaders = ResponseCacheHeaders { /// /// We use this policy mostly for static files, rustdoc toolchain assets, /// or build assets. -pub static FOREVER_IN_CDN_AND_BROWSER: ResponseCacheHeaders = ResponseCacheHeaders { +static FOREVER_IN_CDN_AND_BROWSER: ResponseCacheHeaders = ResponseCacheHeaders { cache_control: Some(HeaderValue::from_static( "public, max-age=31104000, immutable", )), surrogate_control: None, + surrogate_keys: None, needs_cdn_invalidation: false, + is_caching_something: true, }; /// defines the wanted caching behaviour for a web response. #[derive(Debug, Clone)] -#[cfg_attr(test, derive(strum::EnumIter))] pub enum CachePolicy { /// no browser or CDN caching. /// In some cases the browser might still use cached content, @@ -121,33 +135,45 @@ pub enum CachePolicy { /// cache forever in CDN, but not in the browser. /// Since we control the CDN we can actively purge content that is cached like /// this, for example after building a crate. + /// Note: The CDN (Fastly) needs a list of surrogate keys ( = tags )to be able to purge a + /// subset of the pages /// Example usage: `/latest/` rustdoc pages and their redirects. - ForeverInCdn, + ForeverInCdn(SurrogateKeys), /// cache forever in the CDN, but allow stale content in the browser. + /// Note: The CDN (Fastly) needs a list of surrogate keys ( = tags )to be able to purge a + /// subset of the pages /// Example: rustdoc pages with the version in their URL. /// A browser will show the stale content while getting the up-to-date /// version from the origin server in the background. /// This helps building a PWA. - ForeverInCdnAndStaleInBrowser, + ForeverInCdnAndStaleInBrowser(SurrogateKeys), } impl CachePolicy { - pub fn render(&self, config: &Config) -> ResponseCacheHeaders { - match *self { + pub fn render(self, config: &Config) -> anyhow::Result { + let mut headers = match self { CachePolicy::NoCaching => NO_CACHING.clone(), CachePolicy::NoStoreMustRevalidate => NO_STORE_MUST_REVALIDATE.clone(), CachePolicy::ShortInCdnAndBrowser => SHORT.clone(), CachePolicy::ForeverInCdnAndBrowser => FOREVER_IN_CDN_AND_BROWSER.clone(), - CachePolicy::ForeverInCdn => { + CachePolicy::ForeverInCdn(surrogate_keys) => { if config.cache_invalidatable_responses { - FOREVER_IN_FASTLY_CDN.clone() + let mut cache_headers = FOREVER_IN_FASTLY_CDN.clone(); + + cache_headers + .surrogate_keys + .get_or_insert_with(SurrogateKeys::new) + .try_extend(surrogate_keys.into_iter())?; + + cache_headers } else { NO_CACHING.clone() } } - CachePolicy::ForeverInCdnAndStaleInBrowser => { + CachePolicy::ForeverInCdnAndStaleInBrowser(surrogate_keys) => { // when caching invalidatable responses is disabled, this results in NO_CACHING - let mut forever_in_cdn = CachePolicy::ForeverInCdn.render(config); + let mut forever_in_cdn = + CachePolicy::ForeverInCdn(surrogate_keys).render(config)?; if config.cache_invalidatable_responses && let Some(cache_control) = @@ -162,21 +188,20 @@ impl CachePolicy { forever_in_cdn } + }; + + if headers.is_caching_something { + headers + .surrogate_keys + .get_or_insert_with(SurrogateKeys::new) + .try_extend([SURROGATE_KEY_ALL])?; } - } -} -/// All our routes use `{name}` to identify the crate name -/// in routes. -/// With this struct we can extract only that, if it exists. -#[derive(Deserialize)] -pub(crate) struct CrateParam { - name: Option, + Ok(headers) + } } pub(crate) async fn cache_middleware( - Path(param): Path, - matched_route: Option, Extension(config): Extension>, req: AxumHttpRequest, next: Next, @@ -205,103 +230,19 @@ pub(crate) async fn cache_middleware( // We only use cache policies in our successful responses (with content, or redirect), // so any errors (4xx, 5xx) should always get "NoCaching". let cache_policy = response - .extensions() - .get::() - .unwrap_or(&CachePolicy::NoCaching); - let cache_headers = cache_policy.render(&config); - let resp_status = response.status(); - let resp_headers = response.headers_mut(); - - // simple implementation first: - // This is for content we need to invalidate in the CDN level. - // We don't care about content that is filename-hashed and can be cached - // forever, or content that is not cached at all. - // - // Generally Fastly can either purge single URLs, or a whole service. - // When you want to purge the cache for a bigger subset, but not everything, you need to "tag" - // your content with surrogate keys when delivering it to Fastly for caching. - // https://www.fastly.com/documentation/guides/full-site-delivery/purging/working-with-surrogate-keys/ - // - // At some point we should extend this system and make it explicit, so in all places you return - // a cache policy you also return these surrogate keys, probably based on the krate, release - // or other things. For now we stick to invalidating the whole crate on all changes. - // - // For the first version I found an easy "hack" that doesn't need the full refactor across - // all our handlers; - // If the URL contains a crate name, we create a surrogate key based on that. - // Since we always call the crate name (and only the crate name) `{name}` in our routes, - // we're safe here. I added some debug assertions to ensure my assumptions are right, and - // any change to these in the routes would lead to test failures. - let cache_headers = if let Some(ref name) = param.name { - // we could theoretically only run this part when cache_invalidatable_responses and - // cache_headers.needs_cdn_invalidation are true, - // but let's always to this validation and add the surrogate-key to know if - // our "hack" still works. - // - // I didn't think through the possible edge-cases yet, but I feel safer - // always adding a surrogate key if we have one. - debug_assert!( - matched_route - .map(|matched_route| { - let matched_route = matched_route.as_str(); - matched_route.starts_with("/crate/{name}") - || matched_route.starts_with("/{name}") - }) - .unwrap_or(true), - "there shouldn't be a name on any other routes" - ); - if let Ok(krate_name) = name.parse::() { - let keys = SurrogateKeys::from_iter_until_full(vec![krate_name.into()]); - - resp_headers.typed_insert(keys); - - // only allow caching in the CDN when we have a surrogate key to invalidate it later. - // This is just the default for all routes that include a crate name. - // Then we build build & add the surrugate yet. - // It's totally possible that this policy here then states NO_CACHING, - // or FOREVER_IN_CDN_AND_BROWSER, where we wouln't need the surrogate key. - &cache_headers - } else if cache_headers.needs_cdn_invalidation { - // This theoretically shouldn't happen, all current crate names would be valid - // for surrogate keys, and the `KrateName` validation matches the crates.io crate - // publish validation. - // But I'll leave this error log here just in case, until I migrated to using the - // `KrateName` type in all entrypoints (web, builds). - if resp_status.is_success() || resp_status.is_redirection() { - error!( - name = param.name, - ?cache_headers, - "failed to create surrogate key for crate, falling back to NO_CACHING" - ); - } - &NO_CACHING - } else { - &cache_headers + .extensions_mut() + .remove::() + .unwrap_or(CachePolicy::NoCaching); + + let cache_headers = match cache_policy.render(&config) { + Ok(headers) => headers, + Err(e) => { + error!(?e, "couldn't render cache headers for policy"); + NO_CACHING.clone() } - } else { - debug_assert!( - matched_route - .map(|matched_route| { - let matched_route = matched_route.as_str(); - !(matched_route.starts_with("/crate/{name}") - || matched_route.starts_with("/{name}")) - }) - .unwrap_or(true), - "for rustdoc & crate-detail routes the `name` param should always be present" - ); - debug_assert!( - !(config.cache_invalidatable_responses && cache_headers.needs_cdn_invalidation), - "We got to a route without crate name, and a cache policy that needs invalidation. - This doesn't work because Fastly only supports surrogate keys for partial - invalidation." - ); - - // standard case, just use the cache policy, no surrogate keys needed. - &cache_headers }; - cache_headers.set_on_response(resp_headers); - + cache_headers.set_on_response(response.headers_mut()); response } @@ -313,9 +254,9 @@ mod tests { headers::{test_typed_decode, test_typed_encode}, }; use anyhow::{Context as _, Result}; - use axum::{Router, body::Body, http::Request, routing::get}; + use axum::{Router, body::Body, routing::get}; use axum_extra::headers::CacheControl; - use strum::IntoEnumIterator as _; + use http::Request; use test_case::{test_case, test_matrix}; use tower::{ServiceBuilder, ServiceExt as _}; @@ -356,54 +297,59 @@ mod tests { .cache_control_stale_while_revalidate(stale_while_revalidate) .build()?; - for policy in CachePolicy::iter() { - let headers = policy.render(&config); - - if let Some(cache_control) = headers.cache_control { - validate_cache_control(&cache_control).with_context(|| { - format!( - "couldn't validate Cache-Control header syntax for policy {:?}", - policy - ) - })?; + fn validate_headers(headers: &ResponseCacheHeaders) -> Result<()> { + if let Some(ref cache_control) = headers.cache_control { + validate_cache_control(cache_control) + .context("couldn't validate Cache-Control header syntax")?; } - if let Some(surrogate_control) = headers.surrogate_control { - validate_cache_control(&surrogate_control).with_context(|| { - format!( - "couldn't validate Surrogate-Control header syntax for policy {:?}", - policy - ) - })?; + if let Some(ref surrogate_control) = headers.surrogate_control { + validate_cache_control(surrogate_control) + .context("couldn't validate Surrogate-Control header syntax")?; } + Ok(()) + } + + use CachePolicy::*; + + let key = SurrogateKey::from_static("something"); + + for policy in [ + NoCaching, + NoStoreMustRevalidate, + ShortInCdnAndBrowser, + ForeverInCdnAndBrowser, + ForeverInCdn(key.clone().into()), + ForeverInCdnAndStaleInBrowser(key.clone().into()), + ] { + let headers = policy.render(&config)?; + validate_headers(&headers)?; } + Ok(()) } - #[test_case(CachePolicy::NoCaching, Some("max-age=0"), None)] + #[test_case(CachePolicy::NoCaching, Some("max-age=0"), None, false)] #[test_case( CachePolicy::NoStoreMustRevalidate, Some("no-cache, no-store, must-revalidate, max-age=0"), - None + None, + false )] #[test_case( CachePolicy::ForeverInCdnAndBrowser, Some("public, max-age=31104000, immutable"), - None - )] - #[test_case(CachePolicy::ForeverInCdn, Some("max-age=0"), Some("max-age=31536000"))] - #[test_case( - CachePolicy::ForeverInCdnAndStaleInBrowser, - Some("stale-while-revalidate=86400"), - Some("max-age=31536000") + None, + true )] fn render_fastly( cache: CachePolicy, cache_control: Option<&str>, surrogate_control: Option<&str>, + needs_global_surrogate_key: bool, ) -> Result<()> { let config = TestEnvironment::base_config().build()?; - let headers = cache.render(&config); + let headers = cache.render(&config)?; assert_eq!( headers.cache_control, @@ -415,6 +361,66 @@ mod tests { surrogate_control.map(|s| HeaderValue::from_str(s).unwrap()) ); + if needs_global_surrogate_key { + assert_eq!(headers.surrogate_keys.unwrap(), SURROGATE_KEY_ALL.into()); + } else { + assert!(headers.surrogate_keys.is_none()); + } + + Ok(()) + } + + #[test] + fn render_fastly_forever_in_cdn() -> Result<()> { + let config = TestEnvironment::base_config().build()?; + // this surrogate key is user-defined, identifies the crate. + let key = SurrogateKey::from_static("something"); + let headers = CachePolicy::ForeverInCdn(key.clone().into()).render(&config)?; + + // browser or other proxies: mostly no caching + assert_eq!( + headers.cache_control, + Some(HeaderValue::from_static("max-age=0")) + ); + + // CDN: cache forever. + // Fastly will completely ignore cache-control if it finds surrogate-control. + assert_eq!( + headers.surrogate_control, + Some(HeaderValue::from_static("max-age=31536000")) + ); + + // both: our key + the global "all" key. + // So we can purge the CDN for these keys. + assert_eq!( + headers.surrogate_keys.unwrap(), + SurrogateKeys::try_from_iter([key, SURROGATE_KEY_ALL]).unwrap() + ); + + Ok(()) + } + + #[test] + fn render_fastly_forever_in_cdn_stale_in_browser() -> Result<()> { + let config = TestEnvironment::base_config().build()?; + let key = SurrogateKey::from_static("something"); + let headers = + CachePolicy::ForeverInCdnAndStaleInBrowser(key.clone().into()).render(&config)?; + + assert_eq!( + headers.cache_control, + Some(HeaderValue::from_static("stale-while-revalidate=86400")) + ); + assert_eq!( + headers.surrogate_control, + Some(HeaderValue::from_static("max-age=31536000")) + ); + + assert_eq!( + headers.surrogate_keys.unwrap(), + SurrogateKeys::try_from_iter([key, SURROGATE_KEY_ALL]).unwrap() + ); + Ok(()) } @@ -424,7 +430,13 @@ mod tests { .cache_control_stale_while_revalidate(None) .build()?; - let headers = CachePolicy::ForeverInCdnAndStaleInBrowser.render(&config); + let key = SurrogateKey::from_static("something"); + let mut headers = + CachePolicy::ForeverInCdnAndStaleInBrowser(key.clone().into()).render(&config)?; + assert_eq!( + headers.surrogate_keys.take().unwrap(), + SurrogateKeys::try_from_iter([key, SURROGATE_KEY_ALL]).unwrap() + ); assert_eq!(headers, FOREVER_IN_FASTLY_CDN); Ok(()) @@ -436,12 +448,18 @@ mod tests { .cache_control_stale_while_revalidate(Some(666)) .build()?; - let headers = CachePolicy::ForeverInCdnAndStaleInBrowser.render(&config); + let key = SurrogateKey::from_static("something"); + let headers = + CachePolicy::ForeverInCdnAndStaleInBrowser(key.clone().into()).render(&config)?; assert_eq!(headers.cache_control.unwrap(), "stale-while-revalidate=666"); assert_eq!( headers.surrogate_control, FOREVER_IN_FASTLY_CDN.surrogate_control ); + assert_eq!( + headers.surrogate_keys.unwrap(), + SurrogateKeys::try_from_iter([key, SURROGATE_KEY_ALL]).unwrap() + ); Ok(()) } @@ -452,9 +470,11 @@ mod tests { .cache_invalidatable_responses(false) .build()?; - let headers = CachePolicy::ForeverInCdn.render(&config); + let key = SurrogateKey::from_static("something"); + let headers = CachePolicy::ForeverInCdn(key.into()).render(&config)?; assert_eq!(headers.cache_control.unwrap(), "max-age=0"); assert!(headers.surrogate_control.is_none()); + assert!(headers.surrogate_keys.is_none()); Ok(()) } @@ -465,7 +485,8 @@ mod tests { .cache_invalidatable_responses(false) .build()?; - let headers = CachePolicy::ForeverInCdnAndStaleInBrowser.render(&config); + let key = SurrogateKey::from_static("something"); + let headers = CachePolicy::ForeverInCdnAndStaleInBrowser(key.into()).render(&config)?; assert_eq!(headers.cache_control.unwrap(), "max-age=0"); assert!(headers.surrogate_control.is_none()); @@ -474,18 +495,25 @@ mod tests { #[tokio::test] async fn test_middleware_reacts_to_fastly_header_in_crate_route() -> Result<()> { - let config = TestEnvironment::base_config() - .cache_invalidatable_responses(true) - .build()?; + let config = Arc::new( + TestEnvironment::base_config() + .cache_invalidatable_responses(true) + .build()?, + ); + let key = SurrogateKey::from_static("krate"); + let policy = CachePolicy::ForeverInCdn(key.clone().into()); let app = Router::new() .route( "/{name}", - get(move || async move { (Extension(CachePolicy::ForeverInCdn), "Hello, World!") }), + get({ + let policy = policy.clone(); + move || async move { (Extension(policy), "Hello, World!") } + }), ) .layer( ServiceBuilder::new() - .layer(Extension(Arc::new(config))) + .layer(Extension(config.clone())) .layer(axum::middleware::from_fn(cache_middleware)), ); @@ -501,14 +529,14 @@ mod tests { "{}", response.text().await.unwrap(), ); - assert_cache_headers_eq(&response, &FOREVER_IN_FASTLY_CDN); + assert_cache_headers_eq(&response, &policy.render(&config)?); Ok(()) } #[tokio::test] async fn test_middleware_reacts_to_fastly_header_in_other_route() -> Result<()> { - let config = TestEnvironment::base_config().build()?; + let config = Arc::new(TestEnvironment::base_config().build()?); let app = Router::new() .route( @@ -522,7 +550,7 @@ mod tests { ) .layer( ServiceBuilder::new() - .layer(Extension(Arc::new(config))) + .layer(Extension(config.clone())) .layer(axum::middleware::from_fn(cache_middleware)), ); @@ -540,7 +568,10 @@ mod tests { ); // this cache policy leads to the same result in both CDNs - assert_cache_headers_eq(&response, &FOREVER_IN_CDN_AND_BROWSER); + assert_cache_headers_eq( + &response, + &CachePolicy::ForeverInCdnAndBrowser.render(&config)?, + ); Ok(()) } diff --git a/src/web/crate_details.rs b/src/web/crate_details.rs index 6822c51d9..8e89e4614 100644 --- a/src/web/crate_details.rs +++ b/src/web/crate_details.rs @@ -486,10 +486,14 @@ pub(crate) async fn crate_details_handler( let matched_release = match_version(&mut conn, params.name(), params.req_version()) .await? .assume_exact_name()? - .into_canonical_req_version_or_else(|version| { + .into_canonical_req_version_or_else(|confirmed_name, version| { + let params = params + .clone() + .with_confirmed_name(Some(confirmed_name)) + .with_req_version(version); AxumNope::Redirect( - params.clone().with_req_version(version).crate_details_url(), - CachePolicy::ForeverInCdn, + params.crate_details_url(), + CachePolicy::ForeverInCdn(confirmed_name.into()), ) })?; let params = params.apply_matched_release(&matched_release); @@ -497,7 +501,7 @@ pub(crate) async fn crate_details_handler( if params.original_path() != params.crate_details_url().path() { return Err(AxumNope::Redirect( params.crate_details_url(), - CachePolicy::ForeverInCdn, + CachePolicy::ForeverInCdn(matched_release.name.into()), )); } @@ -538,7 +542,7 @@ pub(crate) async fn crate_details_handler( let mut res = CrateDetailsPage { version, - name, + name: name.clone(), owners, metadata, documented_items, @@ -570,9 +574,9 @@ pub(crate) async fn crate_details_handler( .into_response(); res.extensions_mut() .insert::(if is_latest_version { - CachePolicy::ForeverInCdn + CachePolicy::ForeverInCdn(name.into()) } else { - CachePolicy::ForeverInCdnAndStaleInBrowser + CachePolicy::ForeverInCdnAndStaleInBrowser(name.into()) }); Ok(res) } @@ -581,13 +585,16 @@ pub(crate) async fn crate_details_handler( #[template(path = "rustdoc/releases.html")] #[derive(Debug, Clone, PartialEq)] struct ReleaseList { + crate_name: KrateName, releases: Vec, params: RustdocParams, } impl_axum_webpage! { ReleaseList, - cache_policy = |_| CachePolicy::ForeverInCdn, + cache_policy = |page| CachePolicy::ForeverInCdn( + page.crate_name.clone().into() + ), cpu_intensive_rendering = true, } @@ -600,7 +607,7 @@ pub(crate) async fn get_all_releases( // NOTE: we're getting RustDocParams here, where both target and path are optional. let matched_release = match_version(&mut conn, params.name(), params.req_version()) .await? - .into_canonical_req_version_or_else(|_| AxumNope::VersionNotFound)?; + .into_canonical_req_version_or_else(|_, _| AxumNope::VersionNotFound)?; let params = params.apply_matched_release(&matched_release); if matched_release.build_status() != BuildStatus::Success { @@ -612,6 +619,7 @@ pub(crate) async fn get_all_releases( } Ok(ReleaseList { + crate_name: matched_release.name.clone(), releases: matched_release.all_releases, params, } @@ -622,6 +630,7 @@ pub(crate) async fn get_all_releases( #[template(path = "rustdoc/platforms.html")] #[derive(Debug, Clone, PartialEq)] struct PlatformList { + crate_name: KrateName, use_direct_platform_links: bool, current_target: String, params: RustdocParams, @@ -629,7 +638,9 @@ struct PlatformList { impl_axum_webpage! { PlatformList, - cache_policy = |_| CachePolicy::ForeverInCdn, + cache_policy = |page| CachePolicy::ForeverInCdn( + page.crate_name.clone().into() + ), cpu_intensive_rendering = true, } @@ -649,19 +660,20 @@ pub(crate) async fn get_all_platforms_inner( AxumNope::Redirect( params .clone() - .with_name(corrected_name) + .with_confirmed_name(Some(corrected_name)) .with_req_version(req_version) .platforms_partial_url(), CachePolicy::NoCaching, ) })? - .into_canonical_req_version_or_else(|version| { + .into_canonical_req_version_or_else(|confirmed_name, version| { + let params = params + .clone() + .with_confirmed_name(Some(confirmed_name)) + .with_req_version(version); AxumNope::Redirect( - params - .clone() - .with_req_version(version) - .platforms_partial_url(), - CachePolicy::ForeverInCdn, + params.platforms_partial_url(), + CachePolicy::ForeverInCdn(confirmed_name.into()), ) })?; let params = params.apply_matched_release(&matched_release); @@ -670,6 +682,7 @@ pub(crate) async fn get_all_platforms_inner( // when the build wasn't finished, we don't have any target platforms // we could read from. return Ok(PlatformList { + crate_name: matched_release.name.clone(), use_direct_platform_links: is_crate_root, current_target: "".into(), params, @@ -690,6 +703,7 @@ pub(crate) async fn get_all_platforms_inner( }; Ok(PlatformList { + crate_name: matched_release.name.clone(), use_direct_platform_links: is_crate_root, current_target, params, @@ -714,6 +728,7 @@ pub(crate) async fn get_all_platforms( #[cfg(test)] mod tests { use super::*; + use crate::db::types::krate_name::KrateName; use crate::test::{ AxumResponseTestExt, AxumRouterTestExt, FakeBuild, TestDatabase, TestEnvironment, async_wrapper, fake_release_that_failed_before_build, @@ -724,6 +739,7 @@ mod tests { use kuchikiki::traits::TendrilSink; use pretty_assertions::assert_eq; use std::collections::HashMap; + use std::str::FromStr as _; use test_case::test_case; async fn release_build_status( @@ -1163,8 +1179,16 @@ mod tests { .create() .await?; - let response = env.web_app().await.get("/crate/foo/0.0.1").await?; - response.assert_cache_control(CachePolicy::ForeverInCdnAndStaleInBrowser, env.config()); + let krate: KrateName = "foo".parse().unwrap(); + let response = env + .web_app() + .await + .get(&format!("/crate/{krate}/0.0.1")) + .await?; + response.assert_cache_control( + CachePolicy::ForeverInCdnAndStaleInBrowser(krate.into()), + env.config(), + ); assert!( response @@ -1868,7 +1892,10 @@ mod tests { "{}", response.text().await.unwrap() ); - response.assert_cache_control(CachePolicy::ForeverInCdn, env.config()); + response.assert_cache_control( + CachePolicy::ForeverInCdn(KrateName::from_str("dummy").unwrap().into()), + env.config(), + ); let list2 = dbg!(check_links( response.text().await.unwrap(), true, @@ -2045,7 +2072,10 @@ mod tests { let resp = web.get("/crate/dummy/latest").await?; assert!(resp.status().is_success()); - resp.assert_cache_control(CachePolicy::ForeverInCdn, env.config()); + resp.assert_cache_control( + CachePolicy::ForeverInCdn(KrateName::from_str("dummy").unwrap().into()), + env.config(), + ); let body = resp.text().await?; assert!(body.contains(">) -> Self { self.update(|mut params| { - params.confirmed_name = confirmed_name.map(Into::into); + let confirmed_name = confirmed_name.map(Into::into); + + if let Some(ref confirmed_name) = confirmed_name { + params.name = confirmed_name.to_string(); + } + + params.confirmed_name = confirmed_name; params }) } diff --git a/src/web/features.rs b/src/web/features.rs index 633d5d5b8..aee4b310d 100644 --- a/src/web/features.rs +++ b/src/web/features.rs @@ -108,10 +108,13 @@ impl FeaturesPage { impl_axum_webpage! { FeaturesPage, - cache_policy = |page| if page.is_latest_url { - CachePolicy::ForeverInCdn - } else { - CachePolicy::ForeverInCdnAndStaleInBrowser + cache_policy = |page| { + let name = &page.metadata.name; + if page.is_latest_url { + CachePolicy::ForeverInCdn(name.into()) + } else { + CachePolicy::ForeverInCdnAndStaleInBrowser(name.into()) + } }, } @@ -146,10 +149,14 @@ pub(crate) async fn build_features_handler( let matched_release = match_version(&mut conn, params.name(), params.req_version()) .await? .assume_exact_name()? - .into_canonical_req_version_or_else(|version| { + .into_canonical_req_version_or_else(|confirmed_name, version| { + let params = params + .clone() + .with_confirmed_name(Some(confirmed_name)) + .with_req_version(version); AxumNope::Redirect( - params.clone().with_req_version(version).features_url(), - CachePolicy::ForeverInCdn, + params.features_url(), + CachePolicy::ForeverInCdn(confirmed_name.into()), ) })?; let params = params.apply_matched_release(&matched_release); @@ -264,9 +271,13 @@ fn get_sorted_features(raw_features: Vec) -> (Vec, HashSet Self { + if s.is_empty() { + panic!("surrogate key cannot be empty"); + } + if s.len() > 1024 { + panic!("surrogate key exceeds maximum length of 1024 bytes"); + } + + let bytes = s.as_bytes(); + let mut i = 0; + while i < bytes.len() { + let b = bytes[i]; + if b < 0x21 || b > 0x7E { + panic!("invalid character found in surrogate key"); + } + i += 1; + } + + SurrogateKey(HeaderValue::from_static(s)) + } +} + impl FromStr for SurrogateKey { type Err = anyhow::Error; @@ -73,9 +95,36 @@ impl From for SurrogateKey { } } -/// A full Fastly Surrogate-Key header, containing one or more keys. -#[derive(Debug, PartialEq)] -pub struct SurrogateKeys(Vec); +/// A full Fastly Surrogate-Key header, containing zero or more keys. +#[derive(Debug, PartialEq, Clone)] +pub struct SurrogateKeys(BTreeSet); + +impl IntoIterator for SurrogateKeys { + type Item = SurrogateKey; + type IntoIter = as IntoIterator>::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.0.into_iter() + } +} + +impl From for SurrogateKeys { + fn from(key: SurrogateKey) -> Self { + SurrogateKeys(BTreeSet::from_iter(vec![key])) + } +} + +impl From for SurrogateKeys { + fn from(name: KrateName) -> Self { + SurrogateKey::from(name).into() + } +} + +impl From<&KrateName> for SurrogateKeys { + fn from(name: &KrateName) -> Self { + SurrogateKey::from(name.clone()).into() + } +} impl Display for SurrogateKeys { #[allow(unstable_name_collisions)] @@ -101,12 +150,26 @@ impl Header for SurrogateKeys { &SURROGATE_KEY } - fn decode<'i, I>(_values: &mut I) -> Result + fn decode<'i, I>(values: &mut I) -> Result where Self: Sized, I: Iterator, { - unimplemented!(); + let Some(value) = values.next() else { + return Err(headers::Error::invalid()); + }; + + let Ok(value) = value.to_str() else { + return Err(headers::Error::invalid()); + }; + + let keys = value + .split(' ') + .map(SurrogateKey::from_str) + .collect::, _>>() + .map_err(|_| headers::Error::invalid())?; + + Ok(SurrogateKeys(keys)) } fn encode>(&self, values: &mut E) { @@ -119,10 +182,14 @@ impl Header for SurrogateKeys { } impl SurrogateKeys { - /// Build SurrogateKeys from an iterator, de-duplicating keys. - /// Takes only as many elements as would fit into the header, - /// then stops consuming the iterator. - pub fn from_iter_until_full(iter: I) -> Self + pub fn new() -> Self { + Self(BTreeSet::new()) + } + + // Build SurrogateKeys from an iterator, de-duplicating keys. + // Takes only as many elements as would fit into the header, + // then stops consuming the iterator. + pub fn extend_until_full(&mut self, iter: I) where I: IntoIterator, { @@ -135,27 +202,55 @@ impl SurrogateKeys { const MAX_LEN: u64 = 16_384; - let mut current_key_size: u64 = 0; - - SurrogateKeys( - iter.into_iter() - .unique() - .take_while(|key| { - let key_size = key.len() as u64 + 1; // +1 for the space or terminator - if current_key_size + key_size > MAX_LEN { - false - } else { - current_key_size += key_size; - true - } - }) - .collect(), - ) + let mut current_key_size: u64 = self.encoded_len(); + + self.0.extend(iter.into_iter().take_while(|key| { + let key_size = key.len() as u64 + 1; // +1 for the space or terminator + if current_key_size + key_size > MAX_LEN { + false + } else { + current_key_size += key_size; + true + } + })) + } + + pub fn from_iter_until_full(iter: I) -> Self + where + I: IntoIterator, + { + let mut keys = SurrogateKeys::new(); + keys.extend_until_full(iter); + keys + } + + pub fn try_extend(&mut self, iter: I) -> anyhow::Result<()> + where + I: IntoIterator, + { + let mut iter = iter.into_iter().peekable(); + + self.extend_until_full(&mut iter); + + if iter.peek().is_some() { + bail!("adding surrogate key would exceed maximum header length of 16,384 bytes"); + } + + Ok(()) } #[cfg(test)] - pub fn encoded_len(&self) -> usize { - self.0.iter().map(|k| k.0.len() + 1).sum::() + pub fn try_from_iter(iter: I) -> anyhow::Result + where + I: IntoIterator, + { + let mut keys = SurrogateKeys::new(); + keys.try_extend(iter)?; + Ok(keys) + } + + pub fn encoded_len(&self) -> u64 { + self.0.iter().map(|k| (k.0.len() + 1) as u64).sum::() } pub fn key_count(&self) -> usize { @@ -171,7 +266,7 @@ impl FromStr for SurrogateKeys { let keys = s .split(' ') .map(SurrogateKey::from_str) - .collect::, _>>()?; + .collect::, _>>()?; Ok(SurrogateKeys(keys)) } } @@ -179,7 +274,7 @@ impl FromStr for SurrogateKeys { #[cfg(test)] mod tests { use super::*; - use crate::test::headers::test_typed_encode; + use crate::test::headers::{test_typed_decode, test_typed_encode}; use std::ops::RangeInclusive; use test_case::test_case; @@ -189,6 +284,14 @@ mod tests { assert!(SurrogateKey::from_str(&input).is_err()); } + #[test] + #[should_panic] + fn test_parse_surrogate_key_too_long_const() { + const INPUT: [u8; 1025] = [b'X'; 1025]; + let input = std::str::from_utf8(&INPUT).unwrap(); + SurrogateKey::from_static(input); + } + #[test_case(""; "empty")] #[test_case(" "; "space")] #[test_case("\n"; "newline")] @@ -200,8 +303,9 @@ mod tests { #[test_case("1234")] #[test_case("crate-some-crate")] #[test_case("release-some-crate-1.2.3")] - fn test_parse_surrogate_key_ok(input: &str) { + fn test_parse_surrogate_key_ok(input: &'static str) { assert_eq!(SurrogateKey::from_str(input).unwrap(), input); + assert_eq!(SurrogateKey::from_static(input), input); } #[test] @@ -217,7 +321,20 @@ mod tests { assert_eq!( test_typed_encode(SurrogateKeys::from_iter_until_full([k1, k2, k3])), - "key-2 key-1" + "key-1 key-2" + ); + + Ok(()) + } + + #[test] + fn test_decode() -> anyhow::Result<()> { + assert_eq!( + test_typed_decode::("key-1 key-2 key-2")?.unwrap(), + SurrogateKeys::from_iter_until_full([ + SurrogateKey::from_str("key-2").unwrap(), + SurrogateKey::from_str("key-1").unwrap(), + ]), ); Ok(()) diff --git a/src/web/mod.rs b/src/web/mod.rs index 940428a15..19398cae1 100644 --- a/src/web/mod.rs +++ b/src/web/mod.rs @@ -241,7 +241,7 @@ impl MatchedRelease { fn into_exactly_named_or_else(self, f: F) -> Result where - F: FnOnce(&str, &ReqVersion) -> AxumNope, + F: FnOnce(&KrateName, &ReqVersion) -> AxumNope, { if let Some(corrected_name) = self.corrected_name { Err(f(&corrected_name, &self.req_version)) @@ -278,7 +278,7 @@ impl MatchedRelease { /// version specification. fn into_canonical_req_version_or_else(self, f: F) -> Result where - F: FnOnce(&ReqVersion) -> AxumNope, + F: FnOnce(&KrateName, &ReqVersion) -> AxumNope, { let original_req_version = self.req_version.clone(); let canonicalized = self.into_canonical_req_version(); @@ -286,7 +286,7 @@ impl MatchedRelease { if canonicalized.req_version == original_req_version { Ok(canonicalized) } else { - Err(f(&canonicalized.req_version)) + Err(f(&canonicalized.name, &canonicalized.req_version)) } } diff --git a/src/web/rustdoc.rs b/src/web/rustdoc.rs index 098ff008b..ffdce20b0 100644 --- a/src/web/rustdoc.rs +++ b/src/web/rustdoc.rs @@ -2,6 +2,7 @@ use crate::{ AsyncStorage, BUILD_VERSION, Config, RUSTDOC_STATIC_STORAGE_PREFIX, + db::types::krate_name::KrateName, registry_api::OwnerKind, storage::{ CompressionAlgorithm, RustdocJsonFormatVersion, StreamingBlob, rustdoc_archive_path, @@ -303,10 +304,11 @@ pub(crate) async fn rustdoc_redirector_handler( if let Some(description) = DOC_RUST_LANG_ORG_REDIRECTS.get(&*crate_name) { let target_uri = EscapedURI::from_uri(description.href.clone()).append_raw_query(original_query); + let krate_name: KrateName = crate_name.parse().expect("we know these are valid"); return redirect_to_doc( params.original_uri(), target_uri, - CachePolicy::ForeverInCdnAndStaleInBrowser, + CachePolicy::ForeverInCdnAndStaleInBrowser(krate_name.into()), path_in_crate.as_deref(), ); } @@ -376,9 +378,9 @@ pub(crate) async fn rustdoc_redirector_handler( params.original_uri(), params.rustdoc_url().append_raw_query(original_query), if matched_release.is_latest_url() { - CachePolicy::ForeverInCdn + CachePolicy::ForeverInCdn(crate_name.into()) } else { - CachePolicy::ForeverInCdnAndStaleInBrowser + CachePolicy::ForeverInCdnAndStaleInBrowser(crate_name.into()) }, path_in_crate.as_deref(), )? @@ -386,7 +388,7 @@ pub(crate) async fn rustdoc_redirector_handler( } else { Ok(axum_cached_redirect( params.crate_details_url().append_raw_query(original_query), - CachePolicy::ForeverInCdn, + CachePolicy::ForeverInCdn(crate_name.into()), )? .into_response()) } @@ -530,10 +532,12 @@ impl RustdocPage { max_parse_memory: usize, if_none_match: Option<&IfNoneMatch>, ) -> AxumResponse { + let crate_name = &self.metadata.name; + let cache_policy = if self.is_latest_url { - CachePolicy::ForeverInCdn + CachePolicy::ForeverInCdn(crate_name.into()) } else { - CachePolicy::ForeverInCdnAndStaleInBrowser + CachePolicy::ForeverInCdnAndStaleInBrowser(crate_name.into()) }; let robots_tag = (!self.is_latest_url).then_some([(&X_ROBOTS_TAG, "noindex")]); @@ -614,26 +618,31 @@ pub(crate) async fn rustdoc_html_server_handler( AxumNope::Redirect( params .clone() - .with_name(corrected_name) + .with_confirmed_name(Some(corrected_name)) .with_req_version(req_version) .rustdoc_url() .append_raw_query(original_query.as_deref()), CachePolicy::NoCaching, ) })? - .into_canonical_req_version_or_else(|version| { + .into_canonical_req_version_or_else(|confirmed_name, version| { + let params = params + .clone() + .with_confirmed_name(Some(confirmed_name)) + .with_req_version(version); AxumNope::Redirect( - params.clone().with_req_version(version).rustdoc_url(), - CachePolicy::ForeverInCdn, + params.rustdoc_url(), + CachePolicy::ForeverInCdn(confirmed_name.into()), ) })?; let params = params.apply_matched_release(&matched_release); if !matched_release.rustdoc_status() { - return Ok( - axum_cached_redirect(params.crate_details_url(), CachePolicy::ForeverInCdn)? - .into_response(), - ); + return Ok(axum_cached_redirect( + params.crate_details_url(), + CachePolicy::ForeverInCdn(matched_release.name.into()), + )? + .into_response()); } let krate = CrateDetails::from_matched_release(&mut conn, matched_release).await?; @@ -653,7 +662,7 @@ pub(crate) async fn rustdoc_html_server_handler( params .rustdoc_url() .append_raw_query(original_query.as_deref()), - CachePolicy::ForeverInCdn, + CachePolicy::ForeverInCdn(krate.name.into()), )?); } @@ -708,7 +717,7 @@ pub(crate) async fn rustdoc_html_server_handler( params .rustdoc_url() .append_raw_query(original_query.as_deref()), - CachePolicy::ForeverInCdn, + CachePolicy::ForeverInCdn(krate.name.into()), )?); } } @@ -721,7 +730,7 @@ pub(crate) async fn rustdoc_html_server_handler( // but only if the first element after the version is a target? return Ok(axum_cached_redirect( params.target_redirect_url(), - CachePolicy::ForeverInCdn, + CachePolicy::ForeverInCdn(krate.name.into()), )?); } @@ -821,7 +830,7 @@ pub(crate) async fn target_redirect_handler( let matched_release = match_version(&mut conn, params.name(), params.req_version()) .await? - .into_canonical_req_version_or_else(|_| AxumNope::VersionNotFound)?; + .into_canonical_req_version_or_else(|_, _| AxumNope::VersionNotFound)?; let params = params.apply_matched_release(&matched_release); let crate_details = CrateDetails::from_matched_release(&mut conn, matched_release).await?; @@ -854,9 +863,9 @@ pub(crate) async fn target_redirect_handler( Ok(axum_cached_redirect( redirect_uri, if params.req_version().is_latest() { - CachePolicy::ForeverInCdn + CachePolicy::ForeverInCdn(crate_details.name.into()) } else { - CachePolicy::ForeverInCdnAndStaleInBrowser + CachePolicy::ForeverInCdnAndStaleInBrowser(crate_details.name.into()) }, )?) } @@ -901,13 +910,18 @@ pub(crate) async fn json_download_handler( let matched_release = match_version(&mut conn, params.name(), params.req_version()) .await? .assume_exact_name()? - .into_canonical_req_version_or_else(|version| { + .into_canonical_req_version_or_else(|confirmed_name, version| { + let params = params + .clone() + .with_confirmed_name(Some(confirmed_name)) + .with_req_version(version); + AxumNope::Redirect( - params.clone().with_req_version(version).json_download_url( + params.json_download_url( wanted_compression.clone().map(|c| c.0), json_params.format_version.as_deref(), ), - CachePolicy::ForeverInCdn, + CachePolicy::ForeverInCdn(confirmed_name.into()), ) })?; @@ -1002,7 +1016,9 @@ pub(crate) async fn json_download_handler( // StreamingFile::into_response automatically set the default cache-policy for // static assets (ForeverInCdnAndBrowser). // Here we override it with the standard policy for build output. - response.extensions_mut().insert(CachePolicy::ForeverInCdn); + response + .extensions_mut() + .insert(CachePolicy::ForeverInCdn(krate.name.clone().into())); // set content-disposition to attachment to trigger download in browsers // For the attachment filename we can use just the filename without the path, @@ -1020,23 +1036,28 @@ pub(crate) async fn json_download_handler( #[instrument(skip_all)] pub(crate) async fn download_handler( - params: RustdocParams, + mut params: RustdocParams, mut conn: DbConnection, Extension(storage): Extension>, if_none_match: Option>, ) -> AxumResult { - let version = match_version(&mut conn, params.name(), params.req_version()) + let matched_release = match_version(&mut conn, params.name(), params.req_version()) .await? .assume_exact_name()? - .into_canonical_req_version_or_else(|version| { + .into_canonical_req_version_or_else(|confirmed_name, version| { + let params = params + .clone() + .with_confirmed_name(Some(confirmed_name)) + .with_req_version(version); AxumNope::Redirect( - params.clone().with_req_version(version).zip_download_url(), - CachePolicy::ForeverInCdn, + params.zip_download_url(), + CachePolicy::ForeverInCdn(confirmed_name.into()), ) - })? - .into_version(); + })?; + params = params.apply_matched_release(&matched_release); - let archive_path = rustdoc_archive_path(params.name(), &version); + let version = &matched_release.release.version; + let archive_path = rustdoc_archive_path(params.name(), version); let mut response = StreamingFile(storage.get_raw_stream(&archive_path).await?) .into_response(if_none_match.as_deref()); @@ -1044,7 +1065,9 @@ pub(crate) async fn download_handler( // StreamingFile::into_response automatically set the default cache-policy for // static assets (ForeverInCdnAndBrowser). // Here we override it with the standard policy for build output. - response.extensions_mut().insert(CachePolicy::ForeverInCdn); + response + .extensions_mut() + .insert(CachePolicy::ForeverInCdn(matched_release.name.into())); // set content-disposition to attachment to trigger download in browsers response.headers_mut().insert( @@ -1090,7 +1113,7 @@ mod test { use kuchikiki::traits::TendrilSink; use pretty_assertions::assert_eq; use reqwest::StatusCode; - use std::{collections::BTreeMap, io}; + use std::{collections::BTreeMap, io, str::FromStr as _}; use test_case::test_case; use tracing::info; @@ -1108,13 +1131,17 @@ mod test { } async fn try_latest_version_redirect( + krate: &str, path: &str, web: &axum::Router, config: &Config, ) -> Result, anyhow::Error> { web.assert_success(path).await?; let response = web.get(path).await?; - response.assert_cache_control(CachePolicy::ForeverInCdnAndStaleInBrowser, config); + response.assert_cache_control( + CachePolicy::ForeverInCdnAndStaleInBrowser(KrateName::from_str(krate).unwrap().into()), + config, + ); let data = response.text().await?; info!( "fetched path {} and got content {}\nhelp: if this is missing the header, remember to add ", @@ -1129,7 +1156,10 @@ mod test { { let link = elem.attributes.borrow().get("href").unwrap().to_string(); let response = web.get(&link).await?; - response.assert_cache_control(CachePolicy::ForeverInCdn, config); + response.assert_cache_control( + CachePolicy::ForeverInCdn(KrateName::from_str(krate).unwrap().into()), + config, + ); assert!(response.status().is_success() || response.status().is_redirection()); Ok(Some(link)) } else { @@ -1138,11 +1168,12 @@ mod test { } async fn latest_version_redirect( + krate: &str, path: &str, web: &axum::Router, config: &Config, ) -> Result { - try_latest_version_redirect(path, web, config) + try_latest_version_redirect(krate, path, web, config) .await? .with_context(|| anyhow::anyhow!("no redirect found for {}", path)) } @@ -1163,7 +1194,9 @@ mod test { let web = env.web_app().await; web.assert_success_cached( "/krate/0.1.0/help.html", - CachePolicy::ForeverInCdnAndStaleInBrowser, + CachePolicy::ForeverInCdnAndStaleInBrowser( + KrateName::from_str("krate").unwrap().into(), + ), env.config(), ) .await?; @@ -1207,19 +1240,25 @@ mod test { .await?; web.assert_success_cached( "/crate/buggy/0.1.0", - CachePolicy::ForeverInCdnAndStaleInBrowser, + CachePolicy::ForeverInCdnAndStaleInBrowser( + KrateName::from_str("buggy").unwrap().into(), + ), env.config(), ) .await?; web.assert_success_cached( "/buggy/0.1.0/directory_1/index.html", - CachePolicy::ForeverInCdnAndStaleInBrowser, + CachePolicy::ForeverInCdnAndStaleInBrowser( + KrateName::from_str("buggy").unwrap().into(), + ), env.config(), ) .await?; web.assert_success_cached( "/buggy/0.1.0/directory_2.html/index.html", - CachePolicy::ForeverInCdnAndStaleInBrowser, + CachePolicy::ForeverInCdnAndStaleInBrowser( + KrateName::from_str("buggy").unwrap().into(), + ), env.config(), ) .await?; @@ -1231,19 +1270,25 @@ mod test { .await?; web.assert_success_cached( "/buggy/0.1.0/settings.html", - CachePolicy::ForeverInCdnAndStaleInBrowser, + CachePolicy::ForeverInCdnAndStaleInBrowser( + KrateName::from_str("buggy").unwrap().into(), + ), env.config(), ) .await?; web.assert_success_cached( "/buggy/0.1.0/scrape-examples-help.html", - CachePolicy::ForeverInCdnAndStaleInBrowser, + CachePolicy::ForeverInCdnAndStaleInBrowser( + KrateName::from_str("buggy").unwrap().into(), + ), env.config(), ) .await?; web.assert_success_cached( "/buggy/0.1.0/all.html", - CachePolicy::ForeverInCdnAndStaleInBrowser, + CachePolicy::ForeverInCdnAndStaleInBrowser( + KrateName::from_str("buggy").unwrap().into(), + ), env.config(), ) .await?; @@ -1275,14 +1320,16 @@ mod test { let base = "/dummy/0.1.0/dummy/"; web.assert_success_cached( base, - CachePolicy::ForeverInCdnAndStaleInBrowser, + CachePolicy::ForeverInCdnAndStaleInBrowser( + KrateName::from_str("dummy").unwrap().into(), + ), env.config(), ) .await?; web.assert_redirect_cached( "/dummy/0.1.0/x86_64-unknown-linux-gnu/dummy/", base, - CachePolicy::ForeverInCdn, + CachePolicy::ForeverInCdn(KrateName::from_str("dummy").unwrap().into()), env.config(), ) .await?; @@ -1353,7 +1400,10 @@ mod test { .await? .error_for_status()?; - resp.assert_cache_control(CachePolicy::ForeverInCdn, env.config()); + resp.assert_cache_control( + CachePolicy::ForeverInCdn(KrateName::from_str("dummy").unwrap().into()), + env.config(), + ); let body = resp.text().await?; assert!( body.contains(" Vec { @@ -422,7 +434,9 @@ mod tests { let web = env.web_app().await; web.assert_success_cached( "/crate/fake/0.1.0/source/", - CachePolicy::ForeverInCdnAndStaleInBrowser, + CachePolicy::ForeverInCdnAndStaleInBrowser( + KrateName::from_str("fake").unwrap().into(), + ), env.config(), ) .await?; @@ -432,7 +446,12 @@ mod tests { response.headers().get("link").unwrap(), "; rel=\"canonical\"" ); - response.assert_cache_control(CachePolicy::ForeverInCdnAndStaleInBrowser, env.config()); + response.assert_cache_control( + CachePolicy::ForeverInCdnAndStaleInBrowser( + KrateName::from_str("fake").unwrap().into(), + ), + env.config(), + ); assert!(response.text().await?.contains("some_random_content")); Ok(()) }); @@ -466,7 +485,12 @@ mod tests { headers.typed_get::().unwrap(), APPLICATION_PDF.into(), ); - response.assert_cache_control(CachePolicy::ForeverInCdnAndStaleInBrowser, env.config()); + response.assert_cache_control( + CachePolicy::ForeverInCdnAndStaleInBrowser( + KrateName::from_str("fake").unwrap().into(), + ), + env.config(), + ); let etag: ETag = headers.typed_get().unwrap(); @@ -553,7 +577,10 @@ mod tests { .await .get("/crate/fake/latest/source/") .await?; - resp.assert_cache_control(CachePolicy::ForeverInCdn, env.config()); + resp.assert_cache_control( + CachePolicy::ForeverInCdn(KrateName::from_str("fake").unwrap().into()), + env.config(), + ); let body = resp.text().await?; assert!(body.contains("