Skip to content

Commit fe0cceb

Browse files
fix(server): canonicalize an IPv6 host extracted from a port-bearing authority
Review follow-up. The bare-literal short-circuit only inspects the raw input. Once a port is attached it is `urlsplit` that removes the brackets, so the address reached the rest of the function uncompressed and was never folded: normalize_host_key("[2001:DB8::0:1]:443") -> "2001:db8::0:1" normalize_host_key("2001:db8::0:1") -> "2001:db8::1" So a tenant registered under the bare literal was unreachable from the equivalent Host value carrying :443, and the idempotency the docstring claims was simply false -- load-bearing, because InMemorySubdomainTenantRouter normalizes registration keys at construction and normalizes the Host again at lookup, so a non-idempotent key fails to match itself. Re-runs the IP-literal test on the extracted host. Two of the four new tests fail without this change; the other two are regression guards. Refs #990.
1 parent a2194fb commit fe0cceb

2 files changed

Lines changed: 55 additions & 0 deletions

File tree

src/adcp/server/tenant_router.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,21 @@ def normalize_host_key(value: str) -> str:
501501
host = None
502502
if not host:
503503
host = raw.lower() # unparseable authority -> best-effort key
504+
505+
# Re-run the IP-literal test on the EXTRACTED host, not just the raw input.
506+
# The short-circuit above only sees `[2001:DB8::0:1]`; once a port is
507+
# attached, `urlsplit` is what strips the brackets, and the address landed
508+
# here uncompressed. That made the function non-idempotent over its own
509+
# output -- `[2001:DB8::0:1]:443` keyed to `2001:db8::0:1` while the bare
510+
# form keyed to `2001:db8::1` -- so a tenant registered under one was
511+
# unreachable from the other. Idempotency is load-bearing here:
512+
# InMemorySubdomainTenantRouter normalizes registration keys and then
513+
# normalizes the lookup host again.
514+
try:
515+
return str(ipaddress.ip_address(host))
516+
except ValueError:
517+
pass
518+
504519
if host.endswith("."):
505520
host = host[:-1] # single FQDN-root dot, matching canonicalize_host
506521

tests/test_subdomain_tenant_router.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -582,3 +582,43 @@ async def _async_pass_through(scope, receive, send) -> None:
582582
isolation. Never reached when the middleware short-circuits."""
583583
await send({"type": "http.response.start", "status": 200, "headers": []})
584584
await send({"type": "http.response.body", "body": b""})
585+
586+
587+
@pytest.mark.parametrize(
588+
("with_port", "bare"),
589+
[
590+
("[2001:DB8::0:1]:443", "2001:db8::0:1"),
591+
("[::1]:8080", "::1"),
592+
("[2001:db8:0:0:0:0:0:1]:9000", "2001:db8::1"),
593+
],
594+
ids=["uncompressed-upper", "loopback", "fully-expanded"],
595+
)
596+
def test_non_canonical_ipv6_with_port_keys_the_same_as_the_bare_form(
597+
with_port: str, bare: str
598+
) -> None:
599+
"""A port must not change which tenant an IPv6 host resolves to.
600+
601+
The bare-literal short-circuit only sees the raw input. Once a port is
602+
attached, ``urlsplit`` is what removes the brackets, and the address
603+
arrived downstream uncompressed -- so ``[2001:DB8::0:1]:443`` keyed to
604+
``2001:db8::0:1`` while the bare form keyed to ``2001:db8::1``. A tenant
605+
registered under one was unreachable from the other, and the function was
606+
not idempotent over its own output.
607+
608+
Idempotency is load-bearing rather than tidy: ``InMemorySubdomainTenantRouter``
609+
normalizes registration keys at construction and normalizes the Host again
610+
at lookup, so a non-idempotent key silently fails to match itself.
611+
"""
612+
key = normalize_host_key(with_port)
613+
assert key == normalize_host_key(bare)
614+
assert normalize_host_key(key) == key, "normalize_host_key must be idempotent"
615+
616+
617+
def test_ipv6_tenant_is_reachable_with_and_without_a_port() -> None:
618+
"""The end-to-end consequence: same tenant, either Host spelling."""
619+
router = InMemorySubdomainTenantRouter(
620+
tenants={"[2001:DB8::0:1]": Tenant(id="v6", display_name="IPv6 Tenant")}
621+
)
622+
for host in ("[2001:DB8::0:1]", "[2001:db8::1]:443", "2001:db8::0:1"):
623+
resolved = asyncio.run(router.resolve(host))
624+
assert resolved is not None and resolved.id == "v6", f"unreachable via {host!r}"

0 commit comments

Comments
 (0)