-
Notifications
You must be signed in to change notification settings - Fork 12
Exclude GAM paths from Prebid refresh auctions #965
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -260,6 +260,13 @@ pub struct PrebidIntegrationConfig { | |
| /// manages both lists explicitly. | ||
| #[serde(default, deserialize_with = "crate::settings::vec_from_seq_or_map")] | ||
| pub client_side_bidders: Vec<String>, | ||
| /// GAM ad-unit-path suffixes excluded from Trusted Server refresh auctions. | ||
| /// | ||
| /// Matching is exact and case-sensitive. Excluded slots still refresh through | ||
| /// GAM, but are not included in synthetic Prebid refresh ad units. | ||
| #[serde(default, deserialize_with = "crate::settings::vec_from_seq_or_map")] | ||
| #[validate(custom(function = "validate_excluded_gam_ad_unit_path_suffixes"))] | ||
| pub excluded_gam_ad_unit_path_suffixes: Vec<String>, | ||
| /// Compatibility sugar for per-bidder, per-zone param overrides. | ||
| /// | ||
| /// This preserves the natural `bidder -> zone -> params` config shape for | ||
|
|
@@ -338,6 +345,70 @@ impl IntegrationConfig for PrebidIntegrationConfig { | |
| } | ||
| } | ||
|
|
||
| fn excluded_gam_ad_unit_path_suffix_validation_error(message: &'static str) -> ValidationError { | ||
| let mut error = ValidationError::new("invalid_gam_ad_unit_path_suffix"); | ||
| error.message = Some(message.into()); | ||
| error | ||
| } | ||
|
|
||
| fn validate_excluded_gam_ad_unit_path_suffix(value: &str) -> Result<(), ValidationError> { | ||
| if value.trim() != value { | ||
| return Err(excluded_gam_ad_unit_path_suffix_validation_error( | ||
| "excluded_gam_ad_unit_path_suffixes entries must not have surrounding whitespace", | ||
| )); | ||
| } | ||
|
|
||
| if value.is_empty() { | ||
| return Err(excluded_gam_ad_unit_path_suffix_validation_error( | ||
| "excluded_gam_ad_unit_path_suffixes entries must not be empty", | ||
| )); | ||
| } | ||
|
|
||
| if !value.starts_with('/') { | ||
| return Err(excluded_gam_ad_unit_path_suffix_validation_error( | ||
| "excluded_gam_ad_unit_path_suffixes entries must start with '/'", | ||
| )); | ||
| } | ||
|
|
||
| if value == "/" { | ||
| return Err(excluded_gam_ad_unit_path_suffix_validation_error( | ||
| "excluded_gam_ad_unit_path_suffixes entries must identify a non-root path suffix", | ||
| )); | ||
| } | ||
|
|
||
| Ok(()) | ||
| } | ||
|
|
||
| fn validate_excluded_gam_ad_unit_path_suffixes(values: &[String]) -> Result<(), ValidationError> { | ||
| for value in values { | ||
| validate_excluded_gam_ad_unit_path_suffix(value)?; | ||
| } | ||
|
|
||
| Ok(()) | ||
| } | ||
|
|
||
| fn canonicalize_excluded_gam_ad_unit_path_suffixes(config: &mut PrebidIntegrationConfig) { | ||
|
ChristianPavilonis marked this conversation as resolved.
|
||
| let mut canonical = Vec::with_capacity(config.excluded_gam_ad_unit_path_suffixes.len()); | ||
| for suffix in std::mem::take(&mut config.excluded_gam_ad_unit_path_suffixes) { | ||
| if !canonical.contains(&suffix) { | ||
| canonical.push(suffix); | ||
| } | ||
| } | ||
| config.excluded_gam_ad_unit_path_suffixes = canonical; | ||
| } | ||
|
|
||
| fn load_config( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📝 note — This is the right resolution of the earlier canonicalization concern: For the record on what remains: canonicalization is still a property of this helper rather than of deserialization, so a future caller that reaches for |
||
| settings: &Settings, | ||
| ) -> Result<Option<PrebidIntegrationConfig>, Report<TrustedServerError>> { | ||
| let Some(mut config) = | ||
| settings.integration_config::<PrebidIntegrationConfig>(PREBID_INTEGRATION_ID)? | ||
| else { | ||
| return Ok(None); | ||
| }; | ||
| canonicalize_excluded_gam_ad_unit_path_suffixes(&mut config); | ||
| Ok(Some(config)) | ||
| } | ||
|
|
||
| /// Validate enabled Prebid config using the same startup-only checks as runtime registration. | ||
| /// | ||
| /// # Errors | ||
|
|
@@ -347,9 +418,7 @@ impl IntegrationConfig for PrebidIntegrationConfig { | |
| pub fn validate_config_for_startup( | ||
| settings: &Settings, | ||
| ) -> Result<Option<PrebidIntegrationConfig>, Report<TrustedServerError>> { | ||
| let Some(config) = | ||
| settings.integration_config::<PrebidIntegrationConfig>(PREBID_INTEGRATION_ID)? | ||
| else { | ||
| let Some(config) = load_config(settings)? else { | ||
| return Ok(None); | ||
| }; | ||
| BidParamOverrideEngine::try_from_config(&config)?; | ||
|
|
@@ -869,9 +938,7 @@ fn escape_html_attr(value: &str) -> String { | |
| fn build( | ||
| settings: &Settings, | ||
| ) -> Result<Option<Arc<PrebidIntegration>>, Report<TrustedServerError>> { | ||
| let Some(config) = | ||
| settings.integration_config::<PrebidIntegrationConfig>(PREBID_INTEGRATION_ID)? | ||
| else { | ||
| let Some(config) = load_config(settings)? else { | ||
| return Ok(None); | ||
| }; | ||
|
|
||
|
|
@@ -1003,6 +1070,8 @@ impl IntegrationHeadInjector for PrebidIntegration { | |
| bidders: &'a [String], | ||
| #[serde(skip_serializing_if = "<[String]>::is_empty")] | ||
| client_side_bidders: &'a [String], | ||
| #[serde(skip_serializing_if = "<[String]>::is_empty")] | ||
| excluded_gam_ad_unit_path_suffixes: &'a [String], | ||
| } | ||
|
|
||
| let payload = InjectedPrebidClientConfig { | ||
|
|
@@ -1011,6 +1080,7 @@ impl IntegrationHeadInjector for PrebidIntegration { | |
| debug: self.config.debug, | ||
| bidders: &self.config.bidders, | ||
| client_side_bidders: &self.config.client_side_bidders, | ||
| excluded_gam_ad_unit_path_suffixes: &self.config.excluded_gam_ad_unit_path_suffixes, | ||
| }; | ||
|
|
||
| // Escape `</` to prevent breaking out of the script tag. | ||
|
|
@@ -2574,6 +2644,7 @@ mod tests { | |
| external_bundle_sha256: None, | ||
| external_bundle_sri: None, | ||
| client_side_bidders: Vec::new(), | ||
| excluded_gam_ad_unit_path_suffixes: Vec::new(), | ||
| bid_param_zone_overrides: HashMap::default(), | ||
| bid_param_overrides: HashMap::default(), | ||
| bid_param_override_rules: Vec::new(), | ||
|
|
@@ -2884,6 +2955,96 @@ passphrase = "test-secret-key-32-bytes-minimum" | |
| serde_json::from_value(value).expect("should build JSON object") | ||
| } | ||
|
|
||
| #[test] | ||
| fn excluded_gam_ad_unit_path_suffixes_default_to_empty() { | ||
| let config = parse_prebid_toml( | ||
| r#" | ||
| [integrations.prebid] | ||
| server_url = "https://prebid.example/openrtb2/auction" | ||
| "#, | ||
| ); | ||
|
|
||
| assert!( | ||
| config.excluded_gam_ad_unit_path_suffixes.is_empty(), | ||
| "should default to no refresh-auction exclusions" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn startup_validation_and_runtime_build_canonicalize_excluded_gam_ad_unit_path_suffixes() { | ||
| let mut settings = make_settings(); | ||
| settings | ||
| .integrations | ||
| .insert_config( | ||
| PREBID_INTEGRATION_ID, | ||
| &json!({ | ||
| "enabled": true, | ||
| "server_url": "https://prebid.example/openrtb2/auction", | ||
| "external_bundle_url": "https://assets.example/prebid/trusted-prebid.js", | ||
| "excluded_gam_ad_unit_path_suffixes": [ | ||
| "/trackingonly", | ||
| "/measurement-only", | ||
| "/trackingonly" | ||
| ] | ||
| }), | ||
| ) | ||
| .expect("should replace Prebid test configuration"); | ||
|
|
||
| let config = validate_config_for_startup(&settings) | ||
| .expect("should validate Prebid configuration") | ||
| .expect("should return enabled Prebid configuration"); | ||
|
|
||
| assert_eq!( | ||
| config.excluded_gam_ad_unit_path_suffixes, | ||
| ["/trackingonly", "/measurement-only"], | ||
| "should retain only the first declaration of each suffix" | ||
| ); | ||
|
|
||
| let integration = build(&settings) | ||
| .expect("should build Prebid integration") | ||
| .expect("should return enabled Prebid integration"); | ||
| let document_state = IntegrationDocumentState::default(); | ||
| let ctx = IntegrationHtmlContext { | ||
| request_host: "pub.example", | ||
| request_scheme: "https", | ||
| origin_host: "origin.example", | ||
| document_state: &document_state, | ||
| }; | ||
| let inserts = integration.head_inserts(&ctx); | ||
|
|
||
| assert!( | ||
| inserts[0].contains( | ||
| r#""excludedGamAdUnitPathSuffixes":["/trackingonly","/measurement-only"]"# | ||
| ), | ||
| "should inject the canonical suffix list: {}", | ||
| inserts[0] | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn excluded_gam_ad_unit_path_suffixes_reject_invalid_values() { | ||
| for (suffix, expected_message) in [ | ||
| ("", "must not be empty"), | ||
| (" /trackingonly", "must not have surrounding whitespace"), | ||
| ("trackingonly", "must start with '/'"), | ||
| ("/", "must identify a non-root path suffix"), | ||
| ] { | ||
| let error = parse_prebid_toml_result(&format!( | ||
| r#" | ||
| [integrations.prebid] | ||
| server_url = "https://prebid.example/openrtb2/auction" | ||
| excluded_gam_ad_unit_path_suffixes = ["{suffix}"] | ||
| "# | ||
| )) | ||
| .expect_err("should reject an invalid refresh-auction exclusion suffix"); | ||
|
|
||
| assert!( | ||
| error.to_string().contains(expected_message), | ||
| "should report why suffix {suffix:?} is invalid: {error}" | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| #[test] | ||
| fn attribute_rewriter_removes_prebid_scripts() { | ||
| let integration = PrebidIntegration::new(base_config()); | ||
|
|
@@ -3751,6 +3912,36 @@ external_bundle_sri = "sha384-AAAA" | |
| "should include bidders array: {}", | ||
| script | ||
| ); | ||
| assert!( | ||
| !script.contains("excludedGamAdUnitPathSuffixes"), | ||
| "should omit empty refresh-auction exclusions: {}", | ||
| script | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn head_injector_includes_excluded_gam_ad_unit_path_suffixes() { | ||
| let mut config = base_config(); | ||
| config.excluded_gam_ad_unit_path_suffixes = | ||
| vec!["/trackingonly".to_string(), "/measurement-only".to_string()]; | ||
| let integration = PrebidIntegration::new(config); | ||
| let document_state = IntegrationDocumentState::default(); | ||
| let ctx = IntegrationHtmlContext { | ||
| request_host: "pub.example", | ||
| request_scheme: "https", | ||
| origin_host: "origin.example", | ||
| document_state: &document_state, | ||
| }; | ||
|
|
||
| let inserts = integration.head_inserts(&ctx); | ||
| let script = &inserts[0]; | ||
| assert!( | ||
| script.contains( | ||
| r#""excludedGamAdUnitPathSuffixes":["/trackingonly","/measurement-only"]"# | ||
| ), | ||
| "should inject refresh-auction exclusion suffixes: {}", | ||
| script | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -129,6 +129,8 @@ interface InjectedPrebidConfig { | |
| bidders?: string[]; | ||
| /** Bidders that run client-side via native Prebid.js adapters. */ | ||
| clientSideBidders?: string[]; | ||
| /** GAM ad-unit-path suffixes excluded from refresh auctions. */ | ||
| excludedGamAdUnitPathSuffixes?: string[]; | ||
| } | ||
|
|
||
| interface PrebidUserIdDiagnostics { | ||
|
|
@@ -364,6 +366,7 @@ type PrebidUserIdEid = { | |
|
|
||
| type RefreshGptSlot = { | ||
| getSlotElementId?: () => string; | ||
| getAdUnitPath?: () => string; | ||
| getTargeting?: (key: string) => string[]; | ||
| clearTargeting?: (key?: string) => RefreshGptSlot; | ||
| getSizes?: () => unknown[]; | ||
|
|
@@ -690,6 +693,33 @@ function publisherZoneForRefresh(candidateCodes: Array<string | undefined>): str | |
| return match ? match.mediaTypes?.banner?.name : findRefreshSnapshot(candidateCodes)?.zone; | ||
| } | ||
|
|
||
| function isUsableRefreshAuctionExclusionSuffix(suffix: unknown): suffix is string { | ||
| return typeof suffix === 'string' && suffix.startsWith('/') && suffix.length > 1; | ||
| } | ||
|
|
||
| function refreshAuctionExclusionSuffixes(value: unknown): string[] { | ||
| return Array.isArray(value) ? value.filter(isUsableRefreshAuctionExclusionSuffix) : []; | ||
| } | ||
|
|
||
| function isExcludedFromRefreshAuction( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🌱 seedling — No signal is emitted when a slot is excluded, so a too-broad suffix is invisible in the field. Because A single debug line (or a counter surfaced through the GPT diagnostics overlay from #974) would make that misconfiguration diagnosable without a code change: const excluded =
typeof adUnitPath === 'string' &&
excludedGamAdUnitPathSuffixes.some((suffix) => adUnitPath.endsWith(suffix));
if (excluded) {
log.debug(`[tsjs-prebid] refresh auction excluded ${adUnitPath}`);
}
return excluded;Not for this PR if you'd rather keep the predicate allocation-free — noting it as the follow-up that makes the opt-out observable. |
||
| slot: RefreshGptSlot, | ||
| excludedGamAdUnitPathSuffixes: readonly string[] | ||
| ): boolean { | ||
| if (excludedGamAdUnitPathSuffixes.length === 0) return false; | ||
|
|
||
| try { | ||
| const adUnitPath = slot.getAdUnitPath?.(); | ||
| return ( | ||
| typeof adUnitPath === 'string' && | ||
| excludedGamAdUnitPathSuffixes.some((suffix) => adUnitPath.endsWith(suffix)) | ||
| ); | ||
| } catch { | ||
| // GPT path metadata is optional for this optimization. If it is unavailable, | ||
| // preserve normal refresh-auction behavior rather than suppressing demand. | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| function clearRefreshTargeting(slot: RefreshGptSlot): void { | ||
| if (typeof slot.clearTargeting !== 'function') return; | ||
|
|
||
|
|
@@ -1229,9 +1259,21 @@ export function installRefreshHandler(timeoutMs = 1500): void { | |
| return originalRefresh(slots, opts); | ||
| } | ||
|
|
||
| // Clear stale Trusted Server/Prebid targeting from independent slots before | ||
| // filtering so excluded slots still receive a clean GAM refresh. | ||
| independentSlots.forEach(clearRefreshTargeting); | ||
|
|
||
| const adUnits = independentSlots.map((slot) => { | ||
| const excludedGamAdUnitPathSuffixes = refreshAuctionExclusionSuffixes( | ||
| getInjectedConfig()?.excludedGamAdUnitPathSuffixes | ||
| ); | ||
| const auctionSlots = independentSlots.filter( | ||
| (slot) => !isExcludedFromRefreshAuction(slot, excludedGamAdUnitPathSuffixes) | ||
| ); | ||
| if (!auctionSlots.length) { | ||
| return originalRefresh(slots, opts); | ||
| } | ||
|
|
||
| const adUnits = auctionSlots.map((slot) => { | ||
| const injectedSlot = findInjectedSlotForRefresh(slot); | ||
| const code = refreshSlotElementId(slot) ?? 'refresh-slot'; | ||
| // A TS-owned slot may be defined on `${div_id}-container`, so the GPT | ||
|
|
@@ -1291,6 +1333,9 @@ export function installRefreshHandler(timeoutMs = 1500): void { | |
| log.error('[tsjs-prebid] refresh targeting failed', error); | ||
| } | ||
| } | ||
| // Preserve the publisher's original refresh form. In particular, a bare | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤔 thinking — Correct for SRA, but worth naming the remaining cost: in a mixed global refresh the excluded slot's GAM refresh is still withheld until the auction completes (or the That is the right tradeoff (splitting the refresh would break Single Request Architecture and change the publisher's request shape), and the design doc's §5.3 table does describe the ordering. The guide's caveat list is where an operator would look, though: it currently says matching slots "still refresh through GAM" without noting the mixed-refresh delay. One sentence there would set the expectation for anyone opting out a slot for timing reasons rather than for demand reasons. |
||
| // GPT refresh remains bare so GPT resolves its registered slot set when | ||
| // the auction completes. | ||
| originalRefresh(slots, opts); | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🌱 seedling — The validator rejects every input that cannot be a path suffix except one that can never match: a trailing slash.
"/trackingonly/"passes all four checks, injects fine, and then silently matches nothing, since GAM ad-unit paths do not end in/— the PR's own'/123/trackingonly/'test case documents that direction of the asymmetry.Given the design deliberately does no slash normalization (a good call — literal matching is auditable), rejecting the dead form at startup is more consistent than accepting it:
This also subsumes the
value == "/"case. Fine as a follow-up — the current behavior is fail-open, not wrong.