Repository navigation
toolkit: pypcapfile adapter reads dotted-decimal addresses as packed bytes, so 7 of 10 gated tests fail when enabled #743
Description
pcapkit/toolkit/pypcapfile.py treats pypcapfile's IPv4 addresses as packed 4-byte values. They are dotted-decimal ASCII in a ctypes.c_char_p — e.g. b'10.1.1.2'. So every conversion raises.
Sites, on origin/main at 9b2d927c2:
| site | what it does |
|---|---|
:210-211 |
str(ipaddress.IPv4Address(ipv4.src)) / .dst |
:320-321 |
same, in ipv4_reassembly |
:391-393 |
same, in the TCP path |
:104-115 |
feeds ipv4.src/.dst into struct.pack |
ipaddress.IPv4Address(b'10.1.1.2') raises AddressValueError (packed form must be exactly 4 bytes); struct.pack raises struct.error. Confirmed against pypcapfile's own pcapfile/protocols/network/ip.py.
Measured: with pypcapfile installed, 7 of its 10 gated test methods fail outright — reproduced across two independent verification runs plus a direct repro, on Python 3.10 and 3.11.
Why this has never been seen
Three layers hid it, and all three had to hold:
- CI never installed
pypcapfile— tests: seven more skipUnless gates have never executed on any CI path (106 methods) #738's defect class; these tests reported as skips. - The local dev venv is Python 3.14, where
pyproject.toml:201'spython_version < '3.12'marker excludespypcapfileentirely — somake testskipped them too. - The marker means the adapter is only reachable on 3.10/3.11, which nothing routinely exercises.
So engine='pypcapfile' is broken for any user on 3.10 or 3.11 who tries it.
Scope note
Found while proving #740 (which correctly ships crypto/cli/NGAP and excludes PyPCAPFile for exactly this reason). Also correcting my own attribution in #738: the 10 methods are 4 in tests/toolkit/test_pypcapfile_unit.py and 6 in tests/foundation/engines/test_new_engine_parity_runtime.py — the latter is fixture-tier per tests/_tiers.py::fixture_tier_paths(), so it runs only on integration/gate.
Fixing the adapter is the prerequisite for ever enabling HAS_PYPCAPFILE in CI. Turning the extra on without fixing this turns 7 green skips into 7 red failures.
Activity
JarryShaw commentedon Sep 24, 2026
Correcting my own attribution. The count was right, the cause was not.
7 of 10 do fail at baseline — confirmed on Python 3.10.21 with
pypcapfile0.12.0. But only 4 are caused by the address defect (all intests/toolkit/test_pypcapfile_unit.py). The other 3, intests/foundation/engines/test_new_engine_parity_runtime.py, fail for a different and more severe reason and are unaffected by this fix.So after #747: 7 of 10 pass, but not the same 7 — the 4 address ones go green, the 3 parity ones stay red, with identical symptoms before and after.
That second defect is now #746:
pcapkit/foundation/engines/pypcapfile.pyloads savefiles withlayers=0, which inpypcapfilemeans hexlify the whole frame and stop, then feeds that hex-ASCII text straight intoEthernet(packet.packet, layers=1)without un-hexlifying. Every field comes out garbage — MAC, ethertype, IP addresses alike. Strictly worse than this issue, and it needs its own PR.Two things established while fixing this that are worth having on the record:
IP.optandIP.payload, not just addresses — handled in fix(toolkit): parse PyPCAPFile's dotted-decimal IPv4 text, not packed bytes #747 via a_maybe_unhex()helper at three further sites (ipv4_header's options,_transport's TCP segment, andipv4_reassembly's fragment payload).pypcapfileships no IPv6 decoder at all, andipv6_reassembly()already raisesUnsupportedCallunconditionally. My issue left that open as a question; it is answered.The 3 parity tests were deliberately left red rather than adjusted: they correctly detect #746, so nothing in that test file is wrong.
fix(toolkit): parse PyPCAPFile's dotted-decimal IPv4 text, not packed…
docs(changelog): account for the 8 merges since 4530424
docs(changelog): account for the 20 merges since 73f09ae, four breaking
docs(changelog): account for the 20 merges since 73f09ae, six breaking
docs(changelog): account for the 20 merges since 73f09ae, six breaking
docs(changelog): account for the 20 merges since 73f09ae, six breaking
docs(changelog): account for the 20 merges since 73f09ae, six breaking