From dd036a52d955fdcbf9d37392c16ce0933d9a277f Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Fri, 18 Sep 2026 22:46:30 -0400 Subject: [PATCH 1/2] const: accept RFC 2113's Router Alert 0 and IPX's own socket 0 (#492) Three lookup defects under pcapkit/const, all reproduced on the wire: - pcapkit/const/ipv4/router_alert.py, pcapkit/vendor/ipv4/router_alert.py: RouterAlert(0) raised ValueError, rejecting RFC 2113's only defined value (0 = "Router shall examine packet"), the one IGMP/RSVP/MLD actually send. Root cause found by fetching IANA's live CSV directly: the registry ships with NO header row -- its first row IS the value-0 entry -- so the vendor crawler's unconditional `next(reader) # header` silently discarded it. Removed that skip; re-ran the real `pcapkit-vendor ipv4.router_alert` crawler against the live registry and it now regenerates the const file with exactly one new member, `Router_shall_examine_packet = 0`, nothing else changed. - pcapkit/const/ipx/socket.py, pcapkit/vendor/ipx/socket.py: Socket(0) raised ValueError, even though 0x0000 is IPX's own class default for the dst/src address field, so `bytes(IPX(payload=...))` crashed on its own defaults. Added an `Unspecified = 0x0000` member. The Wikipedia table the vendor crawler scrapes for this one has meanwhile lost the well-known-socket-ranges table entirely (only a generic 3-row header-format table remains under `wikitable`, confirmed live) and `requests`' default User-Agent now gets a 403 from Wikipedia regardless -- both pre-existing and out of scope, so the vendor fix prepends the 0x0000 entry unconditionally rather than relying on the scrape to carry it. - pcapkit/const/ftp/return_code.py, pcapkit/vendor/ftp/return_code.py: `ResponseKind._missing_` and `GroupingInformation._missing_` were plain methods, so aenum's `cls._missing_(value)` bound `value` to `cls` and every unregistered value raised TypeError instead of extending the enum. Added the missing `@classmethod`. tests/const/test_const_enum_lookup.py: sweeps all 111 IntEnum classes under pcapkit.const, asserting Enum(0) matches the registry (105 accept, 6 correctly reject -- ReturnCode, StatusCode, ConformanceRequirement, CauseCode, Parameter, MNIDSubtype); asserts every one of the tree's 109 `_missing_` overrides is a classmethod; and parses the RFC 2113 repro packet end-to-end. Verified before/after on the actual repros: RouterAlert(0) and the 28-byte IGMP-over-Router-Alert packet both raised ValueError before, now parse; bytes(IPX(payload=b'\xde\xad\xbe\xef')) raised before, now round-trips; ResponseKind(7)/(0) and GroupingInformation(9) raised TypeError before, now extend correctly. tests/const/: 7 passed, 111 subtests. tests/protocols/internet/test_ipx_unit.py: 3 passed. tests/protocols/internet/test_ipv4_unit.py: 12 passed, 16 subtests. mypy on all six touched source files: identical pre-existing errors only (11, all Liskov/typeshed issues predating this change). --- pcapkit/const/ftp/return_code.py | 2 + pcapkit/const/ipv4/router_alert.py | 3 + pcapkit/const/ipx/socket.py | 3 + pcapkit/vendor/ftp/return_code.py | 2 + pcapkit/vendor/ipv4/router_alert.py | 5 +- pcapkit/vendor/ipx/socket.py | 9 ++ tests/const/__init__.py | 0 tests/const/test_const_enum_lookup.py | 192 ++++++++++++++++++++++++++ 8 files changed, 215 insertions(+), 1 deletion(-) create mode 100644 tests/const/__init__.py create mode 100644 tests/const/test_const_enum_lookup.py diff --git a/pcapkit/const/ftp/return_code.py b/pcapkit/const/ftp/return_code.py index 3fc52be506..05975acdc3 100644 --- a/pcapkit/const/ftp/return_code.py +++ b/pcapkit/const/ftp/return_code.py @@ -41,6 +41,7 @@ class ResponseKind(IntEnum): PermanentNegativeCompletion = 5 Protected = 6 + @classmethod def _missing_(cls, value: 'int') -> 'ResponseKind': """Lookup function used when value is not found. @@ -63,6 +64,7 @@ class GroupingInformation(IntEnum): Unspecified = 4 FileSystem = 5 + @classmethod def _missing_(cls, value: 'int') -> 'GroupingInformation': """Lookup function used when value is not found. diff --git a/pcapkit/const/ipv4/router_alert.py b/pcapkit/const/ipv4/router_alert.py index 5387a7a91e..4376120e8b 100644 --- a/pcapkit/const/ipv4/router_alert.py +++ b/pcapkit/const/ipv4/router_alert.py @@ -18,6 +18,9 @@ class RouterAlert(IntEnum): """[RouterAlert] IPv4 Router Alert Option Values""" + #: Router shall examine packet [:rfc:`2113`] + Router_shall_examine_packet = 0 + #: Aggregated Reservation Nesting Level [:rfc:`3175`] Aggregated_Reservation_Nesting_Level_0 = 1 diff --git a/pcapkit/const/ipx/socket.py b/pcapkit/const/ipx/socket.py index d2be838697..45ab895e71 100644 --- a/pcapkit/const/ipx/socket.py +++ b/pcapkit/const/ipx/socket.py @@ -18,6 +18,9 @@ class Socket(IntEnum): """[Socket] Socket Types""" + #: Unspecified socket; this is IPX's own default for the dst/src socket field. + Unspecified = 0x0000 + #: Routing Information Packet Routing_Information_Packet = 0x0001 diff --git a/pcapkit/vendor/ftp/return_code.py b/pcapkit/vendor/ftp/return_code.py index c371c1be12..ecb7f68058 100644 --- a/pcapkit/vendor/ftp/return_code.py +++ b/pcapkit/vendor/ftp/return_code.py @@ -70,6 +70,7 @@ class ResponseKind(IntEnum): PermanentNegativeCompletion = 5 Protected = 6 + @classmethod def _missing_(cls, value: 'int') -> 'ResponseKind': """Lookup function used when value is not found. @@ -92,6 +93,7 @@ class GroupingInformation(IntEnum): Unspecified = 4 FileSystem = 5 + @classmethod def _missing_(cls, value: 'int') -> 'GroupingInformation': """Lookup function used when value is not found. diff --git a/pcapkit/vendor/ipv4/router_alert.py b/pcapkit/vendor/ipv4/router_alert.py index 41730d12f9..99a7b0f59b 100644 --- a/pcapkit/vendor/ipv4/router_alert.py +++ b/pcapkit/vendor/ipv4/router_alert.py @@ -36,8 +36,11 @@ def process(self, data: 'list[str]') -> 'tuple[list[str], list[str]]': Enumeration fields and missing fields. """ + # NOTE: Unlike most IANA registry CSVs, the ``ipv4-router-alert-option-values`` + # registry ships with no header row -- its very first row is the ``0`` entry + # (RFC 2113's "Router shall examine packet", the only value that RFC defines). + # Skipping a "header" here silently drops that row; see #492. reader = csv.reader(data) - next(reader) # header enum = [] # type: list[str] miss = [] # type: list[str] diff --git a/pcapkit/vendor/ipx/socket.py b/pcapkit/vendor/ipx/socket.py index 9e7bbb39d8..7a47f3708f 100644 --- a/pcapkit/vendor/ipx/socket.py +++ b/pcapkit/vendor/ipx/socket.py @@ -85,6 +85,15 @@ def process(self, soup: 'BeautifulSoup') -> 'tuple[list[str], list[str]]': # py enum = [] # type: list[str] miss = [] # type: list[str] + + # NOTE: 0x0000 is not listed as a well-known socket in the Wikipedia + # registry table scraped below, but it is the IPX protocol's own + # default for the ``dst``/``src`` socket field (an ordinary + # "unspecified socket"), so it must be present regardless of what the + # scraped table contains; see #492. + desc = self.wrap_comment("Unspecified socket; this is IPX's own default for the dst/src socket field.") + enum.append(f'#: {desc}\n Unspecified = 0x0000') + for item in content: line = item.find_all('td') diff --git a/tests/const/__init__.py b/tests/const/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/const/test_const_enum_lookup.py b/tests/const/test_const_enum_lookup.py new file mode 100644 index 0000000000..c5af51467f --- /dev/null +++ b/tests/const/test_const_enum_lookup.py @@ -0,0 +1,192 @@ +# -*- coding: utf-8 -*- +"""Registry-wide regression tests for :mod:`pcapkit.const` enum lookups. + +GitHub issue #492 found three lookup defects hiding under :mod:`pcapkit.const`: +a value that a registry actually defines being rejected by the generated +:class:`~aenum.IntEnum` (:class:`~pcapkit.const.ipv4.router_alert.RouterAlert` +and :class:`~pcapkit.const.ipx.socket.Socket`, both for value ``0``), and a +``_missing_`` lookup method missing its ``@classmethod`` decorator +(:class:`~pcapkit.const.ftp.return_code.ResponseKind` and +:class:`~pcapkit.const.ftp.return_code.GroupingInformation`), which makes +``cls._missing_(value)`` bind ``value`` to ``cls`` and raise :class:`TypeError` +for *every* unregistered value rather than the intended fallback. + +Both defects were found by a sweep instantiating every :class:`~aenum.IntEnum` +under :mod:`pcapkit.const` with value ``0``, and by checking every ``_missing_`` +definition in that tree for the decorator. This module reuses that sweep so the +two defects -- and the fix -- stay pinned: a future regeneration of the +``pcapkit.const`` tree (see :mod:`pcapkit.vendor`) that drops either fix will +fail this suite rather than silently reintroducing GitHub issue #492. + +""" +from __future__ import annotations + +import importlib +import inspect +import pkgutil +import unittest +from typing import TYPE_CHECKING + +from aenum import IntEnum, IntFlag + +import pcapkit.const as const_pkg + +#: Fully qualified names of the :class:`~aenum.IntEnum` classes under +#: :mod:`pcapkit.const` for which rejecting ``0`` is *correct*, because their +#: registries simply do not define ``0`` (three-digit response codes starting +#: at 100, or registries reserving ``0`` and starting real assignments at 1). +#: Confirmed against the IANA/RFC registries backing each one; see GitHub +#: issue #492. +EXPECTED_TO_REJECT_ZERO = frozenset({ + 'pcapkit.const.ftp.return_code.ReturnCode', + 'pcapkit.const.http.status_code.StatusCode', + 'pcapkit.const.ftp.command.ConformanceRequirement', + 'pcapkit.const.sctp.cause_code.CauseCode', + 'pcapkit.const.sctp.parameter.Parameter', + 'pcapkit.const.mh.mn_id_subtype.MNIDSubtype', +}) + + +def _iter_const_int_enums() -> 'list[type]': + """Discover every :class:`~aenum.IntEnum` defined under :mod:`pcapkit.const`. + + Walks every submodule of :mod:`pcapkit.const` and collects the + :class:`~aenum.IntEnum` subclasses (excluding :class:`~aenum.IntFlag`, + which is a different value-lookup contract) that are actually *defined* + in the module being walked, rather than merely imported/re-exported by + it -- so each registry enum is counted exactly once. + + Returns: + The discovered enum classes, in walk order. + + """ + classes = [] # type: list[type] + for module_info in pkgutil.walk_packages(const_pkg.__path__, const_pkg.__name__ + '.'): + module = importlib.import_module(module_info.name) + for _, obj in vars(module).items(): + if (inspect.isclass(obj) and issubclass(obj, IntEnum) + and obj.__module__ == module_info.name + and not issubclass(obj, IntFlag)): + classes.append(obj) + return classes + + +class ConstEnumZeroLookupTests(unittest.TestCase): + """``Enum(0)`` must behave as the backing registry says it should.""" + + if TYPE_CHECKING: + enums: 'list[type]' + + @classmethod + def setUpClass(cls) -> None: + cls.enums = _iter_const_int_enums() + + def test_every_registry_enum_was_discovered(self) -> None: + # Pins the size of the sweep itself: if this drifts, a const enum was + # added, removed, or renamed, and EXPECTED_TO_REJECT_ZERO (and the + # analysis in GitHub issue #492) needs a fresh look rather than a + # silent pass. + names = {f'{obj.__module__}.{obj.__qualname__}' for obj in self.enums} + self.assertEqual(len(self.enums), 111) + self.assertTrue(EXPECTED_TO_REJECT_ZERO.issubset(names), + f'expected reject-list entries missing from the sweep: ' + f'{EXPECTED_TO_REJECT_ZERO - names}') + + def test_zero_lookup_matches_the_registry(self) -> None: + for obj in self.enums: + qualname = f'{obj.__module__}.{obj.__qualname__}' + with self.subTest(enum=qualname): + if qualname in EXPECTED_TO_REJECT_ZERO: + with self.assertRaises(ValueError): + obj(0) + else: + try: + obj(0) + except Exception as error: # pylint: disable=broad-except + self.fail(f'{qualname}(0) unexpectedly raised {error!r}; ' + f'see GitHub issue #492') + + def test_router_alert_accepts_rfc_2113_value(self) -> None: + # The specific HIGH-severity defect: RFC 2113 defines exactly one + # value, 0, and the generated enum did not have it. + from pcapkit.const.ipv4.router_alert import RouterAlert + + self.assertEqual(RouterAlert(0), RouterAlert.Router_shall_examine_packet) + self.assertEqual(RouterAlert(0).value, 0) + + def test_ipx_socket_accepts_unspecified_socket(self) -> None: + # The specific MEDIUM-severity defect: 0x0000 is IPX's own class + # default for the dst/src socket field. + from pcapkit.const.ipx.socket import Socket + + self.assertEqual(Socket(0), Socket.Unspecified) + self.assertEqual(Socket(0).value, 0) + + +class ConstMissingClassmethodTests(unittest.TestCase): + """Every ``_missing_`` under :mod:`pcapkit.const` must be a classmethod. + + aenum invokes lookup failures as ``cls._missing_(value)``. Without + ``@classmethod``, the sole positional argument binds to ``cls`` and + ``value`` goes unfilled, so *every* call raises :class:`TypeError` instead + of running the intended fallback body -- see GitHub issue #492 (LOW + severity there because, at the time, nothing reachable at runtime ever + triggered the broken path). + + """ + + def test_every_missing_is_a_classmethod(self) -> None: + offenders = [] # type: list[str] + checked = 0 + for obj in _iter_const_int_enums(): + if '_missing_' not in obj.__dict__: + continue + checked += 1 + if not isinstance(obj.__dict__['_missing_'], classmethod): + offenders.append(f'{obj.__module__}.{obj.__qualname__}') + + self.assertGreater(checked, 0, 'sweep found no _missing_ overrides at all') + self.assertEqual(offenders, [], + f'_missing_ without @classmethod (GitHub issue #492): {offenders}') + + def test_ftp_return_code_missing_methods_are_classmethods(self) -> None: + # The specific defect: both were plain instance methods. + from pcapkit.const.ftp.return_code import GroupingInformation, ResponseKind + + self.assertIsInstance(ResponseKind.__dict__['_missing_'], classmethod) + self.assertIsInstance(GroupingInformation.__dict__['_missing_'], classmethod) + + # And the actual symptom: every unknown value used to raise TypeError + # rather than extending the enum, not merely value 0. + self.assertEqual(int(ResponseKind(7)), 7) + self.assertEqual(int(ResponseKind(0)), 0) + self.assertEqual(int(GroupingInformation(9)), 9) + + +class RouterAlertPacketParseTests(unittest.TestCase): + """Parse an on-the-wire packet carrying RFC 2113's Router Alert value 0.""" + + def test_parses_igmp_over_router_alert_zero(self) -> None: + from pcapkit.protocols.internet.ipv4 import IPv4 + from pcapkit.const.ipv4.option_number import OptionNumber + from pcapkit.const.ipv4.router_alert import RouterAlert + from pcapkit.protocols.data.internet.ipv4 import RTRALTOption + + # IPv4 header, 28 bytes, carrying a Router Alert option (type 148, + # length 4, value 0) ahead of an IGMP payload -- the packet from + # GitHub issue #492. + wire = bytes.fromhex( + '4600001c00000000000200007f000001000000009404000011000000' + ) + + packet = IPv4(wire) + option = packet.info.options[OptionNumber.RTRALT] # type: ignore[index] + self.assertIsInstance(option, RTRALTOption) + + alert = option.alert # type: ignore[attr-defined] + self.assertEqual(alert, RouterAlert.Router_shall_examine_packet) + self.assertEqual(alert.value, 0) + + +if __name__ == '__main__': + unittest.main() From 725e6a4b76d684c043a8a22d537e7466adea676c Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Fri, 18 Sep 2026 22:51:57 -0400 Subject: [PATCH 2/2] tests: purge sys.modules before the const enum sweep, for full-suite order Passed standalone but failed inside the full suite: tests/const/test_const_enum_lookup.py is the first module to import every submodule under pcapkit.const, including ones nothing else touches (e.g. pcapkit.const.reg.apptype). When tests/cli/test_main.py runs first, its stubbed pcapkit.utilities.compat/exceptions modules are still sitting in sys.modules, and the fresh imports my sweep triggers pick up the stubs instead of the real files -- ImportError: cannot import name 'show_flag_values' / 'SeekError' (unknown location). Every other module in this suite guards against exactly this by calling tests._support.purge_modules(['pcapkit']) before touching pcapkit; this one didn't. Added it to setUp/setUpClass in all three test classes, and moved the pcapkit.const import inside _iter_const_int_enums() so it re-imports fresh after the purge rather than reusing a module-level reference cached at collection time. Reproduced with `pytest tests/cli tests/const/test_const_enum_lookup.py` (3 failed, 4 errored); passes the same way after this fix (13 passed). Full required set unaffected: tests/const/: 7 passed, 111 subtests. tests/protocols/internet/test_ipx_unit.py: 3 passed. tests/protocols/internet/test_ipv4_unit.py: 12 passed, 16 subtests. --- tests/const/test_const_enum_lookup.py | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/tests/const/test_const_enum_lookup.py b/tests/const/test_const_enum_lookup.py index c5af51467f..e710149493 100644 --- a/tests/const/test_const_enum_lookup.py +++ b/tests/const/test_const_enum_lookup.py @@ -18,6 +18,17 @@ ``pcapkit.const`` tree (see :mod:`pcapkit.vendor`) that drops either fix will fail this suite rather than silently reintroducing GitHub issue #492. +Every test class purges :mod:`pcapkit` from :data:`sys.modules` in ``setUp``, +matching the convention every other module in this suite uses (see +:func:`tests._support.purge_modules`). It matters more here than usual: this +module is the first thing in the whole suite to import *every* submodule +under :mod:`pcapkit.const`, including ones nothing else touches (e.g. +:mod:`pcapkit.const.reg.apptype`). ``tests/cli/test_main.py`` stubs pieces of +:mod:`pcapkit.utilities.compat` and :mod:`pcapkit.utilities.exceptions` +straight into :data:`sys.modules` for its own isolation and, depending on +suite order, that stub can still be sitting there when this module runs -- +purging first forces a clean re-import instead of tripping over it. + """ from __future__ import annotations @@ -29,7 +40,7 @@ from aenum import IntEnum, IntFlag -import pcapkit.const as const_pkg +from tests._support import purge_modules #: Fully qualified names of the :class:`~aenum.IntEnum` classes under #: :mod:`pcapkit.const` for which rejecting ``0`` is *correct*, because their @@ -60,6 +71,8 @@ def _iter_const_int_enums() -> 'list[type]': The discovered enum classes, in walk order. """ + import pcapkit.const as const_pkg + classes = [] # type: list[type] for module_info in pkgutil.walk_packages(const_pkg.__path__, const_pkg.__name__ + '.'): module = importlib.import_module(module_info.name) @@ -79,6 +92,7 @@ class ConstEnumZeroLookupTests(unittest.TestCase): @classmethod def setUpClass(cls) -> None: + purge_modules(['pcapkit']) cls.enums = _iter_const_int_enums() def test_every_registry_enum_was_discovered(self) -> None: @@ -135,6 +149,9 @@ class ConstMissingClassmethodTests(unittest.TestCase): """ + def setUp(self) -> None: + purge_modules(['pcapkit']) + def test_every_missing_is_a_classmethod(self) -> None: offenders = [] # type: list[str] checked = 0 @@ -166,6 +183,9 @@ def test_ftp_return_code_missing_methods_are_classmethods(self) -> None: class RouterAlertPacketParseTests(unittest.TestCase): """Parse an on-the-wire packet carrying RFC 2113's Router Alert value 0.""" + def setUp(self) -> None: + purge_modules(['pcapkit']) + def test_parses_igmp_over_router_alert_zero(self) -> None: from pcapkit.protocols.internet.ipv4 import IPv4 from pcapkit.const.ipv4.option_number import OptionNumber