Skip to content

Commit 0568d2e

Browse files
bokelleyclaude
andauthored
fix(compat): warn when brand_manifest non-standard path is flattened to domain (#686)
Resolves silent information loss introduced by the v2.5→v3 adapter: a brand_manifest URL like https://cdn.acmecorp.com/brand-manifest.json is correctly translated to BrandReference.domain but the path is dropped, so v3 sellers will attempt https://cdn.acmecorp.com/.well-known/brand.json which typically 404s with no signal to the operator. Both adapter call sites (get_products.adapt_request and _media_buy_helpers.adapt_brand_manifest_to_brand) now emit a logging.WARNING when urlparse detects a non-standard path. The warning is deduplicated per URL via a module-level set so high-RPS buyers do not saturate log pipelines. Bare-domain inputs (no scheme, urlparse returns hostname=None) are guarded against false positives. Fixes #683. https://claude.ai/code/session_01X44xueoHmfHRYeKL8giv2F Co-authored-by: Claude <noreply@anthropic.com>
1 parent 19be8db commit 0568d2e

4 files changed

Lines changed: 172 additions & 2 deletions

File tree

src/adcp/compat/legacy/v2_5/_media_buy_helpers.py

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,20 @@
1212

1313
from __future__ import annotations
1414

15+
import logging
1516
from typing import Any
17+
from urllib.parse import urlparse
1618

1719
from adcp.compat.legacy.v2_5._url import extract_brand_domain
1820

21+
_logger = logging.getLogger(__name__)
22+
23+
# Paths that v3 sellers reconstruct correctly — no information is lost.
24+
_STANDARD_BRAND_MANIFEST_PATHS = {"", "/", "/.well-known/brand.json"}
25+
26+
# Per-URL dedup so high-RPS v2.5 buyers don't saturate the log pipeline.
27+
_brand_manifest_path_warned: set[str] = set()
28+
1929

2030
def adapt_brand_manifest_to_brand(payload: dict[str, Any]) -> dict[str, Any]:
2131
"""Rewrite v2.5 ``brand_manifest`` (URL string or inline BrandManifest
@@ -34,7 +44,22 @@ def adapt_brand_manifest_to_brand(payload: dict[str, Any]) -> dict[str, Any]:
3444
out = dict(payload)
3545
manifest = out.pop("brand_manifest", None)
3646
if isinstance(manifest, str) and manifest and "brand" not in out:
37-
out["brand"] = {"domain": extract_brand_domain(manifest)}
47+
domain = extract_brand_domain(manifest)
48+
parsed = urlparse(manifest.strip())
49+
if (
50+
parsed.hostname is not None
51+
and parsed.path not in _STANDARD_BRAND_MANIFEST_PATHS
52+
and manifest not in _brand_manifest_path_warned
53+
):
54+
_brand_manifest_path_warned.add(manifest)
55+
_logger.warning(
56+
"brand_manifest at %s uses a non-standard path; "
57+
"v3 sellers derive %s/.well-known/brand.json from BrandReference.domain. "
58+
"Manifest fetch may 404.",
59+
manifest,
60+
domain,
61+
)
62+
out["brand"] = {"domain": domain}
3863
elif isinstance(manifest, dict) and "brand" not in out:
3964
url = manifest.get("url")
4065
if isinstance(url, str) and url.strip():

src/adcp/compat/legacy/v2_5/get_products.py

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,22 @@
3636

3737
from __future__ import annotations
3838

39+
import logging
3940
from typing import Any
41+
from urllib.parse import urlparse
4042

4143
from adcp.compat.legacy import register_adapter
4244
from adcp.compat.legacy.types import AdapterPair
4345
from adcp.compat.legacy.v2_5._url import extract_brand_domain
4446

47+
_logger = logging.getLogger(__name__)
48+
49+
# Paths that v3 sellers reconstruct correctly — no information is lost.
50+
_STANDARD_BRAND_MANIFEST_PATHS = {"", "/", "/.well-known/brand.json"}
51+
52+
# Per-URL dedup so high-RPS v2.5 buyers don't saturate the log pipeline.
53+
_brand_manifest_path_warned: set[str] = set()
54+
4555
# v2.5 channel buckets to v3 channel slugs. Multi-mapped buckets resolve
4656
# to all listed v3 channels; downstream consumers can narrow further via
4757
# ``filters.channels`` if needed.
@@ -135,7 +145,22 @@ def adapt_request(payload: dict[str, Any]) -> dict[str, Any]:
135145
# v3 validation handle the missing field.
136146
brand_manifest = out.pop("brand_manifest", None)
137147
if isinstance(brand_manifest, str) and brand_manifest and "brand" not in out:
138-
out["brand"] = {"domain": extract_brand_domain(brand_manifest)}
148+
domain = extract_brand_domain(brand_manifest)
149+
parsed = urlparse(brand_manifest.strip())
150+
if (
151+
parsed.hostname is not None
152+
and parsed.path not in _STANDARD_BRAND_MANIFEST_PATHS
153+
and brand_manifest not in _brand_manifest_path_warned
154+
):
155+
_brand_manifest_path_warned.add(brand_manifest)
156+
_logger.warning(
157+
"brand_manifest at %s uses a non-standard path; "
158+
"v3 sellers derive %s/.well-known/brand.json from BrandReference.domain. "
159+
"Manifest fetch may 404.",
160+
brand_manifest,
161+
domain,
162+
)
163+
out["brand"] = {"domain": domain}
139164
elif isinstance(brand_manifest, dict) and "brand" not in out:
140165
url = brand_manifest.get("url")
141166
if isinstance(url, str) and url.strip():

tests/test_legacy_adapter_v2_5_get_products.py

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
from __future__ import annotations
44

5+
import logging
56
from typing import Any
67

78
from adcp.compat.legacy import get_legacy_adapter
@@ -139,6 +140,64 @@ def test_brand_manifest_inline_object_brand_wins_when_both_present() -> None:
139140
assert "brand_manifest" not in out
140141

141142

143+
# ---------------------------------------------------------------------------
144+
# Request: brand_manifest non-standard-path warning (issue #683)
145+
# ---------------------------------------------------------------------------
146+
147+
_GP_LOGGER = "adcp.compat.legacy.v2_5.get_products"
148+
149+
150+
def test_brand_manifest_cdn_url_warns(caplog, monkeypatch) -> None: # type: ignore[no-untyped-def]
151+
"""CDN brand_manifest with non-standard path emits a WARNING about the
152+
information loss so operators can debug downstream 404s."""
153+
import adcp.compat.legacy.v2_5.get_products as _gp
154+
155+
monkeypatch.setattr(_gp, "_brand_manifest_path_warned", set())
156+
with caplog.at_level(logging.WARNING, logger=_GP_LOGGER):
157+
out = _adapt({"brand_manifest": "https://cdn.acmecorp.com/brand-manifest.json"})
158+
assert out["brand"] == {"domain": "cdn.acmecorp.com"}
159+
records = [r for r in caplog.records if r.name == _GP_LOGGER]
160+
assert len(records) == 1
161+
msg = records[0].getMessage()
162+
assert "non-standard path" in msg
163+
assert "cdn.acmecorp.com" in msg
164+
165+
166+
def test_brand_manifest_well_known_path_no_warning(caplog, monkeypatch) -> None: # type: ignore[no-untyped-def]
167+
"""Standard /.well-known/brand.json path does NOT warn — v3 derives the
168+
same URL that the buyer originally pointed at."""
169+
import adcp.compat.legacy.v2_5.get_products as _gp
170+
171+
monkeypatch.setattr(_gp, "_brand_manifest_path_warned", set())
172+
with caplog.at_level(logging.WARNING, logger=_GP_LOGGER):
173+
_adapt({"brand_manifest": "https://acme.com/.well-known/brand.json"})
174+
assert not any(r.name == _GP_LOGGER for r in caplog.records)
175+
176+
177+
def test_brand_manifest_bare_domain_no_warning(caplog, monkeypatch) -> None: # type: ignore[no-untyped-def]
178+
"""Bare domain brand_manifest (no scheme) does not false-positive — urlparse
179+
returns hostname=None for schemeless strings so we skip the path check."""
180+
import adcp.compat.legacy.v2_5.get_products as _gp
181+
182+
monkeypatch.setattr(_gp, "_brand_manifest_path_warned", set())
183+
with caplog.at_level(logging.WARNING, logger=_GP_LOGGER):
184+
_adapt({"brand_manifest": "acme.com"})
185+
assert not any(r.name == _GP_LOGGER for r in caplog.records)
186+
187+
188+
def test_brand_manifest_cdn_url_warns_once_dedup(caplog, monkeypatch) -> None: # type: ignore[no-untyped-def]
189+
"""Same CDN URL repeated across requests only logs one warning (dedup)."""
190+
import adcp.compat.legacy.v2_5.get_products as _gp
191+
192+
monkeypatch.setattr(_gp, "_brand_manifest_path_warned", set())
193+
url = "https://cdn.acmecorp.com/brand-manifest.json"
194+
with caplog.at_level(logging.WARNING, logger=_GP_LOGGER):
195+
_adapt({"brand_manifest": url})
196+
_adapt({"brand_manifest": url})
197+
records = [r for r in caplog.records if r.name == _GP_LOGGER]
198+
assert len(records) == 1
199+
200+
142201
# ---------------------------------------------------------------------------
143202
# Request: promoted_offerings → catalog
144203
# ---------------------------------------------------------------------------

tests/test_legacy_adapter_v2_5_media_buy.py

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
from __future__ import annotations
99

10+
import logging
1011
from typing import Any
1112

1213
from adcp.compat.legacy import get_legacy_adapter
@@ -119,6 +120,66 @@ def test_create_media_buy_brand_manifest_inline_object_brand_wins_when_both_pres
119120
assert "brand_manifest" not in out
120121

121122

123+
# ---------------------------------------------------------------------------
124+
# create_media_buy: brand_manifest non-standard-path warning (issue #683)
125+
# ---------------------------------------------------------------------------
126+
127+
_MB_HELPERS_LOGGER = "adcp.compat.legacy.v2_5._media_buy_helpers"
128+
129+
130+
def test_create_media_buy_cdn_url_warns(caplog, monkeypatch) -> None: # type: ignore[no-untyped-def]
131+
"""CDN brand_manifest with non-standard path emits a WARNING about the
132+
information loss so operators can debug downstream 404s."""
133+
import adcp.compat.legacy.v2_5._media_buy_helpers as _mbh
134+
135+
monkeypatch.setattr(_mbh, "_brand_manifest_path_warned", set())
136+
with caplog.at_level(logging.WARNING, logger=_MB_HELPERS_LOGGER):
137+
out = v2_5_cmb.adapt_request(
138+
{"brand_manifest": "https://cdn.acmecorp.com/brand-manifest.json"}
139+
)
140+
assert out["brand"] == {"domain": "cdn.acmecorp.com"}
141+
records = [r for r in caplog.records if r.name == _MB_HELPERS_LOGGER]
142+
assert len(records) == 1
143+
msg = records[0].getMessage()
144+
assert "non-standard path" in msg
145+
assert "cdn.acmecorp.com" in msg
146+
147+
148+
def test_create_media_buy_well_known_path_no_warning(caplog, monkeypatch) -> None: # type: ignore[no-untyped-def]
149+
"""Standard /.well-known/brand.json path does NOT warn."""
150+
import adcp.compat.legacy.v2_5._media_buy_helpers as _mbh
151+
152+
monkeypatch.setattr(_mbh, "_brand_manifest_path_warned", set())
153+
with caplog.at_level(logging.WARNING, logger=_MB_HELPERS_LOGGER):
154+
v2_5_cmb.adapt_request(
155+
{"brand_manifest": "https://acme.com/.well-known/brand.json"}
156+
)
157+
assert not any(r.name == _MB_HELPERS_LOGGER for r in caplog.records)
158+
159+
160+
def test_create_media_buy_bare_domain_no_warning(caplog, monkeypatch) -> None: # type: ignore[no-untyped-def]
161+
"""Bare domain brand_manifest (no scheme) does not false-positive."""
162+
import adcp.compat.legacy.v2_5._media_buy_helpers as _mbh
163+
164+
monkeypatch.setattr(_mbh, "_brand_manifest_path_warned", set())
165+
with caplog.at_level(logging.WARNING, logger=_MB_HELPERS_LOGGER):
166+
v2_5_cmb.adapt_request({"brand_manifest": "acme.com"})
167+
assert not any(r.name == _MB_HELPERS_LOGGER for r in caplog.records)
168+
169+
170+
def test_create_media_buy_cdn_url_warns_once_dedup(caplog, monkeypatch) -> None: # type: ignore[no-untyped-def]
171+
"""Same CDN URL repeated across requests only logs one warning (dedup)."""
172+
import adcp.compat.legacy.v2_5._media_buy_helpers as _mbh
173+
174+
monkeypatch.setattr(_mbh, "_brand_manifest_path_warned", set())
175+
url = "https://cdn.acmecorp.com/brand-manifest.json"
176+
with caplog.at_level(logging.WARNING, logger=_MB_HELPERS_LOGGER):
177+
v2_5_cmb.adapt_request({"brand_manifest": url})
178+
v2_5_cmb.adapt_request({"brand_manifest": url})
179+
records = [r for r in caplog.records if r.name == _MB_HELPERS_LOGGER]
180+
assert len(records) == 1
181+
182+
122183
# ---------------------------------------------------------------------------
123184
# Package request: creative_ids → creative_assignments (both tools)
124185
# ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)