diff --git a/Cargo.lock b/Cargo.lock index b1382659d..411618e8b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1427,7 +1427,7 @@ dependencies = [ [[package]] name = "edgezero-adapter" version = "0.1.0" -source = "git+https://github.com/stackpop/edgezero?tag=v0.0.7#5c9886e51d17e6969531356bacdf27f144ac8a2e" +source = "git+https://github.com/stackpop/edgezero?rev=9c03cc59300363ae5339fc970dded08ede563e98#9c03cc59300363ae5339fc970dded08ede563e98" dependencies = [ "toml", ] @@ -1435,7 +1435,7 @@ dependencies = [ [[package]] name = "edgezero-adapter-axum" version = "0.1.0" -source = "git+https://github.com/stackpop/edgezero?tag=v0.0.7#5c9886e51d17e6969531356bacdf27f144ac8a2e" +source = "git+https://github.com/stackpop/edgezero?rev=9c03cc59300363ae5339fc970dded08ede563e98#9c03cc59300363ae5339fc970dded08ede563e98" dependencies = [ "anyhow", "async-trait", @@ -1463,7 +1463,7 @@ dependencies = [ [[package]] name = "edgezero-adapter-cloudflare" version = "0.1.0" -source = "git+https://github.com/stackpop/edgezero?tag=v0.0.7#5c9886e51d17e6969531356bacdf27f144ac8a2e" +source = "git+https://github.com/stackpop/edgezero?rev=9c03cc59300363ae5339fc970dded08ede563e98#9c03cc59300363ae5339fc970dded08ede563e98" dependencies = [ "anyhow", "async-trait", @@ -1486,7 +1486,7 @@ dependencies = [ [[package]] name = "edgezero-adapter-fastly" version = "0.1.0" -source = "git+https://github.com/stackpop/edgezero?tag=v0.0.7#5c9886e51d17e6969531356bacdf27f144ac8a2e" +source = "git+https://github.com/stackpop/edgezero?rev=9c03cc59300363ae5339fc970dded08ede563e98#9c03cc59300363ae5339fc970dded08ede563e98" dependencies = [ "anyhow", "async-stream", @@ -1515,7 +1515,7 @@ dependencies = [ [[package]] name = "edgezero-adapter-spin" version = "0.1.0" -source = "git+https://github.com/stackpop/edgezero?tag=v0.0.7#5c9886e51d17e6969531356bacdf27f144ac8a2e" +source = "git+https://github.com/stackpop/edgezero?rev=9c03cc59300363ae5339fc970dded08ede563e98#9c03cc59300363ae5339fc970dded08ede563e98" dependencies = [ "anyhow", "async-trait", @@ -1542,7 +1542,7 @@ dependencies = [ [[package]] name = "edgezero-cli" version = "0.1.0" -source = "git+https://github.com/stackpop/edgezero?tag=v0.0.7#5c9886e51d17e6969531356bacdf27f144ac8a2e" +source = "git+https://github.com/stackpop/edgezero?rev=9c03cc59300363ae5339fc970dded08ede563e98#9c03cc59300363ae5339fc970dded08ede563e98" dependencies = [ "chrono", "clap", @@ -1567,7 +1567,7 @@ dependencies = [ [[package]] name = "edgezero-core" version = "0.1.0" -source = "git+https://github.com/stackpop/edgezero?tag=v0.0.7#5c9886e51d17e6969531356bacdf27f144ac8a2e" +source = "git+https://github.com/stackpop/edgezero?rev=9c03cc59300363ae5339fc970dded08ede563e98#9c03cc59300363ae5339fc970dded08ede563e98" dependencies = [ "anyhow", "async-compression", @@ -1598,7 +1598,7 @@ dependencies = [ [[package]] name = "edgezero-macros" version = "0.1.0" -source = "git+https://github.com/stackpop/edgezero?tag=v0.0.7#5c9886e51d17e6969531356bacdf27f144ac8a2e" +source = "git+https://github.com/stackpop/edgezero?rev=9c03cc59300363ae5339fc970dded08ede563e98#9c03cc59300363ae5339fc970dded08ede563e98" dependencies = [ "log", "proc-macro2", diff --git a/Cargo.toml b/Cargo.toml index d6ecdb9ca..39462c7fb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -55,12 +55,14 @@ cssparser = "0.36" derive_more = { version = "2.0", features = ["display", "error"] } directories = "5" ed25519-dalek = { version = "2.2", features = ["rand_core"] } -edgezero-adapter-axum = { git = "https://github.com/stackpop/edgezero", tag = "v0.0.7", default-features = false } -edgezero-adapter-cloudflare = { git = "https://github.com/stackpop/edgezero", tag = "v0.0.7", default-features = false } -edgezero-adapter-fastly = { git = "https://github.com/stackpop/edgezero", tag = "v0.0.7", default-features = false } -edgezero-adapter-spin = { git = "https://github.com/stackpop/edgezero", tag = "v0.0.7", default-features = false } -edgezero-cli = { git = "https://github.com/stackpop/edgezero", tag = "v0.0.7" } -edgezero-core = { git = "https://github.com/stackpop/edgezero", tag = "v0.0.7", default-features = false } +# Temporary integration pin for EdgeZero PR #389. Before merging TS into main, +# replace all six pins with the approved EdgeZero release tag and revalidate. +edgezero-adapter-axum = { git = "https://github.com/stackpop/edgezero", rev = "9c03cc59300363ae5339fc970dded08ede563e98", default-features = false } +edgezero-adapter-cloudflare = { git = "https://github.com/stackpop/edgezero", rev = "9c03cc59300363ae5339fc970dded08ede563e98", default-features = false } +edgezero-adapter-fastly = { git = "https://github.com/stackpop/edgezero", rev = "9c03cc59300363ae5339fc970dded08ede563e98", default-features = false } +edgezero-adapter-spin = { git = "https://github.com/stackpop/edgezero", rev = "9c03cc59300363ae5339fc970dded08ede563e98", default-features = false } +edgezero-cli = { git = "https://github.com/stackpop/edgezero", rev = "9c03cc59300363ae5339fc970dded08ede563e98" } +edgezero-core = { git = "https://github.com/stackpop/edgezero", rev = "9c03cc59300363ae5339fc970dded08ede563e98", default-features = false } env_logger = "0.11" error-stack = "0.6" esi = "0.7.2" diff --git a/crates/trusted-server-adapter-axum/src/timing.rs b/crates/trusted-server-adapter-axum/src/timing.rs index 315ebb5ef..bcf873b8f 100644 --- a/crates/trusted-server-adapter-axum/src/timing.rs +++ b/crates/trusted-server-adapter-axum/src/timing.rs @@ -1,9 +1,10 @@ //! Terminal timing layer for the Axum dev server. //! //! [`TimingService`](crate::timing::TimingService) wraps the tower `Service` -//! boundary the Axum dev server's router sits behind: it creates a +//! boundary the Axum dev server's router sits behind: it reuses the generic +//! handle in request extensions or creates one, exposing it through the //! [`RequestTimings`](trusted_server_core::request_timing::RequestTimings) -//! collector per request, threads it through request extensions so +//! facade so //! downstream core handlers can record into it, and on the way back stamps //! `mark_headers_ready` and appends the `Server-Timing` header via //! [`append_server_timing_if_private`](trusted_server_core::request_timing::append_server_timing_if_private). @@ -21,8 +22,8 @@ //! //! `/health` is excluded by path match before a //! [`RequestTimings`](trusted_server_core::request_timing::RequestTimings) -//! collector is even created: health checks never carry timing data on any -//! adapter. +//! collector is even created: this wrapper bypasses timing for every method +//! on that path, leaving any preinstalled handle unchanged. //! //! Unlike the Fastly adapter (state built per request, adding //! `Phase::AppBuild` to the rendered header), the Axum dev server builds its @@ -40,7 +41,7 @@ use tower::Service; use trusted_server_core::request_timing::{RequestTimings, append_server_timing_if_private}; /// Path excluded from timing collection and `Server-Timing` emission: health -/// checks never carry timing data on any adapter. +/// checks bypass this wrapper for every method. const HEALTH_PATH: &str = "/health"; /// Wraps an inner Axum tower service with the request-phase timing freeze @@ -78,15 +79,18 @@ where fn call(&mut self, mut req: Request) -> Self::Future { let mut inner = self.inner.clone(); - // Excluded before a collector is even created: `/health` never - // carries timing data, on any adapter. + // Bypass installation and finalization for `/health`, regardless of method. + // A preinstalled collector remains available to the inner service. if req.uri().path() == HEALTH_PATH { return Box::pin(async move { inner.call(req).await }); } let server_timing_enabled = self.server_timing_enabled; - let timings = RequestTimings::new(); - req.extensions_mut().insert(timings.clone()); + let timings = RequestTimings::from_extensions(req.extensions()).unwrap_or_else(|| { + let timings = RequestTimings::new(); + req.extensions_mut().insert(timings.handle().clone()); + timings + }); Box::pin(async move { let mut response = inner.call(req).await?; @@ -132,6 +136,69 @@ mod tests { .map(ToOwned::to_owned) } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn axum_preserves_preinstalled_collector_and_origin() { + let timings = RequestTimings::new(); + timings.record( + trusted_server_core::request_timing::Phase::Filter, + std::time::Duration::from_millis(7), + ); + tokio::time::sleep(std::time::Duration::from_millis(20)).await; + let router = RouterService::builder() + .get("/private", |ctx: RequestContext| async move { + let timings = RequestTimings::from_extensions(ctx.request().extensions()) + .expect("should receive preinstalled collector"); + assert_eq!( + timings.snapshot().filter_ms, + Some(7), + "should preserve upstream phase" + ); + timings.mark_auction_dispatched(); + assert!( + timings + .snapshot() + .auction_dispatched_ms + .expect("should mark dispatch") + >= 20, + "should preserve the pre-aged origin" + ); + private_ok_response() + }) + .build(); + let terminal = TimingService::new(EdgeZeroAxumService::new(router), true); + let upstream_timings = timings.clone(); + let mut upstream = service_fn(move |mut req: Request| { + req.extensions_mut() + .insert(upstream_timings.handle().clone()); + let mut service = terminal.clone(); + async move { service.call(req).await } + }); + let request = Request::builder() + .uri("/private") + .body(AxumBody::empty()) + .expect("should build request"); + let response = upstream + .ready() + .await + .expect("should be ready") + .call(request) + .await + .expect("should handle request"); + let value = header(&response, "server-timing").expect("should render private timing"); + assert!( + value.contains("ts-filter;dur=7.0"), + "should render upstream phase: {value}" + ); + assert!( + timings + .snapshot() + .time_elapsed_ms + .expect("should stamp original handle") + >= 20, + "terminal rendering should use the original clock" + ); + } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn axum_emits_header_on_private_response() { let router = RouterService::builder() @@ -201,7 +268,7 @@ mod tests { // here instead of silently losing every phase. let router = RouterService::builder() .get("/private", |ctx: RequestContext| async move { - if let Some(timings) = ctx.request().extensions().get::() { + if let Some(timings) = RequestTimings::from_extensions(ctx.request().extensions()) { timings.record( trusted_server_core::request_timing::Phase::Filter, std::time::Duration::from_millis(7), @@ -283,28 +350,50 @@ mod tests { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn axum_health_is_excluded() { - let router = RouterService::builder() - .get("/health", |_ctx: RequestContext| async { - private_ok_response() - }) - .build(); - let mut service = TimingService::new(EdgeZeroAxumService::new(router), true); - - let request = Request::builder() - .uri("/health") - .body(AxumBody::empty()) - .expect("should build request"); - let response = service - .ready() - .await - .expect("should be ready") - .call(request) - .await - .expect("should not fail"); - - assert!( - header(&response, "server-timing").is_none(), - "/health must never carry a server-timing header" - ); + for method in [axum::http::Method::GET, axum::http::Method::POST] { + for path in ["/health", "/health?probe=1"] { + for preinstalled in [false, true] { + let timings = RequestTimings::new(); + let inner = service_fn(move |req: Request| async move { + assert_eq!( + RequestTimings::from_extensions(req.extensions()).is_some(), + preinstalled, + "health bypass should not install or remove a handle" + ); + Ok::<_, Infallible>( + Response::builder() + .header("cache-control", "private") + .body(AxumBody::empty()) + .expect("should build private response"), + ) + }); + let mut service = TimingService::new(inner, true); + let mut request = Request::builder() + .method(method.clone()) + .uri(path) + .body(AxumBody::empty()) + .expect("should build request"); + if preinstalled { + request.extensions_mut().insert(timings.handle().clone()); + } + let response = service + .ready() + .await + .expect("should be ready") + .call(request) + .await + .expect("should not fail"); + assert!( + header(&response, "server-timing").is_none(), + "health should not emit timing" + ); + assert_eq!( + timings.snapshot().time_elapsed_ms, + None, + "health bypass should not finalize timing" + ); + } + } + } } } diff --git a/crates/trusted-server-adapter-cloudflare/src/app.rs b/crates/trusted-server-adapter-cloudflare/src/app.rs index a3457ef7c..aab63514c 100644 --- a/crates/trusted-server-adapter-cloudflare/src/app.rs +++ b/crates/trusted-server-adapter-cloudflare/src/app.rs @@ -34,6 +34,7 @@ use trusted_server_core::publisher::{ use trusted_server_core::request_signing::{ handle_trusted_server_discovery, handle_verify_signature, }; +use trusted_server_core::request_timing::RequestTimingMiddleware; use trusted_server_core::settings::Settings; use crate::middleware::{AuthMiddleware, FinalizeResponseMiddleware, SanitizeRequestMiddleware}; @@ -467,6 +468,7 @@ fn build_router(state: &Arc) -> RouterService { // any middleware registered ahead of it would observe the // shared-secret authentication header. .middleware(SanitizeRequestMiddleware::new(Arc::clone(&state.settings))) + .middleware(RequestTimingMiddleware::default().with_excluded_paths(&["/health"])) .middleware(FinalizeResponseMiddleware::new(Arc::clone(&state.settings))) .middleware(AuthMiddleware::new(Arc::clone(&state.settings))) .get( diff --git a/crates/trusted-server-adapter-cloudflare/src/middleware.rs b/crates/trusted-server-adapter-cloudflare/src/middleware.rs index 14efed56a..51a5956d1 100644 --- a/crates/trusted-server-adapter-cloudflare/src/middleware.rs +++ b/crates/trusted-server-adapter-cloudflare/src/middleware.rs @@ -56,9 +56,9 @@ impl Middleware for SanitizeRequestMiddleware { /// (injected by the Cloudflare Workers runtime). On the native host target the /// header is absent, so `X-Geo-Info-Available: false` is emitted. /// -/// Registered directly inside [`SanitizeRequestMiddleware`] and ahead of -/// [`AuthMiddleware`] so that every outgoing response — including auth-rejected -/// ones — carries a consistent set of headers. +/// Registered inside [`RequestTimingMiddleware`](trusted_server_core::request_timing::RequestTimingMiddleware) and ahead of [`AuthMiddleware`] +/// so that every outgoing response — including auth-rejected ones — carries a +/// consistent set of headers. pub struct FinalizeResponseMiddleware { settings: Arc, } @@ -161,6 +161,8 @@ pub(crate) fn apply_finalize_headers( #[cfg(test)] mod tests { use super::*; + use edgezero_core::router::RouterService; + use trusted_server_core::request_timing::{RequestTimingMiddleware, RequestTimings}; use std::collections::HashMap; use std::sync::Mutex; @@ -180,10 +182,10 @@ mod tests { .expect("should build empty test response") } - fn empty_ctx() -> RequestContext { + fn ctx_for_path(path: &str) -> RequestContext { let req = request_builder() .method(Method::GET) - .uri("/test") + .uri(path) .header("x-reader-ip", "198.51.100.7") .header("x-reader-ip-auth", "fictional-shared-secret-0123456789") .body(Body::empty()) @@ -191,6 +193,10 @@ mod tests { RequestContext::new(req, PathParams::new(HashMap::new())) } + fn empty_ctx() -> RequestContext { + ctx_for_path("/test") + } + fn settings_with_response_headers(headers: Vec<(&str, &str)>) -> Settings { // Build from explicit test settings: the settings baked into the // binary contain placeholder secrets that `get_settings()` rejects @@ -289,6 +295,77 @@ mod tests { ); } + #[test] + fn request_timing_middleware_preserves_shared_handle_and_health_policy() { + for method in [Method::GET, Method::POST] { + for path in ["/test", "/health", "/health?probe=1", "/health/child"] { + for preinstalled in [false, true] { + let route_path = path.split('?').next().expect("should have path"); + let expected = preinstalled || route_path != "/health"; + let timings = RequestTimings::new(); + timings.record( + trusted_server_core::request_timing::Phase::Filter, + std::time::Duration::from_millis(7), + ); + std::thread::sleep(std::time::Duration::from_millis(2)); + let router = RouterService::builder() + .middleware( + RequestTimingMiddleware::default().with_excluded_paths(&["/health"]), + ) + .middleware( + RequestTimingMiddleware::default().with_excluded_paths(&["/health"]), + ) + .route(route_path, method.clone(), move |ctx: RequestContext| async move { + let installed = RequestTimings::from_extensions(ctx.request().extensions()); + assert_eq!(installed.is_some(), expected, "should preserve exact health policy"); + if let Some(installed) = installed { + if preinstalled { + assert_eq!(installed.snapshot().filter_ms, Some(7), "should retain upstream facts"); + installed.mark_auction_dispatched(); + assert!(installed.snapshot().auction_dispatched_ms.expect("should mark dispatch") >= 2, + "should retain upstream origin"); + } else { + assert_eq!(installed.snapshot(), trusted_server_core::request_timing::TimingSnapshot::default(), + "should install independent empty facts"); + } + installed.record_auction_wait( + trusted_server_core::request_timing::AuctionWaitPlacement::PreHeader, + std::time::Duration::from_millis(3)); + } + Ok::(empty_response()) + }) + .build(); + let mut request = request_builder() + .method(method.clone()) + .uri(path) + .body(Body::empty()) + .expect("should build request"); + if preinstalled { + request.extensions_mut().insert(timings.handle().clone()); + } + let response = + block_on(router.oneshot(request)).expect("should dispatch request"); + assert!( + response.headers().get("server-timing").is_none(), + "attachment should not expose timing" + ); + if preinstalled { + assert_eq!( + timings.snapshot().auction_wait_ms, + Some(3), + "should share handler updates" + ); + assert_eq!( + timings.snapshot().request_elapsed_ms, + None, + "should not fabricate completion" + ); + } + } + } + } + } + #[test] fn sanitize_middleware_strips_configured_trust_headers_before_routing() { let mut settings = settings_with_response_headers(vec![]); diff --git a/crates/trusted-server-adapter-fastly/src/app.rs b/crates/trusted-server-adapter-fastly/src/app.rs index b2221de8d..fdf84f762 100644 --- a/crates/trusted-server-adapter-fastly/src/app.rs +++ b/crates/trusted-server-adapter-fastly/src/app.rs @@ -440,11 +440,7 @@ fn build_ec_request_state( let eids_cookie = crate::extract_cookie_value(req, COOKIE_TS_EIDS); let sharedid_cookie = crate::extract_cookie_value(req, COOKIE_SHAREDID); - let timings = req - .extensions() - .get::() - .cloned() - .unwrap_or_default(); + let timings = RequestTimings::from_extensions(req.extensions()).unwrap_or_default(); let geo_info = { let _span = timings.span(Phase::Geo); services @@ -529,11 +525,7 @@ async fn run_pre_route_filters( ) -> PreRoute { // Only recorded when a filter is actually registered, so unconfigured // deployments omit ts-filter from the Server-Timing header entirely. - let timings = req - .extensions() - .get::() - .cloned() - .unwrap_or_default(); + let timings = RequestTimings::from_extensions(req.extensions()).unwrap_or_default(); let _span = state .registry .has_request_filters() @@ -609,11 +601,8 @@ async fn execute_named( // Deliberately do not use an EC request-state graph: that // copy is bot-gated, while operators use curl for this // authenticated diagnostic. - let timings = req - .extensions() - .get::() - .cloned() - .unwrap_or_default(); + let timings = + RequestTimings::from_extensions(req.extensions()).unwrap_or_default(); let kv = crate::identity_graph_with_timing(&state.settings, &timings); handle_admin_ec_lookup(kv.as_ref(), ®istry, &req) } @@ -685,11 +674,7 @@ async fn run_named_route( if req.method() == Method::OPTIONS { cors_preflight_identify(&state.settings, &req) } else { - let timings = req - .extensions() - .get::() - .cloned() - .unwrap_or_default(); + let timings = RequestTimings::from_extensions(req.extensions()).unwrap_or_default(); let kv = crate::require_identity_graph_with_timing(&state.settings, &timings)?; let partner_registry = PartnerRegistry::from_config(&state.settings.ec.partners)?; handle_identify( @@ -707,11 +692,7 @@ async fn run_named_route( // The auction reads consent data, so the consent KV store must be // available — fail closed with 503 when it is configured but // cannot be opened, matching legacy behavior. - let timings = req - .extensions() - .get::() - .cloned() - .unwrap_or_default(); + let timings = RequestTimings::from_extensions(req.extensions()).unwrap_or_default(); let consent_services = runtime_services_for_consent_route(&state.settings, services, &timings)?; let partner_registry = PartnerRegistry::from_config(&state.settings.ec.partners)?; @@ -741,11 +722,7 @@ async fn run_named_route( // Like the auction, page-bids reads consent data, so the consent KV // store must be available — fail closed with 503 when configured but // unopenable, matching legacy. - let timings = req - .extensions() - .get::() - .cloned() - .unwrap_or_default(); + let timings = RequestTimings::from_extensions(req.extensions()).unwrap_or_default(); let consent_services = runtime_services_for_consent_route(&state.settings, services, &timings)?; let partner_registry = PartnerRegistry::from_config(&state.settings.ec.partners)?; @@ -792,11 +769,7 @@ fn run_batch_sync(state: &AppState, services: &RuntimeServices, req: Request) -> let is_real_browser = device_signals.looks_like_browser(); let eids_cookie = crate::extract_cookie_value(&req, COOKIE_TS_EIDS); let sharedid_cookie = crate::extract_cookie_value(&req, COOKIE_SHAREDID); - let timings = req - .extensions() - .get::() - .cloned() - .unwrap_or_default(); + let timings = RequestTimings::from_extensions(req.extensions()).unwrap_or_default(); let result = crate::require_identity_graph_with_timing(&state.settings, &timings).and_then(|kv| { @@ -962,11 +935,7 @@ async fn dispatch_fallback( // Publisher pages read consent data, so the consent KV store must be // available — fail closed with 503 when it is configured but cannot // be opened, matching legacy behavior. - let timings = req - .extensions() - .get::() - .cloned() - .unwrap_or_default(); + let timings = RequestTimings::from_extensions(req.extensions()).unwrap_or_default(); match runtime_services_for_consent_route(&state.settings, services, &timings) { Ok(publisher_services) => { // Run the server-side auction with the configured creative- @@ -3558,7 +3527,7 @@ mod tests { .build(); let mut req = empty_request(Method::GET, "/some-page"); let timings = RequestTimings::new(); - req.extensions_mut().insert(timings.clone()); + req.extensions_mut().insert(timings.handle().clone()); let _ = block_on(super::run_pre_route_filters( &state, &services, &mut req, None, @@ -3586,7 +3555,7 @@ mod tests { .build(); let mut req = empty_request(Method::GET, "/some-page"); let timings = RequestTimings::new(); - req.extensions_mut().insert(timings.clone()); + req.extensions_mut().insert(timings.handle().clone()); let _ = block_on(super::run_pre_route_filters( &state, &services, &mut req, None, diff --git a/crates/trusted-server-adapter-fastly/src/main.rs b/crates/trusted-server-adapter-fastly/src/main.rs index 8cd2b39d0..4b6230bf5 100644 --- a/crates/trusted-server-adapter-fastly/src/main.rs +++ b/crates/trusted-server-adapter-fastly/src/main.rs @@ -215,7 +215,7 @@ fn edgezero_main(mut req: FastlyRequest, env: &EnvConfig) { core_req.extensions_mut().insert(config_store); core_req.extensions_mut().insert(device_signals); core_req.extensions_mut().insert(client_info); - core_req.extensions_mut().insert(timings.clone()); + core_req.extensions_mut().insert(timings.handle().clone()); match futures::executor::block_on(app.router().oneshot(core_req)) { Ok(response) => response, Err(error) => edge_error_response(error), @@ -1100,6 +1100,21 @@ mod tests { ); } + #[test] + fn health_short_circuit_is_get_only_and_ignores_query() { + for path in ["/health", "/health?probe=1"] { + let url = format!("https://example.com{path}"); + assert!( + health_response(&FastlyRequest::get(&url)).is_some(), + "should bypass GET health" + ); + assert!( + health_response(&FastlyRequest::post(&url)).is_none(), + "should time non-GET health normally" + ); + } + } + #[test] fn health_response_ignores_non_health_paths() { let req = FastlyRequest::get("https://example.com/auction"); diff --git a/crates/trusted-server-adapter-fastly/src/middleware.rs b/crates/trusted-server-adapter-fastly/src/middleware.rs index 7bd577363..17ed2abf3 100644 --- a/crates/trusted-server-adapter-fastly/src/middleware.rs +++ b/crates/trusted-server-adapter-fastly/src/middleware.rs @@ -72,12 +72,8 @@ impl Middleware for FinalizeResponseMiddleware { || FastlyRequestContext::get(ctx.request()).and_then(|c| c.client_ip), |info| info.client_ip, ); - let timings = ctx - .request() - .extensions() - .get::() - .cloned() - .unwrap_or_default(); + let timings = + RequestTimings::from_extensions(ctx.request().extensions()).unwrap_or_default(); let mut response = match next.run(ctx).await { Ok(r) => r, diff --git a/crates/trusted-server-adapter-spin/src/app.rs b/crates/trusted-server-adapter-spin/src/app.rs index 8917f2a65..b21c94a2b 100644 --- a/crates/trusted-server-adapter-spin/src/app.rs +++ b/crates/trusted-server-adapter-spin/src/app.rs @@ -33,6 +33,7 @@ use trusted_server_core::publisher::{ use trusted_server_core::request_signing::{ handle_trusted_server_discovery, handle_verify_signature, }; +use trusted_server_core::request_timing::RequestTimingMiddleware; use trusted_server_core::settings::Settings; use crate::middleware::{ @@ -773,6 +774,7 @@ fn build_router(state: &Arc) -> RouterService { // any middleware registered ahead of it would observe the // shared-secret authentication header. .middleware(SanitizeRequestMiddleware::new(Arc::clone(&state.settings))) + .middleware(RequestTimingMiddleware::default().with_excluded_paths(&["/health"])) .middleware(FinalizeResponseMiddleware::new(Arc::clone(&state.settings))) .middleware(AuthMiddleware::new(Arc::clone(&state.settings))) // Innermost middleware: normalize every routed request (strip diff --git a/crates/trusted-server-adapter-spin/src/middleware.rs b/crates/trusted-server-adapter-spin/src/middleware.rs index d7a09987a..418d63c88 100644 --- a/crates/trusted-server-adapter-spin/src/middleware.rs +++ b/crates/trusted-server-adapter-spin/src/middleware.rs @@ -55,9 +55,9 @@ impl Middleware for SanitizeRequestMiddleware { /// Spin does not expose geo headers to the application, so /// `X-Geo-Info-Available: false` is emitted for every response. /// -/// Registered directly inside [`SanitizeRequestMiddleware`] and ahead of -/// [`AuthMiddleware`] so that every outgoing response — including auth-rejected -/// ones — carries a consistent set of headers. +/// Registered inside [`RequestTimingMiddleware`](trusted_server_core::request_timing::RequestTimingMiddleware) and ahead of [`AuthMiddleware`] +/// so that every outgoing response — including auth-rejected ones — carries a +/// consistent set of headers. pub struct FinalizeResponseMiddleware { settings: Arc, } @@ -188,6 +188,8 @@ pub(crate) fn apply_finalize_headers( #[cfg(test)] mod tests { use super::*; + use edgezero_core::router::RouterService; + use trusted_server_core::request_timing::{RequestTimingMiddleware, RequestTimings}; use std::collections::HashMap; use std::sync::Mutex; @@ -207,10 +209,10 @@ mod tests { .expect("should build empty test response") } - fn empty_ctx() -> RequestContext { + fn ctx_for_path(path: &str) -> RequestContext { let req = request_builder() .method(Method::GET) - .uri("/test") + .uri(path) .header("x-reader-ip", "198.51.100.7") .header("x-reader-ip-auth", "fictional-shared-secret-0123456789") .body(Body::empty()) @@ -218,6 +220,10 @@ mod tests { RequestContext::new(req, PathParams::new(HashMap::new())) } + fn empty_ctx() -> RequestContext { + ctx_for_path("/test") + } + fn settings_with_response_headers(headers: Vec<(&str, &str)>) -> Settings { // Build from explicit test settings: the settings baked into the // binary contain placeholder secrets that `get_settings()` rejects @@ -316,6 +322,77 @@ mod tests { ); } + #[test] + fn request_timing_middleware_preserves_shared_handle_and_health_policy() { + for method in [Method::GET, Method::POST] { + for path in ["/test", "/health", "/health?probe=1", "/health/child"] { + for preinstalled in [false, true] { + let route_path = path.split('?').next().expect("should have path"); + let expected = preinstalled || route_path != "/health"; + let timings = RequestTimings::new(); + timings.record( + trusted_server_core::request_timing::Phase::Filter, + std::time::Duration::from_millis(7), + ); + std::thread::sleep(std::time::Duration::from_millis(2)); + let router = RouterService::builder() + .middleware( + RequestTimingMiddleware::default().with_excluded_paths(&["/health"]), + ) + .middleware( + RequestTimingMiddleware::default().with_excluded_paths(&["/health"]), + ) + .route(route_path, method.clone(), move |ctx: RequestContext| async move { + let installed = RequestTimings::from_extensions(ctx.request().extensions()); + assert_eq!(installed.is_some(), expected, "should preserve exact health policy"); + if let Some(installed) = installed { + if preinstalled { + assert_eq!(installed.snapshot().filter_ms, Some(7), "should retain upstream facts"); + installed.mark_auction_dispatched(); + assert!(installed.snapshot().auction_dispatched_ms.expect("should mark dispatch") >= 2, + "should retain upstream origin"); + } else { + assert_eq!(installed.snapshot(), trusted_server_core::request_timing::TimingSnapshot::default(), + "should install independent empty facts"); + } + installed.record_auction_wait( + trusted_server_core::request_timing::AuctionWaitPlacement::PreHeader, + std::time::Duration::from_millis(3)); + } + Ok::(empty_response()) + }) + .build(); + let mut request = request_builder() + .method(method.clone()) + .uri(path) + .body(Body::empty()) + .expect("should build request"); + if preinstalled { + request.extensions_mut().insert(timings.handle().clone()); + } + let response = + block_on(router.oneshot(request)).expect("should dispatch request"); + assert!( + response.headers().get("server-timing").is_none(), + "attachment should not expose timing" + ); + if preinstalled { + assert_eq!( + timings.snapshot().auction_wait_ms, + Some(3), + "should share handler updates" + ); + assert_eq!( + timings.snapshot().request_elapsed_ms, + None, + "should not fabricate completion" + ); + } + } + } + } + } + #[test] fn sanitize_middleware_strips_configured_trust_headers_before_routing() { let mut settings = settings_with_response_headers(vec![]); diff --git a/crates/trusted-server-core/src/auction/endpoints.rs b/crates/trusted-server-core/src/auction/endpoints.rs index fe42fc6b7..9de8dbab9 100644 --- a/crates/trusted-server-core/src/auction/endpoints.rs +++ b/crates/trusted-server-core/src/auction/endpoints.rs @@ -151,11 +151,7 @@ pub async fn handle_auction( // fetch, and the commit mark lands once the OpenRTB response carrying the // targeting has been built. A defaulted handle records into nothing that // is ever read, so direct-handler tests are unaffected. - let timings = parts - .extensions - .get::() - .cloned() - .unwrap_or_default(); + let timings = RequestTimings::from_extensions(&parts.extensions).unwrap_or_default(); let body_bytes = body.into_bytes().unwrap_or_default(); if body_bytes.len() > MAX_AUCTION_BODY_SIZE { return Response::builder() diff --git a/crates/trusted-server-core/src/integrations/gpt_bootstrap.js b/crates/trusted-server-core/src/integrations/gpt_bootstrap.js index 2475c5082..4d887b970 100644 --- a/crates/trusted-server-core/src/integrations/gpt_bootstrap.js +++ b/crates/trusted-server-core/src/integrations/gpt_bootstrap.js @@ -117,7 +117,7 @@ // and deliberately identical to the bundle scheduler — the impression is // spent on a viewed tab, and the post-hydration guarantee holds whenever // the request is actually issued. - ts.scheduleInitialAdInit = function (initialBids, initialSlots) { + ts.scheduleInitialAdInit = function (initialBids, initialSlots, initialAuctionDiagnostics) { // The bundle may replace this scheduler after the fallback claims the initial // pass. Keep the latch on the shared document API so replacement cannot reset it. if ((ts.navGeneration || 0) !== 0 || ts.initialAdInitScheduled) return; @@ -127,6 +127,9 @@ // would overwrite a committed SPA navigation's slots. if (initialSlots !== undefined) ts.adSlots = initialSlots; if (initialBids !== undefined) ts.bids = initialBids; + if (initialAuctionDiagnostics !== undefined) { + ts.auctionDiagnostics = initialAuctionDiagnostics; + } var fire = function () { if ((ts.navGeneration || 0) !== 0) return; if (typeof ts.adInit === "function") ts.adInit(); diff --git a/crates/trusted-server-core/src/integrations/gpt_diagnostics.rs b/crates/trusted-server-core/src/integrations/gpt_diagnostics.rs index b4a188f2a..b98e3cbbb 100644 --- a/crates/trusted-server-core/src/integrations/gpt_diagnostics.rs +++ b/crates/trusted-server-core/src/integrations/gpt_diagnostics.rs @@ -62,6 +62,7 @@ pub enum GptDiagnosticsCookieAction { #[derive(Clone, Debug, Default, PartialEq, Eq)] pub struct GptDiagnosticsRequestDecision { active: bool, + browser_session_active: bool, clean_browser_path_and_query: Option, cookie_action: GptDiagnosticsCookieAction, } @@ -73,6 +74,16 @@ impl GptDiagnosticsRequestDecision { self.active } + /// Whether this request came from an activated diagnostics browser session. + /// + /// Unlike [`Self::active`], this remains true for non-document requests such + /// as the SPA page-bids fetch. It is captured before the private activation + /// cookie is stripped from the request. + #[must_use] + pub(crate) fn browser_session_active(&self) -> bool { + self.browser_session_active + } + /// Whether the response must be private and non-storeable. #[must_use] pub fn requires_private_no_store(&self) -> bool { @@ -121,6 +132,7 @@ impl GptDiagnosticsRequestDecision { pub(crate) fn active_for_tests() -> Self { Self { active: true, + browser_session_active: true, clean_browser_path_and_query: None, cookie_action: GptDiagnosticsCookieAction::None, } @@ -143,6 +155,7 @@ mod head_seam_invariant_tests { ] { out.push(GptDiagnosticsRequestDecision { active, + browser_session_active: active, clean_browser_path_and_query: clean.clone(), cookie_action, }); @@ -279,12 +292,19 @@ pub fn prepare_request( replace_path_and_query(request, &clean_path)?; } - let mut decision = GptDiagnosticsRequestDecision::default(); + let mut decision = GptDiagnosticsRequestDecision { + browser_session_active: integration_enabled + && directive == QueryDirective::Absent + && cookie_state.occurrences == 1 + && cookie_state.canonical, + ..GptDiagnosticsRequestDecision::default() + }; if integration_enabled && eligible_navigation && had_reserved_query { decision.clean_browser_path_and_query = Some(clean_path); match directive { QueryDirective::Enable => { decision.active = true; + decision.browser_session_active = true; decision.cookie_action = GptDiagnosticsCookieAction::SetSession; } QueryDirective::Disable => { @@ -542,6 +562,22 @@ mod tests { assert_eq!(duplicate.headers()[header::COOKIE], "other=value"); } + #[test] + fn active_cookie_marks_non_document_requests_without_activating_document_behavior() { + let mut request = Request::builder() + .method(Method::GET) + .uri("https://publisher.example/_ts/page-bids?path=/article") + .header(header::COOKIE, "__Host-ts-console=1; other=value") + .body(EdgeBody::empty()) + .expect("should build page-bids request"); + + let decision = prepare_request(&settings(true), &mut request).expect("should prepare"); + + assert!(!decision.active()); + assert!(decision.browser_session_active()); + assert_eq!(request.headers()[header::COOKIE], "other=value"); + } + #[test] fn invalid_duplicate_and_disable_directives_fail_closed() { for query in [ diff --git a/crates/trusted-server-core/src/publisher.rs b/crates/trusted-server-core/src/publisher.rs index e367552e8..03cafe266 100644 --- a/crates/trusted-server-core/src/publisher.rs +++ b/crates/trusted-server-core/src/publisher.rs @@ -3163,6 +3163,44 @@ fn request_origin(scheme: &str, host: &str) -> String { /// JSON for every non-empty map; `serde_json::from_str` failed and `unwrap_or_default()` /// turned the failure into `{}`. Shared modes therefore served **zero bids**, silently, /// on every request that had any. Every fixture had empty bids, so nothing caught it. +#[derive(Clone, Debug, serde::Serialize)] +#[serde(rename_all = "camelCase")] +struct BrowserAuctionDiagnostics { + #[serde(skip_serializing_if = "Option::is_none")] + auction_dispatched_ms: Option, + #[serde(skip_serializing_if = "Option::is_none")] + auction_resolved_ms: Option, + #[serde(skip_serializing_if = "Option::is_none")] + auction_committed_ms: Option, + #[serde(skip_serializing_if = "Option::is_none")] + auction_wait_ms: Option, + #[serde(skip_serializing_if = "Option::is_none")] + auction_wait_placement: Option<&'static str>, +} + +const fn auction_wait_placement_wire(placement: AuctionWaitPlacement) -> &'static str { + match placement { + AuctionWaitPlacement::PreHeader => "pre_header", + AuctionWaitPlacement::InStream => "in_stream", + } +} + +impl BrowserAuctionDiagnostics { + fn from_request_timings(timings: &RequestTimings) -> Option { + let snapshot = timings.snapshot(); + snapshot.auction_dispatched_ms?; + Some(Self { + auction_dispatched_ms: snapshot.auction_dispatched_ms, + auction_resolved_ms: snapshot.auction_resolved_ms, + auction_committed_ms: snapshot.auction_committed_ms, + auction_wait_ms: snapshot.auction_wait_ms, + auction_wait_placement: snapshot + .auction_wait_placement + .map(auction_wait_placement_wire), + }) + } +} + #[derive(Clone, Default)] pub(crate) struct AdBidsState { /// Rendered bids `` sequences inside the string. pub(crate) fn build_bids_script(bid_map: &serde_json::Map) -> String { + build_bids_script_with_diagnostics(bid_map, None) +} + +fn build_bids_script_with_diagnostics( + bid_map: &serde_json::Map, + auction_diagnostics: Option<&BrowserAuctionDiagnostics>, +) -> String { let json = serde_json::to_string(bid_map) .expect("serde_json::to_string of Map should be infallible"); let escaped = html_escape_for_script(&json); @@ -5614,6 +5706,23 @@ pub(crate) fn build_bids_script(bid_map: &serde_json::Map(function(){{\ +var t=window.tsjs=window.tsjs||{{}};\ +var b=JSON.parse(\"{}\");\ +var d=JSON.parse(\"{}\");\ +var s=t.scheduleInitialAdInit;\ +if(typeof s===\"function\")s(b,void 0,d);\ +else{{t.bids=b;t.auctionDiagnostics=d;}}\ +}})();", + escaped, + html_escape_for_script(&diagnostics) + ); + } + format!( "", + html_escape_for_script(slots_json), + html_escape_for_script(&bids), + html_escape_for_script(&diagnostics) + ); + } + format!( "