fix(ipx): order Socket._missing_ range branches narrowest-first

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)

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]]
 
 

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.