interface: add the three missing engine constants, and guard dpkt/scapy tracing

docs/source/pcapkit/interface/core.rst

@@ -64,19 +64,31 @@ 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::
 
       :doc:`../foundation/engines/index` for the full engine list, what each one
       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

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
 

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

pcapkit/foundation/extraction.py

@@ -870,16 +870,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
+            # <pcapkit.dumpkit.pcap.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__
             # <pcapkit.foundation.traceflow.traceflow.TraceFlowBase.__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'

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
 ]

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__ <pcapkit.foundation.extraction.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

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
-    # <pcapkit.foundation.extraction.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 <pcapkit.foundation.extraction.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; "

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()

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()