diff --git a/src/adcp/signing/canonical.py b/src/adcp/signing/canonical.py index 9c7acd6d..befae2a4 100644 --- a/src/adcp/signing/canonical.py +++ b/src/adcp/signing/canonical.py @@ -126,7 +126,17 @@ def canonicalize_target_uri(url: str) -> str: path = "/" # RFC 9421 §2.2.2 + RFC 7230 §5.5: effective request URI excludes the # fragment (client-local, never sent on wire). - return urlunsplit((scheme, netloc, path, parts.query, "")) + target = urlunsplit((scheme, netloc, path, parts.query, "")) + if not parts.query and "?" in url.split("#", 1)[0]: + # `urlsplit` maps both `/p` and `/p?` to `query == ""`, and `urlunsplit` + # emits no `?` for an empty string -- so the two collapse to one + # signature base. A signer that sent `/p?` and a verifier that + # reconstructs `/p` then sign different bytes for different URLs and + # agree, which is exactly the confusion `@target-uri` exists to prevent. + # The distinction has to be recovered from the raw URL because it is + # already lost by the time `parts` exists. + target += "?" + return target def canonicalize_authority(url: str) -> str: diff --git a/tests/conformance/signing/test_canonicalization.py b/tests/conformance/signing/test_canonicalization.py index 96a4da10..3b7c6590 100644 --- a/tests/conformance/signing/test_canonicalization.py +++ b/tests/conformance/signing/test_canonicalization.py @@ -29,11 +29,7 @@ # Cases from canonicalization.json that the SDK does not yet satisfy, mapped to # the issue tracking each gap. Marked strict so a fix XPASSes and forces the # entry to be retired instead of lingering. -KNOWN_CANONICALIZATION_GAPS: dict[str, str] = { - # urlunsplit() cannot distinguish "no query" from "empty query", so the - # trailing '?' is dropped and signer/verifier can disagree on the base. - "trailing-empty-query-preserved": "#979: trailing '?' with an empty query is dropped", -} +KNOWN_CANONICALIZATION_GAPS: dict[str, str] = {} def _vectors_with_expected_base() -> list[tuple[str, Path]]: