Skip to content

Commit e2352ea

Browse files
bluetoothbotbluetoothbot
authored
fix: preserve scope_id when scoped AAAA arrives alongside unscoped (#1764)
1 parent 9470bd6 commit e2352ea

3 files changed

Lines changed: 292 additions & 15 deletions

File tree

‎src/zeroconf/_services/info.pxd‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,9 @@ cdef class ServiceInfo(RecordUpdateListener):
107107
)
108108
cdef bint _process_record_threadsafe(self, object zc, DNSRecord record, double now)
109109

110+
@cython.locals(existing_idx=int, existing=object)
111+
cdef bint _upsert_ipv6_address(self, object ip_addr)
112+
110113
@cython.locals(cache=DNSCache)
111114
cdef cython.list _get_address_records_from_cache_by_type(self, object zc, object _type)
112115

‎src/zeroconf/_services/info.py‎

Lines changed: 71 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
import asyncio
2626
import random
27+
from collections.abc import Sequence
2728
from typing import TYPE_CHECKING, cast
2829

2930
from .._cache import DNSCache
@@ -108,6 +109,36 @@
108109
from .._core import Zeroconf
109110

110111

112+
def _index_of_same_address(
113+
addresses: Sequence[ZeroconfIPv4Address | ZeroconfIPv6Address],
114+
ip_addr: ZeroconfIPv4Address | ZeroconfIPv6Address,
115+
) -> int:
116+
"""Return the index of an existing entry with the same packed bytes, or -1.
117+
118+
Match by ``zc_integer`` so IPv6 addresses that differ only in
119+
scope_id (one observed without scope on an IPv4 socket, another
120+
observed with scope on an IPv6 socket) collapse to a single entry.
121+
"""
122+
target = ip_addr.zc_integer
123+
for idx, existing in enumerate(addresses):
124+
if existing.zc_integer == target:
125+
return idx
126+
return -1
127+
128+
129+
def _has_more_scope_info(
130+
new_addr: ZeroconfIPv4Address | ZeroconfIPv6Address,
131+
existing: ZeroconfIPv4Address | ZeroconfIPv6Address,
132+
) -> bool:
133+
"""True if ``new_addr`` carries a scope_id the ``existing`` entry lacks."""
134+
if new_addr.version != 6:
135+
return False
136+
if TYPE_CHECKING:
137+
assert isinstance(new_addr, ZeroconfIPv6Address)
138+
assert isinstance(existing, ZeroconfIPv6Address)
139+
return new_addr.scope_id is not None and existing.scope_id is None
140+
141+
111142
def instance_name_from_service_info(info: ServiceInfo, strict: bool = True) -> str:
112143
"""Calculate the instance name from the ServiceInfo."""
113144
# This is kind of funky because of the subtype based tests
@@ -453,11 +484,49 @@ def _get_ip_addresses_from_cache_lifo(
453484
if record.is_expired(now):
454485
continue
455486
ip_addr = get_ip_address_object_from_record(record)
456-
if ip_addr is not None and ip_addr not in address_list:
487+
if ip_addr is None:
488+
continue
489+
# The cache keeps scoped and unscoped link-local AAAA
490+
# records as separate entries because DNSAddress equality
491+
# includes scope_id. Collapse them here so each address
492+
# appears once; the scoped variant wins so callers of
493+
# parsed_scoped_addresses() get a %<interface_index>-
494+
# qualified link-local address when one was observed.
495+
existing_idx = _index_of_same_address(address_list, ip_addr)
496+
if existing_idx == -1:
457497
address_list.append(ip_addr)
498+
continue
499+
# Move the re-seen address to the end so the later observation
500+
# wins both in value (scope) and in LIFO position after reverse.
501+
existing = address_list.pop(existing_idx)
502+
address_list.append(ip_addr if _has_more_scope_info(ip_addr, existing) else existing)
458503
address_list.reverse() # Reverse to get LIFO order
459504
return address_list
460505

506+
def _upsert_ipv6_address(self, ip_addr: ZeroconfIPv6Address) -> bool:
507+
"""Insert or update an IPv6 address in LIFO order.
508+
509+
Compares by integer (not IPv6Address equality, which respects
510+
scope_id) so the same link-local address received first without
511+
scope (IPv4 socket) and then with scope (IPv6 socket) collapses
512+
to one entry. The scoped variant wins so parsed_scoped_addresses()
513+
can return a qualified address.
514+
"""
515+
ipv6_addresses = self._ipv6_addresses
516+
existing_idx = _index_of_same_address(ipv6_addresses, ip_addr)
517+
if existing_idx == -1:
518+
ipv6_addresses.insert(0, ip_addr)
519+
return True
520+
existing = ipv6_addresses[existing_idx]
521+
if _has_more_scope_info(ip_addr, existing):
522+
ipv6_addresses.pop(existing_idx)
523+
ipv6_addresses.insert(0, ip_addr)
524+
return True
525+
if existing_idx != 0:
526+
ipv6_addresses.pop(existing_idx)
527+
ipv6_addresses.insert(0, existing)
528+
return False
529+
461530
def _set_ipv6_addresses_from_cache(self, zc: Zeroconf, now: float_) -> None:
462531
"""Set IPv6 addresses from the cache."""
463532
if TYPE_CHECKING:
@@ -532,19 +601,7 @@ def _process_record_threadsafe(self, zc: Zeroconf, record: DNSRecord, now: float
532601

533602
if TYPE_CHECKING:
534603
assert isinstance(ip_addr, ZeroconfIPv6Address)
535-
ipv6_addresses = self._ipv6_addresses
536-
if ip_addr not in self._ipv6_addresses:
537-
ipv6_addresses.insert(0, ip_addr)
538-
return True
539-
# Use int() to compare the addresses as integers
540-
# since by default IPv6Address.__eq__ compares the
541-
# the addresses on version and int which more than
542-
# we need here since we know the version is 6.
543-
if ip_addr.zc_integer != self._ipv6_addresses[0].zc_integer:
544-
ipv6_addresses.remove(ip_addr)
545-
ipv6_addresses.insert(0, ip_addr)
546-
547-
return False
604+
return self._upsert_ipv6_address(ip_addr)
548605

549606
if record_key != self.key:
550607
return False

‎tests/services/test_info.py‎

Lines changed: 218 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@
1818
from zeroconf import DNSAddress, RecordUpdate, const
1919
from zeroconf._protocol.outgoing import DNSOutgoing
2020
from zeroconf._services import info
21-
from zeroconf._services.info import ServiceInfo
21+
from zeroconf._services.info import ServiceInfo, _has_more_scope_info
22+
from zeroconf._utils.ipaddress import ZeroconfIPv4Address
2223
from zeroconf._utils.net import IPVersion
2324
from zeroconf.asyncio import AsyncZeroconf
2425

@@ -790,6 +791,222 @@ def test_scoped_addresses_from_cache():
790791
zeroconf.close()
791792

792793

794+
def test_scoped_address_preferred_when_unscoped_arrives_first_in_cache():
795+
"""A scoped AAAA in the cache wins over an earlier unscoped copy of the same address."""
796+
type_ = "_http._tcp.local."
797+
registration_name = f"scoped-first.{type_}"
798+
zeroconf = r.Zeroconf(interfaces=["127.0.0.1"])
799+
host = "scoped-first.local."
800+
packed = socket.inet_pton(socket.AF_INET6, "fe80::52e:c2f2:bc5f:e9c6")
801+
802+
zeroconf.cache.async_add_records(
803+
[
804+
r.DNSPointer(
805+
type_,
806+
const._TYPE_PTR,
807+
const._CLASS_IN | const._CLASS_UNIQUE,
808+
120,
809+
registration_name,
810+
),
811+
r.DNSService(
812+
registration_name,
813+
const._TYPE_SRV,
814+
const._CLASS_IN | const._CLASS_UNIQUE,
815+
120,
816+
0,
817+
0,
818+
80,
819+
host,
820+
),
821+
r.DNSAddress(
822+
host,
823+
const._TYPE_AAAA,
824+
const._CLASS_IN | const._CLASS_UNIQUE,
825+
120,
826+
packed,
827+
scope_id=None,
828+
),
829+
r.DNSAddress(
830+
host,
831+
const._TYPE_AAAA,
832+
const._CLASS_IN | const._CLASS_UNIQUE,
833+
120,
834+
packed,
835+
scope_id=7,
836+
),
837+
]
838+
)
839+
840+
info = ServiceInfo(type_, registration_name)
841+
info.load_from_cache(zeroconf)
842+
assert info.parsed_scoped_addresses() == ["fe80::52e:c2f2:bc5f:e9c6%7"]
843+
assert info.ip_addresses_by_version(r.IPVersion.V6Only) == [ip_address("fe80::52e:c2f2:bc5f:e9c6%7")]
844+
zeroconf.close()
845+
846+
847+
@pytest.mark.asyncio
848+
async def test_scoped_address_replaces_unscoped_in_live_update():
849+
"""A late-arriving scoped AAAA replaces a previously-stored unscoped variant."""
850+
type_ = "_http._tcp.local."
851+
registration_name = f"scoped-live.{type_}"
852+
aiozc = AsyncZeroconf(interfaces=["127.0.0.1"])
853+
host = "scoped-live.local."
854+
packed = socket.inet_pton(socket.AF_INET6, "fe80::52e:c2f2:bc5f:e9c6")
855+
856+
info = ServiceInfo(type_, registration_name, server=host)
857+
now = r.current_time_millis()
858+
unscoped = r.DNSAddress(
859+
host,
860+
const._TYPE_AAAA,
861+
const._CLASS_IN | const._CLASS_UNIQUE,
862+
120,
863+
packed,
864+
scope_id=None,
865+
)
866+
scoped = r.DNSAddress(
867+
host,
868+
const._TYPE_AAAA,
869+
const._CLASS_IN | const._CLASS_UNIQUE,
870+
120,
871+
packed,
872+
scope_id=9,
873+
)
874+
info.async_update_records(aiozc.zeroconf, now, [RecordUpdate(unscoped, None)])
875+
assert info.parsed_scoped_addresses() == ["fe80::52e:c2f2:bc5f:e9c6"]
876+
info.async_update_records(aiozc.zeroconf, now, [RecordUpdate(scoped, unscoped)])
877+
assert info.parsed_scoped_addresses() == ["fe80::52e:c2f2:bc5f:e9c6%9"]
878+
await aiozc.async_close()
879+
880+
881+
def test_scoped_address_kept_when_unscoped_arrives_after_in_cache():
882+
"""Scoped AAAA seen first in iteration keeps its scope when an unscoped duplicate follows."""
883+
type_ = "_http._tcp.local."
884+
registration_name = f"scoped-after.{type_}"
885+
zeroconf = r.Zeroconf(interfaces=["127.0.0.1"])
886+
host = "scoped-after.local."
887+
packed = socket.inet_pton(socket.AF_INET6, "fe80::52e:c2f2:bc5f:e9c6")
888+
889+
zeroconf.cache.async_add_records(
890+
[
891+
r.DNSPointer(
892+
type_,
893+
const._TYPE_PTR,
894+
const._CLASS_IN | const._CLASS_UNIQUE,
895+
120,
896+
registration_name,
897+
),
898+
r.DNSService(
899+
registration_name,
900+
const._TYPE_SRV,
901+
const._CLASS_IN | const._CLASS_UNIQUE,
902+
120,
903+
0,
904+
0,
905+
80,
906+
host,
907+
),
908+
r.DNSAddress(
909+
host,
910+
const._TYPE_AAAA,
911+
const._CLASS_IN | const._CLASS_UNIQUE,
912+
120,
913+
packed,
914+
scope_id=5,
915+
),
916+
r.DNSAddress(
917+
host,
918+
const._TYPE_AAAA,
919+
const._CLASS_IN | const._CLASS_UNIQUE,
920+
120,
921+
packed,
922+
scope_id=None,
923+
),
924+
]
925+
)
926+
927+
info = ServiceInfo(type_, registration_name)
928+
info.load_from_cache(zeroconf)
929+
assert info.parsed_scoped_addresses() == ["fe80::52e:c2f2:bc5f:e9c6%5"]
930+
assert info.ip_addresses_by_version(r.IPVersion.V6Only) == [ip_address("fe80::52e:c2f2:bc5f:e9c6%5")]
931+
zeroconf.close()
932+
933+
934+
def test_has_more_scope_info_returns_false_for_ipv4():
935+
"""The scope_id helper short-circuits for IPv4 since A records carry no scope."""
936+
ip4 = ZeroconfIPv4Address("192.0.2.1")
937+
assert _has_more_scope_info(ip4, ip4) is False
938+
939+
940+
def test_scope_upgrade_preserves_lifo_recency_order():
941+
"""A scoped AAAA that upgrades an earlier entry becomes the most recent in LIFO order."""
942+
type_ = "_http._tcp.local."
943+
registration_name = f"reorder.{type_}"
944+
zeroconf = r.Zeroconf(interfaces=["127.0.0.1"])
945+
host = "reorder.local."
946+
link_local = socket.inet_pton(socket.AF_INET6, "fe80::52e:c2f2:bc5f:e9c6")
947+
ula = socket.inet_pton(socket.AF_INET6, "fdc8:d776:7cca:46ed::2")
948+
949+
zeroconf.cache.async_add_records(
950+
[
951+
r.DNSPointer(
952+
type_,
953+
const._TYPE_PTR,
954+
const._CLASS_IN | const._CLASS_UNIQUE,
955+
120,
956+
registration_name,
957+
),
958+
r.DNSService(
959+
registration_name,
960+
const._TYPE_SRV,
961+
const._CLASS_IN | const._CLASS_UNIQUE,
962+
120,
963+
0,
964+
0,
965+
80,
966+
host,
967+
),
968+
r.DNSAddress(
969+
host,
970+
const._TYPE_AAAA,
971+
const._CLASS_IN | const._CLASS_UNIQUE,
972+
120,
973+
link_local,
974+
scope_id=None,
975+
),
976+
r.DNSAddress(
977+
host,
978+
const._TYPE_AAAA,
979+
const._CLASS_IN | const._CLASS_UNIQUE,
980+
120,
981+
ula,
982+
scope_id=None,
983+
),
984+
r.DNSAddress(
985+
host,
986+
const._TYPE_AAAA,
987+
const._CLASS_IN | const._CLASS_UNIQUE,
988+
120,
989+
link_local,
990+
scope_id=11,
991+
),
992+
]
993+
)
994+
995+
info = ServiceInfo(type_, registration_name)
996+
info.load_from_cache(zeroconf)
997+
# The scoped link-local upgrade is the most recent observation, so it
998+
# has to come first in LIFO order, ahead of the earlier unrelated ULA.
999+
assert info.ip_addresses_by_version(r.IPVersion.V6Only) == [
1000+
ip_address("fe80::52e:c2f2:bc5f:e9c6%11"),
1001+
ip_address("fdc8:d776:7cca:46ed::2"),
1002+
]
1003+
assert info.parsed_scoped_addresses() == [
1004+
"fe80::52e:c2f2:bc5f:e9c6%11",
1005+
"fdc8:d776:7cca:46ed::2",
1006+
]
1007+
zeroconf.close()
1008+
1009+
7931010
# This test uses asyncio because it needs to access the cache directly
7941011
# which is not threadsafe
7951012
@pytest.mark.asyncio

0 commit comments

Comments
 (0)