diff --git a/pcapkit/foundation/extraction.py b/pcapkit/foundation/extraction.py index a87728e4cc..11a19a7f29 100644 --- a/pcapkit/foundation/extraction.py +++ b/pcapkit/foundation/extraction.py @@ -60,7 +60,13 @@ from pcapkit.protocols.misc.pcapng import PCAPNG from pcapkit.protocols.protocol import ProtocolBase as Protocol - Formats = Literal['pcap', 'json', 'tree', 'plist'] + #: Every key registered in :attr:`Extractor.__output__` and in + #: :attr:`TraceFlowBase.__output__ + #: ` -- the two + #: registries expose the same eight keys. This used to name only four of them, + #: which made ``'cap'`` and the ``'txt'``/``'xml'`` aliases unspellable for a + #: type checker even though every one of them is accepted at runtime. + Formats = Literal['pcap', 'cap', 'json', 'tree', 'text', 'txt', 'plist', 'xml'] # NOTE: this alias is duplicated verbatim in ``pcapkit.interface.misc``; both # copies need updating when a new engine lands. The duplication predates the # engines added here and is left as-is on purpose. diff --git a/pcapkit/interface/misc.py b/pcapkit/interface/misc.py index f827fad7e7..8f3281253f 100644 --- a/pcapkit/interface/misc.py +++ b/pcapkit/interface/misc.py @@ -10,23 +10,41 @@ """ import sys -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, cast from pcapkit.corekit.infoclass import Info, info_final +from pcapkit.foundation.engines.dpkt import DPKT as DPKT_Engine +from pcapkit.foundation.engines.pcap import PCAP as PCAP_Engine +from pcapkit.foundation.engines.pcapng import PCAPNG as PCAPNG_Engine +from pcapkit.foundation.engines.pypcapfile import PyPCAPFile as PyPCAPFile_Engine +from pcapkit.foundation.engines.scapy import Scapy as Scapy_Engine from pcapkit.foundation.extraction import Extractor from pcapkit.foundation.reassembly.tcp import TCP as TCP_Reassembly from pcapkit.utilities.exceptions import stacklevel -from pcapkit.utilities.warnings import EngineWarning, warn +from pcapkit.utilities.warnings import EngineWarning, FormatWarning, warn if TYPE_CHECKING: - from typing import Optional + from typing import Callable, Optional from typing_extensions import Literal from pcapkit.foundation.extraction import Packet + from pcapkit.foundation.reassembly.data.tcp import Packet as TCP_Data + + #: A toolkit's ``tcp_reassembly`` adapter. The pcapkit-native adapters take only + #: the frame; the third-party ones also accept a ``count`` keyword -- hence the + #: open ``...`` parameter list, which lets both call shapes type-check. + ReassemblyAdapter = Callable[..., 'Optional[TCP_Data]'] ByteOrder = Literal['little', 'big'] - Formats = Literal['pcap', 'json', 'tree', 'plist'] + #: Every key registered in :attr:`Extractor.__output__ + #: ` and in + #: :attr:`TraceFlowBase.__output__ + #: ` -- the two + #: registries expose the same eight keys. This used to name only four of them, + #: which made ``'cap'`` and the ``'txt'``/``'xml'`` aliases unspellable for a + #: type checker even though every one of them is accepted at runtime. + Formats = Literal['pcap', 'cap', 'json', 'tree', 'text', 'txt', 'plist', 'xml'] # NOTE: this alias duplicates the one in ``pcapkit.foundation.extraction``; # both copies need updating when a new engine lands. Engines = Literal['default', 'pcapkit', 'dpkt', 'scapy', 'pyshark', 'pypcap', 'pypcapfile'] @@ -83,20 +101,70 @@ def follow_tcp_stream(fin: 'Optional[str]' = None, verbose: 'bool' = False, EngineWarning, stacklevel=stacklevel()) engine = None + # NOTE: the DPKT and Scapy engines hand their frames to the flow tracer as plain + # :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. + 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; " + "using 'json' instead", FormatWarning, stacklevel=stacklevel()) + format = 'json' + extraction = Extractor(fin=fin, fout=None, format=None, auto=True, extension=extension, store=True, files=False, nofile=True, verbose=verbose, engine=engine, layer=None, protocol=None, ip=False, ipv4=False, ipv6=False, tcp=True, reassembly=False, trace=True, trace_fout=fout, trace_format=format, trace_byteorder=byteorder, trace_nanosecond=nanosecond) # type: ignore[var-annotated] - fallback = False - if extraction.engine == 'dpkt': # type: ignore[comparison-overlap] - from pcapkit.toolkit.dpkt import tcp_reassembly # pylint: disable=import-outside-toplevel - elif extraction.engine == 'scapy': # type: ignore[comparison-overlap] - from pcapkit.toolkit.scapy import tcp_reassembly # isort: skip # pylint: disable=import-outside-toplevel + # NOTE: ``Extractor.engine`` returns the running engine *instance* (see + # :meth:`Extractor.engine `), + # never its name -- so the historical ``extraction.engine == 'dpkt'`` compared an + # object against a string and was *always* :data:`False`. Every capture then fell + # through to the pcapkit adapter, which crashed on DPKT frames and silently + # returned no streams on Scapy frames (#399). Dispatch on the engine *type* + # instead, and via :func:`isinstance` so that a third-party engine subclassing a + # built-in still reaches the adapter that matches its frames. + # + # The adapter and its call convention are chosen together: the pcapkit-native + # adapters (:mod:`~pcapkit.toolkit.pcap` and :mod:`~pcapkit.toolkit.pcapng`) read + # the frame number straight off the dissected frame and accept no ``count``, + # whereas the third-party adapters have no such number and must be handed the + # frame index as ``count`` -- so the two forms are not interchangeable. + exeng = extraction.engine + tcp_reassembly = None # type: Optional[ReassemblyAdapter] + pass_count = True + if isinstance(exeng, PCAP_Engine): + from pcapkit.toolkit import pcap as tk_pcap # isort: skip # pylint: disable=import-outside-toplevel + tcp_reassembly, pass_count = cast('ReassemblyAdapter', tk_pcap.tcp_reassembly), False + elif isinstance(exeng, PCAPNG_Engine): + from pcapkit.toolkit import pcapng as tk_pcapng # isort: skip # pylint: disable=import-outside-toplevel + tcp_reassembly, pass_count = cast('ReassemblyAdapter', tk_pcapng.tcp_reassembly), False + elif isinstance(exeng, DPKT_Engine): + from pcapkit.toolkit import dpkt as tk_dpkt # isort: skip # pylint: disable=import-outside-toplevel + tcp_reassembly = cast('ReassemblyAdapter', tk_dpkt.tcp_reassembly) + elif isinstance(exeng, Scapy_Engine): + from pcapkit.toolkit import scapy as tk_scapy # isort: skip # pylint: disable=import-outside-toplevel + tcp_reassembly = cast('ReassemblyAdapter', tk_scapy.tcp_reassembly) + elif isinstance(exeng, PyPCAPFile_Engine): + from pcapkit.toolkit import pypcapfile as tk_pypcapfile # isort: skip # pylint: disable=import-outside-toplevel + tcp_reassembly = cast('ReassemblyAdapter', tk_pypcapfile.tcp_reassembly) else: - from pcapkit.toolkit.pcap import tcp_reassembly # type: ignore[assignment] # isort: skip # pylint: disable=import-outside-toplevel - fallback = True + # A third-party engine pcapkit ships no reassembly adapter for. Falling back + # to the pcapkit adapter is exactly the #399 failure mode -- a wrong or empty + # result indistinguishable from a real one -- so warn and return no streams + # rather than reassemble frames whose shape we cannot parse. + warn(f'unsupported extraction engine for TCP stream following: {exeng.name}; ' + 'returning no streams', EngineWarning, stacklevel=stacklevel()) + return () streams = [] # type: list[Stream] frames = extraction.frame @@ -108,10 +176,10 @@ def follow_tcp_stream(fin: 'Optional[str]' = None, verbose: 'bool' = False, frame = frames[index-1] packets.append(frame) - if fallback: - data = tcp_reassembly(frame) - else: + if pass_count: data = tcp_reassembly(frame, count=index) + else: + data = tcp_reassembly(frame) if data is not None: reassembly(data) diff --git a/tests/interface/test_misc.py b/tests/interface/test_misc.py index 60c9b9adbe..7f940ed824 100644 --- a/tests/interface/test_misc.py +++ b/tests/interface/test_misc.py @@ -1,131 +1,183 @@ from __future__ import annotations -import pathlib -import sys +import importlib.util +import tempfile import types import unittest +import warnings from unittest import mock -from tests._support import ROOT, ensure_package, load_module, purge_modules +from tests._support import purge_modules, sample_path + +#: Packages :mod:`pcapkit` needs before it can parse anything at all; the default +#: engine and therefore every test here depends on them. They are core install +#: dependencies, so this gate only ever skips on a deliberately minimal build. +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 optional +#: extras (``pypcapkit[DPKT]`` / ``[Scapy]``), absent from a plain ``[test]`` +#: install, so the engine-specific cases skip rather than fail on a fresh clone. +HAS_DPKT = importlib.util.find_spec('dpkt') is not None +HAS_SCAPY = importlib.util.find_spec('scapy') is not None + +#: TCP conversations the default (pcapkit-native) engine finds in the committed +#: ``in.pcap`` capture. Measured against ``examples/captures/in.pcap``, and the +#: yardstick every other engine is judged by -- see the class docstring for why +#: the per-engine expected values are *not* uniformly this number. +IN_PCAP_TCP_STREAMS = 3 + + +@unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed') +class FollowTCPStreamTests(unittest.TestCase): + """:func:`~pcapkit.interface.misc.follow_tcp_stream` per extraction engine. + + ``examples/captures/in.pcap`` (committed, so unit-tier safe) holds three TCP + conversations. #399: the reassembly adapter was chosen by comparing the + engine *instance* to a string -- a comparison that is never true -- so every + engine silently used the pcapkit adapter regardless of which engine ran. That + crashed on DPKT frames (``AttributeError: 'dict' object has no attribute + 'packet'``) and returned an empty, misleading result on Scapy frames. + + The expected stream count is asserted per engine rather than as one shared + number, because engines with genuine dissection gaps legitimately differ: + DPKT dissects this capture fully and must match the default engine, whereas + Scapy cannot read its link layer at all and correctly finds nothing. + """ - -class InterfaceMiscTests(unittest.TestCase): def setUp(self) -> None: purge_modules(['pcapkit']) - - def _load_module(self, *, extractor_engine: str = 'default', - stream_indexes: tuple[int, ...] = (2, 1)): - ensure_package('pcapkit', ROOT / 'pcapkit') - ensure_package('pcapkit.foundation', ROOT / 'pcapkit' / 'foundation') - ensure_package('pcapkit.foundation.reassembly', - ROOT / 'pcapkit' / 'foundation' / 'reassembly') - ensure_package('pcapkit.toolkit', ROOT / 'pcapkit' / 'toolkit') - - calls = { - 'pcap': [], - 'dpkt': [], - 'scapy': [], - 'extractor': [], - } - frames = [ - types.SimpleNamespace(index=1, payload=b'one'), - types.SimpleNamespace(index=2, payload=b'two'), - ] - - extraction_module = types.ModuleType('pcapkit.foundation.extraction') - - class Extractor: - def __init__(self, **kwargs): - calls['extractor'].append(kwargs) - self.engine = extractor_engine - self.frame = frames - self.trace = types.SimpleNamespace(tcp=[ - types.SimpleNamespace(index=stream_indexes, fpout='stream.bin'), - ]) - - extraction_module.Extractor = Extractor - sys.modules['pcapkit.foundation.extraction'] = extraction_module - - tcp_module = types.ModuleType('pcapkit.foundation.reassembly.tcp') - - class TCP: - def __init__(self, strict=False): - self.strict = strict - self.datagram = [] - - def __call__(self, packet): - self.datagram.append(types.SimpleNamespace(index=packet.index, - payload=packet.payload)) - - tcp_module.TCP = TCP - sys.modules['pcapkit.foundation.reassembly.tcp'] = tcp_module - - def install_toolkit(name: str, *, uses_count: bool) -> None: - toolkit_module = types.ModuleType(f'pcapkit.toolkit.{name}') - - def tcp_reassembly(frame, *, count=None): - calls[name].append((frame, count)) - if frame.payload == b'two' and count == 2: - return None - index = count if uses_count else frame.index - return types.SimpleNamespace(index=index, payload=frame.payload) - - toolkit_module.tcp_reassembly = tcp_reassembly - sys.modules[f'pcapkit.toolkit.{name}'] = toolkit_module - - install_toolkit('pcap', uses_count=False) - install_toolkit('dpkt', uses_count=True) - install_toolkit('scapy', uses_count=True) - - module = load_module('pcapkit.interface.misc', 'pcapkit/interface/misc.py') - return module, calls - - def test_follow_tcp_stream_falls_back_for_pyshark_and_uses_pcap_helper(self) -> None: - module, calls = self._load_module(extractor_engine='default') - - with mock.patch.object(module, 'warn') as warn: - streams = module.follow_tcp_stream( - fin='in.pcap', - verbose=True, - extension=False, - engine='pyshark', - fout='trace.pcap', - format='pcap', - byteorder='little', - nanosecond=True, - ) - - warn.assert_called_once() - self.assertEqual(calls['extractor'][0]['engine'], None) - self.assertTrue(calls['extractor'][0]['trace']) - self.assertTrue(calls['extractor'][0]['tcp']) - self.assertEqual(calls['extractor'][0]['trace_fout'], 'trace.pcap') - self.assertEqual(calls['extractor'][0]['trace_format'], 'pcap') - self.assertEqual(len(calls['pcap']), 2) - self.assertEqual(calls['pcap'][0][1], None) - self.assertEqual(streams[0].filename, 'stream.bin') - self.assertEqual(streams[0].packets[0].payload, b'two') - self.assertEqual(streams[0].conversations, (b'one', b'two')) - - def test_follow_tcp_stream_uses_dpkt_and_scapy_counted_helpers(self) -> None: - module, calls = self._load_module(extractor_engine='dpkt') - streams = module.follow_tcp_stream(engine='dpkt') - self.assertEqual(calls['extractor'][0]['engine'], 'dpkt') - self.assertEqual(calls['dpkt'], [ - (streams[0].packets[0], 2), - (streams[0].packets[1], 1), - ]) - self.assertEqual(streams[0].conversations, (b'one',)) - - purge_modules(['pcapkit']) - module, calls = self._load_module(extractor_engine='scapy') - streams = module.follow_tcp_stream(engine='scapy') - self.assertEqual(calls['extractor'][0]['engine'], 'scapy') - self.assertEqual(calls['scapy'], [ - (streams[0].packets[0], 2), - (streams[0].packets[1], 1), - ]) - self.assertEqual(streams[0].conversations, (b'one',)) + # follow_tcp_stream always drives the flow tracer, whose output root + # defaults to './tmp' under the working directory when ``fout`` is unset + # (TraceFlow.__init__). Point it at a scratch directory so a test run + # writes nothing into the tree. + tmp = tempfile.TemporaryDirectory(prefix='pcapkit-follow-') + self.addCleanup(tmp.cleanup) + self.tmp_dir = tmp.name + + def _follow(self, **kwargs: object) -> tuple: + from pcapkit.interface.misc import follow_tcp_stream + + kwargs.setdefault('fout', self.tmp_dir) + return follow_tcp_stream(fin=sample_path('in.pcap'), **kwargs) # type: ignore[arg-type] + + def test_default_engine_finds_every_tcp_stream(self) -> None: + streams = self._follow() + self.assertEqual(len(streams), IN_PCAP_TCP_STREAMS) + # Each detected flow carries the frames traceflow grouped into it, and names + # the file its trace was written to. in.pcap's three TCP flows are one frame + # each and single-segment, so there is no multi-segment payload to + # reassemble -- the empty conversations are a property of this capture, not + # of the reassembly, which is why the parity test below compares them rather + # than requiring them non-empty. + for stream in streams: + self.assertGreaterEqual(len(stream.packets), 1) + self.assertIsNotNone(stream.filename) + + @unittest.skipUnless(HAS_DPKT, 'dpkt not installed') + def test_dpkt_engine_matches_the_default_engine(self) -> None: + # DPKT dissects in.pcap's Ethernet/IP/TCP in full, so it must agree with the + # native engine on both the stream count and the reassembled bytes. This is + # the regression guard for #399: before the fix the DPKT frame reached the + # pcapkit adapter and raised AttributeError, and the counted-vs-uncounted + # call convention is what makes the reassembled numbering line up. + with warnings.catch_warnings(): + warnings.simplefilter('ignore') + native = self._follow() + foreign = self._follow(engine='dpkt') + + self.assertEqual(len(foreign), IN_PCAP_TCP_STREAMS) + self.assertEqual([len(stream.packets) for stream in foreign], + [len(stream.packets) for stream in native]) + self.assertEqual([stream.conversations for stream in foreign], + [stream.conversations for stream in native]) + + @unittest.skipUnless(HAS_SCAPY, 'scapy not installed') + def test_scapy_engine_finds_no_streams_for_this_capture(self) -> None: + # Scapy does not recognise this capture's link-layer type and hands every + # frame back as one opaque ``Raw`` layer with no TCP inside, so it correctly + # finds no TCP stream to follow. Zero is therefore a capability gap, not the + # #399 bug -- and the second assertion pins the *cause*, so a future scapy + # that learns this link type fails here loudly instead of silently drifting. + import pcapkit + + with warnings.catch_warnings(): + warnings.simplefilter('ignore') + streams = self._follow(engine='scapy') + extractor = pcapkit.extract(fin=sample_path('in.pcap'), engine='scapy', + store=True, nofile=True) + + self.assertEqual(len(streams), 0) + self.assertFalse( + any(frame.haslayer('TCP') for frame in extractor.frame), + 'scapy dissected a TCP layer from in.pcap; the empty-stream assertion ' + 'above is no longer a capability gap and this test needs revisiting', + ) + + def test_pyshark_and_pypcap_fall_back_to_the_default_engine(self) -> None: + # Neither engine can trace TCP flows (PyShark has no reassembly adapter, + # PyPCAP does no dissection), so follow_tcp_stream redirects them to the + # default engine with an EngineWarning rather than raising. No PyShark or + # PyPCAP install is needed: the redirect happens before extraction begins. + from pcapkit.utilities.warnings import EngineWarning + + for engine in ('pyshark', 'pypcap'): + with self.subTest(engine=engine): + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + streams = self._follow(engine=engine) + self.assertTrue(any(issubclass(w.category, EngineWarning) for w in caught)) + self.assertEqual(len(streams), IN_PCAP_TCP_STREAMS) + + @unittest.skipUnless(HAS_DPKT, 'dpkt not installed') + def test_dpkt_explicit_pcap_trace_format_is_downgraded_with_a_warning(self) -> None: + # The PCAP trace dumper cannot serialise DPKT's dict frames, so an explicit + # 'pcap' trace format is replaced with a dict-capable one, with a + # FormatWarning -- and the stream is still followed rather than crashing the + # extraction the way it did before #399 was fixed. + from pcapkit.utilities.warnings import FormatWarning + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + streams = self._follow(engine='dpkt', format='pcap') + + self.assertTrue(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_dpkt_unset_trace_format_is_upgraded_quietly(self) -> None: + # An unset trace format is not a request, so upgrading it to a dict-capable + # format for DPKT does not warn -- only an explicit, unusable one does. + from pcapkit.utilities.warnings import FormatWarning + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + streams = self._follow(engine='dpkt') + + self.assertFalse(any(issubclass(w.category, FormatWarning) for w in caught)) + self.assertEqual(len(streams), IN_PCAP_TCP_STREAMS) + + 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 + # pcapkit adapter, which is precisely the #399 failure mode. It warns and + # returns nothing. Extractor is stubbed so the branch can be reached without + # registering a real engine. + from pcapkit.interface import misc + from pcapkit.utilities.warnings import EngineWarning + + class _FakeExtractor: + def __init__(self, **kwargs: object) -> None: + self.engine = types.SimpleNamespace(name='ThirdParty') + self.frame = () # type: tuple + self.trace = types.SimpleNamespace(tcp=[]) + + with mock.patch.object(misc, 'Extractor', _FakeExtractor): + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + streams = misc.follow_tcp_stream(fin='ignored.pcap') + + self.assertEqual(streams, ()) + self.assertTrue(any(issubclass(w.category, EngineWarning) for w in caught)) if __name__ == '__main__': diff --git a/tests/utilities/test_quiet_exceptions.py b/tests/utilities/test_quiet_exceptions.py index e72ae6d70e..e130b156ff 100644 --- a/tests/utilities/test_quiet_exceptions.py +++ b/tests/utilities/test_quiet_exceptions.py @@ -1,4 +1,4 @@ -"""Regression tests for GH-362 -- ``quiet=True`` must mean *emit nothing*. +"""Regression tests for #362 -- ``quiet=True`` must mean *emit nothing*. ``BaseError.__init__`` used to treat ``quiet`` as "log at ``ERROR`` instead of ``CRITICAL``", so every absent-key lookup through diff --git a/tests/utilities/test_warning_emission.py b/tests/utilities/test_warning_emission.py index 119fb33eab..1dc38c8b97 100644 --- a/tests/utilities/test_warning_emission.py +++ b/tests/utilities/test_warning_emission.py @@ -1,4 +1,4 @@ -"""Regression tests for GH-363 -- one :func:`~pcapkit.utilities.warnings.warn` +"""Regression tests for #363 -- one :func:`~pcapkit.utilities.warnings.warn` call must produce exactly one record on each channel. The emission model, which these tests pin: @@ -13,7 +13,7 @@ Before the fix the counts were: 1 outside development mode (the :mod:`warnings` emission was swallowed by the filter the constructor installed -- -see GH-364) and 3 in development mode (two log records, from ``warn()`` and again +see #364) and 3 in development mode (two log records, from ``warn()`` and again from ``BaseWarning.__init__``, plus the :mod:`warnings` emission). """ diff --git a/tests/utilities/test_warning_filters.py b/tests/utilities/test_warning_filters.py index efcd5fb510..c19e9e8cff 100644 --- a/tests/utilities/test_warning_filters.py +++ b/tests/utilities/test_warning_filters.py @@ -1,4 +1,4 @@ -"""Regression tests for GH-364 -- constructing a pcapkit warning must not touch +"""Regression tests for #364 -- constructing a pcapkit warning must not touch the process-global :data:`warnings.filters`. ``BaseWarning.__init__`` used to call ``warnings.simplefilter('ignore',