From 5e0aa58726ba660a22abc30cf3ed626c726915ac Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Sun, 1 Nov 2026 14:03:42 -0500 Subject: [PATCH] fix(vendor): restore sys.path when an IPX crawler import fails (#2589) pcapkit.vendor.ipx and its packet and socket crawlers pop sys.path[0] around their imports: run as a script, a crawler has the package directory first on sys.path, and the sibling socket.py would shadow the socket module requests imports. The entry went back only on success, so a failed import lost it, and pkgutil.walk_packages, retrying on every walk, drained sys.path until pop raised IndexError without requests installed. Restore the entry in a finally in all three modules. The new tests/vendor/test_vendor_ipx_sys_path_2589_unit.py hides requests and checks sys.path after a failed package import and a failed script-mode run of each crawler; it fails without the fix. --- pcapkit/vendor/ipx/__init__.py | 13 +-- pcapkit/vendor/ipx/packet.py | 21 ++-- pcapkit/vendor/ipx/socket.py | 21 ++-- .../test_vendor_ipx_sys_path_2589_unit.py | 106 ++++++++++++++++++ 4 files changed, 132 insertions(+), 29 deletions(-) create mode 100644 tests/vendor/test_vendor_ipx_sys_path_2589_unit.py diff --git a/pcapkit/vendor/ipx/__init__.py b/pcapkit/vendor/ipx/__init__.py index 28fe01ee54..e42b64e352 100644 --- a/pcapkit/vendor/ipx/__init__.py +++ b/pcapkit/vendor/ipx/__init__.py @@ -28,13 +28,12 @@ # :mod:`pcapkit.vendor.__main__` builds, so this module-level temporary must not # read as a public name of the package. _path = sys.path.pop(0) -############################################################################### - -from pcapkit.vendor.ipx.packet import Packet as IPX_Packet -from pcapkit.vendor.ipx.socket import Socket as IPX_Socket - -############################################################################### -sys.path.insert(0, _path) +# NOTE: restored in ``finally``, so a failed import does not lose the entry (#2589). +try: + from pcapkit.vendor.ipx.packet import Packet as IPX_Packet + from pcapkit.vendor.ipx.socket import Socket as IPX_Socket +finally: + sys.path.insert(0, _path) ############################################################################### __all__ = ['IPX_Packet', 'IPX_Socket'] diff --git a/pcapkit/vendor/ipx/packet.py b/pcapkit/vendor/ipx/packet.py index 7b5b14cfd7..74dd677783 100644 --- a/pcapkit/vendor/ipx/packet.py +++ b/pcapkit/vendor/ipx/packet.py @@ -11,22 +11,21 @@ """ ############################################################################### -# NOTE: fix duplicated name of ``socket`` +# NOTE: fix duplicated name of ``socket``; restored in ``finally``, so a failed +# import does not lose the entry (#2589). import sys path = sys.path.pop(0) -############################################################################### - -import collections -from typing import TYPE_CHECKING +try: + import collections + from typing import TYPE_CHECKING -from pcapkit.vendor.default import Vendor + from pcapkit.vendor.default import Vendor -if TYPE_CHECKING: - from collections import Counter - -############################################################################### -sys.path.insert(0, path) + if TYPE_CHECKING: + from collections import Counter +finally: + sys.path.insert(0, path) ############################################################################### __all__ = ['Packet'] diff --git a/pcapkit/vendor/ipx/socket.py b/pcapkit/vendor/ipx/socket.py index 081e0dca5d..1844ef164b 100644 --- a/pcapkit/vendor/ipx/socket.py +++ b/pcapkit/vendor/ipx/socket.py @@ -11,22 +11,21 @@ """ ############################################################################### -# NOTE: fix duplicated name of ``socket`` +# NOTE: fix duplicated name of ``socket``; restored in ``finally``, so a failed +# import does not lose the entry (#2589). import sys path = sys.path.pop(0) -############################################################################### - -import collections -from typing import TYPE_CHECKING +try: + import collections + from typing import TYPE_CHECKING -from pcapkit.vendor.default import Vendor + from pcapkit.vendor.default import Vendor -if TYPE_CHECKING: - from collections import Counter - -############################################################################### -sys.path.insert(0, path) + if TYPE_CHECKING: + from collections import Counter +finally: + sys.path.insert(0, path) ############################################################################### __all__ = ['Socket'] diff --git a/tests/vendor/test_vendor_ipx_sys_path_2589_unit.py b/tests/vendor/test_vendor_ipx_sys_path_2589_unit.py new file mode 100644 index 0000000000..87059b987d --- /dev/null +++ b/tests/vendor/test_vendor_ipx_sys_path_2589_unit.py @@ -0,0 +1,106 @@ +# -*- coding: utf-8 -*- +"""A failed IPX crawler import leaves :data:`sys.path` as it found it. + +GitHub issue #2589. :mod:`pcapkit.vendor.ipx`, :mod:`pcapkit.vendor.ipx.packet` +and :mod:`pcapkit.vendor.ipx.socket` each pop ``sys.path[0]`` before their +imports and put it back after them. Run as a script, a crawler there has the +package directory as ``sys.path[0]``, and its sibling ``socket.py`` would then +shadow the standard library's :mod:`socket`, which ``requests`` imports through +:mod:`urllib3`. Without a ``finally``, an import that raises -- ``requests`` +missing, say -- loses the popped entry. :func:`pkgutil.walk_packages` swallows +that :exc:`ImportError` and retries the import on every walk, so each walk loses +more, until ``sys.path.pop(0)`` raises :exc:`IndexError`. + +``requests`` is hidden by binding it to :data:`None` in :data:`sys.modules`, so +importing it raises :exc:`ModuleNotFoundError` just as a missing package does, +whether or not it is installed here. The whole of :mod:`pcapkit.vendor` is +purged first, so each import runs its code again, and put back afterwards. The +package itself is in that region: an import that is first to load it does so +with ``requests`` hidden, which records ``requests`` in its ``_MISSING``. + +Without the ``finally`` blocks every test here fails on the :data:`sys.path` +comparison. + +""" +from __future__ import annotations + +import importlib +import importlib.util +import pathlib +import sys +import unittest + +from tests._support import isolate_modules + +#: Test tier (#1958): import bookkeeping in pure Python, the same on every interpreter. +__tier__ = 'LATEST' + +#: The package directory, which is ``sys.path[0]`` when a crawler in it runs as a script. +IPX_DIR = pathlib.Path(__file__).resolve().parents[2] / 'pcapkit' / 'vendor' / 'ipx' + +#: The crawler modules of :mod:`pcapkit.vendor.ipx`, each of which pops and +#: restores ``sys.path[0]`` around its own imports. +CRAWLERS = ('packet', 'socket') + +#: What the failing import raises on, and so the module this file hides. +HIDDEN = 'requests' + +#: Marks :data:`HIDDEN` as absent from :data:`sys.modules` before the test. +_ABSENT = object() + + +class FailedImportKeepsSysPathTests(unittest.TestCase): + """An IPX import that raises puts back the :data:`sys.path` entry it popped.""" + + def setUp(self) -> None: + isolate_modules(self, ('pcapkit.vendor',)) + + # On an unfixed tree the import loses entries; restoring them keeps that + # failure from breaking every import after this test. + saved = list(sys.path) + self.addCleanup(sys.path.__setitem__, slice(None), saved) + + held = sys.modules.get(HIDDEN, _ABSENT) + sys.modules[HIDDEN] = None + self.addCleanup(self._unhide, held) + + @staticmethod + def _unhide(held: object) -> None: + """Put :data:`HIDDEN` back in :data:`sys.modules` as it was before the test.""" + if held is _ABSENT: + sys.modules.pop(HIDDEN, None) + else: + sys.modules[HIDDEN] = held + + def test_failed_package_import_keeps_sys_path(self) -> None: + # The import pkgutil.walk_packages makes, and retries on every walk. + expected = list(sys.path) + with self.assertRaises(ImportError): + importlib.import_module('pcapkit.vendor.ipx') + self.assertEqual(sys.path, expected, + 'a failed import of pcapkit.vendor.ipx lost a sys.path entry') + + def test_failed_crawler_run_as_script_keeps_sys_path(self) -> None: + # Loaded from its file with the package directory first on sys.path, as + # `python pcapkit/vendor/ipx/.py` has it. The package import + # above never reaches the socket crawler, as the packet crawler fails first. + for name in CRAWLERS: + with self.subTest(crawler=name): + sys.path.insert(0, str(IPX_DIR)) + expected = list(sys.path) + spec = importlib.util.spec_from_file_location( + f'_ipx_{name}_as_script', IPX_DIR / f'{name}.py') + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + try: + with self.assertRaises(ImportError): + spec.loader.exec_module(module) + self.assertEqual(sys.path, expected, + f'a failed run of the {name} crawler lost a sys.path entry') + finally: + if sys.path and sys.path[0] == str(IPX_DIR): + del sys.path[0] + + +if __name__ == '__main__': + unittest.main()