From 342c4c81c617dbea8aa828011ee1604ed3a8392d Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Wed, 16 Sep 2026 09:51:28 -0400 Subject: [PATCH] interface: add the three missing engine constants, and guard dpkt/scapy tracing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `interface/core.py` exposed constants for the engines it knew about -- `DPKT`, `Scapy`, `PCAPKit`, `PyShark` -- but not for `PyPCAP`, `PCAP_CT` or `PyPCAPFile`, so three of the seven shipped engines could only be selected by a bare string while their siblings had a name. All seven now resolve from `pcapkit`, `pcapkit.interface` and `pcapkit.all`. That required three files beyond `interface/core.py`: the constants are re-exported by explicit name, so `pcapkit.PyPCAP` would not have existed while `pcapkit.DPKT` did. Checked for shadowing -- `pcapkit.foundation` does not re-export the engine *classes*, so the string macros collide with nothing. `docs/source/pcapkit/interface/core.rst` said the newer engines were string-selected only and never mentioned `PCAP_CT` at all; it now documents all three and says only runtime-registered engines lack a constant. **The trace-format guard now covers dpkt and scapy.** Its own NOTE said they were "deliberately *not* listed here… outside the scope of this change", and that scope has passed. Their flow-tracing adapters report each frame as a plain `dict`, which the PCAP trace dumper cannot re-serialise -- it reaches for `frame.packet` and raises `AttributeError: 'dict' object has no attribute 'packet'` at `dumpkit/pcap.py:139`. `None` stays in the format tuple because `TraceFlow.__init__` substitutes `'pcap'` for it. dpkt reproduced directly. **scapy did not, and that is worth recording**: its crash was *masked* by #406 -- the engine imported only `scapy.sendrecv`, so every frame dissected as `Raw`, no TCP layer was found, and the tracer was never fed. With the layer registry loaded it crashes identically. So the guard is written from what the adapters produce rather than from which engines happen to crash today. `follow_tcp_stream`'s local workaround is **kept**, having measured both paths: they pick the same replacement format and write byte-identical trace files, but differ in when they complain. The core guard warns for every substitution including the unset default, while `follow_tcp_stream` upgrades an unset `format` silently and warns only on an explicitly unusable one -- a distinction already pinned by `test_dpkt_unset_trace_format_is_upgraded_quietly`. Removing it would make `follow_tcp_stream(engine='dpkt')` warn about a default the caller never chose, using a message that names a `trace_format=` argument the function does not expose. Its stale NOTE is corrected and a test pins the difference. Negative controls, because a constant that silently falls back is worse than a missing one: setting `PyPCAP = 'pypcap-typo'` fails both the registry-divergence test and the selection test with `'PCAP' != 'PyPCAP'`, and reverting the guard tuple fails 12 subtests plus the end-to-end test with the real `AttributeError`. Five constants get a real extraction asserting `__engine_name__`. `dumpkit/pcap.py` is unchanged -- the fix belongs in the guard -- but a dumpkit test now pins *why* (`PCAPIO` raises `AttributeError` on a mapping), so the guard's justification is checked rather than asserted in a comment. Verified: fixture-free CI 660 passed / 8 skipped against a 644/8 baseline, the delta being the new tests; all seven constants resolve to their registry keys. --- docs/source/pcapkit/interface/core.rst | 24 ++- pcapkit/__init__.py | 1 + pcapkit/all.py | 1 + pcapkit/foundation/extraction.py | 25 ++- pcapkit/interface/__init__.py | 6 +- pcapkit/interface/core.py | 12 ++ pcapkit/interface/misc.py | 22 ++- tests/dumpkit/test_common_unit.py | 21 +++ tests/foundation/test_extraction.py | 159 +++++++++++++++- tests/interface/test_core.py | 243 +++++++++++++++++++++++++ tests/interface/test_misc.py | 38 ++++ 11 files changed, 528 insertions(+), 24 deletions(-) diff --git a/docs/source/pcapkit/interface/core.rst b/docs/source/pcapkit/interface/core.rst index 8b75d0b2dd..898544b959 100644 --- a/docs/source/pcapkit/interface/core.rst +++ b/docs/source/pcapkit/interface/core.rst @@ -64,14 +64,25 @@ Extration Engines .. data:: PyShark :value: 'pyshark' +.. data:: PyPCAP + :value: 'pypcap' + +.. data:: PCAP_CT + :value: 'pcap_ct' + +.. data:: PyPCAPFile + :value: 'pypcapfile' + .. note:: - These constants predate the `PyPCAP`_ and `PyPCAPFile`_ engines and no - equivalents were added for them, so those two are selected by their literal - ``engine=`` values -- ``'pypcap'`` and ``'pypcapfile'`` -- rather than through - a named constant. Any engine registered at runtime with - :func:`~pcapkit.foundation.registry.foundation.register_extractor_engine` is - likewise addressed by its string name. + Every engine :mod:`pcapkit` ships now has a constant here. The `PyPCAP`_, + `pcap-ct`_ and `PyPCAPFile`_ ones were added after the first four, so code + written against an earlier release may still select them by their literal + ``engine=`` values -- ``'pypcap'``, ``'pcap_ct'`` and ``'pypcapfile'``. That + keeps working: each constant *is* that string, so the two spellings are + interchangeable. An engine registered at runtime with + :func:`~pcapkit.foundation.registry.foundation.register_extractor_engine` has + no constant and is addressed by the name it was registered under. .. seealso:: @@ -79,4 +90,5 @@ Extration Engines supports, and the installation prerequisites the third-party ones carry. .. _PyPCAP: https://github.com/pynetwork/pypcap +.. _pcap-ct: https://pypi.org/project/pcap-ct .. _PyPCAPFile: https://github.com/kisom/pypcapfile diff --git a/pcapkit/__init__.py b/pcapkit/__init__.py index 7c6e706b6d..e1fed082ae 100644 --- a/pcapkit/__init__.py +++ b/pcapkit/__init__.py @@ -102,6 +102,7 @@ 'TREE', 'JSON', 'PLIST', 'PCAP', # Format Macros 'LINK', 'INET', 'TRANS', 'APP', 'RAW', # Layer Macros 'DPKT', 'Scapy', 'PyShark', 'PCAPKit', # Engine Macros + 'PyPCAP', 'PCAP_CT', 'PyPCAPFile', # Engine Macros 'LINKTYPE', 'ETHERTYPE', 'TRANSTYPE', 'APPTYPE', # Protocol Numbers diff --git a/pcapkit/all.py b/pcapkit/all.py index c4c179f1ea..8f57831f12 100644 --- a/pcapkit/all.py +++ b/pcapkit/all.py @@ -102,6 +102,7 @@ 'TREE', 'JSON', 'PLIST', 'PCAP', # Format Macros 'LINK', 'INET', 'TRANS', 'APP', 'RAW', # Layer Macros 'DPKT', 'Scapy', 'PyShark', 'PCAPKit', # Engine Macros + 'PyPCAP', 'PCAP_CT', 'PyPCAPFile', # Engine Macros # pcapkit.protocols 'LINKTYPE', 'ETHERTYPE', 'TRANSTYPE', 'APPTYPE', # Protocol Numbers diff --git a/pcapkit/foundation/extraction.py b/pcapkit/foundation/extraction.py index f31a928ef0..5917dec89b 100644 --- a/pcapkit/foundation/extraction.py +++ b/pcapkit/foundation/extraction.py @@ -869,16 +869,27 @@ def __init__(self, # NOTE: these engines' flow tracing adapters report the frame as a # plain :obj:`dict`, which the PCAP trace dumper cannot re-serialise - # -- it reaches for ``frame.packet``. ``None`` has to be caught along - # with ``'pcap'`` here, and replaced by a format that *can* take a - # mapping, because :meth:`TraceFlow.__init__ + # -- :meth:`PCAPIO._append_value + # ` reaches for + # ``frame.packet`` and dies with ``AttributeError: 'dict' object has no + # attribute 'packet'``. ``None`` has to be caught along with ``'pcap'`` + # here, and replaced by a format that *can* take a mapping, because + # :meth:`TraceFlow.__init__ # ` # itself substitutes ``'pcap'`` for ``None``. # - # The DPKT and Scapy engines report the frame as a mapping too and are - # deliberately *not* listed here: they are affected by the same defect - # on this revision, but they are outside the scope of this change. - if self._exnam in ('pyshark', 'pypcapfile') and trace_format in ('pcap', 'cap', None): + # DPKT and Scapy belong on this list and were once left off it. Their + # adapters build the frame with ``packet2dict`` exactly as the other two + # do, so both crash the same way -- but only DPKT did so visibly. The + # Scapy engine imports just :mod:`scapy.sendrecv`, which leaves the L2 + # link types unregistered, so every frame dissects as ``Raw``, no TCP + # layer is ever found, and the tracer is never fed at all (#406). That + # hides this defect rather than avoiding it: register the link types -- + # as importing :mod:`scapy.all` does -- and the same ``AttributeError`` + # appears. So the guard is written from what the adapters produce, not + # from which engines happen to crash today. + if (self._exnam in ('dpkt', 'scapy', 'pyshark', 'pypcapfile') + and trace_format in ('pcap', 'cap', None)): warn(f"'Extractor(engine={self._exnam})' does not support 'trace_format={trace_format}'; " "using 'trace_format=\"json\"' instead", FormatWarning, stacklevel=stacklevel()) trace_format = 'json' diff --git a/pcapkit/interface/__init__.py b/pcapkit/interface/__init__.py index 9d9c37bc64..2f2dcda840 100644 --- a/pcapkit/interface/__init__.py +++ b/pcapkit/interface/__init__.py @@ -11,12 +11,14 @@ """ -from pcapkit.interface.core import (APP, DPKT, INET, JSON, LINK, PCAP, PLIST, RAW, TRANS, TREE, - PCAPKit, PyShark, Scapy, extract, reassemble, trace) +from pcapkit.interface.core import (APP, DPKT, INET, JSON, LINK, PCAP, PCAP_CT, PLIST, RAW, TRANS, + TREE, PCAPKit, PyPCAP, PyPCAPFile, PyShark, Scapy, extract, + reassemble, trace) __all__ = [ 'extract', 'reassemble', 'trace', # interface functions 'TREE', 'JSON', 'PLIST', 'PCAP', # format macros 'LINK', 'INET', 'TRANS', 'APP', 'RAW', # layer macros 'DPKT', 'Scapy', 'PyShark', 'PCAPKit', # engine macros + 'PyPCAP', 'PCAP_CT', 'PyPCAPFile', # engine macros ] diff --git a/pcapkit/interface/core.py b/pcapkit/interface/core.py index dbdc0213dc..d3272e1493 100644 --- a/pcapkit/interface/core.py +++ b/pcapkit/interface/core.py @@ -36,6 +36,7 @@ 'TREE', 'JSON', 'PLIST', 'PCAP', # format macros 'LINK', 'INET', 'TRANS', 'APP', 'RAW', # layer macros 'DPKT', 'Scapy', 'PyShark', 'PCAPKit', # engine macros + 'PyPCAP', 'PCAP_CT', 'PyPCAPFile', # engine macros ] # output file formats @@ -52,10 +53,21 @@ APP = 'application' # extraction engines +# +# NOTE: each value is the key the engine is registered under in +# :attr:`Extractor.__engine__ `, +# not its display name -- ``Extractor`` looks the requested engine up in that +# mapping, and an unknown key only warns and falls back to the default engine, so a +# constant whose value does not match the key would silently do nothing. The +# identifiers, by contrast, follow each engine's ``__engine_name__``, which is why +# the casing of the two sides differs. DPKT = 'dpkt' Scapy = 'scapy' PCAPKit = 'default' PyShark = 'pyshark' +PyPCAP = 'pypcap' +PCAP_CT = 'pcap_ct' +PyPCAPFile = 'pypcapfile' def extract(fin: 'Optional[str | IO[bytes]]' = None, fout: 'Optional[str]' = None, format: 'Optional[Formats]' = None, # basic settings # pylint: disable=redefined-builtin diff --git a/pcapkit/interface/misc.py b/pcapkit/interface/misc.py index ce63e96d26..e540fcce36 100644 --- a/pcapkit/interface/misc.py +++ b/pcapkit/interface/misc.py @@ -106,14 +106,20 @@ def follow_tcp_stream(fin: 'Optional[str]' = None, verbose: 'bool' = False, # :obj:`dict`\\ s, which the PCAP trace dumper cannot re-serialise -- it reaches # for ``frame.packet`` and dies with ``AttributeError: 'dict' object has no # attribute 'packet'`` (#399). The tracer defaults an unset ``format`` to - # ``'pcap'``, so following a stream through either engine crashes *during - # extraction*, before the reassembly below ever runs. :class:`Extractor - # ` already substitutes a dict-capable - # format for the PyShark and PyPCAPFile engines but deliberately leaves DPKT and - # Scapy out (see the note at its ``trace`` setup); apply the same remedy here so - # the stream is followed rather than crashed on. A caller that never asked for a - # trace format (``None``) is quietly upgraded; an explicit but unusable one is - # replaced with a warning, since it is a request that cannot be honoured. + # ``'pcap'``, so following a stream through either engine would crash *during + # extraction*, before the reassembly below ever runs. + # + # :class:`Extractor ` now guards both + # engines itself, so this is no longer what keeps the extraction alive -- it is + # what keeps it *quiet*. The two guards choose the same replacement format and so + # produce byte-identical traces; they differ only in when they complain. The + # Extractor warns for every substitution it makes, including the one nobody asked + # for, whereas here an unset ``format`` is not a request and is upgraded silently, + # and only an explicit but unusable one draws a warning -- with a message naming + # the engine's limitation rather than the ``trace_format=`` argument this function + # does not expose. Removing this therefore would not change any trace file, but it + # would make ``follow_tcp_stream(engine='dpkt')`` warn about a default the caller + # never chose. if engine is not None and engine.lower() in ('dpkt', 'scapy') and format in ('pcap', 'cap', None): if format is not None: warn(f"extraction engine {engine} cannot write '{format}' trace files; " diff --git a/tests/dumpkit/test_common_unit.py b/tests/dumpkit/test_common_unit.py index a44a85de1b..89de1be39f 100644 --- a/tests/dumpkit/test_common_unit.py +++ b/tests/dumpkit/test_common_unit.py @@ -154,6 +154,27 @@ def test_null_dumper_noops_and_pcap_dumper_writes_file(self) -> None: self.assertIs(dumper(frame), dumper) self.assertGreater(pcap_path.stat().st_size, header_size) + def test_pcap_dumper_cannot_serialise_a_mapping_frame(self) -> None: + # Why Extractor substitutes a dict-capable trace format for the DPKT, Scapy, + # PyShark and PyPCAPFile engines: their flow-tracing adapters report each + # frame as a plain dict from ``packet2dict``, and this dumper reads + # ``value.packet`` and ``value.frame_info`` off a dissected Frame. Pinned + # here so the guard's justification is checked rather than asserted in a + # comment -- if this dumper ever learns to take a mapping, the guard is what + # should be revisited. + from pcapkit.const.reg.linktype import LinkType + from pcapkit.dumpkit.pcap import PCAPIO + + with tempfile.TemporaryDirectory() as tempdir: + pcap_path = pathlib.Path(tempdir) / 'mapping.pcap' + dumper = PCAPIO(str(pcap_path), protocol=LinkType.ETHERNET, + byteorder='little', nanosecond=False) + + # The shape ``packet2dict`` produces: keys, not attributes. + with self.assertRaises(AttributeError) as caught: + dumper({'frame_info': {'ts_sec': 1}, 'packet': b'abcd'}, name='Frame 1') + self.assertIn('packet', str(caught.exception)) + if __name__ == '__main__': unittest.main() diff --git a/tests/foundation/test_extraction.py b/tests/foundation/test_extraction.py index 71142821fd..dc75f68246 100644 --- a/tests/foundation/test_extraction.py +++ b/tests/foundation/test_extraction.py @@ -7,12 +7,17 @@ import tempfile import types import unittest +import warnings from unittest import mock -from tests._support import purge_modules +from tests._support import purge_modules, sample_path RUNTIME_DEPS = ('tbtrim', 'aenum', 'chardet', 'dictdumper') HAS_RUNTIME = all(importlib.util.find_spec(name) is not None for name in RUNTIME_DEPS) +#: Whether the optional DPKT engine can be selected. It is an extra +#: (``pypcapkit[DPKT]``), absent from a plain ``[test]`` install, so the end-to-end +#: case below skips rather than fails on a fresh clone. +HAS_DPKT = importlib.util.find_spec('dpkt') is not None class ClosableBytesIO(io.BytesIO): @@ -554,6 +559,158 @@ def test_constructor_configuration_branches_with_run_patched(self) -> None: self.assertTrue(hasattr(unknown_output, '_ofile')) unknown_output._ifile.close() + def _traced(self, temp: pathlib.Path, tag: str, **kwargs: object): + """An ``Extractor`` built for tracing, with ``run`` patched out. + + Nothing is extracted, so no engine needs to be installed -- the format + substitution under test happens in the constructor. Returns the extractor + and the patched module-level ``warn``. + + """ + from pcapkit.foundation.extraction import Extractor + + with mock.patch.object(Extractor, 'run'): + with mock.patch('pcapkit.foundation.extraction.warn') as warn: + extractor = Extractor(str(temp / 'sample.pcap'), str(temp / f'out-{tag}'), + format='json', auto=False, nofile=True, trace=True, + tcp=True, trace_fout=str(temp / f'flows-{tag}'), + **kwargs) # type: ignore[arg-type] + self.addCleanup(extractor._ifile.close) + return extractor, warn + + def test_pcap_trace_format_is_replaced_for_every_dict_frame_engine(self) -> None: + # The flow-tracing adapters of these four engines report each frame as a + # plain dict, which the PCAP trace dumper cannot re-serialise: it reaches for + # ``frame.packet`` and raises AttributeError from + # pcapkit.dumpkit.pcap.PCAPIO._append_value. DPKT and Scapy were once left + # off this list even though ``packet2dict`` builds their frames exactly as it + # builds the other two's. + # + # ``None`` is asserted alongside 'pcap' and 'cap' because TraceFlow.__init__ + # substitutes 'pcap' for it, so an unset format routes to the same dumper. + # The chosen dumper is read off the tracer's own file extension rather than + # inferred from the warning: '.pcap' there means the crash is still reachable + # however loudly the constructor complained. + with tempfile.TemporaryDirectory() as tempdir: + temp = pathlib.Path(tempdir) + (temp / 'sample.pcap').write_bytes(b'\xa1\xb2\xc3\xd4payload') + + for engine in ('dpkt', 'scapy', 'pyshark', 'pypcapfile'): + for trace_format in ('pcap', 'cap', None): + with self.subTest(engine=engine, trace_format=trace_format): + tag = f'{engine}-{trace_format}' + traced, warn = self._traced(temp, tag, engine=engine, + trace_format=trace_format) + + self.assertEqual(traced._trace.tcp._fdpext, '.json') + warn.assert_called_once() + message = warn.call_args.args[0] + self.assertIn(f'engine={engine}', message) + self.assertIn(f'trace_format={trace_format}', message) + + def test_a_dict_capable_trace_format_is_left_alone(self) -> None: + # Only the formats that route to the PCAP dumper are substituted. A format + # that can already take a mapping is the caller's choice and is honoured + # silently -- otherwise every traced extraction on these engines would warn. + with tempfile.TemporaryDirectory() as tempdir: + temp = pathlib.Path(tempdir) + (temp / 'sample.pcap').write_bytes(b'\xa1\xb2\xc3\xd4payload') + + for trace_format, extension in (('json', '.json'), ('tree', '.txt'), + ('plist', '.plist')): + with self.subTest(trace_format=trace_format): + traced, warn = self._traced(temp, f'dpkt-{trace_format}', engine='dpkt', + trace_format=trace_format) + + self.assertEqual(traced._trace.tcp._fdpext, extension) + warn.assert_not_called() + + def test_an_engine_with_real_frames_keeps_the_pcap_trace_format(self) -> None: + # The guard is about the frame shape an engine's adapter produces, not about + # tracing in general: the default engine hands the tracer a dissected Frame, + # which the PCAP dumper serialises perfectly well, so 'pcap' must survive. + with tempfile.TemporaryDirectory() as tempdir: + temp = pathlib.Path(tempdir) + (temp / 'sample.pcap').write_bytes(b'\xa1\xb2\xc3\xd4payload') + + for trace_format in ('pcap', None): + with self.subTest(trace_format=trace_format): + traced, warn = self._traced(temp, f'default-{trace_format}', + trace_format=trace_format) + + self.assertEqual(traced._trace.tcp._fdpext, '.pcap') + warn.assert_not_called() + + +@unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed') +class DictFrameTraceEndToEndTests(unittest.TestCase): + """A traced extraction on a dict-frame engine must complete, not crash. + + The constructor-level cases above prove the format was substituted; this proves + the substitution is *sufficient* -- the tracer runs, the dumper is handed the + mapping and writes it. Before the guard covered DPKT, this raised + ``AttributeError: 'dict' object has no attribute 'packet'`` from + :meth:`pcapkit.dumpkit.pcap.PCAPIO._append_value`. + + What makes this reachable is that the tracer is actually fed: the dumper runs + only once a flow has been recorded, so the capture has to hold real TCP frames + and both ``trace=True`` and ``tcp=True`` have to be set. ``nofile=True`` is + orthogonal -- it suppresses the *frame* output, not the trace output, so it + neither causes nor prevents the crash. + + Only DPKT is exercised end to end. Scapy's adapter produces the same mapping and + is covered by the constructor cases, but reaching its dumper needs the L2 link + types registered -- measured: with only :mod:`scapy.sendrecv` imported, as the + engine does today, every frame of ``in.pcap`` dissects as ``Raw``, no TCP layer + is found and the tracer is never fed (#406). Importing :mod:`scapy.all` to force + it is a *global and irreversible* change to ``scapy.conf``: measured on + ``in.pcap``, frames 3-5 gain a TCP layer afterwards. That would silently + invalidate the Scapy case in :mod:`tests.interface.test_misc`, which asserts the + opposite, depending on which of the two ran first. So it is deliberately not done + here. + """ + + def setUp(self) -> None: + purge_modules(['pcapkit']) + + @unittest.skipUnless(HAS_DPKT, 'dpkt not installed') + def test_dpkt_traced_extraction_writes_flows_instead_of_crashing(self) -> None: + import pcapkit + + for trace_format in ('pcap', 'cap', None): + with self.subTest(trace_format=trace_format): + with tempfile.TemporaryDirectory() as tempdir: + flows = pathlib.Path(tempdir) / 'flows' + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + extraction = pcapkit.extract( + fin=sample_path('in.pcap'), engine='dpkt', store=True, + nofile=True, tcp=True, trace=True, trace_fout=str(flows), + trace_format=trace_format, + ) + + # The engine that ran is the one asked for -- a fallback to the + # default engine would not exercise the dict-frame path at all. + self.assertEqual(type(extraction.engine).__engine_name__, 'DPKT') + self.assertTrue(extraction.trace.tcp, + 'no TCP flow was traced, so the dumper never ran ' + 'and this test proves nothing') + + # Each traced flow named a file, and every one of them exists and + # holds the JSON the substituted format produces. + written = sorted(path for path in flows.rglob('*') if path.is_file()) + self.assertTrue(written) + for path in written: + self.assertEqual(path.suffix, '.json') + self.assertGreater(path.stat().st_size, 0) + + self.assertTrue( + any('json' in str(w.message) for w in caught + if w.category.__name__ == 'FormatWarning'), + 'the format substitution was not announced', + ) + if __name__ == '__main__': unittest.main() diff --git a/tests/interface/test_core.py b/tests/interface/test_core.py index 07784bafa0..15e09cb256 100644 --- a/tests/interface/test_core.py +++ b/tests/interface/test_core.py @@ -1,9 +1,73 @@ from __future__ import annotations +import importlib.util +import sys +import types import unittest +import warnings +from unittest import mock from tests._support import load_module, purge_modules, sample_path +RUNTIME_DEPS = ('tbtrim', 'aenum', 'chardet', 'dictdumper') +HAS_RUNTIME = all(importlib.util.find_spec(name) is not None for name in RUNTIME_DEPS) +#: Whether the optional DPKT / Scapy engines can be selected. Both are extras +#: (``pypcapkit[DPKT]`` / ``[Scapy]``), absent from a plain ``[test]`` install. +HAS_DPKT = importlib.util.find_spec('dpkt') is not None +HAS_SCAPY = importlib.util.find_spec('scapy') is not None + + +class FakeHandle: + """Stand-in for :class:`pcap.pcap`, yielding ``(timestamp, bytes)`` pairs. + + The engines construct this as ``pcap.pcap(name=..., promisc=False)``, so both + keywords have to be accepted even though neither is used. + """ + + def __init__(self, name=None, promisc=False) -> None: + self._iter = iter([(1.5, b'payload')]) + + def datalink(self) -> int: + return 1 # LINKTYPE_ETHERNET + + def __iter__(self): + return self + + def __next__(self): + return next(self._iter) + + def close(self) -> None: + pass + + +class FakePacket: + """Stand-in for :class:`pcapfile.structs.pcap_packet`.""" + + def __init__(self, header, timestamp, timestamp_us, capture_len, packet_len, packet) -> None: + self.header = header + self.timestamp = timestamp + self.timestamp_us = timestamp_us + self.capture_len = capture_len + self.packet_len = packet_len + self.packet = packet + + +class FakeSaveFile: + """Stand-in for :class:`pcapfile.savefile.pcap_savefile`.""" + + def __init__(self) -> None: + self.header = types.SimpleNamespace(ll_type=1, ns_resolution=False) + self.packets = [FakePacket(None, 1, 500000, 7, 7, b'payload')] + + +class FakeDecoded: + """Stand-in for a decoded link layer frame.""" + + def __init__(self, packet, layers=0) -> None: + self.raw = packet + self.layers = layers + self.payload = b'decoded-payload' + class InterfaceCoreTests(unittest.TestCase): def setUp(self) -> None: @@ -122,5 +186,184 @@ def test_unsupported_protocols_raise_format_error(self) -> None: module.trace('UDP', fout=None, format=None) +@unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed') +class EngineConstantTests(unittest.TestCase): + """The ``engine=`` constants, and whether each selects the engine it names. + + The constants are only worth having if they *work*, and a wrong one fails + quietly: :meth:`Extractor.run ` + answers an unknown engine name with an :class:`EngineWarning + ` and the default engine, so an + extraction driven by a misspelled constant still succeeds and still returns + frames. Every case here therefore asserts on the engine that actually ran. + """ + + #: Constant name in :mod:`pcapkit.interface.core` -> the ``__engine_name__`` + #: of the engine it has to select. + ENGINES = { + 'PCAPKit': 'PCAP', + 'DPKT': 'DPKT', + 'Scapy': 'Scapy', + 'PyShark': 'PyShark', + 'PyPCAP': 'PyPCAP', + 'PCAP_CT': 'PCAP_CT', + 'PyPCAPFile': 'PyPCAPFile', + } + + def setUp(self) -> None: + purge_modules(['pcapkit']) + + def test_every_shipped_engine_has_a_constant(self) -> None: + # The guard against the next engine landing without one. ``'default'`` is the + # pcapkit-native engine, which Extractor.run handles directly rather than + # looking up, so it is not a key in the registry; ``'pcapkit'`` is an accepted + # alias for it and deliberately has no constant of its own. + from pcapkit.foundation.extraction import Extractor + from pcapkit.interface import core + + declared = {} + for name in self.ENGINES: + self.assertIn(name, core.__all__, + f'{name} is not exported from pcapkit.interface.core') + declared[name] = getattr(core, name) + + self.assertEqual(set(declared.values()), set(Extractor.__engine__) | {'default'}, + 'the engine constants and the engine registry have diverged') + + def test_constants_are_reachable_from_the_package_root(self) -> None: + # ``pcapkit.DPKT`` has always worked, so ``pcapkit.PyPCAP`` has to as well -- + # a constant that exists only in the submodule is a constant nobody finds. + # Both re-export lists name their symbols explicitly, so neither picks a new + # one up on its own. + import pcapkit + from pcapkit.interface import core + + for name in self.ENGINES: + with self.subTest(constant=name): + for module in (pcapkit, pcapkit.interface): + self.assertIn(name, module.__all__, + f'{name} missing from {module.__name__}.__all__') + self.assertEqual(getattr(module, name), getattr(core, name)) + + def _extract(self, engine: str): + """A real extraction of the committed capture, driven by ``engine``.""" + import pcapkit + + with warnings.catch_warnings(): + # An engine that cannot run warns and falls back; the caller asserts on + # which engine ran, so the warning itself is noise here. + warnings.simplefilter('ignore') + return pcapkit.extract(fin=sample_path('in.pcap'), engine=engine, + store=True, nofile=True) + + def _assert_selects(self, constant: str) -> None: + from pcapkit.interface import core + + extraction = self._extract(getattr(core, constant)) + self.assertEqual(type(extraction.engine).__engine_name__, self.ENGINES[constant]) + + def test_pcapkit_constant_selects_the_native_engine(self) -> None: + # ``PCAPKit`` is 'default', which resolves by magic number rather than by + # registry lookup -- in.pcap is a PCAP savefile, hence the 'PCAP' engine. + self._assert_selects('PCAPKit') + + @unittest.skipUnless(HAS_DPKT, 'dpkt not installed') + def test_dpkt_constant_selects_the_dpkt_engine(self) -> None: + self._assert_selects('DPKT') + + @unittest.skipUnless(HAS_SCAPY, 'scapy not installed') + def test_scapy_constant_selects_the_scapy_engine(self) -> None: + self._assert_selects('Scapy') + + # NOTE: ``PyShark`` has no selection case of its own. It is covered by the two + # tests above, but running it needs the :program:`tshark` *binary* as well as the + # :mod:`pyshark` package, and the engine drives it as a subprocess -- so a + # stand-in would be a stand-in for the whole extraction rather than for a module, + # and would stop testing the routing this class is about. Its constant predates + # this change and its value is asserted against the registry above, which is what + # a wrong constant would break. + + def _fake_pcap_module(self, *, is_pcap_ct: bool): + """A stand-in :mod:`pcap` module, as one distribution or the other. + + ``pcap-ct`` ships :mod:`pcap` as a package whose ``__init__`` does + ``from ._pcap import *``, so the submodule ends up bound as an attribute; + upstream ``pypcap`` ships a single extension module with no such attribute. + That difference is what + :func:`pcapkit.foundation.engines._pcap_backend.identify` keys on, so the + stand-in only has to reproduce it. + + """ + module = types.ModuleType('pcap') + module.pcap = FakeHandle # type: ignore[attr-defined] + module.__version__ = '1.3.0b3' if is_pcap_ct else '1.3.0' # type: ignore[attr-defined] + module.__file__ = ('/stub/site-packages/pcap/__init__.py' if is_pcap_ct + else '/stub/site-packages/pcap.cpython-310.so') + modules = {'pcap': module} + if is_pcap_ct: + submodule = types.ModuleType('pcap._pcap') + module._pcap = submodule # type: ignore[attr-defined] + # ``PCAP_CT.__engine_module__`` is 'pcap._pcap', so the import test + # resolves that name rather than 'pcap'. + modules['pcap._pcap'] = submodule + return modules + + def _assert_selects_with_stub_pcap(self, constant: str, *, distribution: str, + is_pcap_ct: bool) -> None: + """Select a ``pcap``-backed engine against a stand-in for :mod:`pcap`. + + Neither ``pypcap`` nor ``pcap-ct`` is installable everywhere -- upstream + needs a compiler and 3.11 or older, ``pcap-ct`` needs a system + :manpage:`libpcap(3)` -- so the routing is asserted against a stand-in. + The metadata lookup is patched alongside the module because the engines + decide *which* distribution owns :mod:`pcap` from both. + + """ + from pcapkit.foundation.engines import _pcap_backend + + with mock.patch.dict(sys.modules, self._fake_pcap_module(is_pcap_ct=is_pcap_ct)): + with mock.patch.object(_pcap_backend, 'installed_distributions', + return_value=(distribution,)): + self._assert_selects(constant) + + def test_pypcap_constant_selects_the_pypcap_engine(self) -> None: + self._assert_selects_with_stub_pcap('PyPCAP', distribution='pypcap', + is_pcap_ct=False) + + def test_pcap_ct_constant_selects_the_pcap_ct_engine(self) -> None: + self._assert_selects_with_stub_pcap('PCAP_CT', distribution='pcap-ct', + is_pcap_ct=True) + + def test_pypcapfile_constant_selects_the_pypcapfile_engine(self) -> None: + # ``pypcapfile`` 0.12.0 cannot run on Python 3.12+ at all -- its linklayer + # module imports ``imp`` -- and PyPCAPFile.unsupported_reason refuses the + # engine there, so the ceiling is lifted for the duration of the routing + # check. Raising it is what keeps this test meaningful on a modern + # interpreter; the ceiling itself has its own coverage in + # tests/foundation/engines/test_pypcapfile_engine.py. + from pcapkit.foundation.engines.pypcapfile import PyPCAPFile + + package = types.ModuleType('pcapfile') + savefile = types.ModuleType('pcapfile.savefile') + savefile.load_savefile = lambda *a, **kw: FakeSaveFile() # type: ignore[attr-defined] + linklayer = types.ModuleType('pcapfile.linklayer') + linklayer.clookup = lambda linktype: FakeDecoded # type: ignore[attr-defined] + structs = types.ModuleType('pcapfile.structs') + structs.pcap_packet = FakePacket # type: ignore[attr-defined] + + # Both bindings are needed: ``import pcapfile.savefile`` is satisfied by + # sys.modules, but the engine then reaches the submodules as *attributes* + # of the package it stored in ``_expkg``. + package.savefile = savefile # type: ignore[attr-defined] + package.linklayer = linklayer # type: ignore[attr-defined] + package.structs = structs # type: ignore[attr-defined] + + modules = {'pcapfile': package, 'pcapfile.savefile': savefile, + 'pcapfile.linklayer': linklayer, 'pcapfile.structs': structs} + with mock.patch.dict(sys.modules, modules): + with mock.patch.object(PyPCAPFile, 'PYTHON_CEILING', (99, 0)): + self._assert_selects('PyPCAPFile') + + if __name__ == '__main__': unittest.main() diff --git a/tests/interface/test_misc.py b/tests/interface/test_misc.py index 59c8e363e8..b8f7e80416 100644 --- a/tests/interface/test_misc.py +++ b/tests/interface/test_misc.py @@ -203,6 +203,44 @@ def test_dpkt_unset_trace_format_is_upgraded_quietly(self) -> None: self.assertFalse(any(issubclass(w.category, FormatWarning) for w in caught)) self.assertEqual(len(streams), IN_PCAP_TCP_STREAMS) + @unittest.skipUnless(HAS_DPKT, 'dpkt not installed') + def test_the_local_format_upgrade_is_not_redundant(self) -> None: + # Extractor now guards DPKT and Scapy itself, which makes the local upgrade + # in follow_tcp_stream look removable. It is not, and this pins the reason so + # that deleting it fails here rather than only changing what users see. + # + # The two guards pick the same replacement format, so the traces are + # identical either way; they differ in when they complain. The Extractor + # announces every substitution, including for an unset format -- which + # follow_tcp_stream does not even expose, since its argument is ``format``. + # Here an unset format is upgraded silently. Remove the local guard and + # ``follow_tcp_stream(engine='dpkt')`` starts warning about a default the + # caller never chose. + import pcapkit + from pcapkit.utilities.warnings import FormatWarning + + def format_warnings(caught): + return [str(w.message) for w in caught if issubclass(w.category, FormatWarning)] + + with warnings.catch_warnings(record=True) as local: + warnings.simplefilter('always') + streams = self._follow(engine='dpkt', format=None) + + with warnings.catch_warnings(record=True) as core: + warnings.simplefilter('always') + extraction = pcapkit.extract(fin=sample_path('in.pcap'), engine='dpkt', + store=True, nofile=True, tcp=True, trace=True, + trace_fout=self.tmp_dir, trace_format=None) + + # Same work done either way ... + self.assertEqual(len(streams), IN_PCAP_TCP_STREAMS) + self.assertEqual(len(extraction.trace.tcp), IN_PCAP_TCP_STREAMS) + + # ... and the difference is only in the reporting. + self.assertEqual(format_warnings(local), []) + self.assertEqual(len(format_warnings(core)), 1) + self.assertIn('trace_format=None', format_warnings(core)[0]) + def test_engine_without_a_reassembly_adapter_returns_no_streams(self) -> None: # An engine pcapkit ships no reassembly adapter for -- a third-party one, or # a built-in that grows dict frames -- must not be silently routed to the