From 072c9ddacf1aeedcd226c7090556bc89316de5af Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Sun, 27 Sep 2026 08:24:21 -0400 Subject: [PATCH] fix(ipx): order Socket._missing_ range branches narrowest-first - pcapkit/vendor/ipx/socket.py: RANGES listed the wide "Registered by Xerox" and "Dynamically Assigned" rows before the narrower ranges they fully contain, so Experimental (0x0020-0x003F), Dynamically Assigned Socket Numbers (0x4000-0x4FFF) and Statically Assigned Socket Numbers (0x8000-0xFFFF) could never be reached in the generated `_missing_`. Reorder each subset range ahead of the wider range that masks it. - pcapkit/const/ipx/socket.py: regenerated from the fixed vendor module; only the branch order changes. - tests/vendor/test_ipx_socket_unit.py: update EXPECTED_RANGES and EXPECTED_MISSING_NAMES, which pinned the old (shadowed) behaviour, and add test_previously_shadowed_ranges_are_reachable pinning 0x0030, 0x4080 and 0x8100 to their now-correct names. Verified `python -m coverage run -m pytest tests/vendor/test_ipx_socket_unit.py` passes (10 passed, 19 subtests), and Socket(0x0000) still resolves as Unspecified. Closes #841. --- pcapkit/const/ipx/socket.py | 12 +++--- pcapkit/vendor/ipx/socket.py | 23 ++++++----- tests/vendor/test_ipx_socket_unit.py | 62 +++++++++++++++++++--------- 3 files changed, 62 insertions(+), 35 deletions(-) diff --git a/pcapkit/const/ipx/socket.py b/pcapkit/const/ipx/socket.py index 2e5b077bd2..e7005e61f6 100644 --- a/pcapkit/const/ipx/socket.py +++ b/pcapkit/const/ipx/socket.py @@ -139,19 +139,19 @@ def _missing_(cls, value: 'int') -> 'Socket': """ if not (isinstance(value, int) and 0x0000 <= value <= 0xFFFF): raise ValueError('%r is not a valid %s' % (value, cls.__name__)) - if 0x0001 <= value <= 0x0BB8: - #: Registered by Xerox - return extend_enum(cls, 'Registered by Xerox_0x%s' % hex(value)[2:].upper().zfill(4), value) if 0x0020 <= value <= 0x003F: #: Experimental return extend_enum(cls, 'Experimental_0x%s' % hex(value)[2:].upper().zfill(4), value) - if 0x0BB9 <= value <= 0xFFFF: - #: Dynamically Assigned - return extend_enum(cls, 'Dynamically Assigned_0x%s' % hex(value)[2:].upper().zfill(4), value) + if 0x0001 <= value <= 0x0BB8: + #: Registered by Xerox + return extend_enum(cls, 'Registered by Xerox_0x%s' % hex(value)[2:].upper().zfill(4), value) if 0x4000 <= value <= 0x4FFF: #: Dynamically Assigned Socket Numbers return extend_enum(cls, 'Dynamically Assigned Socket Numbers_0x%s' % hex(value)[2:].upper().zfill(4), value) if 0x8000 <= value <= 0xFFFF: #: Statically Assigned Socket Numbers return extend_enum(cls, 'Statically Assigned Socket Numbers_0x%s' % hex(value)[2:].upper().zfill(4), value) + if 0x0BB9 <= value <= 0xFFFF: + #: Dynamically Assigned + return extend_enum(cls, 'Dynamically Assigned_0x%s' % hex(value)[2:].upper().zfill(4), value) return super()._missing_(value) diff --git a/pcapkit/vendor/ipx/socket.py b/pcapkit/vendor/ipx/socket.py index 6caaefae74..1820bb2e60 100644 --- a/pcapkit/vendor/ipx/socket.py +++ b/pcapkit/vendor/ipx/socket.py @@ -231,20 +231,23 @@ #: processes." ``(0x4000, 0x4FFF)`` is *contradicted*, not merely rounded: the #: preceding sentence reads "Socket numbers between 0x4000 and 0x7FFF are #: dynamic sockets", so the upper bound is 0x7FFF. The transcribed 0x4FFF is -#: kept because widening it would change the generated ``_missing_``; the -#: archived table is what is wrong here, and this range is masked by -#: ``(0x0BB9, 0xFFFF)`` in any case (see the note below), so the bound has no -#: observable effect today. +#: kept because widening it would change the generated ``_missing_``. RANGES = [ - # NOTE: order is significant, and is the order the rows appeared in. The - # generated ``_missing_`` tests these in sequence and returns on the first - # match, so the wide ranges here mask the narrow ones that follow them -- - # reordering the list silently changes which name an unlisted socket gets. - (0x0001, 0x0BB8, 'Registered by Xerox'), + # NOTE: order is significant. The generated ``_missing_`` tests these in + # sequence and returns on the first match, so a wide range placed before a + # narrower one it fully contains would mask that narrower one -- it would + # never be reached, and the socket would be misdescribed under the wide + # range's name instead. See GitHub issue #841, which found exactly that: + # ``(0x0020, 0x003F)`` is a strict subset of ``(0x0001, 0x0BB8)``, and both + # ``(0x4000, 0x4FFF)`` and ``(0x8000, 0xFFFF)`` are strict subsets of + # ``(0x0BB9, 0xFFFF)``. Each subset range is therefore listed ahead of the + # wider range that contains it, rather than in the order the rows appeared + # in the archived revision. (0x0020, 0x003F, 'Experimental'), - (0x0BB9, 0xFFFF, 'Dynamically Assigned'), + (0x0001, 0x0BB8, 'Registered by Xerox'), (0x4000, 0x4FFF, 'Dynamically Assigned Socket Numbers'), (0x8000, 0xFFFF, 'Statically Assigned Socket Numbers'), + (0x0BB9, 0xFFFF, 'Dynamically Assigned'), ] # type: list[tuple[int, int, str]] diff --git a/tests/vendor/test_ipx_socket_unit.py b/tests/vendor/test_ipx_socket_unit.py index 5a7ea26eec..16b51ce319 100644 --- a/tests/vendor/test_ipx_socket_unit.py +++ b/tests/vendor/test_ipx_socket_unit.py @@ -79,15 +79,22 @@ #: The socket number ranges, in the order :meth:`Socket.process` emits them and #: therefore the order the generated ``_missing_`` tests them in. Pinned because -#: that order decides which name an unlisted socket is given: the wide ranges -#: mask the narrow ones after them, so reordering the list changes behaviour -#: without changing any member. +#: that order decides which name an unlisted socket is given: a wide range +#: placed before a narrower one it fully contains masks that narrower one, so +#: reordering the list changes behaviour without changing any member. +#: +#: GitHub issue #841: three ranges used to be listed after a wider range that +#: fully contained them -- ``(0x0020, 0x003F)`` after ``(0x0001, 0x0BB8)``, and +#: both ``(0x4000, 0x4FFF)`` and ``(0x8000, 0xFFFF)`` after ``(0x0BB9, 0xFFFF)`` +#: -- which made those three branches of the generated ``_missing_`` +#: unreachable. Each subset range is now listed ahead of the wider range that +#: contains it, so every branch fires for at least the values only it covers. EXPECTED_RANGES = ( - (0x0001, 0x0BB8, 'Registered by Xerox'), (0x0020, 0x003F, 'Experimental'), - (0x0BB9, 0xFFFF, 'Dynamically Assigned'), + (0x0001, 0x0BB8, 'Registered by Xerox'), (0x4000, 0x4FFF, 'Dynamically Assigned Socket Numbers'), (0x8000, 0xFFFF, 'Statically Assigned Socket Numbers'), + (0x0BB9, 0xFFFF, 'Dynamically Assigned'), ) #: Sampled sockets and the member name each is expected to resolve to, at the @@ -97,27 +104,27 @@ #: only the name says which branch ran, and asserting the value alone would pass #: just as happily with three of the five branches deleted. #: -#: Three of them are in fact unreachable -- ``(0x0001, 0x0BB8)`` masks -#: ``(0x0020, 0x003F)``, and ``(0x0BB9, 0xFFFF)`` masks both -#: ``(0x4000, 0x4FFF)`` and ``(0x8000, 0xFFFF)``. That is preserved scrape -#: behaviour rather than a defect this change introduces, and it is documented on -#: :data:`pcapkit.vendor.ipx.socket.RANGES`; pinning the names is what makes that -#: documentation fail here if it ever stops being true. +#: Before GitHub issue #841 was fixed, three of these were unreachable -- +#: ``(0x0001, 0x0BB8)`` masked ``(0x0020, 0x003F)``, and ``(0x0BB9, 0xFFFF)`` +#: masked both ``(0x4000, 0x4FFF)`` and ``(0x8000, 0xFFFF)`` -- so ``0x0020``, +#: ``0x003F``, ``0x4000``, ``0x4FFF``, ``0x8000``, ``0x8061``, ``0x9094`` and +#: ``0xFFFF`` all resolved under the wrong wide range's name. See +#: :data:`pcapkit.vendor.ipx.socket.RANGES` for the reordering that fixed it. EXPECTED_MISSING_NAMES = { 0x0000: 'Unspecified', # a defined member 0x0001: 'Routing_Information_Packet', # a defined member 0x0004: 'Registered by Xerox_0x0004', - 0x0020: 'Registered by Xerox_0x0020', # masks 'Experimental' - 0x003F: 'Registered by Xerox_0x003F', # masks 'Experimental' + 0x0020: 'Experimental_0x0020', + 0x003F: 'Experimental_0x003F', 0x0BB8: 'Registered by Xerox_0x0BB8', 0x0BB9: 'Dynamically Assigned_0x0BB9', - 0x4000: 'Dynamically Assigned_0x4000', # masks 'Dynamically Assigned Socket Numbers' - 0x4FFF: 'Dynamically Assigned_0x4FFF', # masks 'Dynamically Assigned Socket Numbers' + 0x4000: 'Dynamically Assigned Socket Numbers_0x4000', + 0x4FFF: 'Dynamically Assigned Socket Numbers_0x4FFF', 0x7FFF: 'Dynamically Assigned_0x7FFF', - 0x8000: 'Dynamically Assigned_0x8000', # masks 'Statically Assigned Socket Numbers' - 0x8061: 'Dynamically Assigned_0x8061', # masks 'Statically Assigned Socket Numbers' - 0x9094: 'Dynamically Assigned_0x9094', # masks 'Statically Assigned Socket Numbers' - 0xFFFF: 'Dynamically Assigned_0xFFFF', # masks 'Statically Assigned Socket Numbers' + 0x8000: 'Statically Assigned Socket Numbers_0x8000', + 0x8061: 'Statically Assigned Socket Numbers_0x8061', + 0x9094: 'Statically Assigned Socket Numbers_0x9094', + 0xFFFF: 'Statically Assigned Socket Numbers_0xFFFF', } @@ -254,6 +261,23 @@ def test_unspecified_socket_survives_the_retirement(self) -> None: def test_range_order_is_preserved(self) -> None: self.assertEqual(tuple(self.vendor_module.RANGES), EXPECTED_RANGES) + def test_previously_shadowed_ranges_are_reachable(self) -> None: + # GitHub issue #841: under the old ``RANGES`` order each of these three + # values resolved as the wider range that masked its own -- 0x0030 as + # 'Registered by Xerox_0x0030', 0x4080 and 0x8100 both as + # 'Dynamically Assigned_0x...'. Pinned separately from + # test_unlisted_sockets_still_resolve so the regression this issue + # describes has a test that names it. + for value, name in ( + (0x0030, 'Experimental_0x0030'), + (0x4080, 'Dynamically Assigned Socket Numbers_0x4080'), + (0x8100, 'Statically Assigned Socket Numbers_0x8100'), + ): + with self.subTest(socket=f'0x{value:04X}'): + member = self.const_module.Socket(value) + self.assertEqual(int(member), value) + self.assertEqual(member.name, name) + def test_unlisted_sockets_still_resolve(self) -> None: # _missing_ has to cover the whole 16-bit space, so no legal wire value # raises -- and it has to reach the branch it looks like it reaches.