Skip to content

fix: preserve scope_id when scoped AAAA arrives alongside unscoped#1764

Merged
bdraco merged 3 commits into
python-zeroconf:masterfrom
bluetoothbot:koan/fix-issue-1567
May 21, 2026
Merged

fix: preserve scope_id when scoped AAAA arrives alongside unscoped#1764
bdraco merged 3 commits into
python-zeroconf:masterfrom
bluetoothbot:koan/fix-issue-1567

Conversation

@bluetoothbot
Copy link
Copy Markdown
Contributor

@bluetoothbot bluetoothbot commented May 20, 2026

Summary

ServiceInfo.parsed_scoped_addresses() is documented to qualify link-local IPv6 addresses with %<interface_index> when available, but the qualifier was being dropped when the same AAAA arrived as two records — once via the IPv4 listener (no scope captured) and once via the IPv6 listener (scope from the receiving socket). The cache stored both records as distinct entries (DNSAddress equality includes scope_id), so dedup at the ServiceInfo layer either kept both copies side-by-side or, depending on arrival order, silently chose the unscoped one.

Match addresses on their packed integer (zc_integer, scope-agnostic) when collapsing duplicates in _get_ip_addresses_from_cache_lifo and _process_record_threadsafe, and prefer whichever variant carries a scope_id. The fix is local to the ServiceInfo layer — DNS records remain hashed by scope_id so the cache still distinguishes them — and addresses the dedup half of #1567 without touching socket setup. The complementary work for the IPv4-only-arrival case (per-interface sockets or IP_PKTINFO) remains for a follow-up.

Fixes #1567

Changes

  • Add _index_of_same_address / _has_more_scope_info helpers that compare addresses ignoring scope_id and tell whether a new record carries scope info the stored variant lacks.
  • Rewrite the cache-LIFO and live-update paths in ServiceInfo so a later-arriving scoped AAAA upgrades the stored entry instead of being deduped away (or appended as a duplicate).
  • Extract the AAAA-add branch of _process_record_threadsafe into _upsert_ipv6_address, declared in info.pxd.

Test plan

  • New test_scoped_address_preferred_when_unscoped_arrives_first_in_cache — both variants cached, parsed_scoped_addresses() returns the %<idx>-qualified form only.
  • New test_scoped_address_replaces_unscoped_in_live_update — exercises async_update_records with unscoped-then-scoped arrival order.
  • SKIP_CYTHON=1 poetry run pytest tests --timeout=60 → 391 passed, 2 skipped.

Generated by Kōan /fix


Quality Report

Changes: 3 files changed, 159 insertions(+), 14 deletions(-)

Code scan: clean

Tests: passed (4 PASSED)

Branch hygiene: clean

Generated by Kōan post-mission quality pipeline

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 20, 2026

Merging this PR will not alter performance

✅ 14 untouched benchmarks


Comparing bluetoothbot:koan/fix-issue-1567 (8c356dd) with master (137a5d6)1

Open in CodSpeed

Footnotes

  1. No successful run was found on master (9470bd6) during the generation of this report, so 137a5d6 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.77%. Comparing base (137a5d6) to head (8c356dd).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1764   +/-   ##
=======================================
  Coverage   99.77%   99.77%           
=======================================
  Files          33       33           
  Lines        3511     3536   +25     
  Branches      494      498    +4     
=======================================
+ Hits         3503     3528   +25     
  Misses          5        5           
  Partials        3        3           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@bdraco
Copy link
Copy Markdown
Member

bdraco commented May 20, 2026

local testing scripts

"""Discover services on this LAN and verify scope_id preservation on link-local IPv6."""

from __future__ import annotations

import asyncio
import socket
from ipaddress import IPv6Address

from zeroconf import IPVersion, ServiceInfo
from zeroconf.asyncio import (
    AsyncServiceBrowser,
    AsyncServiceInfo,
    AsyncZeroconf,
    AsyncZeroconfServiceTypes,
)


async def main() -> None:
    aiozc = AsyncZeroconf(ip_version=IPVersion.All)

    print("discovering service types for 5s...")
    types = list(await AsyncZeroconfServiceTypes.async_find(aiozc=aiozc, timeout=5))
    print(f"  found {len(types)} type(s): {types}")

    seen: dict[str, AsyncServiceInfo] = {}

    def on_service_state_change(zeroconf, service_type, name, state_change):
        if state_change.name != "Added":
            return
        seen[name] = AsyncServiceInfo(service_type, name)

    if not types:
        print("no service types found on this network")
        await aiozc.async_close()
        return

    browser = AsyncServiceBrowser(
        aiozc.zeroconf, types, handlers=[on_service_state_change]
    )
    await asyncio.sleep(6)
    await browser.async_cancel()

    print(f"\nresolving {len(seen)} service(s)...")
    link_local_with_scope = 0
    link_local_without_scope = 0
    dup_check_ok = 0
    dup_check_bad = 0

    for name, info in seen.items():
        ok = await info.async_request(aiozc.zeroconf, 3000)
        if not ok:
            continue
        scoped = info.parsed_scoped_addresses()
        v6 = info.ip_addresses_by_version(IPVersion.V6Only)
        v4 = info.ip_addresses_by_version(IPVersion.V4Only)

        ll_with_scope = []
        ll_no_scope = []
        for addr in scoped:
            if addr.startswith("fe80:") or "%" in addr:
                if "%" in addr:
                    ll_with_scope.append(addr)
                else:
                    ll_no_scope.append(addr)

        # Verify no packed-byte duplicates among v6 addresses.
        packed_seen = set()
        dup_in_packed = False
        for a in v6:
            key = int(IPv6Address(str(a).split("%", 1)[0]))
            if key in packed_seen:
                dup_in_packed = True
                break
            packed_seen.add(key)
        if dup_in_packed:
            dup_check_bad += 1
        else:
            dup_check_ok += 1

        link_local_with_scope += len(ll_with_scope)
        link_local_without_scope += len(ll_no_scope)

        marker = ""
        if ll_with_scope:
            marker = "  <-- scoped link-local"
        elif ll_no_scope:
            marker = "  <-- unscoped link-local (would be a regression)"

        print(f"  {name}{marker}")
        print(f"    v4: {[str(a) for a in v4]}")
        print(f"    v6: {[str(a) for a in v6]}")
        print(f"    parsed_scoped_addresses: {scoped}")

    print()
    print("summary:")
    print(f"  resolved: {len(seen)}")
    print(f"  link-local IPv6 with %scope:  {link_local_with_scope}")
    print(f"  link-local IPv6 without %scope: {link_local_without_scope}")
    print(f"  services with no packed-byte duplicates: {dup_check_ok}")
    print(f"  services with packed-byte duplicates (BAD): {dup_check_bad}")

    await aiozc.async_close()


if __name__ == "__main__":
    asyncio.run(main())
"""Pinpoint services where master returns a packed-byte duplicate IPv6 address."""

from __future__ import annotations

import asyncio
from ipaddress import IPv6Address

from zeroconf import IPVersion
from zeroconf.asyncio import (
    AsyncServiceBrowser,
    AsyncServiceInfo,
    AsyncZeroconf,
    AsyncZeroconfServiceTypes,
)


async def main() -> None:
    aiozc = AsyncZeroconf(ip_version=IPVersion.All)
    types = list(await AsyncZeroconfServiceTypes.async_find(aiozc=aiozc, timeout=5))
    seen: dict[str, AsyncServiceInfo] = {}

    def on_state(zeroconf, service_type, name, state_change):
        if state_change.name == "Added":
            seen[name] = AsyncServiceInfo(service_type, name)

    browser = AsyncServiceBrowser(aiozc.zeroconf, types, handlers=[on_state])
    await asyncio.sleep(8)
    await browser.async_cancel()

    print(f"scanning {len(seen)} services for v6 packed-byte duplicates...")
    for name, info in seen.items():
        if not await info.async_request(aiozc.zeroconf, 3000):
            continue
        v6 = info.ip_addresses_by_version(IPVersion.V6Only)
        packed = [int(IPv6Address(str(a).split("%", 1)[0])) for a in v6]
        if len(packed) != len(set(packed)):
            print(f"\nDUPLICATE in {name}")
            print(f"  raw v6: {[str(a) for a in v6]}")
            print(f"  parsed_scoped: {info.parsed_scoped_addresses()}")
    await aiozc.async_close()


if __name__ == "__main__":
    asyncio.run(main())

confirmed reproduction on master and confirmed fixed with this pr

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes ServiceInfo’s IPv6 link-local scoping behavior when the same AAAA address is observed both with and without scope_id (e.g., via separate IPv4/IPv6 listeners). The intent is to ensure parsed_scoped_addresses() prefers and preserves the %<interface_index>-qualified form when available, while still allowing the DNS cache to keep scoped/unscoped records distinct.

Changes:

  • Add _index_of_same_address / _has_more_scope_info helpers to dedupe AAAA addresses ignoring scope_id and prefer the variant that contains scope info.
  • Update cache-load and live-update paths so later-arriving scoped AAAA “upgrades” an existing unscoped entry instead of being dropped or duplicated.
  • Extract IPv6 address insert/update logic into _upsert_ipv6_address (declared in info.pxd) and add new tests covering cache and live-update ordering.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
tests/services/test_info.py Adds regression tests ensuring scoped AAAA is preferred/replaces unscoped variants.
src/zeroconf/_services/info.py Implements scope-agnostic dedup + “prefer scoped” selection for IPv6 addresses.
src/zeroconf/_services/info.pxd Declares the new _upsert_ipv6_address method for Cython builds.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/zeroconf/_services/info.py Outdated
@bdraco
Copy link
Copy Markdown
Member

bdraco commented May 20, 2026

@bluetoothbot rr

@bluetoothbot
Copy link
Copy Markdown
Contributor Author

PR Review — fix: preserve scope_id when scoped AAAA arrives alongside unscoped

Targeted fix that correctly addresses the dedup half of #1567. The cache and live-update paths now collapse same-packed-bytes AAAA records via zc_integer and prefer the scoped variant when one arrives. The LIFO-recency concern Copilot/@bdraco raised on the previous iteration is fixed — re-seen addresses are popped and re-appended in _get_ip_addresses_from_cache_lifo, and test_scope_upgrade_preserves_lifo_recency_order pins the behaviour with an interleaved unscoped/ULA/scoped sequence. Cython wiring in info.pxd looks consistent (existing_idx=int, existing=object); CodSpeed reports no regression. Only suggestions are style-level: trim the module-helper docstrings to single lines per CLAUDE.md, and consider whether the scoped→different-scope case in _has_more_scope_info is an explicit non-goal. Merge-ready.


🟢 Suggestions

1. Helper docstrings exceed the project's terse style (`src/zeroconf/_services/info.py`, L112-126)

CLAUDE.md is explicit: "Docstrings: terse, default to single-line" and "Rationale / motivation / 'why we used to do X' — that's the PR description and the commit message." The _index_of_same_address and _upsert_ipv6_address docstrings restate the scope-collapse rationale that already lives in the PR body. Consider trimming to one-line contracts, e.g.:

def _index_of_same_address(...) -> int:
    """Return the index of an entry with the same packed bytes, or -1."""

def _upsert_ipv6_address(self, ip_addr) -> bool:
    """Insert or upgrade an IPv6 address in LIFO order, preferring scope."""

The inline comment block in _get_ip_addresses_from_cache_lifo (lines 487–494) is in a grey area — it explains a non-obvious cache invariant (DNSAddress.__eq__ includes scope_id), which is the legitimate why case the style guide reserves comments for. That one is probably fine; it's the multi-paragraph docstrings on the new module-level helpers that drift from convention.

def _index_of_same_address(
    addresses: Sequence[ZeroconfIPv4Address | ZeroconfIPv6Address],
    ip_addr: ZeroconfIPv4Address | ZeroconfIPv6Address,
) -> int:
    """Return the index of an existing entry with the same packed bytes, or -1.

    Match by ``zc_integer`` so IPv6 addresses that differ only in
    scope_id ...
    """

Checklist

  • Correctness of scope_id preservation
  • LIFO recency ordering preserved on upgrade
  • Cython .pxd updated for new method
  • Test coverage for cache + live-update paths
  • No hardcoded secrets / unsafe deserialization
  • Docstring style matches CLAUDE.md (terse, single-line default) — suggestion #1
  • Behavioural return-value shift documented — suggestion #3

Summary

Targeted fix that correctly addresses the dedup half of #1567. The cache and live-update paths now collapse same-packed-bytes AAAA records via zc_integer and prefer the scoped variant when one arrives. The LIFO-recency concern Copilot/@bdraco raised on the previous iteration is fixed — re-seen addresses are popped and re-appended in _get_ip_addresses_from_cache_lifo, and test_scope_upgrade_preserves_lifo_recency_order pins the behaviour with an interleaved unscoped/ULA/scoped sequence. Cython wiring in info.pxd looks consistent (existing_idx=int, existing=object); CodSpeed reports no regression. Only suggestions are style-level: trim the module-helper docstrings to single lines per CLAUDE.md, and consider whether the scoped→different-scope case in _has_more_scope_info is an explicit non-goal. Merge-ready.


Automated review by Kōanead2b2d
7a8102f
60708f9
c31118f

bluetoothbot and others added 3 commits May 20, 2026 23:11
When the same link-local IPv6 address arrives as two DNSAddress
records — once over an IPv4 socket (scope_id=None) and once over an
IPv6 socket (scope_id=interface index) — the cache stored both and
ServiceInfo would either return both copies or silently drop the
scoped variant, leaving parsed_scoped_addresses() unable to return
the %<interface_index>-qualified address its docstring promises.

Collapse the two variants in _get_ip_addresses_from_cache_lifo() and
_process_record_threadsafe() by matching addresses on their packed
integer (ignoring scope_id) and keeping the entry that carries a
scope_id when one is available.

Fixes python-zeroconf#1567
Add a cache LIFO case where a scoped AAAA is encountered before its
unscoped twin so the unscoped variant fails the _has_more_scope_info
check and the scoped entry stays in place. Add a direct call against
_has_more_scope_info with an IPv4 address so its non-IPv6 short-circuit
return is exercised.
In _get_ip_addresses_from_cache_lifo, an in place replacement of an
existing address kept the upgraded entry at its original insertion index.
After the final reverse to produce LIFO order, an unrelated address that
arrived between the unscoped and the scoped variant ended up ahead of
the upgrade, breaking the recency guarantee. Pop the existing entry and
re append so the later observation wins both in value and in LIFO
position.
@bluetoothbot
Copy link
Copy Markdown
Contributor Author

Rebase with requested adjustments

Branch koan/fix-issue-1567 was rebased onto master and review feedback was applied.

Stats

3 files changed, 292 insertions(+), 15 deletions(-)
Actions performed
  • Already-solved check: negative (confidence=high, reasoning=No recent master commit addresses scope_id preservation in ServiceInfo address dedup for issue Link-local addresses returned without scope. #1567)
  • Rebased koan/fix-issue-1567 onto upstream/master
  • Pre-push CI check: previous run passed
  • Force-pushed koan/fix-issue-1567 to origin
  • CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.


Automated by Kōan

@bdraco bdraco marked this pull request as ready for review May 21, 2026 02:58
@bdraco bdraco merged commit e2352ea into python-zeroconf:master May 21, 2026
36 checks passed
@bluetoothbot bluetoothbot deleted the koan/fix-issue-1567 branch May 21, 2026 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Link-local addresses returned without scope.

3 participants