From ef138d9b7616baf87a378c1e3ae01621cf2ffcef Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Tue, 1 Sep 2026 14:22:33 +0530 Subject: [PATCH 1/8] Match CSS url() quoting rules when rewriting creative styles Treat a url() value as quoted only when a matching closing quote is present, and end a quoted value at that quote rather than at the first paren, so a value is rewritten the way a browser reads it. Derive srcset candidate state as the scan advances rather than from the candidate prefix at each comma. --- crates/trusted-server-core/src/creative.rs | 196 ++++++++++++++++++--- 1 file changed, 176 insertions(+), 20 deletions(-) diff --git a/crates/trusted-server-core/src/creative.rs b/crates/trusted-server-core/src/creative.rs index a4d641bdf..ec8234c01 100644 --- a/crates/trusted-server-core/src/creative.rs +++ b/crates/trusted-server-core/src/creative.rs @@ -73,6 +73,26 @@ pub(super) fn to_abs(settings: &Settings, u: &str) -> Option { Some(absolute) } +/// Returns the index of the quote that closes a CSS string opened with `quote` and +/// starting at `from`, or `None` when the string is unterminated. +/// +/// A backslash escapes the byte that follows it, and a raw newline ends a CSS string +/// as a bad-string, so neither can close it. Returning `None` leaves the caller on its +/// plain scan for the next `)`, which keeps a malformed value unrewritten rather than +/// guessing at its extent. +fn css_string_end(bytes: &[u8], from: usize, quote: u8) -> Option { + let mut i = from; + while i < bytes.len() { + match bytes[i] { + b'\\' => i += 2, + b'\n' | b'\r' => return None, + byte if byte == quote => return Some(i), + _ => i += 1, + } + } + None +} + // Helper: rewrite url(...) occurrences inside a CSS style string to first-party proxy. // `base_origin` is prefixed onto the proxy path — empty for root-relative output, // `https://` for absolute output (see [`build_proxy_url`]). @@ -87,15 +107,28 @@ pub(super) fn rewrite_style_urls(settings: &Settings, style: &str, base_origin: let open = start + 4; // after 'url(' // write prefix including 'url(' out.push_str(&style[write_pos..open]); + let bytes = style.as_bytes(); + // A quoted CSS string may legally contain `)`, so when the value opens with a + // quote the closing paren is searched for after the matching closing quote. + // Scanning from `open` would end the value at a `)` the browser keeps as part + // of the URL, leaving it unrewritten. + let mut value_start = open; + while value_start < bytes.len() && bytes[value_start].is_ascii_whitespace() { + value_start += 1; + } + let search_from = bytes + .get(value_start) + .filter(|byte| **byte == b'"' || **byte == b'\'') + .and_then(|quote| css_string_end(bytes, value_start + 1, *quote)) + .map_or(open, |string_end| string_end + 1); // find closing ')' - let close = if let Some(c) = lower[open..].find(')') { - open + c + let close = if let Some(c) = lower[search_from..].find(')') { + search_from + c } else { out.push_str(&style[open..]); return out; }; // trim spaces and quotes - let bytes = style.as_bytes(); let mut s = open; while s < close && bytes[s].is_ascii_whitespace() { s += 1; @@ -104,10 +137,18 @@ pub(super) fn rewrite_style_urls(settings: &Settings, style: &str, base_origin: while e > s && bytes[e - 1].is_ascii_whitespace() { e -= 1; } + // Only treat the value as quoted when a matching closing quote is actually + // present. Stepping back from `e` on the assumption that one is there can + // land inside a multi-byte character, and it silently rewrites the closing + // delimiter when the quotes do not match. let mut quoted = false; - let (qs, qe) = if s < e && (bytes[s] == b'"' || bytes[s] == b'\'') { + let (qs, qe) = if s < e + && (bytes[s] == b'"' || bytes[s] == b'\'') + && e > s + 1 + && bytes[e - 1] == bytes[s] + { quoted = true; - (s + 1, if e > s + 1 { e - 1 } else { e }) + (s + 1, e - 1) } else { (s, e) }; @@ -118,7 +159,7 @@ pub(super) fn rewrite_style_urls(settings: &Settings, style: &str, base_origin: url_val.to_owned() }; if quoted { - let q = style.as_bytes()[s] as char; + let q = bytes[s] as char; out.push(q); out.push_str(&new_val); out.push(q); @@ -227,26 +268,35 @@ pub(super) fn proxy_if_abs(settings: &Settings, val: &str, base_origin: &str) -> /// - Splits on commas that separate candidates; whitespace after the comma is optional /// - Avoids splitting on the mediatype/data comma of a leading `data:` URL /// (e.g., `data:image/png;base64,AAAA 1x, ...`). -/// Note: this implementation only protects the first mediatype/data comma; it does not -/// attempt to handle additional commas inside a `data:` payload (rare in ad creatives). +/// Note: commas are treated as part of the `data:` URL until whitespace appears in the +/// candidate, so a payload containing several commas stays in one candidate. A `data:` +/// payload with whitespace before a comma is split there. pub(super) fn split_srcset_candidates(s: &str) -> Vec<&str> { let bytes = s.as_bytes(); let mut items = Vec::new(); let mut start = 0_usize; let mut i = 0_usize; + // Whether the candidate beginning at `start` uses the `data:` scheme, and whether + // any whitespace has followed its first non-whitespace byte. Both are properties of + // the candidate rather than of each comma, so they are tracked as the scan advances + // instead of being re-derived from the whole prefix at every comma. + let mut candidate_is_data_scheme = starts_with_data_scheme(&s[start..]); + let mut seen_non_whitespace = false; + let mut seen_whitespace_after_content = false; while i < bytes.len() { - if bytes[i] == b',' { - // Determine if this comma is the mediatype/data separator in a data: URL. - // Look at the current candidate prefix from `start` to `i` and see if it begins with - // `data:` (ignoring leading whitespace) and has no whitespace before this comma. - let prefix = &s[start..i]; - let trimmed = prefix.trim_start(); - let lower = trimmed.to_ascii_lowercase(); - let is_data_scheme = lower.starts_with("data:"); - let has_ws_before_comma = trimmed.chars().any(|c| c.is_ascii_whitespace()); - let comma_is_data_delim = is_data_scheme && !has_ws_before_comma; - if comma_is_data_delim { - // Skip splitting at this comma; it's within the data: URL itself + let byte = bytes[i]; + if byte.is_ascii_whitespace() { + if seen_non_whitespace { + seen_whitespace_after_content = true; + } + i += 1; + continue; + } + if byte == b',' { + // A comma inside a `data:` URL that carries no whitespace yet is the + // mediatype/data separator, not a candidate separator. + if candidate_is_data_scheme && !seen_whitespace_after_content { + seen_non_whitespace = true; i += 1; continue; } @@ -259,8 +309,12 @@ pub(super) fn split_srcset_candidates(s: &str) -> Vec<&str> { i += 1; } start = i; + candidate_is_data_scheme = starts_with_data_scheme(&s[start..]); + seen_non_whitespace = false; + seen_whitespace_after_content = false; continue; } + seen_non_whitespace = true; i += 1; } if start < bytes.len() { @@ -269,6 +323,16 @@ pub(super) fn split_srcset_candidates(s: &str) -> Vec<&str> { items } +/// Returns `true` when `candidate` begins with the `data:` scheme, ignoring leading +/// whitespace and ASCII case. +fn starts_with_data_scheme(candidate: &str) -> bool { + candidate + .trim_start() + .as_bytes() + .get(..5) + .is_some_and(|scheme| scheme.eq_ignore_ascii_case(b"data:")) +} + /// Helper: rewrite a `srcset`/`imagesrcset` attribute value. /// - Proxies absolute or protocol-relative candidates via first-party endpoint /// - Preserves descriptors (e.g., `1x`, `1.5x`, `100w`) @@ -1306,6 +1370,79 @@ mod tests { assert!(out.contains("url(/local/border.png)")); } + #[test] + fn rewrite_style_urls_handles_unterminated_quote_before_multibyte() { + let settings = crate::test_support::tests::create_test_settings(); + let css = "background:url(\"café)"; + let out = rewrite_style_urls(&settings, css, ""); + assert_eq!( + out, css, + "should leave an unterminated quoted url unchanged" + ); + } + + #[test] + fn rewrite_style_urls_keeps_paren_inside_quoted_url() { + let settings = crate::test_support::tests::create_test_settings(); + let css = "background:url(\"https://cdn.example/a)b.png\")"; + let out = rewrite_style_urls(&settings, css, ""); + let expected = super::build_proxy_url(&settings, "https://cdn.example/a)b.png", ""); + assert!( + out.contains(&expected), + "the whole quoted value, parens included, should be proxied: {out}" + ); + } + + #[test] + fn rewrite_style_urls_honors_escaped_quote_inside_quoted_url() { + let settings = crate::test_support::tests::create_test_settings(); + let css = r#"background:url("https://cdn.example/a\")b.png")"#; + let out = rewrite_style_urls(&settings, css, ""); + let truncated = super::build_proxy_url(&settings, r#"https://cdn.example/a\"#, ""); + assert!( + !out.contains(&truncated), + "an escaped quote should not end the string early: {out}" + ); + assert!( + out.contains("/first-party/proxy?tsurl="), + "the value should still be proxied: {out}" + ); + } + + #[test] + fn rewrite_style_urls_ends_unquoted_url_at_first_paren() { + let settings = crate::test_support::tests::create_test_settings(); + let css = "background:url(https://cdn.example/a)b.png)"; + let out = rewrite_style_urls(&settings, css, ""); + assert!( + out.contains("/first-party/proxy?tsurl="), + "an unquoted url still ends at the first paren: {out}" + ); + assert!( + out.ends_with("b.png)"), + "the trailing text is preserved: {out}" + ); + } + + #[test] + fn rewrite_style_urls_leaves_string_broken_by_newline_unchanged() { + let settings = crate::test_support::tests::create_test_settings(); + let css = "background:url(\"https://cdn.example/a\nb)"; + let out = rewrite_style_urls(&settings, css, ""); + assert_eq!( + out, css, + "a string a newline has already ended should be left alone" + ); + } + + #[test] + fn rewrite_style_urls_handles_mismatched_quotes() { + let settings = crate::test_support::tests::create_test_settings(); + let css = "background:url('/local/a.png\")"; + let out = rewrite_style_urls(&settings, css, ""); + assert_eq!(out, css, "should leave a mismatched quoted url unchanged"); + } + #[test] fn rewrites_style_1x1_px() { use crate::http_util::encode_url; @@ -1589,6 +1726,25 @@ mod tests { assert!(items[1].trim().starts_with("//cdn.example/b.png 2x")); } + #[test] + fn split_srcset_keeps_consecutive_data_url_commas_in_one_candidate() { + let s = "data:text/plain;charset=utf-8,a,b,c 1x, /local/b.png 2x"; + let items = super::split_srcset_candidates(s); + assert_eq!(items.len(), 2, "{items:?}"); + assert_eq!(items[0], "data:text/plain;charset=utf-8,a,b,c 1x"); + } + + #[test] + fn split_srcset_handles_long_data_url_comma_run() { + let mut s = String::from("data:image/png;base64,"); + s.push_str(&",".repeat(100_000)); + s.push_str(" 1x, https://cdn.example/b.png 2x"); + let items = super::split_srcset_candidates(&s); + assert_eq!(items.len(), 2, "{items:?}"); + assert!(items[0].starts_with("data:image/png;base64,")); + assert!(items[1].trim().starts_with("https://cdn.example/b.png")); + } + #[test] fn link_rel_case_and_multi_values_rewritten() { let settings = crate::test_support::tests::create_test_settings(); From d2be7032b9409af4b6b53cc4e7bcc21dbe8a5a20 Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Tue, 1 Sep 2026 21:05:58 +0530 Subject: [PATCH 2/8] Leave a CSS url() value alone when its bytes cannot be resolved The rewriter does not resolve CSS escapes, so a value carrying a backslash cannot be mapped to the resource the page will actually request; proxying the raw bytes points somewhere else. A raw newline, which preprocessing also produces from a carriage return or a form feed, makes the value a bad string the browser discards, so rewriting it proxies a URL that is never fetched. Both are now passed through untouched. Also fold an escaped CRLF into a single escaped newline when locating the end of a quoted string, and end the string at a form feed, so the extent matches what preprocessing produces. --- crates/trusted-server-core/src/creative.rs | 81 ++++++++++++++++++++-- 1 file changed, 75 insertions(+), 6 deletions(-) diff --git a/crates/trusted-server-core/src/creative.rs b/crates/trusted-server-core/src/creative.rs index ec8234c01..9afae7849 100644 --- a/crates/trusted-server-core/src/creative.rs +++ b/crates/trusted-server-core/src/creative.rs @@ -80,12 +80,37 @@ pub(super) fn to_abs(settings: &Settings, u: &str) -> Option { /// as a bad-string, so neither can close it. Returning `None` leaves the caller on its /// plain scan for the next `)`, which keeps a malformed value unrewritten rather than /// guessing at its extent. +/// Whether a `url()` value can be mapped to the URL the browser will request. +/// +/// CSS escapes are not resolved here, so a value carrying a backslash cannot be +/// resolved to the resource the page actually asks for, and proxying the raw +/// bytes would point somewhere else. A raw newline — which preprocessing also +/// produces from a carriage return or a form feed — makes the value a bad +/// string the browser discards, so rewriting it would proxy a URL that is never +/// requested. Both are left untouched. +fn css_value_is_resolvable(value: &str) -> bool { + !value + .bytes() + .any(|byte| matches!(byte, b'\\' | b'\n' | b'\r' | b'\x0c')) +} + fn css_string_end(bytes: &[u8], from: usize, quote: u8) -> Option { let mut i = from; while i < bytes.len() { match bytes[i] { - b'\\' => i += 2, - b'\n' | b'\r' => return None, + b'\\' => { + // A backslash escapes what follows. CSS preprocessing folds a + // CRLF pair into one newline, so an escaped CRLF is a single + // escaped newline and both bytes belong to the escape. + if bytes.get(i + 1) == Some(&b'\r') && bytes.get(i + 2) == Some(&b'\n') { + i += 3; + } else { + i += 2; + } + } + // Preprocessing turns a carriage return and a form feed into a + // newline, and a raw newline ends a string as a bad string. + b'\n' | b'\r' | b'\x0c' => return None, byte if byte == quote => return Some(i), _ => i += 1, } @@ -153,7 +178,9 @@ pub(super) fn rewrite_style_urls(settings: &Settings, style: &str, base_origin: (s, e) }; let url_val = &style[qs..qe]; - let new_val = if let Some(abs) = to_abs(settings, url_val) { + let new_val = if !css_value_is_resolvable(url_val) { + url_val.to_owned() + } else if let Some(abs) = to_abs(settings, url_val) { build_proxy_url(settings, &abs, base_origin) } else { url_val.to_owned() @@ -1403,9 +1430,9 @@ mod tests { !out.contains(&truncated), "an escaped quote should not end the string early: {out}" ); - assert!( - out.contains("/first-party/proxy?tsurl="), - "the value should still be proxied: {out}" + assert_eq!( + out, css, + "an escape cannot be resolved here, so the value is left alone" ); } @@ -1435,6 +1462,48 @@ mod tests { ); } + #[test] + fn rewrite_style_urls_leaves_a_value_carrying_an_escape_unrewritten() { + let settings = crate::test_support::tests::create_test_settings(); + // The rewriter does not resolve CSS escapes, so it cannot know which URL + // the browser will actually request. Emitting a proxy token for the raw + // bytes would point at a different resource than the page asked for. + for css in [ + "background:url(\"https://cdn.example/a\\2e png\")", + "background:url(\"https://cdn.example/a\\\r\nb.png\")", + "background:url(\"https://cdn.example/a\\\r\nb)c.png\")", + ] { + let out = rewrite_style_urls(&settings, css, ""); + assert_eq!(out, css, "should leave an escaped value alone: {css}"); + } + } + + #[test] + fn rewrite_style_urls_treats_a_form_feed_as_ending_the_string() { + let settings = crate::test_support::tests::create_test_settings(); + // CSS preprocessing turns a form feed into a newline, which makes this a + // bad string the browser discards. Proxying it would rewrite a URL that + // is never requested. + let css = "background:url(\"https://cdn.example/a\u{c}b.png\")"; + + let out = rewrite_style_urls(&settings, css, ""); + + assert_eq!(out, css, "should not proxy a value the browser discards"); + } + + #[test] + fn rewrite_style_urls_still_proxies_a_plain_quoted_value() { + let settings = crate::test_support::tests::create_test_settings(); + let css = "background:url(\"https://cdn.example/plain.png\")"; + + let out = rewrite_style_urls(&settings, css, ""); + + assert!( + out.contains("/first-party/proxy?tsurl="), + "an ordinary value must still be proxied: {out}" + ); + } + #[test] fn rewrite_style_urls_handles_mismatched_quotes() { let settings = crate::test_support::tests::create_test_settings(); From 5927289fd2039a3e6a2e10eadb8ccd892b740cbc Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Tue, 1 Sep 2026 21:24:02 +0530 Subject: [PATCH 3/8] Restore the string-scanner documentation to its own function Inserting the resolvability helper above css_string_end left that function's doc comment attached to the new helper. --- crates/trusted-server-core/src/creative.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/trusted-server-core/src/creative.rs b/crates/trusted-server-core/src/creative.rs index 9afae7849..a0601a5a2 100644 --- a/crates/trusted-server-core/src/creative.rs +++ b/crates/trusted-server-core/src/creative.rs @@ -73,13 +73,6 @@ pub(super) fn to_abs(settings: &Settings, u: &str) -> Option { Some(absolute) } -/// Returns the index of the quote that closes a CSS string opened with `quote` and -/// starting at `from`, or `None` when the string is unterminated. -/// -/// A backslash escapes the byte that follows it, and a raw newline ends a CSS string -/// as a bad-string, so neither can close it. Returning `None` leaves the caller on its -/// plain scan for the next `)`, which keeps a malformed value unrewritten rather than -/// guessing at its extent. /// Whether a `url()` value can be mapped to the URL the browser will request. /// /// CSS escapes are not resolved here, so a value carrying a backslash cannot be @@ -94,6 +87,13 @@ fn css_value_is_resolvable(value: &str) -> bool { .any(|byte| matches!(byte, b'\\' | b'\n' | b'\r' | b'\x0c')) } +/// Returns the index of the quote that closes a CSS string opened with `quote` and +/// starting at `from`, or `None` when the string is unterminated. +/// +/// A backslash escapes what follows, and a raw newline ends a CSS string as a +/// bad-string, so neither can close it. Returning `None` leaves the caller on its +/// plain scan for the next `)`, which keeps a malformed value unrewritten rather than +/// guessing at its extent. fn css_string_end(bytes: &[u8], from: usize, quote: u8) -> Option { let mut i = from; while i < bytes.len() { From a8d4e099e41176822f7a4410820726f3e4935d52 Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Wed, 2 Sep 2026 19:48:21 +0530 Subject: [PATCH 4/8] Take CSS URL references from the grammar, not a quote scan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The scan bounded a value by the next quote and paren, so a span could fuse across declarations, a missing paren abandoned the rest of the input, an escape became a URL nobody requests, and a value ending at input was skipped. It also stepped back from a computed index, which slices a multi-byte character and aborts the guest. Reading with a tokenizer supplies both the extent and the resolved value, and leaves a malformed value — which a browser discards anyway — on its original bytes. Cover the other references a browser fetches: src(), a bare string candidate in image-set(), and an @import prelude, which takes one URL and reads later strings as media queries. A string counts as a URL only in those places, so font-family and content keep theirs, and an @import is a rule only where a top-level rule may start — the same token is data inside a declaration, in another prelude, and in a style attribute, which is not a stylesheet. The walk recurses per scope through upstream-supplied CSS, so it is bounded, and CSS past the bound is rejected rather than passed through below it. cssparser already built here as a transitive dependency. --- Cargo.lock | 1 + Cargo.toml | 1 + crates/trusted-server-core/Cargo.toml | 1 + crates/trusted-server-core/src/creative.rs | 944 ++++++++++++++++++--- 4 files changed, 818 insertions(+), 129 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e29380b77..085fcd1ee 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5453,6 +5453,7 @@ dependencies = [ "config", "cookie", "criterion", + "cssparser 0.36.0", "derive_more", "ed25519-dalek", "edgezero-core", diff --git a/Cargo.toml b/Cargo.toml index 25c367181..1e843c656 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -51,6 +51,7 @@ clap = { version = "4", features = ["derive"] } config = "0.15.19" cookie = "0.18.1" criterion = { version = "0.5", default-features = false, features = ["cargo_bench_support"] } +cssparser = "0.36" derive_more = { version = "2.0", features = ["display", "error"] } directories = "5" ed25519-dalek = { version = "2.2", features = ["rand_core"] } diff --git a/crates/trusted-server-core/Cargo.toml b/crates/trusted-server-core/Cargo.toml index e44d46f77..01780dd39 100644 --- a/crates/trusted-server-core/Cargo.toml +++ b/crates/trusted-server-core/Cargo.toml @@ -19,6 +19,7 @@ bytes = { workspace = true } chacha20poly1305 = { workspace = true } chrono = { workspace = true } cookie = { workspace = true } +cssparser = { workspace = true } derive_more = { workspace = true } ed25519-dalek = { workspace = true } edgezero-core = { workspace = true } diff --git a/crates/trusted-server-core/src/creative.rs b/crates/trusted-server-core/src/creative.rs index a0601a5a2..983f87985 100644 --- a/crates/trusted-server-core/src/creative.rs +++ b/crates/trusted-server-core/src/creative.rs @@ -73,132 +73,315 @@ pub(super) fn to_abs(settings: &Settings, u: &str) -> Option { Some(absolute) } -/// Whether a `url()` value can be mapped to the URL the browser will request. +/// Maximum number of nested parser scopes [`rewrite_style_urls`] will enter. /// -/// CSS escapes are not resolved here, so a value carrying a backslash cannot be -/// resolved to the resource the page actually asks for, and proxying the raw -/// bytes would point somewhere else. A raw newline — which preprocessing also -/// produces from a carriage return or a form feed — makes the value a bad -/// string the browser discards, so rewriting it would proxy a URL that is never -/// requested. Both are left untouched. -fn css_value_is_resolvable(value: &str) -> bool { - !value - .bytes() - .any(|byte| matches!(byte, b'\\' | b'\n' | b'\r' | b'\x0c')) -} +/// The walk recurses into blocks, functions and `@import` preludes, and the CSS +/// it reads is supplied by the upstream creative, so an input of nothing but +/// `{` would otherwise decide how deep the stack goes — and an overflow aborts +/// the guest, turning a 2 KB style attribute into a failed response. Measured on +/// `wasm32-wasip1`, the walk survives 1,000 nested blocks and overflows by +/// 1,100, so this leaves room for a stack an order of magnitude smaller than +/// the one measured. It is still far above any real stylesheet, where nesting +/// is a declaration list inside a handful of blocks. Entering this many +/// scopes costs one more stack frame than the bound, since the outermost walk +/// has entered none. +/// +/// An iterative walk would need no bound, but the tokenizer only exposes a +/// block through a closure, so each nested slice would be re-read from its +/// start — quadratic in the input, which [`MAX_REWRITABLE_BODY_SIZE`] allows +/// to be 10 MB. Recursing stays linear and bounds the stack instead. +const MAX_CSS_NESTING_DEPTH: usize = 64; -/// Returns the index of the quote that closes a CSS string opened with `quote` and -/// starting at `from`, or `None` when the string is unterminated. +/// Rewrites URL references inside a CSS string to the first-party proxy. /// -/// A backslash escapes what follows, and a raw newline ends a CSS string as a -/// bad-string, so neither can close it. Returning `None` leaves the caller on its -/// plain scan for the next `)`, which keeps a malformed value unrewritten rather than -/// guessing at its extent. -fn css_string_end(bytes: &[u8], from: usize, quote: u8) -> Option { - let mut i = from; - while i < bytes.len() { - match bytes[i] { - b'\\' => { - // A backslash escapes what follows. CSS preprocessing folds a - // CRLF pair into one newline, so an escaped CRLF is a single - // escaped newline and both bytes belong to the escape. - if bytes.get(i + 1) == Some(&b'\r') && bytes.get(i + 2) == Some(&b'\n') { - i += 3; - } else { - i += 2; +/// Covers every form the browser fetches: `url()` and `src()`, a bare string +/// candidate in `image-set()`, and an `@import` prelude string. +/// +/// `base_origin` is prefixed onto the proxy path — empty for root-relative +/// output, `https://` for absolute output (see [`build_proxy_url`]). +/// +/// Values are read with a CSS tokenizer rather than by scanning for quotes, so +/// the extent of a value comes from the grammar and escapes are already +/// resolved. That matters in both directions: a value whose escapes hide an +/// absolute URL (`url("https://t.example/\70 ixel.gif")`) is still proxied, and +/// a malformed value — which the tokenizer reports as a bad URL or bad string, +/// exactly what a browser discards — is left untouched rather than guessed at. +/// +/// A rewritten reference is re-emitted as `url("…")` whichever form it came +/// in as, so the output is normalized rather than byte-preserved — `url()` +/// is accepted everywhere the other forms are. Anything not rewritten keeps +/// its original bytes. +/// +/// CSS nested past [`MAX_CSS_NESTING_DEPTH`] is rejected outright (empty string +/// returned), matching [`MAX_CREATIVE_SIZE`]. The alternative — keeping the +/// rewrite of everything above the cap and passing the deeper bytes through — +/// turns the bound into a way around the rewrite: a `url()` placed below the +/// cap is never inspected and reaches the browser untouched, which is the leak +/// this exists to close. Rejecting costs the styling of CSS no real page +/// produces; passing through would cost the guarantee. +pub(super) fn rewrite_style_urls(settings: &Settings, style: &str, base_origin: &str) -> String { + rewrite_style_urls_in_context(settings, style, base_origin, true) +} + +/// Rewrites a style attribute, where `@import` is ordinary declaration data. +fn rewrite_style_attribute_urls(settings: &Settings, style: &str, base_origin: &str) -> String { + rewrite_style_urls_in_context(settings, style, base_origin, false) +} + +fn rewrite_style_urls_in_context( + settings: &Settings, + style: &str, + base_origin: &str, + allows_import_rules: bool, +) -> String { + let mut rewriter = CssUrlRewriter { + settings, + style, + base_origin, + allows_import_rules, + out: String::with_capacity(style.len() + 16), + write_pos: 0, + depth_exceeded: false, + }; + let mut input = cssparser::ParserInput::new(style); + let mut parser = cssparser::Parser::new(&mut input); + rewriter.walk(&mut parser, 0, BareStringUrls::Never); + if rewriter.depth_exceeded { + log::warn!("Rejecting a stylesheet nested past the supported depth"); + return String::new(); + } + rewriter.out.push_str(&style[rewriter.write_pos..]); + rewriter.out +} + +/// Splices rewritten `url()` values into a copy of the original CSS. +struct CssUrlRewriter<'a> { + settings: &'a Settings, + style: &'a str, + base_origin: &'a str, + /// Whether the outermost scope is a stylesheet that may contain `@import`. + allows_import_rules: bool, + out: String, + write_pos: usize, + /// Set when the walk refused to descend further, which invalidates the + /// output because a `url()` below the cap was never inspected. + depth_exceeded: bool, +} + +impl CssUrlRewriter<'_> { + /// Visits every token at this nesting level, descending into blocks and + /// functions because a `url()` may appear at any depth. + /// + /// `depth` is the number of parser scopes already entered; descending past + /// [`MAX_CSS_NESTING_DEPTH`] sets `depth_exceeded` instead of recursing. + /// + /// `strings_are_urls` marks a context where a bare quoted string is itself + /// a URL the browser fetches: an `image-set()` argument list, or an + /// `@import` prelude. It is false almost everywhere else — a `font-family` + /// or `content` string must not be touched. The mode is local to this + /// parser scope, so no malformed at-rule can carry it into a sibling. + fn walk( + &mut self, + parser: &mut cssparser::Parser<'_, '_>, + depth: usize, + strings_are_urls: BareStringUrls, + ) { + // Scope-local, so it cannot carry into a sibling or later scope. + let mut bare_strings = strings_are_urls; + // Only the start of a top-level stylesheet rule can introduce an + // `@import`; the same token is ordinary data in declarations and in + // another at-rule's prelude. + let is_stylesheet_root = depth == 0 && self.allows_import_rules; + let mut rule_may_start = is_stylesheet_root; + loop { + let token_start = parser.position().byte_index(); + // Owned so the token stops borrowing the parser before the body of + // a block or function is read. + let found = match parser.next_including_whitespace_and_comments() { + Ok(cssparser::Token::UnquotedUrl(value)) => { + CssToken::Url(value.as_ref().to_owned()) + } + Ok(cssparser::Token::QuotedString(value)) + if bare_strings != BareStringUrls::Never => + { + CssToken::Url(value.as_ref().to_owned()) + } + Ok(cssparser::Token::Function(name)) if is_url_function(name.as_ref()) => { + CssToken::UrlFunction + } + Ok(cssparser::Token::Function(name)) => CssToken::Block( + if takes_bare_string_urls(name.as_ref()) { + BareStringUrls::Every + } else { + BareStringUrls::Never + }, + false, + ), + Ok(cssparser::Token::ParenthesisBlock | cssparser::Token::SquareBracketBlock) => { + CssToken::Block(BareStringUrls::Never, false) + } + Ok(cssparser::Token::CurlyBracketBlock) => { + CssToken::Block(BareStringUrls::Never, is_stylesheet_root) } + // `@import "…";` names a stylesheet the browser loads, so its + // prelude reads a bare string as a URL. + Ok(cssparser::Token::AtKeyword(name)) + if rule_may_start && name.eq_ignore_ascii_case("import") => + { + CssToken::ImportPrelude + } + Ok(cssparser::Token::Semicolon) => CssToken::RuleBoundary, + Ok(cssparser::Token::CDO | cssparser::Token::CDC) if is_stylesheet_root => { + CssToken::Skippable + } + Ok(cssparser::Token::WhiteSpace(_) | cssparser::Token::Comment(_)) => { + CssToken::Skippable + } + Ok(_) => CssToken::Other, + Err(_) => break, + }; + let carries_a_value = !matches!(found, CssToken::Skippable); + let ends_rule = matches!(found, CssToken::RuleBoundary | CssToken::Block(_, true)); + + match found { + CssToken::Url(value) => self.rewrite(&value, token_start, parser), + CssToken::UrlFunction => { + if depth >= MAX_CSS_NESTING_DEPTH { + self.depth_exceeded = true; + return; + } + if let Some(value) = quoted_url_argument(parser) { + self.rewrite(&value, token_start, parser); + } + } + CssToken::ImportPrelude => { + if depth >= MAX_CSS_NESTING_DEPTH { + self.depth_exceeded = true; + return; + } + // Ends at the `;` or `{` that ends the at-rule, and the + // delimiter itself is left for this loop to read, so the + // prelude cannot reach a later declaration. + let _ = parser.parse_until_before( + cssparser::Delimiter::Semicolon | cssparser::Delimiter::CurlyBracketBlock, + |prelude| -> Result<(), cssparser::ParseError<'_, ()>> { + self.walk(prelude, depth + 1, BareStringUrls::FirstValue); + Ok(()) + }, + ); + } + CssToken::Block(nested_strings_are_urls, _) => { + if depth >= MAX_CSS_NESTING_DEPTH { + self.depth_exceeded = true; + return; + } + // `parse_nested_block` must be called to consume the body; + // skipping it would leave the block unvisited. + let _ = parser.parse_nested_block( + |inner| -> Result<(), cssparser::ParseError<'_, ()>> { + self.walk(inner, depth + 1, nested_strings_are_urls); + Ok(()) + }, + ); + } + CssToken::RuleBoundary | CssToken::Skippable | CssToken::Other => {} + } + + // The prelude's URL is its first value, so once a value has been + // read no later string in this scope is one. + if carries_a_value && bare_strings == BareStringUrls::FirstValue { + bare_strings = BareStringUrls::Never; + } + if ends_rule { + rule_may_start = is_stylesheet_root; + } else if carries_a_value { + rule_may_start = false; } - // Preprocessing turns a carriage return and a form feed into a - // newline, and a raw newline ends a string as a bad string. - b'\n' | b'\r' | b'\x0c' => return None, - byte if byte == quote => return Some(i), - _ => i += 1, } } - None -} -// Helper: rewrite url(...) occurrences inside a CSS style string to first-party proxy. -// `base_origin` is prefixed onto the proxy path — empty for root-relative output, -// `https://` for absolute output (see [`build_proxy_url`]). -pub(super) fn rewrite_style_urls(settings: &Settings, style: &str, base_origin: &str) -> String { - // naive url(...) rewrite for absolute/protocol-relative URLs - let lower = style.to_ascii_lowercase(); - let mut out = String::with_capacity(style.len() + 16); - let mut write_pos = 0_usize; - let mut scan = 0_usize; - while let Some(off) = lower[scan..].find("url(") { - let start = scan + off; - let open = start + 4; // after 'url(' - // write prefix including 'url(' - out.push_str(&style[write_pos..open]); - let bytes = style.as_bytes(); - // A quoted CSS string may legally contain `)`, so when the value opens with a - // quote the closing paren is searched for after the matching closing quote. - // Scanning from `open` would end the value at a `)` the browser keeps as part - // of the URL, leaving it unrewritten. - let mut value_start = open; - while value_start < bytes.len() && bytes[value_start].is_ascii_whitespace() { - value_start += 1; - } - let search_from = bytes - .get(value_start) - .filter(|byte| **byte == b'"' || **byte == b'\'') - .and_then(|quote| css_string_end(bytes, value_start + 1, *quote)) - .map_or(open, |string_end| string_end + 1); - // find closing ')' - let close = if let Some(c) = lower[search_from..].find(')') { - search_from + c - } else { - out.push_str(&style[open..]); - return out; + /// Replaces the span just consumed with a proxied `url()`, or leaves it. + fn rewrite(&mut self, value: &str, token_start: usize, parser: &cssparser::Parser<'_, '_>) { + let Some(absolute) = to_abs(self.settings, value) else { + return; }; - // trim spaces and quotes - let mut s = open; - while s < close && bytes[s].is_ascii_whitespace() { - s += 1; - } - let mut e = close; - while e > s && bytes[e - 1].is_ascii_whitespace() { - e -= 1; - } - // Only treat the value as quoted when a matching closing quote is actually - // present. Stepping back from `e` on the assumption that one is there can - // land inside a multi-byte character, and it silently rewrites the closing - // delimiter when the quotes do not match. - let mut quoted = false; - let (qs, qe) = if s < e - && (bytes[s] == b'"' || bytes[s] == b'\'') - && e > s + 1 - && bytes[e - 1] == bytes[s] - { - quoted = true; - (s + 1, e - 1) - } else { - (s, e) - }; - let url_val = &style[qs..qe]; - let new_val = if !css_value_is_resolvable(url_val) { - url_val.to_owned() - } else if let Some(abs) = to_abs(settings, url_val) { - build_proxy_url(settings, &abs, base_origin) - } else { - url_val.to_owned() - }; - if quoted { - let q = bytes[s] as char; - out.push(q); - out.push_str(&new_val); - out.push(q); - } else { - out.push_str(&new_val); - } - out.push(')'); - write_pos = close + 1; - scan = write_pos; + let token_end = parser.position().byte_index(); + self.out.push_str(&self.style[self.write_pos..token_start]); + self.out.push_str("url("); + cssparser::serialize_string( + &build_proxy_url(self.settings, &absolute, self.base_origin), + &mut self.out, + ) + .expect("should write a serialized URL into a String"); + self.out.push(')'); + self.write_pos = token_end; } - out.push_str(&style[write_pos..]); - out +} + +/// What a token turned out to be while scanning for `url()` references. +enum CssToken { + /// An unquoted `url(...)`, carrying its resolved value. + Url(String), + /// The opening of a `url(` function, whose argument has yet to be read. + UrlFunction, + /// A block or other function, whose body may contain a `url()`. Carries + /// whether a bare string inside it is itself a URL and whether its closing + /// returns the outer parser to a top-level rule boundary. + Block(BareStringUrls, bool), + /// The start of an `@import`, whose prelude names a stylesheet. + ImportPrelude, + /// Whitespace or a comment, which carries no value. + Skippable, + /// A semicolon that ends the current top-level rule. + RuleBoundary, + /// Anything else. + Other, +} + +/// Where a bare quoted string in a scope is itself a URL. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum BareStringUrls { + /// A string is never a URL — a `content` or `font-family` value. + Never, + /// Every candidate is a URL, as in an `image-set()` argument list. + Every, + /// Only the first value is, as in an `@import` prelude: the grammar takes + /// one stylesheet URL, and anything after it is a layer, a supports + /// condition or a media query. + FirstValue, +} + +/// Whether a function's argument is a URL, as `url()` and `src()` both are. +fn is_url_function(name: &str) -> bool { + name.eq_ignore_ascii_case("url") || name.eq_ignore_ascii_case("src") +} + +/// Whether a function accepts a bare string as a URL, rather than only `url()`. +/// +/// `image-set()` takes each candidate as either a `url()` or a plain string, +/// and the browser fetches the string form just the same. +fn takes_bare_string_urls(name: &str) -> bool { + name.eq_ignore_ascii_case("image-set") || name.eq_ignore_ascii_case("-webkit-image-set") +} + +/// Reads the argument of a `url(` or `src(` function when it is a single string. +/// +/// Returns `None` for any other shape — a bad string, or a string carrying +/// anything after it — leaving the original bytes in place rather than +/// guessing at the intended value. The trailing case covers both a malformed +/// value and the `` grammar, which no engine acts on today; if +/// one does, such a value would need proxying rather than skipping. +fn quoted_url_argument(parser: &mut cssparser::Parser<'_, '_>) -> Option { + parser + .parse_nested_block( + |inner| -> Result, cssparser::ParseError<'_, ()>> { + Ok(match inner.next() { + Ok(cssparser::Token::QuotedString(value)) => Some(value.as_ref().to_owned()), + _ => None, + }) + }, + ) + .unwrap_or(None) } #[inline] @@ -918,7 +1101,7 @@ fn rewrite_creative_html_impl( // Inline style url(...) element!("[style]", |el| { if let Some(st) = el.get_attribute("style") { - let rewritten = rewrite_style_urls(settings, &st, base_origin); + let rewritten = rewrite_style_attribute_urls(settings, &st, base_origin); if rewritten != st { let _ = el.set_attribute("style", &rewritten); } @@ -1423,16 +1606,19 @@ mod tests { #[test] fn rewrite_style_urls_honors_escaped_quote_inside_quoted_url() { let settings = crate::test_support::tests::create_test_settings(); + // An escaped quote does not end the string, so the whole value is the + // URL and it resolves with a literal quote in the path. let css = r#"background:url("https://cdn.example/a\")b.png")"#; + let out = rewrite_style_urls(&settings, css, ""); - let truncated = super::build_proxy_url(&settings, r#"https://cdn.example/a\"#, ""); + assert!( - !out.contains(&truncated), - "an escaped quote should not end the string early: {out}" - ); - assert_eq!( - out, css, - "an escape cannot be resolved here, so the value is left alone" + out.contains(&super::build_proxy_url( + &settings, + "https://cdn.example/a\")b.png", + "" + )), + "should proxy the resolved value: {out}" ); } @@ -1463,21 +1649,46 @@ mod tests { } #[test] - fn rewrite_style_urls_leaves_a_value_carrying_an_escape_unrewritten() { + fn rewrite_style_urls_resolves_an_escape_before_deciding() { let settings = crate::test_support::tests::create_test_settings(); - // The rewriter does not resolve CSS escapes, so it cannot know which URL - // the browser will actually request. Emitting a proxy token for the raw - // bytes would point at a different resource than the page asked for. + // The browser resolves `\\2e ` to `.` and requests the result, so an + // escape must not hide an absolute URL from the rewriter — leaving it + // alone would send the request to the third party with the visitor's + // address and cookies. for css in [ "background:url(\"https://cdn.example/a\\2e png\")", "background:url(\"https://cdn.example/a\\\r\nb.png\")", "background:url(\"https://cdn.example/a\\\r\nb)c.png\")", ] { let out = rewrite_style_urls(&settings, css, ""); - assert_eq!(out, css, "should leave an escaped value alone: {css}"); + assert!( + out.contains("/first-party/proxy?tsurl="), + "an escaped absolute URL must still be proxied: {css} -> {out}" + ); } } + #[test] + fn rewrite_style_urls_proxies_the_url_the_browser_will_request() { + let settings = crate::test_support::tests::create_test_settings(); + // `\\70 ` is `p`, so the browser asks for `pixel.gif`. The proxied + // token must name that, not the raw bytes. + let out = rewrite_style_urls( + &settings, + "background:url(\"https://tracker.example/\\70 ixel.gif\")", + "", + ); + + assert!( + out.contains(&super::build_proxy_url( + &settings, + "https://tracker.example/pixel.gif", + "" + )), + "should proxy the resolved URL: {out}" + ); + } + #[test] fn rewrite_style_urls_treats_a_form_feed_as_ending_the_string() { let settings = crate::test_support::tests::create_test_settings(); @@ -1504,6 +1715,464 @@ mod tests { ); } + #[test] + fn rewrite_style_urls_leaves_a_bad_string_byte_for_byte() { + let settings = crate::test_support::tests::create_test_settings(); + // An unterminated string puts the tokenizer into the same recovery a + // browser performs, so the declaration is discarded rather than + // interpreted. Previously the scan fused the span with a quote from a + // later declaration and emitted a proxy token for the joined text. + let css = "a{background:url(\"https://cdn.example/a.png);\ +b{background:url(\"https://cdn.example/c.png\")}"; + + let out = rewrite_style_urls(&settings, css, ""); + + assert_eq!(out, css, "should not invent a value out of a bad string"); + assert!( + !out.contains("%29%3B"), + "should not emit a token built from a fused span: {out}" + ); + } + + #[test] + fn rewrite_style_urls_proxies_a_url_token_unterminated_at_end_of_input() { + let settings = crate::test_support::tests::create_test_settings(); + // A url token that runs to the end of input is still a url token, and + // the browser fetches it, so the old bail-out was a standing bypass. + let out = rewrite_style_urls( + &settings, + "a{background:url('https://tracker.example/pixel.gif'", + "", + ); + + assert!( + out.contains("/first-party/proxy?tsurl="), + "should proxy a url token that reaches end of input: {out}" + ); + } + + /// Splices a large number of adversarial inputs to prove the rewrite never + /// slices a multi-byte character. + /// + /// The rewrite copies the original bytes around each token it replaces, so + /// every offset it uses has to fall on a character boundary. Slicing + /// between the bytes of one character panics, and the guest aborts on + /// panic, so such an input would answer a request with a failure. The + /// generator is a fixed-seed xorshift rather than a random source, so a + /// failure reproduces exactly. + #[test] + fn rewrite_style_urls_never_slices_a_character_in_half() { + let settings = crate::test_support::tests::create_test_settings(); + // Mixes CSS structure with multi-byte characters, including a combining + // mark and a zero-width mark, so a boundary lands mid-character often. + let alphabet = [ + "url(", + ")", + "\"", + "'", + "{", + "}", + "/*", + "*/", + "\\", + ";", + ":", + ",", + " ", + "\n", + "\r", + "\t", + "\u{0c}", + "https://cdn.example/", + "//cdn.example/", + "a", + "-", + "@import", + "image-set(", + "var(--x)", + "data:image/png;base64,", + "\u{e9}", + "\u{4e2d}\u{6587}", + "\u{1f600}", + "\u{301}", + "\u{feff}", + ]; + let mut state: u64 = 0x2545_F491_4F6C_DD1D; + let mut next = || { + state ^= state << 13; + state ^= state >> 7; + state ^= state << 17; + state + }; + + for case in 0..20_000u32 { + let segments = (next() % 24) as usize + 1; + let mut css = String::new(); + for _ in 0..segments { + css.push_str(alphabet[(next() as usize) % alphabet.len()]); + } + + let out = rewrite_style_urls(&settings, &css, ""); + + // Output either keeps the original bytes, is rejected outright, + // or carries the proxy token it was rewritten for. Anything else + // means bytes were dropped or duplicated by the splice. + assert!( + out == css || out.is_empty() || out.contains("/first-party/proxy?tsurl="), + "case {case} changed {css:?} into {out:?} without a proxy token" + ); + } + } + + #[test] + fn rewrite_style_urls_proxies_a_bare_string_candidate_in_image_set() { + let settings = crate::test_support::tests::create_test_settings(); + // `image-set()` takes each candidate as a `url()` or a plain string, + // and the browser fetches the string form identically. + for css in [ + "background:image-set(\"https://tracker.example/a.png\" 1x)", + "background:-webkit-image-set(\"https://tracker.example/a.png\" 1x)", + ] { + let out = rewrite_style_urls(&settings, css, ""); + + assert!( + out.contains(&super::build_proxy_url( + &settings, + "https://tracker.example/a.png", + "" + )), + "should proxy a bare image-set candidate: {css} -> {out}" + ); + } + } + + #[test] + fn rewrite_style_urls_proxies_every_image_set_candidate() { + let settings = crate::test_support::tests::create_test_settings(); + let out = rewrite_style_urls( + &settings, + "background:image-set(\"https://tracker.example/a.png\" 1x,\"https://tracker.example/b.png\" 2x)", + "", + ); + + for name in ["a.png", "b.png"] { + assert!( + out.contains(&super::build_proxy_url( + &settings, + &format!("https://tracker.example/{name}"), + "" + )), + "should proxy {name}: {out}" + ); + } + } + + #[test] + fn rewrite_style_urls_proxies_a_src_function() { + let settings = crate::test_support::tests::create_test_settings(); + let out = rewrite_style_urls( + &settings, + "@font-face{src:src(\"https://tracker.example/a.woff2\")}", + "", + ); + + assert!( + out.contains(&super::build_proxy_url( + &settings, + "https://tracker.example/a.woff2", + "" + )), + "should proxy a src() argument: {out}" + ); + } + + #[test] + fn rewrite_style_urls_proxies_a_bare_import_prelude() { + let settings = crate::test_support::tests::create_test_settings(); + // `@import "…";` loads a whole third-party stylesheet. + let out = rewrite_style_urls(&settings, "@import \"https://tracker.example/s.css\";", ""); + + assert!( + out.contains(&super::build_proxy_url( + &settings, + "https://tracker.example/s.css", + "" + )), + "should proxy an @import prelude string: {out}" + ); + } + + #[test] + fn rewrite_style_urls_leaves_strings_that_are_not_urls_alone() { + let settings = crate::test_support::tests::create_test_settings(); + // A string is only a URL in the few contexts that fetch it. Rewriting + // one anywhere else would corrupt the declaration. + for css in [ + "font-family:\"https://tracker.example/a.png\"", + "content:\"https://tracker.example/a.png\"", + "background:linear-gradient(\"https://tracker.example/a.png\")", + "@media \"https://tracker.example/a.png\"{a{color:red}}", + "@import \"https://tracker.example/s.css\";content:\"https://tracker.example/a.png\"", + ] { + let out = rewrite_style_urls(&settings, css, ""); + + assert!( + !out.contains("content:\"/first-party") + && !out.contains("font-family:\"/first-party") + && !out.contains("linear-gradient(\"/first-party") + && !out.contains("@media \"/first-party"), + "should not treat a non-URL string as a URL: {css} -> {out}" + ); + } + } + + #[test] + fn rewrite_style_attribute_urls_leaves_a_leading_import_rule_alone() { + let settings = crate::test_support::tests::create_test_settings(); + // A style attribute is a declaration list, so an at-rule is not valid + // there and the browser never loads it. The same bytes in a stylesheet + // are a real `@import`, which is why the two entry points differ. + let css = "@import \"https://tracker.example/x.css\""; + + let attribute = super::rewrite_style_attribute_urls(&settings, css, ""); + let stylesheet = rewrite_style_urls(&settings, css, ""); + + assert_eq!( + attribute, css, + "should not read an at-rule in a style attribute: {attribute}" + ); + assert!( + stylesheet.contains(&super::build_proxy_url( + &settings, + "https://tracker.example/x.css", + "" + )), + "should still read the same bytes as a rule in a stylesheet: {stylesheet}" + ); + } + + #[test] + fn rewrite_style_urls_confines_an_import_prelude_to_its_first_value() { + let settings = crate::test_support::tests::create_test_settings(); + // `@import` takes one stylesheet URL; a layer, supports condition or + // media query may follow, and none of those is a URL. + let out = rewrite_style_urls( + &settings, + "@import \"https://cdn.example/x.css\" \"https://tracker.example/z.css\";", + "", + ); + + assert!( + out.contains(&super::build_proxy_url( + &settings, + "https://cdn.example/x.css", + "" + )), + "should proxy the prelude's URL: {out}" + ); + assert!( + out.contains("\"https://tracker.example/z.css\""), + "should leave a later prelude string alone: {out}" + ); + } + + #[test] + fn rewrite_style_urls_keeps_an_unterminated_import_out_of_later_declarations() { + let settings = crate::test_support::tests::create_test_settings(); + // An `@import` with no semicolon runs to the end of its block, so a + // following string sits inside the at-rule. It is still not a URL, and + // rewriting it would rewrite a `content` value. + let css = "@import \"https://cdn.example/x.css\" content:\"https://legit.example/y.png\""; + let out = rewrite_style_urls(&settings, css, ""); + + assert!( + !out.contains("content:url("), + "should not turn a content value into a URL: {css} -> {out}" + ); + assert_eq!( + out.matches("/first-party/proxy?tsurl=").count(), + 1, + "should proxy only the stylesheet URL: {out}" + ); + } + + #[test] + fn rewrite_style_urls_leaves_an_import_token_in_a_custom_property_alone() { + let settings = crate::test_support::tests::create_test_settings(); + let css = ".a{--metadata:@import \"https://tracker.example/text\";}"; + + let out = rewrite_style_urls(&settings, css, ""); + + assert_eq!( + out, css, + "should not treat @import inside a declaration as a stylesheet rule" + ); + } + + #[test] + fn rewrite_style_urls_leaves_an_import_token_in_another_at_rule_prelude_alone() { + let settings = crate::test_support::tests::create_test_settings(); + let css = "@unknown @import \"https://tracker.example/text\";"; + + let out = rewrite_style_urls(&settings, css, ""); + + assert_eq!( + out, css, + "should recognize @import only where a top-level rule may start" + ); + } + + #[test] + fn rewrite_style_urls_reads_an_import_value_past_whitespace_and_comments() { + let settings = crate::test_support::tests::create_test_settings(); + let out = rewrite_style_urls( + &settings, + "@import /*c*/ \"https://cdn.example/x.css\";", + "", + ); + + assert!( + out.contains(&super::build_proxy_url( + &settings, + "https://cdn.example/x.css", + "" + )), + "should not spend the prelude's value on whitespace: {out}" + ); + } + + #[test] + fn rewrite_style_urls_reads_an_import_past_legacy_stylesheet_wrappers() { + let settings = crate::test_support::tests::create_test_settings(); + + for css in [ + " @import \"https://cdn.example/x.css\";", + ] { + let out = rewrite_style_urls(&settings, css, ""); + + assert!( + out.contains(&super::build_proxy_url( + &settings, + "https://cdn.example/x.css", + "" + )), + "should ignore a top-level CDO/CDC token before @import: {css} -> {out}" + ); + } + } + + #[test] + fn rewrite_style_urls_does_not_count_an_import_as_the_parent_of_later_css() { + let settings = crate::test_support::tests::create_test_settings(); + let css = format!( + "@import \"https://cdn.example/x.css\";{}background:url(https://tracker.example/y.png){}", + "a{".repeat(super::MAX_CSS_NESTING_DEPTH), + "}".repeat(super::MAX_CSS_NESTING_DEPTH) + ); + let out = rewrite_style_urls(&settings, &css, ""); + + for url in ["https://cdn.example/x.css", "https://tracker.example/y.png"] { + assert!( + out.contains(&super::build_proxy_url(&settings, url, "")), + "should proxy {url} without treating later CSS as part of the import: {out}" + ); + } + } + + #[test] + fn rewrite_style_urls_rejects_a_quoted_url_past_the_supported_depth() { + let settings = crate::test_support::tests::create_test_settings(); + let css = format!( + "{}background:url(\"https://cdn.example/a.png\"){}", + "a{".repeat(super::MAX_CSS_NESTING_DEPTH), + "}".repeat(super::MAX_CSS_NESTING_DEPTH) + ); + + let out = rewrite_style_urls(&settings, &css, ""); + + assert!( + out.is_empty(), + "should count the quoted url() parser frame against the depth bound" + ); + } + + #[test] + fn rewrite_style_urls_still_proxies_image_set_after_an_unexpected_token() { + let settings = crate::test_support::tests::create_test_settings(); + // Every candidate in an `image-set()` is a URL, so an unexpected token + // in the list must not switch that off for the ones after it. + let out = rewrite_style_urls( + &settings, + "background:image-set(@x \"https://tracker.example/a.png\" 1x)", + "", + ); + + assert!( + out.contains(&super::build_proxy_url( + &settings, + "https://tracker.example/a.png", + "" + )), + "should proxy a candidate following an unexpected token: {out}" + ); + } + + #[test] + fn rewrite_style_urls_rejects_css_nested_past_the_supported_depth() { + let settings = crate::test_support::tests::create_test_settings(); + // Well past the cap and well past what the guest stack survives, so an + // unbounded walk would abort the request rather than answer it. + let css = "{".repeat(super::MAX_CSS_NESTING_DEPTH * 40); + + let out = rewrite_style_urls(&settings, &css, ""); + + assert!( + out.is_empty(), + "should reject rather than return partly inspected CSS" + ); + } + + #[test] + fn rewrite_style_urls_rewrites_at_the_deepest_supported_nesting() { + let settings = crate::test_support::tests::create_test_settings(); + // One url() at exactly the cap, to pin that the bound admits the depth + // it advertises rather than stopping one level short. + let css = format!( + "{}background:url(https://cdn.example/a.png){}", + "a{".repeat(super::MAX_CSS_NESTING_DEPTH), + "}".repeat(super::MAX_CSS_NESTING_DEPTH) + ); + + let out = rewrite_style_urls(&settings, &css, ""); + + assert!( + out.contains(&super::build_proxy_url( + &settings, + "https://cdn.example/a.png", + "" + )), + "should still rewrite at the deepest supported level: {out}" + ); + } + + #[test] + fn rewrite_style_urls_rewrites_inside_ordinary_nesting() { + let settings = crate::test_support::tests::create_test_settings(); + let css = "@media screen{.a{background:url(https://cdn.example/a.png)}}"; + + let out = rewrite_style_urls(&settings, css, ""); + + assert!( + out.contains(&super::build_proxy_url( + &settings, + "https://cdn.example/a.png", + "" + )), + "should rewrite a url() inside a media query: {out}" + ); + } + #[test] fn rewrite_style_urls_handles_mismatched_quotes() { let settings = crate::test_support::tests::create_test_settings(); @@ -1736,6 +2405,23 @@ mod tests { assert!(!out.contains("https://cdn.example/bg.png")); } + #[test] + fn leaves_an_import_token_and_string_in_an_inline_custom_property_alone() { + let settings = crate::test_support::tests::create_test_settings(); + let html = r#"
ad
"#; + + let out = rewrite_creative_html(&settings, html); + + assert!( + !out.contains("/first-party/proxy?tsurl="), + "should not treat @import inside an inline declaration as an at-rule: {out}" + ); + assert!( + out.contains("https://tracker.example/text"), + "should preserve the custom-property string: {out}" + ); + } + #[test] fn rewrites_style_block_url_variants() { let settings = crate::test_support::tests::create_test_settings(); From ff6472277a5f97dff9315041ffa2b5433f56b1ae Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Wed, 2 Sep 2026 20:29:45 +0530 Subject: [PATCH 5/8] Re-emit a rewritten CSS reference as the function it read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A rewritten value went out as url() whatever it arrived as. For src() that changes what the browser does rather than where it points: an engine that ignores src() leaves the declaration inert, so emitting url() starts a request the origin never made. Keep the name and proxy the value. Read a var() fallback in the context around it, so a candidate written image-set(var(--c, "https://…") 1x) is proxied like the plain string it becomes. The propagation is deliberately narrow: substitution applies to declaration values, so the same fallback in a content value or an @import prelude stays untouched. A URL assembled from a separate custom-property declaration is left alone and documented — pairing the two is the cascade's work, not a rewriter's. --- crates/trusted-server-core/src/creative.rs | 146 ++++++++++++++++++++- 1 file changed, 140 insertions(+), 6 deletions(-) diff --git a/crates/trusted-server-core/src/creative.rs b/crates/trusted-server-core/src/creative.rs index 983f87985..b77987225 100644 --- a/crates/trusted-server-core/src/creative.rs +++ b/crates/trusted-server-core/src/creative.rs @@ -112,6 +112,14 @@ const MAX_CSS_NESTING_DEPTH: usize = 64; /// is accepted everywhere the other forms are. Anything not rewritten keeps /// its original bytes. /// +/// A URL that only exists after custom-property substitution is out of reach: +/// `--c:"https://t.example/a.png"` used as `image-set(var(--c) 1x)` is a URL to +/// the browser, but the string and its use are separate declarations and +/// resolving one against the other is the cascade's job, not a rewriter's. The +/// inline fallback form, `image-set(var(--c, "https://t.example/a.png") 1x)`, +/// is substituted in place and is rewritten. A `url()` token in a custom +/// property is also rewritten, since it is a URL wherever it lands. +/// /// CSS nested past [`MAX_CSS_NESTING_DEPTH`] is rejected outright (empty string /// returned), matching [`MAX_CREATIVE_SIZE`]. The alternative — keeping the /// rewrite of everything above the cap and passing the deeper bytes through — @@ -207,11 +215,17 @@ impl CssUrlRewriter<'_> { CssToken::Url(value.as_ref().to_owned()) } Ok(cssparser::Token::Function(name)) if is_url_function(name.as_ref()) => { - CssToken::UrlFunction + CssToken::UrlFunction(url_function_name(name.as_ref())) } Ok(cssparser::Token::Function(name)) => CssToken::Block( if takes_bare_string_urls(name.as_ref()) { BareStringUrls::Every + } else if bare_strings == BareStringUrls::Every + && name.eq_ignore_ascii_case("var") + { + // A `var()` fallback is substituted in place, so a + // string there is read in the context around it. + BareStringUrls::Every } else { BareStringUrls::Never }, @@ -245,13 +259,13 @@ impl CssUrlRewriter<'_> { match found { CssToken::Url(value) => self.rewrite(&value, token_start, parser), - CssToken::UrlFunction => { + CssToken::UrlFunction(function) => { if depth >= MAX_CSS_NESTING_DEPTH { self.depth_exceeded = true; return; } if let Some(value) = quoted_url_argument(parser) { - self.rewrite(&value, token_start, parser); + self.rewrite_as(&value, token_start, parser, function); } } CssToken::ImportPrelude => { @@ -302,12 +316,30 @@ impl CssUrlRewriter<'_> { /// Replaces the span just consumed with a proxied `url()`, or leaves it. fn rewrite(&mut self, value: &str, token_start: usize, parser: &cssparser::Parser<'_, '_>) { + self.rewrite_as(value, token_start, parser, "url"); + } + + /// Replaces the span just consumed with a proxied reference named + /// `function`, or leaves it. + /// + /// The name is preserved rather than normalized to `url`, because the two + /// are not interchangeable to a browser: rewriting a `src()` the engine + /// ignores into a `url()` it honours would start a request the origin + /// never made. + fn rewrite_as( + &mut self, + value: &str, + token_start: usize, + parser: &cssparser::Parser<'_, '_>, + function: &str, + ) { let Some(absolute) = to_abs(self.settings, value) else { return; }; let token_end = parser.position().byte_index(); self.out.push_str(&self.style[self.write_pos..token_start]); - self.out.push_str("url("); + self.out.push_str(function); + self.out.push('('); cssparser::serialize_string( &build_proxy_url(self.settings, &absolute, self.base_origin), &mut self.out, @@ -322,8 +354,9 @@ impl CssUrlRewriter<'_> { enum CssToken { /// An unquoted `url(...)`, carrying its resolved value. Url(String), - /// The opening of a `url(` function, whose argument has yet to be read. - UrlFunction, + /// The opening of a `url(` or `src(` function, whose argument has yet to + /// be read. Carries the name to re-emit, which is not always `url`. + UrlFunction(&'static str), /// A block or other function, whose body may contain a `url()`. Carries /// whether a bare string inside it is itself a URL and whether its closing /// returns the outer parser to a top-level rule boundary. @@ -351,6 +384,15 @@ enum BareStringUrls { FirstValue, } +/// The name to re-emit for a URL function, preserving `src()` as itself. +fn url_function_name(name: &str) -> &'static str { + if name.eq_ignore_ascii_case("src") { + "src" + } else { + "url" + } +} + /// Whether a function's argument is a URL, as `url()` and `src()` both are. fn is_url_function(name: &str) -> bool { name.eq_ignore_ascii_case("url") || name.eq_ignore_ascii_case("src") @@ -2118,6 +2160,98 @@ b{background:url(\"https://cdn.example/c.png\")}"; ); } + #[test] + fn rewrite_style_urls_keeps_a_src_function_as_src() { + let settings = crate::test_support::tests::create_test_settings(); + // `src()` and `url()` are not interchangeable: an engine that ignores + // `src()` makes the declaration inert, so emitting `url()` would start + // a request the origin never made. + let out = rewrite_style_urls( + &settings, + "background-image:src(\"https://tracker.example/a.png\")", + "", + ); + + assert!( + out.starts_with("background-image:src("), + "should re-emit the function it read: {out}" + ); + assert!( + out.contains(&super::build_proxy_url( + &settings, + "https://tracker.example/a.png", + "" + )), + "should still proxy the value: {out}" + ); + } + + #[test] + fn rewrite_style_urls_proxies_a_var_fallback_in_a_bare_string_context() { + let settings = crate::test_support::tests::create_test_settings(); + // A `var()` fallback is substituted in place, so inside `image-set()` + // the fallback string is a URL candidate the browser fetches. + let out = rewrite_style_urls( + &settings, + "background-image:image-set(var(--missing, \"https://tracker.example/a.png\") 1x)", + "", + ); + + assert!( + out.contains(&super::build_proxy_url( + &settings, + "https://tracker.example/a.png", + "" + )), + "should proxy a var() fallback candidate: {out}" + ); + } + + #[test] + fn rewrite_style_urls_leaves_a_var_fallback_outside_a_url_context_alone() { + let settings = crate::test_support::tests::create_test_settings(); + // The same fallback in a non-URL context is ordinary text, and an + // `@import` prelude takes no substitution at all. + for css in [ + "content:var(--missing, \"https://tracker.example/a.png\")", + "@import var(--missing, \"https://tracker.example/x.css\");", + ] { + let out = rewrite_style_urls(&settings, css, ""); + + assert_eq!( + out, css, + "should not read a fallback as a URL outside a URL context: {css}" + ); + } + } + + #[test] + fn rewrite_style_urls_cannot_resolve_a_custom_property_used_as_a_candidate() { + let settings = crate::test_support::tests::create_test_settings(); + // Documents a known limit: the string and its use are separate + // declarations, and pairing them is the cascade's job. A `url()` token + // in a custom property is still rewritten, since it is a URL anywhere. + let indirect = rewrite_style_urls( + &settings, + "a{--c:\"https://tracker.example/a.png\";background-image:image-set(var(--c) 1x)}", + "", + ); + assert!( + !indirect.contains("/first-party/proxy?tsurl="), + "substitution is out of reach, so nothing is claimed: {indirect}" + ); + + let token = rewrite_style_urls(&settings, "a{--c:url(https://tracker.example/a.png)}", ""); + assert!( + token.contains(&super::build_proxy_url( + &settings, + "https://tracker.example/a.png", + "" + )), + "a url() token in a custom property is still a URL: {token}" + ); + } + #[test] fn rewrite_style_urls_rejects_css_nested_past_the_supported_depth() { let settings = crate::test_support::tests::create_test_settings(); From 14dd0fc223214bec668dc22704884d2fa1735d82 Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Wed, 2 Sep 2026 21:20:24 +0530 Subject: [PATCH 6/8] Keep the shape a rewritten CSS reference was read in Bare-string references still went out wrapped in url(). That is valid in the places they are read, so nothing broke, but it is not valid once the same candidate is substituted into a src() argument, which has to stay a string. Re-emit each reference in the shape it arrived in and replace only the value. That makes the remaining src() gap fixable: src() takes a normal value list, so a var() there is substituted and its fallback is the string the engine ends up with. Walk it and rewrite the fallback where it sits, leaving both calls intact. url() is deliberately excluded, since an engine does not substitute inside it and a fallback there is never requested. The entry-point note claimed everything was re-emitted as url(), which the earlier src() change had already made untrue. --- crates/trusted-server-core/src/creative.rs | 195 +++++++++++++++------ 1 file changed, 141 insertions(+), 54 deletions(-) diff --git a/crates/trusted-server-core/src/creative.rs b/crates/trusted-server-core/src/creative.rs index b77987225..252a3639a 100644 --- a/crates/trusted-server-core/src/creative.rs +++ b/crates/trusted-server-core/src/creative.rs @@ -107,10 +107,11 @@ const MAX_CSS_NESTING_DEPTH: usize = 64; /// a malformed value — which the tokenizer reports as a bad URL or bad string, /// exactly what a browser discards — is left untouched rather than guessed at. /// -/// A rewritten reference is re-emitted as `url("…")` whichever form it came -/// in as, so the output is normalized rather than byte-preserved — `url()` -/// is accepted everywhere the other forms are. Anything not rewritten keeps -/// its original bytes. +/// A rewritten reference keeps the shape it was read in — `url()` as `url()`, +/// `src()` as `src()`, a bare string as a bare string — because the forms are +/// not interchangeable to a browser. Only the value inside is replaced, and +/// it is re-quoted, so the output is normalized in that respect rather than +/// byte-preserved. Anything not rewritten keeps its original bytes. /// /// A URL that only exists after custom-property substitution is out of reach: /// `--c:"https://t.example/a.png"` used as `image-set(var(--c) 1x)` is a URL to @@ -207,12 +208,12 @@ impl CssUrlRewriter<'_> { // a block or function is read. let found = match parser.next_including_whitespace_and_comments() { Ok(cssparser::Token::UnquotedUrl(value)) => { - CssToken::Url(value.as_ref().to_owned()) + CssToken::Url(value.as_ref().to_owned(), UrlShape::Function("url")) } Ok(cssparser::Token::QuotedString(value)) if bare_strings != BareStringUrls::Never => { - CssToken::Url(value.as_ref().to_owned()) + CssToken::Url(value.as_ref().to_owned(), UrlShape::BareString) } Ok(cssparser::Token::Function(name)) if is_url_function(name.as_ref()) => { CssToken::UrlFunction(url_function_name(name.as_ref())) @@ -258,15 +259,13 @@ impl CssUrlRewriter<'_> { let ends_rule = matches!(found, CssToken::RuleBoundary | CssToken::Block(_, true)); match found { - CssToken::Url(value) => self.rewrite(&value, token_start, parser), + CssToken::Url(value, shape) => self.rewrite(&value, token_start, parser, shape), CssToken::UrlFunction(function) => { if depth >= MAX_CSS_NESTING_DEPTH { self.depth_exceeded = true; return; } - if let Some(value) = quoted_url_argument(parser) { - self.rewrite_as(&value, token_start, parser, function); - } + self.rewrite_url_function(parser, token_start, function, depth); } CssToken::ImportPrelude => { if depth >= MAX_CSS_NESTING_DEPTH { @@ -314,49 +313,80 @@ impl CssUrlRewriter<'_> { } } - /// Replaces the span just consumed with a proxied `url()`, or leaves it. - fn rewrite(&mut self, value: &str, token_start: usize, parser: &cssparser::Parser<'_, '_>) { - self.rewrite_as(value, token_start, parser, "url"); - } - - /// Replaces the span just consumed with a proxied reference named - /// `function`, or leaves it. - /// - /// The name is preserved rather than normalized to `url`, because the two - /// are not interchangeable to a browser: rewriting a `src()` the engine - /// ignores into a `url()` it honours would start a request the origin - /// never made. - fn rewrite_as( + /// Replaces the span just consumed with a proxied reference in `shape`, or + /// leaves it. + fn rewrite( &mut self, value: &str, token_start: usize, parser: &cssparser::Parser<'_, '_>, - function: &str, + shape: UrlShape, ) { let Some(absolute) = to_abs(self.settings, value) else { return; }; let token_end = parser.position().byte_index(); self.out.push_str(&self.style[self.write_pos..token_start]); - self.out.push_str(function); - self.out.push('('); - cssparser::serialize_string( - &build_proxy_url(self.settings, &absolute, self.base_origin), - &mut self.out, - ) - .expect("should write a serialized URL into a String"); - self.out.push(')'); + let proxied = build_proxy_url(self.settings, &absolute, self.base_origin); + if let UrlShape::Function(name) = shape { + self.out.push_str(name); + self.out.push('('); + } + cssparser::serialize_string(&proxied, &mut self.out) + .expect("should write a serialized URL into a String"); + if matches!(shape, UrlShape::Function(_)) { + self.out.push(')'); + } self.write_pos = token_end; } + + /// Rewrites a `url(` or `src(` call, or descends into its argument. + /// + /// The argument is normally a single string. A `src()` may instead hold a + /// `var()`, whose fallback is substituted in place, so that is walked and + /// the fallback rewritten where it sits — leaving the call's own shape + /// alone, since a substituted `src()` argument has to stay a string. A + /// `url()` argument is not walked: an engine does not substitute inside + /// it, so a fallback there is never the URL that gets requested. + fn rewrite_url_function( + &mut self, + parser: &mut cssparser::Parser<'_, '_>, + token_start: usize, + function: &'static str, + depth: usize, + ) { + let substitutes = function == "src"; + let mut single_string = None; + let _ = parser.parse_nested_block(|inner| -> Result<(), cssparser::ParseError<'_, ()>> { + let start = inner.state(); + if let Ok(cssparser::Token::QuotedString(read)) = inner.next() { + let read = read.as_ref().to_owned(); + if inner.is_exhausted() { + single_string = Some(read); + return Ok(()); + } + } + inner.reset(&start); + if substitutes { + self.walk(inner, depth + 1, BareStringUrls::Every); + } + Ok(()) + }); + if let Some(value) = single_string { + self.rewrite(&value, token_start, parser, UrlShape::Function(function)); + } + } } /// What a token turned out to be while scanning for `url()` references. enum CssToken { - /// An unquoted `url(...)`, carrying its resolved value. - Url(String), + /// A reference read as a value in its own right, carrying its resolved + /// value and the shape to write back. + Url(String, UrlShape), /// The opening of a `url(` or `src(` function, whose argument has yet to /// be read. Carries the name to re-emit, which is not always `url`. UrlFunction(&'static str), + /// A block or other function, whose body may contain a `url()`. Carries /// whether a bare string inside it is itself a URL and whether its closing /// returns the outer parser to a top-level rule boundary. @@ -384,6 +414,21 @@ enum BareStringUrls { FirstValue, } +/// How a rewritten reference is written back. +/// +/// The shape is preserved rather than normalized, because the forms are not +/// interchangeable: `src()` is honoured where `url()` is not, and a bare +/// string is the only form an `image-set()` candidate may take once it has +/// been substituted into a `src()` argument. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum UrlShape { + /// A function call, written with the name it was read as. + Function(&'static str), + /// A bare quoted string, as an `image-set()` candidate or an `@import` + /// prelude uses. + BareString, +} + /// The name to re-emit for a URL function, preserving `src()` as itself. fn url_function_name(name: &str) -> &'static str { if name.eq_ignore_ascii_case("src") { @@ -406,26 +451,6 @@ fn takes_bare_string_urls(name: &str) -> bool { name.eq_ignore_ascii_case("image-set") || name.eq_ignore_ascii_case("-webkit-image-set") } -/// Reads the argument of a `url(` or `src(` function when it is a single string. -/// -/// Returns `None` for any other shape — a bad string, or a string carrying -/// anything after it — leaving the original bytes in place rather than -/// guessing at the intended value. The trailing case covers both a malformed -/// value and the `` grammar, which no engine acts on today; if -/// one does, such a value would need proxying rather than skipping. -fn quoted_url_argument(parser: &mut cssparser::Parser<'_, '_>) -> Option { - parser - .parse_nested_block( - |inner| -> Result, cssparser::ParseError<'_, ()>> { - Ok(match inner.next() { - Ok(cssparser::Token::QuotedString(value)) => Some(value.as_ref().to_owned()), - _ => None, - }) - }, - ) - .unwrap_or(None) -} - #[inline] fn build_signed_url_for( settings: &Settings, @@ -2252,6 +2277,68 @@ b{background:url(\"https://cdn.example/c.png\")}"; ); } + #[test] + fn rewrite_style_urls_proxies_a_var_fallback_inside_src() { + let settings = crate::test_support::tests::create_test_settings(); + // `src()` takes a normal value list, so a `var()` there is substituted + // and its fallback is the string the engine ends up with. + let out = rewrite_style_urls( + &settings, + "@font-face{src:src(var(--missing, \"https://tracker.example/a.woff2\"))}", + "", + ); + + assert!( + out.contains(&super::build_proxy_url( + &settings, + "https://tracker.example/a.woff2", + "" + )), + "should proxy the fallback: {out}" + ); + // Both calls must survive: substitution replaces the `var()` with the + // string, and a `src()` argument has to stay a string. + assert!( + out.contains("src:src(var(--missing, \"") && !out.contains("src(url("), + "should rewrite in place and keep both calls: {out}" + ); + } + + #[test] + fn rewrite_style_urls_leaves_a_var_fallback_inside_url_alone() { + let settings = crate::test_support::tests::create_test_settings(); + // An engine does not substitute inside `url()`, so a fallback there is + // never the URL it requests, and rewriting it would claim otherwise. + let css = "background:url(var(--missing, \"https://tracker.example/a.png\"))"; + + let out = rewrite_style_urls(&settings, css, ""); + + assert_eq!(out, css, "should leave a url() argument unsubstituted"); + } + + #[test] + fn rewrite_style_urls_re_emits_a_bare_string_as_a_bare_string() { + let settings = crate::test_support::tests::create_test_settings(); + // Wrapping these in `url()` would be valid here, but it stops being + // valid once the same candidate is substituted into a `src()`. + let candidate = rewrite_style_urls( + &settings, + "background:image-set(\"https://tracker.example/a.png\" 1x)", + "", + ); + assert!( + candidate.starts_with("background:image-set(\"") && !candidate.contains("set(url("), + "an image-set candidate stays a string: {candidate}" + ); + + let prelude = + rewrite_style_urls(&settings, "@import \"https://tracker.example/s.css\";", ""); + assert!( + prelude.starts_with("@import \"") && !prelude.contains("@import url("), + "an @import prelude stays a string: {prelude}" + ); + } + #[test] fn rewrite_style_urls_rejects_css_nested_past_the_supported_depth() { let settings = crate::test_support::tests::create_test_settings(); From 83e629d2309d67e5d3004895372b9eb7b6573221 Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Thu, 3 Sep 2026 10:10:23 +0530 Subject: [PATCH 7/8] Follow an env() fallback like a var() fallback Both resolve to their fallback when the name is not set, so a string written there is a string the engine ends up with. Only var() was followed, which left image-set(env(--x, "https://...")) unproxied - and that is a supported feature reached by an unrecognised name, which is the case the fallback exists for, not a future one. Follow both, and pin that this reaches no further: a string in a gradient nested inside image-set is still not a URL. --- crates/trusted-server-core/src/creative.rs | 76 +++++++++++++++++++++- 1 file changed, 73 insertions(+), 3 deletions(-) diff --git a/crates/trusted-server-core/src/creative.rs b/crates/trusted-server-core/src/creative.rs index 252a3639a..2de85b7ef 100644 --- a/crates/trusted-server-core/src/creative.rs +++ b/crates/trusted-server-core/src/creative.rs @@ -222,10 +222,10 @@ impl CssUrlRewriter<'_> { if takes_bare_string_urls(name.as_ref()) { BareStringUrls::Every } else if bare_strings == BareStringUrls::Every - && name.eq_ignore_ascii_case("var") + && substitutes_a_fallback(name.as_ref()) { - // A `var()` fallback is substituted in place, so a - // string there is read in the context around it. + // The fallback is substituted in place, so a string + // there is read in the context around it. BareStringUrls::Every } else { BareStringUrls::Never @@ -443,6 +443,18 @@ fn is_url_function(name: &str) -> bool { name.eq_ignore_ascii_case("url") || name.eq_ignore_ascii_case("src") } +/// Whether a function substitutes a fallback argument in place. +/// +/// `var()` and `env()` both resolve to their fallback when the name they +/// reference is not set, so a string written there is a string the engine ends +/// up with. `env()` matters as much as `var()` despite being the rarer of the +/// two: an unrecognised name is exactly the case its fallback exists for, and +/// `image-set()` — where such a fallback is a URL candidate — is supported +/// everywhere today. +fn substitutes_a_fallback(name: &str) -> bool { + name.eq_ignore_ascii_case("var") || name.eq_ignore_ascii_case("env") +} + /// Whether a function accepts a bare string as a URL, rather than only `url()`. /// /// `image-set()` takes each candidate as either a `url()` or a plain string, @@ -2339,6 +2351,64 @@ b{background:url(\"https://cdn.example/c.png\")}"; ); } + #[test] + fn rewrite_style_urls_proxies_an_env_fallback_like_a_var_fallback() { + let settings = crate::test_support::tests::create_test_settings(); + // `env()` resolves to its fallback when the name is not recognised, + // which is the usual case for a custom name, and `image-set()` is + // supported everywhere — so this candidate is really fetched. + for css in [ + "background:image-set(env(--nope, \"https://tracker.example/a.png\") 1x)", + "background:image-set(env(--a, var(--b, \"https://tracker.example/a.png\")) 1x)", + ] { + let out = rewrite_style_urls(&settings, css, ""); + + assert!( + out.contains(&super::build_proxy_url( + &settings, + "https://tracker.example/a.png", + "" + )), + "should proxy a substituted fallback: {css} -> {out}" + ); + } + } + + #[test] + fn rewrite_style_urls_only_follows_functions_that_substitute() { + let settings = crate::test_support::tests::create_test_settings(); + // Inside `image-set()` a bare string is a URL, but that does not carry + // into any function nested there — only into one whose fallback is + // substituted in its place. A gradient's string is not a URL. + for css in [ + "background:image-set(linear-gradient(\"https://tracker.example/a.png\") 1x)", + "background:image-set(counter(x, \"https://tracker.example/a.png\") 1x)", + ] { + let out = rewrite_style_urls(&settings, css, ""); + + assert_eq!( + out, css, + "should not read a nested function's string as a URL: {css}" + ); + } + } + + #[test] + fn rewrite_style_urls_leaves_an_env_fallback_outside_a_url_context_alone() { + let settings = crate::test_support::tests::create_test_settings(); + for css in [ + "content:env(--nope, \"https://tracker.example/a.png\")", + "background:url(env(--nope, \"https://tracker.example/a.png\"))", + ] { + let out = rewrite_style_urls(&settings, css, ""); + + assert_eq!( + out, css, + "a fallback is only a URL where the surrounding context makes it one: {css}" + ); + } + } + #[test] fn rewrite_style_urls_rejects_css_nested_past_the_supported_depth() { let settings = crate::test_support::tests::create_test_settings(); From 60b5e992c004969836febef3efd5340ceaa85d1c Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Fri, 4 Sep 2026 10:58:43 +0530 Subject: [PATCH 8/8] Bound CSS nesting by scope, and report a refused stylesheet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reading the single string argument of `url()` or `src()` opened a parser scope, so `url("https://…")` was charged a nesting level that the identical `url(https://…)` was not. At exactly the cap that decided whether the whole stylesheet survived, which is not a distinction the constant ever claimed to make. That grammar is terminal and costs no recursion, so it is no longer charged, and every recursion the walk makes now runs through one `descend` that no arm can bypass. A form that does open a scope, such as `image-set()`, still costs a level, and the bound now follows the grammar rather than how a URL is spelled. A refused stylesheet on the CSS proxy path was returned as an empty 200, indistinguishable from a stylesheet the origin legitimately served empty and attributable only from an edge-side log. `rewrite_css_body` now reports the refusal to its caller, so the response carries a status the way the oversized-body path already does. Markup still drops only the offending `