From fe72cfa03409c0654a5b153afdd3ae639ca03df6 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Fri, 23 Oct 2026 22:21:42 -0400 Subject: [PATCH] fix(foundation): let the pypcap engine fall back when the pcap module cannot load (#2132) - Add _pcap_backend.engine_reason(): the wrong-backend verdict plus the installed-but-unusable one PCAP_CT already gave. - PyPCAP._reason_for and PCAP_CT._reason_for both return it, so extract(engine='pypcap') now warns and falls back to the default engine instead of raising the loader's OSError, and the two cannot drift again. - Document the new verdict in PyPCAP's docstrings and the engines docs. Tests: tests/foundation/engines/test_pypcap_unusable_module_2132_unit.py, with a fake pcap package raising OSError in a child interpreter; fails on origin/main, passes here, foundation leg green. --- .../pcapkit/foundation/engines/3rdparty.rst | 8 + .../pcapkit/foundation/engines/index.rst | 4 +- pcapkit/foundation/engines/_pcap_backend.py | 36 +++- pcapkit/foundation/engines/pcap_ct.py | 12 +- pcapkit/foundation/engines/pypcap.py | 23 ++- .../test_pypcap_unusable_module_2132_unit.py | 189 ++++++++++++++++++ 6 files changed, 252 insertions(+), 20 deletions(-) create mode 100644 tests/foundation/engines/test_pypcap_unusable_module_2132_unit.py diff --git a/docs/source/pcapkit/foundation/engines/3rdparty.rst b/docs/source/pcapkit/foundation/engines/3rdparty.rst index 394fd5c742..0c8e96beba 100644 --- a/docs/source/pcapkit/foundation/engines/3rdparty.rst +++ b/docs/source/pcapkit/foundation/engines/3rdparty.rst @@ -207,6 +207,12 @@ support, as is used by :class:`pcapkit.foundation.extraction.Extractor`. not the one in use, and the two have different install requirements and different version coverage to report. + A |pcap|_ module that is installed but cannot load -- its import raises + :exc:`OSError`, say, because no :manpage:`libpcap(3)` can be found -- is reported + the same way :class:`~pcapkit.foundation.engines.pcap_ct.PCAP_CT` reports it, so the + extraction falls back to the :term:`default engine` with a warning instead of + aborting (:issue:`2132`). + .. important:: `PyPCAP`_ is a :manpage:`libpcap(3)` binding aimed primarily at live capture. @@ -502,6 +508,8 @@ cycle. .. autofunction:: pcapkit.foundation.engines._pcap_backend.wrong_backend_reason +.. autofunction:: pcapkit.foundation.engines._pcap_backend.engine_reason + .. autofunction:: pcapkit.foundation.engines._pcap_backend.collision_reason Internal Definitions diff --git a/docs/source/pcapkit/foundation/engines/index.rst b/docs/source/pcapkit/foundation/engines/index.rst index 6d0837a5f9..f2b6024f9c 100644 --- a/docs/source/pcapkit/foundation/engines/index.rst +++ b/docs/source/pcapkit/foundation/engines/index.rst @@ -235,7 +235,9 @@ actually got, via name the distribution, version and file actually in use, so a bug report about "the pypcap engine" says which one ran; * declines to run on the other one, through ``unsupported_reason()``, with a - message naming the ``engine=`` string that does want it; and + message naming the ``engine=`` string that does want it, and likewise on a + |pcap|_ that is installed but cannot load, falling back to the + :term:`default engine` instead of letting the import error escape; and * warns with an :class:`~pcapkit.utilities.warnings.EngineWarning` when it finds both installed, since that state leaves one engine permanently unselectable and nothing else would explain why. diff --git a/pcapkit/foundation/engines/_pcap_backend.py b/pcapkit/foundation/engines/_pcap_backend.py index eda46e1b96..7b170370f7 100644 --- a/pcapkit/foundation/engines/_pcap_backend.py +++ b/pcapkit/foundation/engines/_pcap_backend.py @@ -38,7 +38,7 @@ __all__ = [ 'PYPCAP', 'PCAP_CT', 'DISTRIBUTIONS', 'ENGINE_NAMES', 'Probe', 'Memo', 'probe', 'identify', 'installed_distributions', - 'wrong_backend_reason', 'collision_reason', + 'wrong_backend_reason', 'engine_reason', 'collision_reason', ] if TYPE_CHECKING: @@ -308,6 +308,40 @@ def wrong_backend_reason(wanted: 'str', found: 'Probe') -> 'Optional[str]': f'({found.version or "unknown version"}), not {wanted}{suggestion}') +def engine_reason(wanted: 'str', found: 'Probe') -> 'Optional[str]': + """Why the engine driving ``wanted`` cannot run on ``found``, if it cannot. + + The verdict both engines' ``unsupported_reason()`` give, kept in one place so + that they agree (:issue:`2132`). + + Args: + wanted: The distribution the calling engine drives -- :data:`PYPCAP` or + :data:`PCAP_CT`. + found: The result of :func:`probe`. + + Returns: + :func:`wrong_backend_reason`'s answer when another distribution owns + |pcap|_; a phrase carrying the import failure when |pcap|_ is installed + but raised something other than :exc:`ImportError` -- ``OSError: Cannot + find libpcap.so library`` with no system :manpage:`libpcap(3)`, say; and + :data:`None` otherwise. An absent module is still :data:`None`, since + :meth:`Extractor.import_test + ` reports that. + + """ + wrong = wrong_backend_reason(wanted, found) + if wrong is not None: + return wrong + + # ``missing`` separates "not installed", which the import test reports + # well, from a failure that would otherwise escape it -- the missing + # ``libpcap.so`` being the case this exists for. + if found.name is None and not found.missing: + return f'the `pcap` module is installed but unusable -- {found.failure}' + + return None + + def collision_reason(found: 'Probe') -> 'Optional[str]': """A description of both distributions being installed at once, if they are. diff --git a/pcapkit/foundation/engines/pcap_ct.py b/pcapkit/foundation/engines/pcap_ct.py index d03c0009d1..4e6e4f464b 100644 --- a/pcapkit/foundation/engines/pcap_ct.py +++ b/pcapkit/foundation/engines/pcap_ct.py @@ -166,17 +166,7 @@ def _reason_for(cls, found: '_pcap_backend.Probe') -> 'Optional[str]': import after a failed one does not reproduce the same error. """ - wrong = _pcap_backend.wrong_backend_reason(cls.__engine_distribution__, found) - if wrong is not None: - return wrong - - # ``missing`` separates "not installed", which the import test reports - # well, from a failure that would otherwise escape it -- the missing - # ``libpcap.so`` being the case this exists for. - if found.name is None and not found.missing: - return f'the `pcap` module is installed but unusable -- {found.failure}' - - return None + return _pcap_backend.engine_reason(cls.__engine_distribution__, found) @classmethod def unsupported_reason(cls) -> 'Optional[str]': diff --git a/pcapkit/foundation/engines/pypcap.py b/pcapkit/foundation/engines/pypcap.py index 7a3c637893..924079bcb7 100644 --- a/pcapkit/foundation/engines/pypcap.py +++ b/pcapkit/foundation/engines/pypcap.py @@ -130,7 +130,7 @@ def _reason_for(cls, found: '_pcap_backend.Probe') -> 'Optional[str]': import after a failed one does not reproduce the same error. """ - return _pcap_backend.wrong_backend_reason(cls.__engine_distribution__, found) + return _pcap_backend.engine_reason(cls.__engine_distribution__, found) @classmethod def unsupported_reason(cls) -> 'Optional[str]': @@ -150,6 +150,16 @@ def unsupported_reason(cls) -> 'Optional[str]': interpreter, and would say nothing useful on 3.10 and 3.11, where both distributions install happily and either could be the one in place. + It also reports a |pcap|_ that is installed but cannot load -- one whose + import raises something other than :exc:`ImportError`, such as the + :exc:`OSError` a loader raises when it cannot find :manpage:`libpcap(3)`. + :meth:`Extractor.import_test + ` lets that through, + so without this the extraction would die rather than fall back to the + :term:`default engine` (:issue:`2132`). The verdict is + :func:`~pcapkit.foundation.engines._pcap_backend.engine_reason`, shared + with :class:`~pcapkit.foundation.engines.pcap_ct.PCAP_CT`. + Returns: A phrase naming the real cause, or :data:`None`. An absent |pcap|_ returns :data:`None`, since the import test reports that case in its @@ -195,9 +205,9 @@ def __init__(self, extractor: 'Extractor') -> 'None': Raises: UnsupportedCall: If the installed |pcap|_ is `pcap-ct`_ rather - than upstream `PyPCAP`_. This is the same condition - :meth:`unsupported_reason` reports, kept here as well because that - hook is only consulted by + than upstream `PyPCAP`_, or is installed but cannot load. These + are the conditions :meth:`unsupported_reason` reports, kept here as + well because that hook is only consulted by :meth:`Extractor.run `; constructing the engine directly bypasses it, and running `pcap-ct`_ under the :class:`~pcapkit.foundation.engines.pypcap.PyPCAP` name would @@ -208,9 +218,8 @@ def __init__(self, extractor: 'Extractor') -> 'None': """ # NOTE: the probe comes *before* ``import pcap`` so that this engine and # :class:`~pcapkit.foundation.engines.pcap_ct.PCAP_CT` behave the same way - # here. It matters more for the latter, whose import can raise - # ``OSError`` rather than ``ImportError``, but a shared order is one fewer - # difference between them. + # here. Either import can raise ``OSError`` rather than ``ImportError`` + # (#2132), which the probe turns into a reason instead of a crash. self._backend = _pcap_backend.probe() collision = _pcap_backend.collision_reason(self._backend) diff --git a/tests/foundation/engines/test_pypcap_unusable_module_2132_unit.py b/tests/foundation/engines/test_pypcap_unusable_module_2132_unit.py new file mode 100644 index 0000000000..afe144b8de --- /dev/null +++ b/tests/foundation/engines/test_pypcap_unusable_module_2132_unit.py @@ -0,0 +1,189 @@ +"""The pypcap engine falls back when the ``pcap`` module cannot load (#2132). + +A ``pcap`` module can be installed and still fail to import with something other +than :exc:`ImportError` -- ``OSError: Cannot find libpcap.so library`` with no +system libpcap, for one. :meth:`Extractor.import_test +` lets that through, so the +engine's ``unsupported_reason()`` has to report it. ``PCAP_CT`` did and ``PyPCAP`` +did not, so ``extract(engine='pypcap')`` raised the loader's error where +``engine='pcap_ct'`` warned and fell back. Both now ask +:func:`pcapkit.foundation.engines._pcap_backend.engine_reason`. + +""" +from __future__ import annotations + +import importlib.util +import json +import os +import pathlib +import subprocess # nosec: B404 +import sys +import tempfile +import types +import unittest +from unittest import mock + +from tests._support import scale_timeout +from tests._tiers import ROOT + +RUNTIME_DEPS = ('tbtrim', 'aenum', 'chardet', 'dictdumper') +HAS_RUNTIME = all(importlib.util.find_spec(name) is not None for name in RUNTIME_DEPS) + +#: What the fake ``pcap`` package raises on import: the libpcap loader's message. +LOADER_ERROR = 'Cannot find libpcap.so library' +#: The verdict ``PCAP_CT`` already gave on that failure, which ``PyPCAP`` now shares. +REASON = f'the `pcap` module is installed but unusable -- OSError: {LOADER_ERROR}' +#: Hang guard for the child interpreter, in seconds. +TIMEOUT = 120 + +#: Child script: run one extraction with the fake ``pcap`` first on the path and +#: report, as JSON, which ``pcapkit`` ran, what was warned and what happened. +CHILD = ''' +import json, sys, warnings +engine, capture = sys.argv[1], sys.argv[2] +import pcapkit +from pcapkit.foundation.engines.pcap_ct import PCAP_CT +from pcapkit.foundation.engines.pypcap import PyPCAP +result = { + 'pcapkit': pcapkit.__file__, + 'pypcap_reason': PyPCAP.unsupported_reason(), + 'pcap_ct_reason': PCAP_CT.unsupported_reason(), +} +with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + try: + extractor = pcapkit.extract(fin=capture, engine=engine, nofile=True) + except BaseException as exc: + result['error'] = f'{type(exc).__name__}: {exc}' + else: + result['engine'] = extractor._exnam + result['frames'] = len(extractor.frame) +result['warnings'] = [f'{w.category.__name__}: {w.message}' for w in caught] +print(json.dumps(result)) +''' + + +@unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed') +class EngineReasonTests(unittest.TestCase): + """The shared verdict, and both engines giving it.""" + + def probe(self, **overrides): + from pcapkit.foundation.engines import _pcap_backend + + values = dict(name=None, version=None, origin=None, + failure=f'OSError: {LOADER_ERROR}', missing=False, installed=()) + values.update(overrides) + return _pcap_backend.Probe(**values) + + def test_an_installed_but_unusable_module_is_a_reason_for_either_engine(self) -> None: + from pcapkit.foundation.engines import _pcap_backend + + for wanted in _pcap_backend.DISTRIBUTIONS: + with self.subTest(wanted=wanted): + self.assertEqual(_pcap_backend.engine_reason(wanted, self.probe()), REASON) + + def test_an_absent_module_is_left_to_the_import_test(self) -> None: + from pcapkit.foundation.engines import _pcap_backend + + found = self.probe(failure='no module named `pcap`', missing=True) + self.assertIsNone(_pcap_backend.engine_reason(_pcap_backend.PYPCAP, found)) + + def test_the_wrong_backend_keeps_its_own_reason(self) -> None: + from pcapkit.foundation.engines import _pcap_backend + + found = self.probe(name=_pcap_backend.PCAP_CT, version='1.3.0b3', + origin='/stub/pcap/__init__.py', failure=None) + self.assertEqual(_pcap_backend.engine_reason(_pcap_backend.PYPCAP, found), + _pcap_backend.wrong_backend_reason(_pcap_backend.PYPCAP, found)) + + def broken_import(self): + from pcapkit.foundation.engines import _pcap_backend + + return (mock.patch('importlib.import_module', side_effect=OSError(LOADER_ERROR)), + mock.patch.object(_pcap_backend, 'installed_distributions', + return_value=(_pcap_backend.PYPCAP,))) + + def test_pypcap_reports_the_module_that_cannot_load(self) -> None: + from pcapkit.foundation.engines.pcap_ct import PCAP_CT + from pcapkit.foundation.engines.pypcap import PyPCAP + + importer, installed = self.broken_import() + with importer, installed: + reason = PyPCAP.unsupported_reason() + peer = PCAP_CT.unsupported_reason() + + self.assertEqual(reason, REASON) + self.assertEqual(reason, peer) + + def test_pypcap_constructor_refuses_instead_of_raising_the_oserror(self) -> None: + from pcapkit.foundation.engines.pypcap import PyPCAP + from pcapkit.utilities.exceptions import UnsupportedCall + + importer, installed = self.broken_import() + with importer, installed: + with self.assertRaises(UnsupportedCall) as caught: + PyPCAP(types.SimpleNamespace()) # type: ignore[arg-type] + + self.assertIn(REASON, str(caught.exception)) + + +@unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed') +class FakePcapPackageTests(unittest.TestCase): + """End to end, with a real ``pcap`` package on :data:`sys.path` that cannot load. + + Run in a child interpreter so that the fake package never reaches this + process's :data:`sys.modules` or import caches. + + """ + + @classmethod + def setUpClass(cls) -> None: + tmp = tempfile.TemporaryDirectory() + cls.addClassCleanup(tmp.cleanup) + package = pathlib.Path(tmp.name) / 'pcap' + package.mkdir() + (package / '__init__.py').write_text(f'raise OSError({LOADER_ERROR!r})\n') + cls.site = tmp.name + + def extract(self, engine: str) -> dict: + env = dict(os.environ) + env.pop('PYTHONSAFEPATH', None) # cwd must win, so the tree under test runs + env['PYTHONPATH'] = os.pathsep.join( + filter(None, (self.site, env.get('PYTHONPATH')))) + capture = str(ROOT / 'examples' / 'captures' / 'in.pcap') + completed = subprocess.run( # nosec: B603 + [sys.executable, '-c', CHILD, engine, capture], + cwd=str(ROOT), env=env, capture_output=True, text=True, + timeout=scale_timeout(TIMEOUT), check=False, + ) + self.assertEqual(completed.returncode, 0, completed.stderr) + result = json.loads(completed.stdout.strip().splitlines()[-1]) + loaded = os.path.realpath(result['pcapkit']) + self.assertEqual(os.path.commonpath([loaded, str(ROOT)]), str(ROOT), loaded) + return result + + def assert_falls_back(self, engine: str, name: str) -> None: + result = self.extract(engine) + + self.assertNotIn('error', result) + self.assertEqual(result['engine'], 'default') + self.assertGreater(result['frames'], 0) + self.assertIn(f'EngineWarning: engine {name} is not supported on this ' + f'interpreter ({REASON}); using default engine instead', + result['warnings']) + + def test_pcap_ct_falls_back(self) -> None: + self.assert_falls_back('pcap_ct', 'PCAP_CT') + + def test_pypcap_falls_back_like_pcap_ct(self) -> None: + self.assert_falls_back('pypcap', 'PyPCAP') + + def test_pypcap_unsupported_reason_names_the_loader_error(self) -> None: + result = self.extract('pypcap') + + self.assertEqual(result['pypcap_reason'], REASON) + self.assertEqual(result['pypcap_reason'], result['pcap_ct_reason']) + + +if __name__ == '__main__': + unittest.main()