Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.86%. Comparing base (77f8cf0) to head (2f17ae8).
⚠️ Report is 1 commits behind head on master.

@@            Coverage Diff             @@
##           master    #1829      +/-   ##
==========================================
+ Coverage   99.81%   99.86%   +0.05%     
==========================================
  Files          33       33              
  Lines        3839     3839              
  Branches      553      553              
==========================================
+ Hits         3832     3834       +2     
+ Misses          5        3       -2     
  Partials        2        2              

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

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Merging this PR will not alter performance

✅ 24 untouched benchmarks

Comparing fuzz_incoming (2f17ae8) with master (77f8cf0)1

Footnotes

1. No successful run was found on master (228902a) during the generation of this report, so 77f8cf0 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

Previous review — superseded by a newer review below.

PR Review — test: add fuzzing coverage for incoming packet parsing

All six findings from the previous round are addressed; what remains are two nits. Merge-ready.

The follow-up commit is a clean response rather than a minimal one: the Hypothesis profile registration moved into tests/conftest.py so it is session-wide config instead of a single module's import side effect; the availability guard is now importlib.util.find_spec("hypothesis"), which covers both PyPy and a non-dev install with one condition, and the accompanying comment's stated "why" now matches reality (I verified .github/workflows/ci.yml:60 pins pypy-3.10 and the lock ships only pp311 wheels). MAX_PACKET_SIZE is derived from const._MAX_MSG_ABSOLUTE with the slack explained, _parse now reprs and hashes every returned record, and test_corpus_parses_cleanly gives the suite the positive control it was missing — without it, a regression that made the parser reject everything would have left all six tests green. The poetry.lock version churn flagged last time is gone (lock-version stays at 2.1), and exceptiongroup already carries a python_version == "3.10" marker wide enough for Hypothesis's backport requirement, so the 3.10 matrix entry resolves.

I re-verified the two properties the suite leans on: DNSRecord.__eq__ compares wire fields only (_dns.py:283-292, 575-594) and excludes created, so the determinism assertion at line 70 is stable; and _seen_logs is capped by _mark_seen (_logger.py:62), while tests/test_logger.py and tests/test_protocol.py both .clear() it up front, so the fuzz run cannot leak state into them.

• _parse still never calls repr(incoming) — the one call that covers truncated, the counts, and every DNSQuestion repr — and always uses the bare DNSIncoming(data) form, while production passes source/scope_id/now (_listener.py:187).
• start = pos % (len(data) - 1) at line 103 breaks on any future corpus entry shorter than 2 bytes.
• Noted, not a finding: nothing in .github/workflows/ sets HYPOTHESIS_PROFILE=long, so CI replays the same derandomized 200 examples and the deep run stays manual — the description now says so explicitly, which is the author's call to make.

✅ Resolved since last review (6)

• tests/test_fuzz_incoming.py:19 Hypothesis profile is registered/loaded as a test-module import side effect
• tests/test_fuzz_incoming.py:20 derandomize=True plus no scheduled long run makes this a fixed 200-example regression suite
• tests/test_fuzz_incoming.py:46 _parse stops at answers() and never touches the returned records
• tests/test_fuzz_incoming.py:43 No positive control — the suite passes vacuously if every packet becomes invalid
• tests/test_fuzz_incoming.py:24 MAX_PACKET_SIZE = 9194 is a magic number with no counterpart in the codebase
• tests/conftest.py:19 Skip comment's stated reason is inaccurate, and the guard keys on interpreter rather than importability

🟢 Suggestions

The helper now touches the returned records (good — that closes the earlier gap), but two surfaces the parser exposes in production are still untouched.

• repr(incoming) (src/zeroconf/_protocol/incoming.py:206-220) is the widest single call: it formats truncated, all four counts, every DNSQuestion repr, and answers(). Question reprs are the only record-adjacent repr the suite never exercises — _read_questions is reachable from the garbage/mutation strategies (any 12-byte prefix yields some qdcount), so the objects are being built and then dropped untested. Handlers log msg at debug level, so this repr does run on adversarial input in production.
• Constructor arguments. _listener.py:187 calls DNSIncoming(data, addr_port, scope, now); the fuzzer only ever calls DNSIncoming(data). scope_id is threaded into every DNSAddress built from an AAAA record (incoming.py:379-390) and into its _hash, so the scoped-address construction path is never fuzzed.

Both are cheap to add:

def _parse(data: bytes, scope_id: int | None = None) -> list[DNSRecord]:
    incoming = DNSIncoming(data, ("127.0.0.1", 5353), scope_id)
    ...
    repr(incoming)
    return answers

Note repr(incoming) calls answers() internally, so order it after the existing loop to keep the record-level asserts first.

    answers = incoming.answers()
    assert isinstance(answers, list)
    for record in answers:
        repr(record)
        hash(record)
    return answers

start = pos % (len(data) - 1) is only safe because every current CORPUS entry is a full multi-record packet. A 1-byte entry gives pos % 0 → ZeroDivisionError; an empty one gives pos % -1 → 0, and then data[1] = ... raises IndexError.

Why it matters: the corpus is a shared, growable module global (CORPUS = [*generate_packets(), _nsec_packet()]). The next person adding a deliberately tiny packet — exactly the kind of thing you'd want in a fuzz corpus — gets a crash inside the strategy body rather than a fuzz finding, and the failure will read as a Hypothesis bug rather than a corpus one.

Either clamp (start = pos % max(len(data) - 1, 1)) or assert the precondition once next to CORPUS (assert all(len(p) >= 2 for p in CORPUS)).

    data = bytearray(packet)
    start = pos % (len(data) - 1)
    data[start] = 0xC0 | (target >> 8)
    data[start + 1] = target & 0xFF

Checklist

• No hardcoded secrets or credentials
• No unsafe deserialization / eval / exec
• Error handling — no silently swallowed exceptions introduced
• No unbounded collections (_seen_logs capped by _mark_seen; pre-existing tests clear it)
• Test isolation — Hypothesis profile now set session-wide in conftest, no cross-module global leakage
• Positive control present — corpus liveness pinned, suite cannot pass vacuously
• Edge case and downstream coverage of new tests — suggestion ＃1, suggestion ＃2
• Tests verify observable behavior, not source inspection
• Python-specific: no mutable defaults, no is vs == misuse
• Free-threaded 3.14t / PyPy matrix not broken (cp314t wheels locked; PyPy skipped via find_spec)
• Dependency lock is coherent (content-hash updated, exceptiongroup marker covers 3.10)
• Diff matches PR description, no scope creep

Automated review by Kōan (Claude) HEAD=2f17ae8 4 min 41s