Add the pypcap and pypcapfile extraction engines - #386
Conversation
Both candidates the Help Wanted page names are now selectable as
engine='pypcap' and engine='pypcapfile', each with a matching pcapkit.toolkit
module, a pyproject extra, docs and tests.
They are registered as built-ins in Extractor.__engine__ rather than through
Engine.__init_subclass__, because the public auto-registration path cannot work
for an engine that ships with the library: pcapkit/__init__.py never imports
pcapkit.foundation.engines, so the hook would not fire for a bare `import
pcapkit` and engine='pypcap' would silently fall through to the built-in
parser - the exact trap these engines are meant to avoid. ModuleDescriptor also
keeps the third-party import lazy. The public path stays the documented route
for user engines and keeps its own coverage.
Capability gaps are surfaced rather than papered over. pypcap is a libpcap
binding with no dissection: reassembly and flow tracing are switched off with a
warning and the toolkit adapters raise UnsupportedCall; it is PCAP-only, since
libpcap opens a PCAP-NG file happily and then yields zero frames, so the engine
checks the magic number and raises FormatError instead of reporting an empty
capture; and it needs a file on disk. pypcapfile decodes Ethernet/IPv4/TCP/UDP
only, so IPv6 reassembly raises rather than returning None, which would be
indistinguishable from "no fragment here".
Neither library installs cleanly from PyPI on a current Python, and pep.rst
says so instead of claiming otherwise: released pypcapfile 0.12.0 imports the
`imp` module removed in 3.12, so the engine was verified against upstream
master; and pypcap 1.3.0 ships pre-generated Cython C that no longer compiles
against the 3.12+ C API, plus a setup.py that only searches fixed prefixes for
pcap.h. Both were genuinely executed here, on a locally rebuilt wheel.
Parity is asserted per frame - capture length, original length, timestamp and
the full Ethernet header - across four captures, and each test proves the
engine actually ran rather than being replaced by the fallback, by asserting no
EngineWarning was raised and that the extractor reports the engine by name.
One pre-existing crash had to be fixed for pypcapfile to work at all:
Extractor(trace=True) died with AttributeError because the guard listed only
pyshark and matched trace_format == 'pcap', while TraceFlow substitutes 'pcap'
for None, so it never fired. It now matches ('pcap', 'cap', None), which
incidentally fixes pyshark. dpkt and scapy remain broken there by design and
are called out in a comment.
There was a problem hiding this comment.
🟢 Approval recommended
The changes appear functionally complete with extensive unit/parity coverage, and the only noted issues are minor typos in user-facing strings/docstrings.
Pull request overview
Adds two new selectable third-party extraction engines (pypcap, pypcapfile) to PyPCAPKit/pcapkit, including lazy engine registration, matching toolkit adapters, documentation, and extensive unit + parity test coverage to ensure the requested engine actually ran and matches the default engine on key wire-level fields.
Changes:
- Register
pypcapandpypcapfileas built-in engines inExtractor.__engine__, and adjust extractor trace-format handling for engines that emit mapping frames. - Introduce new engine implementations and toolkit adapters for
pypcapandpypcapfile, including explicit capability-gap behavior (warnings/exceptions). - Add docs updates and a broad test suite (unit tests + end-to-end parity tests), plus packaging extras for the new dependencies.
File summaries
| File | Description |
|---|---|
| tests/toolkit/test_pypcapfile_unit.py | Unit tests for PyPCAPFile toolkit helpers and adapters (stand-ins + real-decoder gated tests). |
| tests/toolkit/test_pypcap_unit.py | Unit tests for PyPCAP toolkit helpers and “refuse loudly” adapters. |
| tests/foundation/engines/test_pypcapfile_engine.py | Engine-level unit tests for PyPCAPFile behavior using stand-ins/mocks. |
| tests/foundation/engines/test_pypcap_engine.py | Engine-level unit tests for PyPCAP behavior using a fake pcap handle. |
| tests/foundation/engines/test_new_engine_parity.py | End-to-end parity tests proving the requested engine ran and matches default on key fields. |
| pyproject.toml | Adds PyPCAP/PyPCAPFile extras and includes them in all. |
| Pipfile | Adds pypcap and pypcapfile to dev dependencies. |
| pcapkit/toolkit/pypcapfile.py | New toolkit adapter module for pypcapfile packets, including IPv4/TCP support and explicit IPv6 refusal. |
| pcapkit/toolkit/pypcap.py | New toolkit adapter module for pypcap raw frames and explicit refusal for higher-level adapters. |
| pcapkit/toolkit/init.py | Documents (commented) import surface for the new toolkit modules. |
| pcapkit/interface/misc.py | Updates engine type alias and adds pypcap to follow_tcp_stream fallback list. |
| pcapkit/foundation/extraction.py | Adds new engines to typing/registration; adjusts trace-format fallback for engines emitting dict frames. |
| pcapkit/foundation/engines/pypcapfile.py | New PyPCAPFile engine implementation (PCAP-only, IPv6 gap surfaced, decode-depth control). |
| pcapkit/foundation/engines/pypcap.py | New PyPCAP engine implementation (PCAP-only, file-on-disk only, no dissection, disables reassembly/trace). |
| pcapkit/foundation/engines/init.py | Exposes the new engines from the engines package. |
| docs/source/pep.rst | Updates “New Engines” section to reflect completion and document installability caveats. |
| docs/source/pcapkit/toolkit/3rdparty.rst | Adds Sphinx docs entries for the new toolkit modules. |
| docs/source/pcapkit/foundation/engines/index.rst | Updates engine index and adds a capability-gap table including the new engines. |
| docs/source/pcapkit/foundation/engines/3rdparty.rst | Adds Sphinx docs entries for the new engine classes and internals. |
| docs/source/ext.rst | Updates extraction engine support table and adds notes about capability gaps. |
Review details
Suppressed comments (1)
pcapkit/foundation/engines/pypcap.py:167
- Typo in warning message: "dose" should be "does".
warn("'Extractor(engine=pypcap)' object dose not support flow tracing; "
f"so 'tcp={ext._tcp}' will be ignored", AttributeWarning, stacklevel=stacklevel())
- Files reviewed: 20/20 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
@copilot Fix the code for all comments in this review thread. When a review comment includes a suggested change, apply the suggestion exactly. Do not make changes beyond what is described in the linked review thread. |
Co-authored-by: JarryShaw <15666417+JarryShaw@users.noreply.github.com>
Fixed all four typos ("dose" → "does" in |
`pypcap` 1.3.0 publishes no wheels at all -- only an sdist -- so installing it
compiles a C extension against libpcap and needs both the headers and the shared
library. Listing it in `all` therefore made `pip install pypcapkit[all]` demand a
compiler and libpcap development files from every user, on every platform.
CI proved it before any user could: deploy-pages failed on this PR at
`Getting requirements to build wheel ... error`, with pypcap's setup.py saying
Found pcap headers in /Applications/Xcode.app/.../MacOSX26.sdk/usr/include/pcap.h
None of the following found: ['libpcap.a', 'libpcap.so', 'libpcap.dylib', 'wpcap.lib']
The macOS runner has the header and no library, which is exactly the case that
fails. deploy-pages was only the first to run: `.[all]` is also installed by
cron-conda (x3), cron-vendor and create-release (x2), so the release and conda
paths would have broken on merge too.
`pypcap` stays available as its own opt-in extra, `pip install pypcapkit[PyPCAP]`,
and is documented as needing libpcap present. `pypcapfile` is sdist-only as well
but pure Python, so it installs anywhere and stays in `all`.
Also dropped `pypcap` from the Pipfile's dev-packages, where it would have broken
`pipenv install --dev` -- and `pipenv lock`, which has to build its metadata -- on
any machine without libpcap. This one included: it has no pcap.h either.
The docs already warned that `pip install pypcapkit[PyPCAP]` can fail to build
while pep.rst simultaneously claimed both extras were in `all`; that
contradiction is now resolved in favour of what the packaging actually does.
Verified: `pip install --dry-run '.[all]'` on this libpcap-less host now resolves
and would install only pypcapfile. Unit tier 438 passed, 14 skipped (the
pypcap/pypcapfile-gated ones); engines and toolkit 74 passed, 14 skipped.
|
Copilot's four typo comments were already fixed by its own commit
So the release and conda paths would have broken on merge too, and every user running
Worth noting Verified: Unrelated and left alone: |
Closes the New Engines item on the Help Wanted page — both candidates it names are now selectable, each with a matching
pcapkit.toolkitmodule, apyproject.tomlextra, docs and tests.Registration: built-in, not the public hook
Both are registered in
Extractor.__engine__asModuleDescriptors, likedpkt/scapy/pyshark. That is not merely for consistency — the public auto-registration path cannot work for an engine shipped with the library.pcapkit/__init__.pynever importspcapkit.foundation.engines, soEngine.__init_subclass__would never fire for a bareimport pcapkit;Extractor(engine='pypcap')would then fall through to "unsupported engine" and silently use the built-in parser — precisely the trap these engines exist to avoid.ModuleDescriptoralso keeps the third-party import lazy, soimport pcapkitstays free ofpcap/pcapfile. The public path remains the documented route for user engines and keeps its own coverage.Capability gaps are surfaced, not papered over
pypcapis a libpcap binding with zero dissection:run()clears the flags, replaces the managers, and warns; the four toolkit adapters raiseUnsupportedCall.datalink()==1, and yields 0 frames — a silently empty capture. So the engine gates on the magic number and raisesFormatErrorinstead.pcap_open_offlineopens by name), so non-file input raisesUnsupportedCall.follow_tcp_stream(engine='pypcap')now falls back with anEngineWarning, as it cannot trace.pypcapfiledecodes Ethernet/IPv4/TCP/UDP only:run()clears the flag and warns, andipv6_reassemblyraises rather than returningNone, which would be indistinguishable from "no fragment here".layers=0and decodes to depth 2 itself, because pypcapfile's decoders replace payload bytes; stopping at the network layer keeps the TCP segment verbatim so the header/payload split is exact rather than reconstructed. The IPv4 header is reconstructed, and that reconstruction is asserted byte-exact against real packets including options and fragment flags.Neither installs cleanly from PyPI, and
pep.rstsays sopypcapfile0.12.0 importsimp, removed in Python 3.12, so it cannot be imported at all on 3.12+. Upstream master (0.12.1, unreleased) fixes it, and that is what the engine was verified against.pypcap1.3.0 ships a pre-generatedpcap.cfrom an old Cython that no longer compiles against the 3.12+ C API (ob_digit,_PyLong_AsByteArray), and itssetup.pysearches a fixed list of prefixes forpcap.h. It builds once the headers are visible andpcap.cis regenerated with Cython 3.Both were genuinely executed here, on a locally rebuilt wheel — neither is claimed as unverified. The caveats are about installability, not coverage, and the note in
pep.rststates them rather than claiming both are simply done.Parity, and proving the engine actually ran
tests/foundation/engines/test_new_engine_parity.pyruns overin.pcap,arp.pcap,tcp.pcap,ipv4.pcap(1137 frames) and asserts, per frame: capture length, original length, timestamp, and the full Ethernet header — all exact. Every parity test goes through a helper that patcheswarn, asserts noEngineWarningwas raised, and assertsextractor._exnamis the engine requested — becauseExtractordoes not raise when an engine's package is missing, it warns and silently falls back, so a naive test would happily report parity for work the built-in parser did.pypcapfileIPv4 reassembly is additionally asserted byte-identical to the default engine.One pre-existing crash fixed, because nothing worked without it
Extractor(trace=True)died withAttributeError: 'dict' object has no attribute 'packet'. The guard atextraction.pylisted onlypysharkand only matchedtrace_format == 'pcap', whileTraceFlow.__init__substitutes'pcap'forNone— so it never fired. It now matches('pcap', 'cap', None), which incidentally fixespysharktoo.dpktandscapyremain broken there by design and are called out in a code comment; that is its own change.Also worth knowing for future tests:
BaseWarning.__init__callswarnings.simplefilter('ignore', type(self))outside devmode, soassertWarnscan never fire on a pcapkit warning. These tests patchwarnand inspect the call instead. (That behaviour is #364.)Verification
56 new test functions across 5 files. Full suite on this branch: 550 passed, 18 skipped, 335 subtests — the 18 skips are exactly the availability-gated engine tests, since the project venv deliberately does not carry either library. In a venv with both installed the same suite runs 8 more tests.