20260518-053619 Review fixes for #680

crates/trusted-server-adapter-fastly/src/main.rs

@@ -42,6 +42,39 @@ use crate::platform::{build_runtime_services, open_kv_store, UnavailableKvStore}
 
 const CREATIVE_OPPORTUNITIES_TOML: &str = include_str!("../../../creative-opportunities.toml");
 
+/// Parses the embedded `creative-opportunities.toml` at most once per Wasm
+/// instance.
+///
+/// On parse failure, logs an error and falls back to an empty
+/// [`CreativeOpportunitiesFile`] — i.e. the documented "feature disabled"
+/// state — instead of panicking the request hot path. The build-time
+/// validator in `crates/trusted-server-core/build.rs` catches every realistic
+/// authoring mistake; this fallback exists so a CI-bypassed binary patch or a
+/// future schema change can't take the entire fleet down with a per-request
+/// panic.
+static SLOTS_FILE: std::sync::LazyLock<
+    trusted_server_core::creative_opportunities::CreativeOpportunitiesFile,
+> = std::sync::LazyLock::new(|| {
+    let mut file = match toml::from_str::<
+        trusted_server_core::creative_opportunities::CreativeOpportunitiesFile,
+    >(CREATIVE_OPPORTUNITIES_TOML)
+    {
+        Ok(file) => file,
+        Err(err) => {
+            log::error!(
+                "creative-opportunities.toml failed to parse at startup; \
+                     falling back to an empty slots file (server-side ad-slot \
+                     templates disabled): {err}"
+            );
+            trusted_server_core::creative_opportunities::CreativeOpportunitiesFile::default()
+        }
+    };
+    // Pre-compile glob patterns once so per-request `matches_path` doesn't
+    // re-invoke `Pattern::new` on every page hit.
+    file.compile();
+    file
+});
+
 /// Entry point for the Fastly Compute program.
 ///
 /// Uses an undecorated `main()` with `Request::from_client()` instead of
@@ -94,9 +127,7 @@ fn main() {
         }
     };
 
-    let slots_file: trusted_server_core::creative_opportunities::CreativeOpportunitiesFile =
-        toml::from_str(CREATIVE_OPPORTUNITIES_TOML)
-            .expect("should parse creative-opportunities.toml");
+    let slots_file = &*SLOTS_FILE;
 
     let integration_registry = match IntegrationRegistry::new(&settings) {
         Ok(r) => r,
@@ -121,7 +152,7 @@ fn main() {
         &orchestrator,
         &integration_registry,
         &runtime_services,
-        &slots_file,
+        slots_file,
         req,
     )) {
         response.send_to_client();

crates/trusted-server-core/src/auction/endpoints.rs

@@ -6,7 +6,7 @@ use fastly::{Request, Response};
 use crate::auction::formats::AdRequest;
 use crate::compat;
 use crate::consent;
-use crate::cookies::handle_request_cookies;
+use crate::cookies::{handle_request_cookies, parse_ts_eids_cookie};
 use crate::edge_cookie::get_or_generate_ec_id_from_http_request;
 use crate::error::TrustedServerError;
 use crate::platform::RuntimeServices;
@@ -125,8 +125,8 @@ pub async fn handle_auction(
             .map(|_| services.kv_store()),
     });
 
-    // Convert tsjs request format to auction request
-    let auction_request = convert_tsjs_to_auction_request(
+    // Convert tsjs request format to auction request.
+    let mut auction_request = convert_tsjs_to_auction_request(
         &body,
         settings,
         services,
@@ -135,6 +135,10 @@ pub async fn handle_auction(
         &ec_id,
         geo,
     )?;
+    // Forward Extended User IDs from the `ts-eids` cookie so programmatic
+    // callers (slim-Prebid, native apps) get parity with the publisher /
+    // page-bids paths, both of which already do this.
+    auction_request.user.eids = parse_ts_eids_cookie(cookie_jar.as_ref());
 
     // Create auction context
     let context = AuctionContext {

crates/trusted-server-core/src/auction/orchestrator.rs

@@ -540,32 +540,29 @@ impl AuctionOrchestrator {
         }
 
         let starting_count = winning_bids.len();
-        winning_bids.retain(|slot_id, bid| match floor_prices.get(slot_id) {
-            Some(floor) => {
-                // price=None means the SSP returned an encoded price (e.g. APS amznbid).
-                // In the parallel-only path this bid cannot yet be floor-checked; it passes
-                // through and will be decoded (and re-checked) by the mediation layer.
-                // In the mediation path, mediation decodes prices before calling this
-                // function, so any bid still carrying price=None is dropped upstream.
-                match bid.price {
-                    Some(price) if price >= *floor => true,
-                    Some(_) => {
-                        log::info!(
-                            "Dropping winning bid below floor price for slot '{}'",
-                            slot_id
-                        );
-                        false
-                    }
-                    None => {
-                        log::debug!(
-                            "Passing encoded-price bid for slot '{}' - price not yet decoded",
-                            slot_id
-                        );
-                        true
-                    }
-                }
+        winning_bids.retain(|slot_id, bid| match (floor_prices.get(slot_id), bid.price) {
+            (Some(floor), Some(price)) if price >= *floor => true,
+            (Some(_), Some(_)) => {
+                log::info!(
+                    "Dropping winning bid below floor price for slot '{}'",
+                    slot_id
+                );
+                false
             }
-            None => true,
+            (_, None) => {
+                // Any caller that needs to keep an undecoded (encoded-price)
+                // bid must decode it *before* invoking this function — both
+                // `select_winning_bids` and the mediator path already do.
+                // Letting `None`-price bids through here would cause
+                // `winning_bids.len()` to overcount what `build_bid_map`
+                // downstream is willing to emit, so they get dropped instead.
+                log::debug!(
+                    "Dropping bid for slot '{}' - no decoded price (caller must decode before apply_floor_prices)",
+                    slot_id
+                );
+                false
+            }
+            (None, Some(_)) => true,
         });
 
         if winning_bids.len() != starting_count {
@@ -872,7 +869,14 @@ impl AuctionOrchestrator {
                         remaining,
                         mediator.timeout_ms(),
                     );
-                    let placeholder = fastly::Request::get("https://placeholder.invalid/");
+                    // The mediator runs on the collect path. See the doc-comment on
+                    // `AuctionContext::request`: the real client request was already
+                    // consumed by `send_async` during dispatch, so we substitute a
+                    // canonical placeholder URL. Any future mediator that needs real
+                    // client headers must snapshot them at dispatch time onto
+                    // `DispatchedAuction` rather than reading `context.request` here.
+                    let placeholder =
+                        fastly::Request::get(crate::auction::types::MEDIATOR_PLACEHOLDER_URL);
                     let mediator_context = AuctionContext {
                         settings: context.settings,
                         request: &placeholder,
@@ -1256,9 +1260,14 @@ mod tests {
     }
 
     #[test]
-    fn test_apply_floor_prices_allows_none_prices_for_encoded_bids() {
-        // Test that bids with None prices (APS-style) pass through floor pricing
-        // This is correct behavior for parallel-only strategy where mediation happens later
+    fn test_apply_floor_prices_drops_bids_with_undecoded_price() {
+        // Bids that reach apply_floor_prices with `price=None` cannot have a
+        // floor compared against them — and they would not survive downstream
+        // (build_bid_map filters them) — so apply_floor_prices drops them so
+        // the count it reports matches what eventually ships to the client.
+        // Both production paths (select_winning_bids and the mediator filter)
+        // already decode/skip None prices before calling this function; this
+        // test pins the contract.
         let orchestrator = AuctionOrchestrator::new(AuctionConfig::default());
         let mut floor_prices = HashMap::new();
         floor_prices.insert("slot-1".to_string(), 1.00);
@@ -1268,7 +1277,7 @@ mod tests {
             "slot-1".to_string(),
             Bid {
                 slot_id: "slot-1".to_string(),
-                price: None, // APS bid with encoded price
+                price: None,
                 currency: "USD".to_string(),
                 creative: Some("<div>Ad</div>".to_string()),
                 adomain: None,
@@ -1289,25 +1298,15 @@ mod tests {
             },
         );
 
-        // Apply floor pricing - should pass through with None price
         let filtered = orchestrator.apply_floor_prices(winning_bids, &floor_prices);
 
-        assert_eq!(
-            filtered.len(),
-            1,
-            "APS bid with None price should pass through floor check"
-        );
         assert!(
-            filtered.contains_key("slot-1"),
-            "Slot-1 should still be present"
+            filtered.is_empty(),
+            "bid with None price should be dropped by apply_floor_prices"
         );
         assert!(
-            filtered
-                .get("slot-1")
-                .expect("slot-1 should be present")
-                .price
-                .is_none(),
-            "Price should still be None (not decoded yet)"
+            !filtered.contains_key("slot-1"),
+            "slot-1 should not survive when its bid has no decoded price"
         );
     }
 

crates/trusted-server-core/src/auction/types.rs

@@ -115,6 +115,29 @@ pub struct SiteInfo {
 }
 
 /// Context passed to auction providers.
+///
+/// # The `request` field is path-dependent
+///
+/// `request` carries the **real downstream client request** in the dispatch
+/// path ([`AuctionOrchestrator::run_auction`][run] and
+/// [`dispatch_auction`][dispatch]). Providers there can read client headers
+/// (DNT, User-Agent, cookies, X-* customs) directly off it.
+///
+/// In the **collect path** ([`collect_dispatched_auction`][collect]) the
+/// mediator is invoked with a synthetic placeholder request
+/// (`https://placeholder.invalid/`), because the real client request has
+/// already been consumed by `send_async` during dispatch and the host pipeline
+/// can't lend it across the `.await`. **Mediators must not depend on reading
+/// client state from `context.request`** — the placeholder has none of the
+/// real headers. If a future mediator needs that data, snapshot it into a new
+/// field on this struct at dispatch time and stash it on the
+/// [`DispatchedAuction`] token so collect can attach it to the mediator's
+/// context. See <https://github.com/IABTechLab/trusted-server/issues/680>
+/// (P2-1) for the open follow-up.
+///
+/// [run]: crate::auction::AuctionOrchestrator::run_auction
+/// [dispatch]: crate::auction::AuctionOrchestrator::dispatch_auction
+/// [collect]: crate::auction::AuctionOrchestrator::collect_dispatched_auction
 pub struct AuctionContext<'a> {
     pub settings: &'a Settings,
     pub request: &'a Request,
@@ -127,6 +150,12 @@ pub struct AuctionContext<'a> {
     pub services: &'a RuntimeServices,
 }
 
+/// URL used by the orchestrator when invoking a mediator from the collect
+/// path. Providers can `debug_assert` against this value to catch a mediator
+/// that has accidentally started depending on `context.request` carrying real
+/// client headers.
+pub const MEDIATOR_PLACEHOLDER_URL: &str = "https://placeholder.invalid/";
+
 /// Response from a single auction provider.
 #[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct AuctionResponse {