iframe-proxy

bdraco · 2026-08-28T20:44:28Z

Summary

Adds hypothesis based fuzzing for DNSIncoming ahead of planned parser performance work; the constructor contract is that any byte sequence parses or fails quietly with valid = False, never raising.

Details

new tests/test_fuzz_incoming.py covers raw garbage, mutated and truncated valid packets, spliced compression pointers, adversarial name sections built from label and pointer tokens, and NSEC bitmap mutations; every parsed record is also touched via repr and hash, and a positive control pins that the corpus itself parses fully
the CI profile is derandomized with bounded examples so runs stay fast and reproducible, making CI a fixed regression suite; the actual exploration happens through HYPOTHESIS_PROFILE=long deep runs, which are manual for now
hypothesis joins the dev dependency group; .hypothesis/ is gitignored
verified on both the compiled extension and the pure python fallback; a 300k example deep run passed clean

Test plan

poetry run pytest tests passes, 490 passed
fuzz file passes against the pure python path with the extensions removed
HYPOTHESIS_PROFILE=long run, 50k examples per test, no failures

codecov · 2026-08-28T20:46:23Z

codspeed · 2026-08-28T20:46:29Z

Merging this PR will not alter performance

✅ 24 untouched benchmarks

_{Comparing fuzz_incoming (2f17ae8) with master (77f8cf0)¹}

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. ↩

bluetoothbot · 2026-08-28T21:14:12Z

~~Previous review~~ — superseded by a newer review below.

bluetoothbot

Tip

No blocking issues found — ready to merge.

Copilot

Pull request overview

Adds reproducible Hypothesis fuzz coverage for DNSIncoming packet parsing.

Changes:

Tests malformed, mutated, truncated, compressed-name, and NSEC inputs.
Configures bounded CI and extended local fuzzing profiles.
Adds Hypothesis and ignores its generated state.

Reviewed changes

Copilot reviewed 3 out of 5 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
`tests/test_fuzz_incoming.py`	Adds parser fuzz tests and valid-packet controls.
`tests/conftest.py`	Configures Hypothesis profiles and optional collection.
`pyproject.toml`	Adds the Hypothesis development dependency.
`poetry.lock`	Locks Hypothesis and its dependency.
`.gitignore`	Ignores Hypothesis state.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

bluetoothbot · 2026-08-28T22:00:56Z

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)

Previously-flagged issues verified fixed

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

1. `_parse` misses `repr(incoming)` and the production constructor shape
tests/test_fuzz_incoming.py:42-55

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

2. `pos % (len(data) - 1)` assumes every corpus entry is at least 2 bytes
tests/test_fuzz_incoming.py:103-105

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

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

bluetoothbot

Tip

No blocking issues found — ready to merge.

test: add fuzzing coverage for incoming packet parsing

02137cb

test: skip hypothesis fuzzing on pypy

a17aaf1

bdraco marked this pull request as ready for review August 28, 2026 20:56

bluetoothbot approved these changes Aug 28, 2026

View reviewed changes

bdraco mentioned this pull request Aug 28, 2026

feat: speed up incoming packet parsing #1830

Merged

5 tasks done

test: address fuzzing review feedback

2f17ae8

bdraco requested a balanced review from Copilot August 28, 2026 21:26

Copilot started reviewing on behalf of bdraco August 28, 2026 21:27

View session

Copilot AI reviewed Aug 28, 2026

View reviewed changes

bluetoothbot approved these changes Aug 28, 2026

View reviewed changes

bdraco merged commit 868391c into master Aug 28, 2026
39 checks passed

bdraco deleted the fuzz_incoming branch August 28, 2026 22:02

Sunbelt Computer Software

PL/B Language Development and Support

Navigation Menu

Repository navigation

test: add fuzzing coverage for incoming packet parsing - #1829

test: add fuzzing coverage for incoming packet parsing#1829
bdraco merged 3 commits into
masterfrom
fuzz_incoming

bdraco commented Aug 28, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Aug 28, 2026 •

edited

Loading

Uh oh!

codspeed Bot commented Aug 28, 2026 •

edited

Loading

Uh oh!

bluetoothbot commented Aug 28, 2026 •

edited

Loading

Uh oh!

bluetoothbot left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

bluetoothbot commented Aug 28, 2026

Uh oh!

bluetoothbot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Sunbelt Computer Software

PL/B Language Development and Support

Conversation

bdraco commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Details

Test plan

Uh oh!

codecov Bot commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

codspeed Bot commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Footnotes

Uh oh!

bluetoothbot commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bluetoothbot left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

bluetoothbot commented Aug 28, 2026

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

✅ Resolved since last review (6)

🟢 Suggestions

Checklist

Uh oh!

bluetoothbot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bdraco commented Aug 28, 2026 •

edited

Loading

codecov Bot commented Aug 28, 2026 •

edited

Loading

codspeed Bot commented Aug 28, 2026 •

edited

Loading

bluetoothbot commented Aug 28, 2026 •

edited

Loading