diff --git a/CHANGELOG.md b/CHANGELOG.md index c7a0eb1ec..d1bec9248 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Breaking** — `bid_param_zone_overrides` inner values must now be JSON objects; previously non-object or empty values (`"header" = "x"`, `"header" = {}`) were accepted and silently produced a dead rule at runtime. They now fail at startup with a configuration error. Operators upgrading should audit their `bid_param_zone_overrides` config for non-object zone entries. - **Breaking** — Integration configuration strings are no longer globally reinterpreted as JSON scalars. Operators upgrading should audit `[integrations.*]` settings and use native TOML/typed-config booleans and numbers (for example, `enabled = true`, not `enabled = "true"`); quoted numeric and boolean scalars now fail validation instead of silently converting. - **Breaking** — Sourcepoint browser module inclusion now requires explicit `[integrations.sourcepoint].enabled = true`; operators relying on the previous unconditional Sourcepoint module should enable the integration before upgrading. -- **Breaking** — Auction creative sanitization is now opt-in: the new `[auction].sanitize_creatives` defaults to `false` because unconditional sanitization blanked script-based creatives (the majority of programmatic display) while recording normal impressions. `[auction].rewrite_creatives` keeps its `true` default. The per-creative cap is now enforced on rewritten output as well as raw input and in every processing mode (1 MiB for auction `adm`; proxied HTML documents keep the proxy's own 10 MiB bound), rewriting fails closed on parser errors instead of emitting partial output and never turns a rejected creative into a runtime-only `adm`, and `hb_cache_host`/`hb_cache_path` are emitted only for bids that supplied no creative — any bid carrying its own `adm` ships without them, so a processed or rejected creative can never be re-fetched raw from PBS Cache. Creative markup with no `
` token now receives the click-guard runtime, and bidder `
AdExcludedMail"#;
+ const MATRIX_FIXTURE_FRAGMENT: &str = r#"
AdExcludedMail"#;
+ const INLINE_ORIGIN: &str = "https://www.example.com";
+
+ #[derive(Debug, Clone, Copy)]
+ enum CreativePath {
+ Auction,
+ Inline,
+ }
+
+ fn process_for_path(settings: &Settings, path: CreativePath, html: &str) -> String {
+ match path {
+ CreativePath::Auction => process_auction_creative(settings, html),
+ CreativePath::Inline => {
+ super::process_inline_auction_creative(settings, INLINE_ORIGIN, html)
+ }
+ }
+ }
+
+ fn matrix_settings(assets: bool, clicks: Option
AdExcludedMail"#;
+ const MATRIX_DEFAULT_INLINE_OUTPUT: &str = r#"
AdExcludedMail"#;
+
+ #[test]
+ fn base_target_is_kept_when_the_base_url_is_dropped() {
+ let fixture = r#"
Ad"#;
+ for path in [CreativePath::Auction, CreativePath::Inline] {
+ for (assets, clicks) in [(true, true), (true, false), (false, true), (false, false)] {
+ let settings = matrix_settings(assets, Some(clicks));
+ let label = format!("{path:?} assets={assets} clicks={clicks}");
+
+ let out = process_for_path(&settings, path, fixture);
+
+ if assets || clicks {
+ assert!(
+ out.contains(r#"
"#),
+ "{label}: relative image should stay raw: {out}"
+ );
+ assert!(
+ out.contains(r#""#),
+ "{label}: relative anchor should stay raw: {out}"
+ );
+ }
+ }
+ }
+
+ #[test]
+ fn default_settings_rewrite_matrix_fixture_byte_for_byte_as_before() {
+ let settings = matrix_settings(true, None);
+
+ for (path, expected) in [
+ (CreativePath::Auction, MATRIX_DEFAULT_AUCTION_OUTPUT),
+ (CreativePath::Inline, MATRIX_DEFAULT_INLINE_OUTPUT),
+ ] {
+ assert_eq!(
+ process_for_path(&settings, path, MATRIX_FIXTURE_WITH_BODY),
+ expected,
+ "{path:?}: default settings should match the pre-rewrite_clicks output exactly"
+ );
+ }
+ }
+
+ fn normalized(raw: &str) -> Option
"#;
- // Should exclude base domain
- assert_eq!(to_abs(&settings, "https://cloudflare.com/cdn.js"), None);
+ let out = rewrite_creative_html(&settings, html);
- // Should exclude subdomain
- assert_eq!(
- to_abs(&settings, "https://cdnjs.cloudflare.com/lib.js"),
- None
- );
- assert_eq!(
- to_abs(&settings, "//cloudflare.com/cdn.js"),
- None,
- "should exclude a protocol-relative wildcard base domain"
- );
- assert_eq!(
- to_abs(&settings, "//cdnjs.cloudflare.com/lib.js"),
- None,
- "should exclude a protocol-relative wildcard subdomain"
+ assert!(
+ out.contains(r#""#),
+ "should leave a link raw when its host matches a mixed-case entry: {out}"
);
-
- // Should not exclude different domain
- assert_eq!(
- to_abs(&settings, "https://notcloudflare.com/lib.js"),
- Some("https://notcloudflare.com/lib.js".to_owned())
+ assert!(
+ out.contains(r#"
"#),
+ "should leave an asset raw when its host matches a mixed-case entry: {out}"
);
}
diff --git a/crates/trusted-server-core/src/proxy.rs b/crates/trusted-server-core/src/proxy.rs
index 6a30645ab..34d5f4774 100644
--- a/crates/trusted-server-core/src/proxy.rs
+++ b/crates/trusted-server-core/src/proxy.rs
@@ -1692,7 +1692,8 @@ pub async fn handle_first_party_click(
/// # Errors
///
/// Returns an error if JSON parsing fails, the URL cannot be parsed, the URL uses an
-/// unsupported scheme, the URL lacks a host, or the host violates `proxy.allowed_domains`.
+/// unsupported scheme, the URL lacks a host, the host matches
+/// `rewrite.exclude_domains`, or the host violates `proxy.allowed_domains`.
pub async fn handle_first_party_proxy_sign(
settings: &Settings,
_services: &RuntimeServices,
@@ -1741,37 +1742,31 @@ pub async fn handle_first_party_proxy_sign(
};
let trimmed = payload.url.trim();
- let abs = if trimmed.starts_with("//") {
- format!("{}:{}", request_scheme, trimmed)
+ let protocol_relative;
+ // A protocol-relative target inherits the signing request's scheme before
+ // normalization, so `//` and absolute input reach the same policy check.
+ let candidate = if trimmed.starts_with("//") {
+ protocol_relative = format!("{request_scheme}:{trimmed}");
+ protocol_relative.as_str()
} else {
- crate::creative::to_abs(settings, trimmed).ok_or_else(|| {
- Report::new(TrustedServerError::Proxy {
- message: "unsupported url".to_string(),
- })
- })?
+ trimmed
};
-
- if settings.rewrite.is_excluded(&abs) {
- return Err(Report::new(TrustedServerError::Proxy {
+ let target = crate::creative::normalize_creative_url(candidate).ok_or_else(|| {
+ Report::new(TrustedServerError::Proxy {
message: "unsupported url".to_string(),
- }));
- }
-
- let parsed = url::Url::parse(&abs).change_context(TrustedServerError::Proxy {
- message: "invalid url".to_string(),
+ })
})?;
- let scheme = parsed.scheme();
- if scheme != "http" && scheme != "https" {
- return Err(Report::new(TrustedServerError::Proxy {
- message: "unsupported scheme".to_string(),
- }));
- }
-
- let host = parsed.host_str().ok_or_else(|| {
+ let host = target.host_str().ok_or_else(|| {
Report::new(TrustedServerError::Proxy {
message: "missing host".to_string(),
})
})?;
+ if !settings.rewrite.should_proxy_asset(host) {
+ log::debug!("sign request for `{host}` declined by rewrite policy");
+ return Err(Report::new(TrustedServerError::Proxy {
+ message: "unsupported url".to_string(),
+ }));
+ }
if !is_host_permitted(&settings.proxy.allowed_domains, host) {
log::warn!(
"sign request for `{}` blocked: host not in proxy.allowed_domains",
@@ -1791,10 +1786,10 @@ pub async fn handle_first_party_proxy_sign(
.to_string();
let extras = vec![(String::from("tsexp"), tsexp)];
- let mut base = parsed.clone();
+ let mut base = target.clone();
base.set_query(None);
base.set_fragment(None);
- let proxied = crate::creative::build_proxy_url_with_extras(settings, &abs, &extras);
+ let proxied = crate::creative::build_proxy_url_with_extras(settings, target.as_str(), &extras);
let resp = ProxySignResp {
href: proxied,
@@ -2766,20 +2761,58 @@ mod tests {
let mut settings = create_test_settings();
settings.rewrite.exclude_domains = vec!["cdn.example".to_owned()];
- for url in ["https://cdn.example/asset.js", "//cdn.example/asset.js"] {
- let body = serde_json::json!({ "url": url });
- let req =
- build_http_post_json_request("https://edge.example/first-party/sign", &body);
- let err: Report
Excluded"#;
+ // (rewrite_creatives, rewrite_clicks, expect wrapped clicks)
+ let cases = [
+ (true, None, true),
+ (false, None, true),
+ (true, Some(true), true),
+ (true, Some(false), false),
+ (false, Some(false), false),
+ (false, Some(true), true),
+ ];
+
+ for (rewrite_creatives, rewrite_clicks, expect_clicks) in cases {
+ let mut settings = create_test_settings();
+ settings.auction.rewrite_creatives = rewrite_creatives;
+ settings.auction.rewrite_clicks = rewrite_clicks;
+ settings.rewrite.exclude_domains = vec!["excluded.example.com".to_owned()];
+ let label =
+ format!("rewrite_creatives={rewrite_creatives} rewrite_clicks={rewrite_clicks:?}");
+ let req = build_http_request(Method::GET, "https://edge.example.com/first-party/proxy");
+ let mut response = build_http_response(StatusCode::OK, EdgeBody::from(html));
+ response.headers_mut().insert(
+ header::CONTENT_TYPE,
+ HeaderValue::from_static("text/html; charset=utf-8"),
+ );
+
+ let body = response_body_string(
+ finalize(
+ &settings,
+ &req,
+ "https://cdn.example.com/creative.html",
+ response,
+ )
+ .expect("should finalize proxied HTML"),
+ );
+
+ assert!(
+ body.contains("/first-party/proxy?tsurl="),
+ "{label}: proxied HTML always proxies assets: {body}"
+ );
+ // A wrapped landing link carries the click URL in href and data-tsclick.
+ assert_eq!(
+ body.matches("/first-party/click?tsurl=").count(),
+ if expect_clicks { 2 } else { 0 },
+ "{label}: click wrapping in proxied HTML: {body}"
+ );
+ assert_eq!(
+ body.matches("data-tsclick").count(),
+ usize::from(expect_clicks),
+ "{label}: data-tsclick in proxied HTML: {body}"
+ );
+ if !expect_clicks {
+ let landing_href = body
+ .split("Excluded"#),
+ "{label}: excluded link always stays raw: {body}"
+ );
+ assert!(
+ !body.contains("
AdExcludedMail"#;
+ const MATRIX_FIXTURE_FRAGMENT: &str = r#"
AdExcludedMail"#;
+ const INLINE_ORIGIN: &str = "https://www.example.com";
+
+ #[derive(Debug, Clone, Copy)]
+ enum CreativePath {
+ Auction,
+ Inline,
+ }
+
+ fn process_for_path(settings: &Settings, path: CreativePath, html: &str) -> String {
+ match path {
+ CreativePath::Auction => process_auction_creative(settings, html),
+ CreativePath::Inline => {
+ super::process_inline_auction_creative(settings, INLINE_ORIGIN, html)
+ }
+ }
+ }
+
+ fn matrix_settings(assets: bool, clicks: Option
AdExcludedMail"#;
+ const MATRIX_DEFAULT_INLINE_OUTPUT: &str = r#"
AdExcludedMail"#;
+
+ #[test]
+ fn base_target_is_kept_when_the_base_url_is_dropped() {
+ let fixture = r#"
Ad"#;
+ for path in [CreativePath::Auction, CreativePath::Inline] {
+ for (assets, clicks) in [(true, true), (true, false), (false, true), (false, false)] {
+ let settings = matrix_settings(assets, Some(clicks));
+ let label = format!("{path:?} assets={assets} clicks={clicks}");
+
+ let out = process_for_path(&settings, path, fixture);
+
+ if assets || clicks {
+ assert!(
+ out.contains(r#"
"#),
+ "{label}: relative image should stay raw: {out}"
+ );
+ assert!(
+ out.contains(r#""#),
+ "{label}: relative anchor should stay raw: {out}"
+ );
+ }
+ }
+ }
+
+ #[test]
+ fn default_settings_rewrite_matrix_fixture_byte_for_byte_as_before() {
+ let settings = matrix_settings(true, None);
+
+ for (path, expected) in [
+ (CreativePath::Auction, MATRIX_DEFAULT_AUCTION_OUTPUT),
+ (CreativePath::Inline, MATRIX_DEFAULT_INLINE_OUTPUT),
+ ] {
+ assert_eq!(
+ process_for_path(&settings, path, MATRIX_FIXTURE_WITH_BODY),
+ expected,
+ "{path:?}: default settings should match the pre-rewrite_clicks output exactly"
+ );
+ }
+ }
+```
+
+- [ ] **Step 3: End-to-end tests**
+
+`auction/formats.rs`, before `sanitize_creatives_defaults_to_disabled`:
+
+```rust
+ #[test]
+ fn convert_to_openrtb_response_wraps_clicks_without_rewriting_assets() {
+ let mut settings = make_settings();
+ settings.auction.sanitize_creatives = false;
+ settings.auction.rewrite_creatives = false;
+ settings.auction.rewrite_clicks = Some(true);
+ let auction_request = make_auction_request();
+ let result = make_result(make_complete_creative_bid());
+
+ let response = convert_to_openrtb_response(&result, &settings, &auction_request, false)
+ .expect("should convert creative with click rewriting only");
+ let adm = response_adm(response);
+
+ assert!(
+ adm.contains("/first-party/click?tsurl=") && adm.contains("data-tsclick"),
+ "should wrap the landing link: {adm}"
+ );
+ assert!(
+ !adm.contains("/first-party/proxy?tsurl="),
+ "should not proxy any asset: {adm}"
+ );
+ assert!(
+ adm.contains(r#"src="https://cdn.example.com/ad.png""#),
+ "should keep the image URL direct: {adm}"
+ );
+ assert!(
+ adm.contains("tsjs-unified.min.js"),
+ "should inject the creative runtime for the click guard: {adm}"
+ );
+ }
+```
+
+`publisher.rs`, before `build_bid_map_uses_request_origin_for_inline_urls`:
+
+```rust
+ #[test]
+ fn build_bid_map_keeps_inline_anchors_raw_when_clicks_are_off() {
+ let mut settings = test_settings();
+ settings.auction.rewrite_creatives = true;
+ settings.auction.rewrite_clicks = Some(false);
+ settings.publisher.domain = "example.com".to_string();
+
+ let mut winning_bids = HashMap::new();
+ let mut bid = make_bid(
+ "atf_sidebar_ad",
+ 1.50,
+ "examplessp",
+ "abc123",
+ "https://ssp.example.com/win",
+ "https://ssp.example.com/bill",
+ );
+ bid.creative = Some(
+ "
Excluded"#;
+ // (rewrite_creatives, rewrite_clicks, expect wrapped clicks)
+ let cases = [
+ (true, None, true),
+ (false, None, true),
+ (true, Some(true), true),
+ (true, Some(false), false),
+ (false, Some(false), false),
+ (false, Some(true), true),
+ ];
+
+ for (rewrite_creatives, rewrite_clicks, expect_clicks) in cases {
+ let mut settings = create_test_settings();
+ settings.auction.rewrite_creatives = rewrite_creatives;
+ settings.auction.rewrite_clicks = rewrite_clicks;
+ settings.rewrite.exclude_domains = vec!["excluded.example.com".to_owned()];
+ let label =
+ format!("rewrite_creatives={rewrite_creatives} rewrite_clicks={rewrite_clicks:?}");
+ let req = build_http_request(Method::GET, "https://edge.example.com/first-party/proxy");
+ let mut response = build_http_response(StatusCode::OK, EdgeBody::from(html));
+ response.headers_mut().insert(
+ header::CONTENT_TYPE,
+ HeaderValue::from_static("text/html; charset=utf-8"),
+ );
+
+ let body = response_body_string(
+ finalize(
+ &settings,
+ &req,
+ "https://cdn.example.com/creative.html",
+ response,
+ )
+ .expect("should finalize proxied HTML"),
+ );
+
+ assert!(
+ body.contains("/first-party/proxy?tsurl="),
+ "{label}: proxied HTML always proxies assets: {body}"
+ );
+ // A wrapped landing link carries the click URL in href and data-tsclick.
+ assert_eq!(
+ body.matches("/first-party/click?tsurl=").count(),
+ if expect_clicks { 2 } else { 0 },
+ "{label}: click wrapping in proxied HTML: {body}"
+ );
+ assert_eq!(
+ body.matches("data-tsclick").count(),
+ usize::from(expect_clicks),
+ "{label}: data-tsclick in proxied HTML: {body}"
+ );
+ if !expect_clicks {
+ let landing_href = body
+ .split("Excluded"#),
+ "{label}: excluded link always stays raw: {body}"
+ );
+ assert!(
+ !body.contains("