feat: speed up incoming packet parsing by bdraco · Pull Request #1830 · python-zeroconf/python-zeroconf · GitHub
Skip to content

feat: speed up incoming packet parsing - #1830

Merged
bdraco merged 12 commits into
masterfrom
speed_up_incoming_parser
Aug 28, 2026
Merged

bdraco merged 12 commits into
masterfrom
speed_up_incoming_parser

Conversation

@bdraco

@bdraco bdraco commented Aug 28, 2026

Copy link
Copy Markdown
Member

Summary

Speeds up DNSIncoming parsing by 47 percent; the benchmark packet drops from 9.3µs to 4.9µs on the compiled build. Stacked on #1829 so the fuzzing suite guards every change.

Details

  • names that are a single RFC 1035 §4.1.4 compression pointer to an already decoded name now return the cached finished string; the benchmark packet builds 45 names but only 8 are distinct, so 37 list, set, join, and dict operations disappear, and identical names across a packet share one str object
  • the loop detection set is only allocated when a name actually follows an uncached pointer
  • _read_bitmap indexes the buffer directly instead of allocating a bytes slice per NSEC record, and skips zero bytes
  • the memoryview attribute is replaced by a raw buffer pointer with an explicit bounds check at every read site; the memoryview's implicit IndexError was load bearing, so each site now carries its own guard, and messages beyond the 64 KiB DNS limit are rejected up front so no unsigned arithmetic can wrap
  • DNSIncoming now refuses pickling, data is readonly, both closing lifetime holes the raw pointer would otherwise open

Test plan

  • two adversarial security audits of every buffer read verified all 15 sites bounded on all paths, including a 200k name differential fuzz of the name cache against a cache free reference decoder with zero mismatches
  • hypothesis fuzzing from test: add fuzzing coverage for incoming packet parsing #1829: 300k example deep runs pass on both the compiled and pure python builds
  • new tests pin string sharing across records, all pointer flows, overlong name rejection, pickle refusal, and oversized message rejection
  • poetry run pytest tests passes, 495 passed; pre-commit clean; Cython annotate maps show the added score confined to cold raise blocks and the 8x slow path
  • verified on a live network; all services resolve with correct properties

@codspeed

codspeed Bot commented Aug 28, 2026 •

Copy link
Copy Markdown

@codecov

codecov Bot commented Aug 28, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.87%. Comparing base (28f8047) to head (59acf2c).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1830   +/-   ##
=======================================
  Coverage   99.86%   99.87%           
=======================================
  Files          33       33           
  Lines        3839     3883   +44     
  Branches      553      567   +14     
=======================================
+ Hits         3834     3878   +44     
  Misses          3        3           
  Partials        2        2           

☔ View full report in Codecov by Harness.
📢 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.

@bdraco
bdraco force-pushed the speed_up_incoming_parser branch from 59015cd to 8448b77 Compare August 28, 2026 21:24
@bdraco

bdraco commented Aug 28, 2026

Copy link
Copy Markdown
Member Author

Just waiting for ASAN locally

@bdraco

bdraco commented Aug 28, 2026

Copy link
Copy Markdown
Member Author

local ASan 50,000 hostile packets + all 14 audit boundary packets, zero sanitizer reports.

@bdraco
bdraco marked this pull request as ready for review August 28, 2026 21:56
Base automatically changed from fuzz_incoming to master August 28, 2026 22:02
@bluetoothbot

Copy link
Copy Markdown
Contributor

@bdraco: local ASan 50,000 hostile packets + all 14 audit boundary packets, zero sanitizer reports.

Matches static audit. Traced all buffer reads independently: header, questions, record header, SRV, character string, string, NSEC bitmap, name fast path, label pointer. Each guarded before dereference. Label slice correctly stays on self.data, avoiding pointer over-read.

One gap: fuzz strategies cap at _MAX_MSG_ABSOLUTE + 256, roughly 9 KiB. ASan corpus likely same range. Nothing reaches offsets near 65535, where no-wrap argument for MAX_MSG_LEN matters. Worth one 65530-byte packet with record header at tail.

@bluetoothbot

bluetoothbot commented Aug 28, 2026 •

Copy link
Copy Markdown
Contributor

Previous review — superseded by a newer review below.

@bluetoothbot bluetoothbot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tip

No blocking issues found — ready to merge.

@bdraco

bdraco commented Aug 28, 2026

Copy link
Copy Markdown
Member Author
  1. Pickle refusal + readonly data are user-visible restrictions on a re-exported class
    src/zeroconf/_protocol/incoming.py:140-143

Nobody is pickling DNSIncoming, that would be a crazy use case. Its not intended to be a public API. This is an internal safety

@bluetoothbot

Copy link
Copy Markdown
Contributor

@bdraco: > 1. Pickle refusal + readonly data are user-visible restrictions on a re-exported class

Fair — dropping that finding. Grepped repo: nothing pickles or copies DNSIncoming, so no in-tree breakage. Only note: __reduce__ also blocks copy.copy and copy.deepcopy, which your test already pins. Won't re-raise.

@bluetoothbot

Copy link
Copy Markdown
Contributor

@bluetoothbot bluetoothbot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tip

No blocking issues found — ready to merge.

@bdraco
bdraco merged commit b3634f2 into master Aug 28, 2026
33 checks passed
@bdraco
bdraco deleted the speed_up_incoming_parser branch August 28, 2026 22:48
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.

2 participants