fix(corekit): resolve PortEnumField misses for any proto; restate its peek (#1774)

pcapkit/corekit/fields/numbers.py

@@ -850,12 +850,13 @@ def _unregistered_member(namespace: 'Type[StdlibEnum] | Type[AenumEnum]',
             this way, which is what keeps this safe to return from
             ``post_process``. A direct call to
             :meth:`~pcapkit.const.reg.apptype.apptype.AppType.get` for the same port
-            is a different matter and deliberately so: asking the
-            registry for a name is an explicit request for a named member, so
-            it still mints one -- measured on this tree,
-            ``AppType.get(54321, proto=tcp)`` returns ``PORT_54321_tcp`` and
-            takes ``TCP.__members__`` from 6147 to 6148, and a second call
-            with the same port returns that member rather than raising. Two
+            does not come through here, and it does not mint either: it
+            builds an unregistered member of its own, through the registry's
+            ``_unregistered_member`` -- measured on this tree,
+            ``AppType.get(54321, proto=tcp)`` returns
+            ``<TCP.unknown: 54321 [tcp]>``, ``TCP.__members__`` stays at
+            6147, and a second call with the same port returns an equal
+            member rather than the same one. Two
             unregistered members for the same value also compare equal
             without being identical, since
             :class:`~pcapkit.const.reg.apptype.apptype.AppType` and
@@ -1087,18 +1088,31 @@ def post_process(self, value: 'int | bytes', packet: 'dict[str, Any]') -> 'AppTy
             the port itself, when the registry declares none.
 
         Notes:
-            :meth:`~pcapkit.const.reg.apptype.apptype.AppType.get` mints a fresh
-            member -- via :func:`aenum.extend_enum` -- for any port neither an
-            existing row nor one of :meth:`_missing_`'s documented IANA spans
-            accounts for, which in practice means the ephemeral/dynamic range.
-            Calling it on every parsed port would therefore grow the registry
-            without bound. This peeks at the registry
+            This peeks at what
             :meth:`~pcapkit.const.reg.apptype.apptype.AppType.get` itself would
-            consult -- its per-port rows via ``__registry__.getlist``, then
-            its documented spans via ``_missing_`` -- and only calls
-            :meth:`~pcapkit.const.reg.apptype.apptype.AppType.get` once one of those is
-            already known to hold, so a genuine miss gets
-            :meth:`EnumField._unregistered_member` instead of a mint.
+            consult -- the per-port rows via ``__registry__.getlist``, then the
+            documented IANA spans via
+            :meth:`~pcapkit.const.reg.apptype.apptype.AppType._missing_` -- and
+            only calls ``get`` once one of those is known to hold. It is not a
+            guard against growth: ``get`` answers a port neither accounts for
+            with an unregistered member rather than minting one (GitHub issue
+            :issue:`860`), and the registry does not grow either way
+            (:issue:`1774`). It exists because that member is not the one a
+            parse needs. ``get`` builds it through
+            ``AppType._unregistered_member``, which installs no
+            ``__reduce_ex__``, so ``pickle.loads(pickle.dumps(...))`` raises
+            ``ValueError: 'unknown [50000 - tcp]' is not a valid TCP``; and its
+            ``.name`` is ``'unknown'``, so it dumps as ``TCP::unknown [...]``
+            rather than as the ``TCP::<unassigned> [...]`` of every other
+            unassigned wire value. A genuine miss therefore builds its member
+            through :meth:`EnumField._unregistered_member` instead.
+
+            That member carries the registry's own ``__transport__``, as
+            ``get``'s does, rather than ``self._proto``. The two are the same
+            member for each of the three schemas, but a composite built by hand
+            such as ``TransportProtocol.tcp | TransportProtocol.udp`` is a bare
+            :class:`int` (``3``, which dispatches to SCTP), and reading ``.name``
+            off it would raise :exc:`AttributeError` on a miss ``get`` resolves.
 
             A port outside this field's own width is rejected *before* any of
             that, rather than being let through to :meth:`_missing_` and
@@ -1149,8 +1163,11 @@ def post_process(self, value: 'int | bytes', packet: 'dict[str, Any]') -> 'AppTy
                 # DictDumper.object_hook renders a member's addon keys straight
                 # out of its ``__dict__`` in insertion order -- so any other
                 # order here would make an unassigned port dump ``port`` before
-                # ``svc`` while every declared one dumps ``svc`` first.
+                # ``svc`` while every declared one dumps ``svc`` first. The
+                # transport is ``owner.__transport__``, as AppType.get uses,
+                # since ``proto`` may be a bare int with no ``.name`` (#1774).
+                transport = owner.__transport__
                 return self._unregistered_member(
-                    owner, f'unknown [{value:d} - {proto.name}]',
-                    svc='unknown', port=value, proto=proto)
+                    owner, f'unknown [{value:d} - {transport.name}]',
+                    svc='unknown', port=value, proto=transport)
         return self._namespace.get(value, proto=proto)

tests/corekit/test_fields_numbers_port_enum_field_peek_1774_unit.py

@@ -0,0 +1,167 @@
+# -*- coding: utf-8 -*-
+"""Why :meth:`~pcapkit.corekit.fields.numbers.PortEnumField.post_process` still peeks.
+
+GitHub issue #1774. The field looks a port up in the registry rows and in
+``AppType._missing_``'s spans before calling ``AppType.get``, so that a genuine
+miss builds its member through ``EnumField._unregistered_member``. That peek was
+written to stop ``get`` minting a member per ephemeral port. Since #860 ``get``
+no longer mints, so the growth it guarded against is gone; but the member ``get``
+builds on a miss is still not the one a parse needs. These tests pin both
+halves, so that the peek's documented reason fails here the day it stops being
+true, and pin that a composite ``proto`` resolves a miss as ``get`` does.
+
+"""
+import pickle
+import unittest
+from typing import TYPE_CHECKING
+
+from tests._support import reimport_once_per_class
+
+if TYPE_CHECKING:
+    from typing import Any
+
+#: An unassigned port on all four transports, in none of ``_missing_``'s spans.
+PORT = 50000
+
+
+class PortEnumFieldPeekTests(unittest.TestCase):
+    """The peek's reason, over every transport registry."""
+
+    def setUp(self) -> None:
+        reimport_once_per_class(self)
+
+    def _protos(self) -> 'tuple[Any, ...]':
+        from pcapkit.const.reg.apptype import TransportProtocol
+
+        return (TransportProtocol.tcp, TransportProtocol.udp,
+                TransportProtocol.sctp, TransportProtocol.dccp)
+
+    def _field(self, proto: 'Any') -> 'Any':
+        from pcapkit.const.reg.apptype import AppType
+        from pcapkit.corekit.fields.numbers import PortEnumField
+
+        return PortEnumField(length=2, namespace=AppType, proto=proto)
+
+    @staticmethod
+    def _tables(owner: 'Any') -> 'tuple[Any, ...]':
+        by_value = dict(owner._value2member_map_)  # pylint: disable=protected-access
+        rows = [(port, list(owner.__registry__.getlist(port))) for port in owner.__registry__]
+        return (dict(owner.__members__), by_value, rows)
+
+    def test_neither_the_field_nor_get_grows_the_registry(self) -> None:
+        """The peek's original reason is gone: ``get`` itself no longer mints."""
+        from pcapkit.const.reg.apptype import AppType
+
+        for proto in self._protos():
+            with self.subTest(proto=proto.name):
+                owner = AppType.__registries__[proto]
+                before = self._tables(owner)
+
+                via_get = AppType.get(PORT, proto=proto)
+                via_field = self._field(proto).unpack(PORT.to_bytes(2, 'big'), {})
+
+                self.assertEqual(self._tables(owner), before)
+                for member in (via_get, via_field):
+                    self.assertIsInstance(member, owner)
+                    self.assertEqual((member.svc, member.port, member.proto),
+                                     ('unknown', PORT, proto))
+
+    def test_a_miss_through_the_field_pickles_and_renders_as_unassigned(self) -> None:
+        """What the peek buys: the member a parse returns survives pickling and
+        dumps as ``<unassigned>``, like every other unassigned wire value."""
+        from pcapkit.const.reg.apptype import AppType
+        from pcapkit.dumpkit.common import render_enum
+
+        for proto in self._protos():
+            owner = AppType.__registries__[proto]
+            member = self._field(proto).unpack(PORT.to_bytes(2, 'big'), {})
+            with self.subTest(proto=proto.name):
+                self.assertEqual(member.name, '<unassigned>')
+                expected = f'{owner.__name__}::<unassigned> [unknown [{PORT} - {proto.name}]]'
+                self.assertEqual(render_enum(member), expected)
+            for protocol in range(pickle.HIGHEST_PROTOCOL + 1):
+                with self.subTest(proto=proto.name, protocol=protocol):
+                    restored = pickle.loads(pickle.dumps(member, protocol))
+                    self.assertIsInstance(restored, owner)
+                    self.assertEqual((restored.svc, restored.port, restored.proto),
+                                     ('unknown', PORT, proto))
+
+    def test_gets_own_member_for_a_miss_is_not_that_member(self) -> None:
+        """Why the field cannot just call ``get``: its miss member has no reducer
+        and another name. If this starts failing, ``get``'s member has become
+        what a parse needs, and the peek can go -- see #1774."""
+        from pcapkit.const.reg.apptype import AppType
+
+        for proto in self._protos():
+            with self.subTest(proto=proto.name):
+                member = AppType.get(PORT, proto=proto)
+
+                self.assertEqual(member.name, 'unknown')
+                with self.assertRaises(ValueError):
+                    pickle.loads(pickle.dumps(member))
+
+    def test_a_composite_proto_resolves_a_miss_as_get_does(self) -> None:
+        """``tcp | udp`` is a bare :class:`int` (``3``, which is SCTP's value).
+
+        ``get`` dispatches it to SCTP for a hit and a miss alike. The field's
+        own miss branch used to read ``.name`` off it, raising
+        :exc:`AttributeError` for a port ``get`` resolves; it now takes the
+        transport from the registry, as ``get`` does.
+
+        """
+        from pcapkit.const.reg.apptype import AppType, TransportProtocol
+
+        for label, proto in (('tcp|udp', TransportProtocol.tcp | TransportProtocol.udp),
+                             ('int 1', 1)):
+            with self.subTest(proto=label):
+                self.assertIsInstance(proto, int)
+                self.assertNotIsInstance(proto, TransportProtocol)
+                field = self._field(proto)
+                expected = AppType.get(PORT, proto=proto)
+                owner = type(expected)
+                before = self._tables(owner)
+
+                miss = field.unpack(PORT.to_bytes(2, 'big'), {})
+                hit = field.unpack(b'\x00\x50', {})  # 80
+
+                self.assertIsInstance(miss, owner)
+                self.assertEqual((miss.svc, miss.port, miss.proto),
+                                 (expected.svc, expected.port, expected.proto))
+                self.assertEqual(miss.name, '<unassigned>')
+                self.assertIs(hit, AppType.get(80, proto=proto))
+                self.assertEqual(self._tables(owner), before)
+
+    def test_a_str_proto_resolves_a_miss_as_get_does(self) -> None:
+        """``'tcp'`` names its registry, as ``get`` lets it, hit and miss alike."""
+        from pcapkit.const.reg.apptype import AppType, TransportProtocol
+
+        tcp = AppType.__registries__[TransportProtocol.tcp]
+        field = self._field('tcp')
+        before = self._tables(tcp)
+
+        miss = field.unpack(PORT.to_bytes(2, 'big'), {})
+        hit = field.unpack(b'\x00\x50', {})  # 80
+
+        self.assertIsInstance(miss, tcp)
+        self.assertEqual((miss.svc, miss.port, miss.proto),
+                         ('unknown', PORT, TransportProtocol.tcp))
+        self.assertEqual(miss.name, '<unassigned>')
+        self.assertIs(hit, AppType.get(80, proto='tcp'))
+        self.assertEqual(self._tables(tcp), before)
+
+    def test_a_composite_naming_no_registry_is_refused_as_get_refuses_it(self) -> None:
+        """``tcp | dccp`` is ``5``, which names no registry, hit or miss."""
+        from pcapkit.const.reg.apptype import AppType, TransportProtocol
+
+        proto = TransportProtocol.tcp | TransportProtocol.dccp
+        field = self._field(proto)
+        for port in (80, PORT):
+            with self.subTest(port=port):
+                with self.assertRaises(ValueError):
+                    AppType.get(port, proto=proto)
+                with self.assertRaises(ValueError):
+                    field.unpack(port.to_bytes(2, 'big'), {})
+
+
+if __name__ == '__main__':
+    unittest.main()