fix(const): reject an out-of-range port in AppType.register (#1792)

docs/source/contributing/conventions/mint-criterion.rst

@@ -10,7 +10,7 @@ a ``_missing_`` override, which decides what happens when a value has no member.
 MRO, so they carry no declared-but-unassigned range to resolve. 121 define one
 themselves; the other four are the transport subclasses of
 :class:`~pcapkit.const.reg.apptype.apptype.AppType`, which inherit
-``AppType._missing_`` (:file:`pcapkit/const/reg/apptype/apptype.py:2958`). Three
+``AppType._missing_`` (:file:`pcapkit/const/reg/apptype/apptype.py:2994`). Three
 behaviours follow from it, and which one a given range gets is a **design decision, not
 a style preference**:
 

pcapkit/const/reg/apptype/apptype.py

@@ -17,6 +17,7 @@
 
 from pcapkit.corekit.enum import EnumLookup, EnumRegistry
 from pcapkit.corekit.sentinels import NO_DEFAULT
+from pcapkit.utilities.exceptions import EnumValueError
 
 __all__ = ['TransportProtocol', 'AppType']
 
@@ -2773,6 +2774,27 @@ def _sanitize_identifier(cls, name: 'str') -> 'str':
             identifier = f'{identifier}_'
         return identifier
 
+    @classmethod
+    def _validate_value(cls, value: 'Any') -> 'None':
+        """Reject ``value`` unless it is a port number in ``0``-``65535``.
+
+        Overrides :meth:`~pcapkit.corekit.enum.EnumLookup._validate_value`
+        with the check :meth:`_missing_` makes, and the message it raises, so
+        :meth:`register` refuses exactly the ports :meth:`get` refuses, in the
+        same words (:issue:`1792`).
+
+        Args:
+            value: Port number about to be minted.
+
+        Raises:
+            EnumValueError: If ``value`` is not an :class:`int` in
+                ``0``-``65535``. A :exc:`ValueError`, so an ``except
+                ValueError`` caller is unaffected.
+
+        """
+        if not (isinstance(value, int) and 0 <= value <= 65535):
+            raise EnumValueError(f'{value!r} is not a valid {cls.__name__}')
+
     @classmethod
     def register(cls, port: 'int', name: 'str') -> 'AppType':  # pylint: disable=arguments-renamed
         """Mint the first, canonical member at ``port``, in this registry alone.
@@ -2793,6 +2815,10 @@ def register(cls, port: 'int', name: 'str') -> 'AppType':  # pylint: disable=arg
         the operation where guessing the right transport for the caller is
         the wrong default.
 
+        Routes ``port`` through :meth:`_validate_value` after the duplicate
+        check, as :meth:`~pcapkit.corekit.enum.EnumRegistry.register` does, so
+        a port :meth:`get` refuses cannot be minted here either.
+
         Args:
             port: Port number to register under. Must not already carry a
                 member of this registry -- see :meth:`register_alias` for
@@ -2811,6 +2837,10 @@ def register(cls, port: 'int', name: 'str') -> 'AppType':  # pylint: disable=arg
                 is not a valid Python identifier at all; or if the identifier
                 it does sanitise to already names an unrelated member of this
                 registry.
+            EnumValueError: If ``port`` is not an :class:`int` in
+                ``0``-``65535``, from :meth:`_validate_value`. A
+                :exc:`ValueError`, carrying the message :meth:`get` raises for
+                the same port.
 
         """
         if cls.__registry__ is None:
@@ -2823,6 +2853,7 @@ def register(cls, port: 'int', name: 'str') -> 'AppType':  # pylint: disable=arg
                              f'({", ".join(member.svc for member in existing)}); use '
                              f'{cls.__name__}.register_alias() to add a further service name '
                              'for it')
+        cls._validate_value(port)
 
         identifier = cls._sanitize_identifier(name)
         if not identifier.isidentifier():

pcapkit/corekit/enum.py

@@ -218,9 +218,9 @@ def _validate_value(cls, value: 'Any') -> 'None':
         Of the registries that override :meth:`EnumRegistry.register` to mint
         through a path of their own,
         :meth:`OptionType.register <pcapkit.const.pcapng.option_type.OptionType.register>`
-        calls it at the same point and
+        and
         :meth:`AppType.register <pcapkit.const.reg.apptype.apptype.AppType.register>`
-        does not.
+        both call it at the same point.
 
         :meth:`EnumRegistry.register_alias` does not call it, and does not need
         to: it refuses any ``value`` that is not already registered, so the value

pcapkit/vendor/reg/apptype/apptype.py

@@ -110,6 +110,7 @@
 
 from pcapkit.corekit.enum import EnumLookup, EnumRegistry
 from pcapkit.corekit.sentinels import NO_DEFAULT
+from pcapkit.utilities.exceptions import EnumValueError
 
 __all__ = ['TransportProtocol', '{NAME}']
 
@@ -728,6 +729,27 @@ def _sanitize_identifier(cls, name: 'str') -> 'str':
             identifier = f'{{identifier}}_'
         return identifier
 
+    @classmethod
+    def _validate_value(cls, value: 'Any') -> 'None':
+        """Reject ``value`` unless it is a port number in ``0``-``65535``.
+
+        Overrides :meth:`~pcapkit.corekit.enum.EnumLookup._validate_value`
+        with the check :meth:`_missing_` makes, and the message it raises, so
+        :meth:`register` refuses exactly the ports :meth:`get` refuses, in the
+        same words (:issue:`1792`).
+
+        Args:
+            value: Port number about to be minted.
+
+        Raises:
+            EnumValueError: If ``value`` is not an :class:`int` in
+                ``0``-``65535``. A :exc:`ValueError`, so an ``except
+                ValueError`` caller is unaffected.
+
+        """
+        if not ({FLAG}):
+            raise EnumValueError(f'{{value!r}} is not a valid {{cls.__name__}}')
+
     @classmethod
     def register(cls, port: 'int', name: 'str') -> '{NAME}':  # pylint: disable=arguments-renamed
         """Mint the first, canonical member at ``port``, in this registry alone.
@@ -748,6 +770,10 @@ def register(cls, port: 'int', name: 'str') -> '{NAME}':  # pylint: disable=argu
         the operation where guessing the right transport for the caller is
         the wrong default.
 
+        Routes ``port`` through :meth:`_validate_value` after the duplicate
+        check, as :meth:`~pcapkit.corekit.enum.EnumRegistry.register` does, so
+        a port :meth:`get` refuses cannot be minted here either.
+
         Args:
             port: Port number to register under. Must not already carry a
                 member of this registry -- see :meth:`register_alias` for
@@ -766,6 +792,10 @@ def register(cls, port: 'int', name: 'str') -> '{NAME}':  # pylint: disable=argu
                 is not a valid Python identifier at all; or if the identifier
                 it does sanitise to already names an unrelated member of this
                 registry.
+            EnumValueError: If ``port`` is not an :class:`int` in
+                ``0``-``65535``, from :meth:`_validate_value`. A
+                :exc:`ValueError`, carrying the message :meth:`get` raises for
+                the same port.
 
         """
         if cls.__registry__ is None:
@@ -778,6 +808,7 @@ def register(cls, port: 'int', name: 'str') -> '{NAME}':  # pylint: disable=argu
                              f'({{", ".join(member.svc for member in existing)}}); use '
                              f'{{cls.__name__}}.register_alias() to add a further service name '
                              'for it')
+        cls._validate_value(port)
 
         identifier = cls._sanitize_identifier(name)
         if not identifier.isidentifier():

tests/const/test_const_apptype_register_range_1792_unit.py

@@ -0,0 +1,131 @@
+# -*- coding: utf-8 -*-
+"""``AppType.register`` refuses a port that ``get`` refuses.
+
+GitHub issue #1792. :meth:`~pcapkit.const.reg.apptype.apptype.AppType.register`
+neither called nor overrode
+:meth:`~pcapkit.corekit.enum.EnumLookup._validate_value`, so ``TCP.register(70000,
+'x')`` minted a member, and ``AppType.get(70000, proto='tcp')`` then returned it
+although ``get`` raises :exc:`ValueError` for a port outside ``0``-``65535``
+(#758). ``AppType`` now overrides the hook with :meth:`_missing_`'s own check and
+``register`` calls it after its duplicate check, as the base ``register`` does.
+The override raises :exc:`~pcapkit.utilities.exceptions.EnumValueError`, as the
+registry-protocol convention asks of one, with the message ``get`` raises.
+
+Each test imports the registries after
+:func:`~tests._support.reimport_once_per_class`, whose ``restore`` puts the
+class's import back afterwards, so a member minted here does not leak.
+
+"""
+from __future__ import annotations
+
+import itertools
+import unittest
+
+from tests._support import reimport_once_per_class
+
+#: Serial for probe names, so that no refusal is a name collision instead.
+_SERIAL = itertools.count()
+
+#: Ports outside ``0``-``65535``, on both sides of the range.
+OUT_OF_RANGE = (-1, 65536, 70000)
+
+#: Ports that are not an :class:`int` at all, and hash like no registered port.
+NOT_AN_INT = ('8080', 1.5)
+
+
+def _registries() -> 'tuple[type, ...]':
+    from pcapkit.const.reg.apptype import DCCP, SCTP, TCP, UDP
+
+    return TCP, UDP, SCTP, DCCP
+
+
+class AppTypeRegisterRange1792Tests(unittest.TestCase):
+    """``register`` and ``get`` agree on what a port number is."""
+
+    def setUp(self) -> None:
+        reimport_once_per_class(self, restore=True)
+
+    def assertRefusedWithoutMinting(self, registry: 'type', port: 'object') -> 'str':
+        """Assert ``registry.register(port, ...)`` raises and mints nothing.
+
+        The refusal is an :exc:`~pcapkit.utilities.exceptions.EnumValueError`,
+        and so still a :exc:`ValueError` to an ``except ValueError`` caller.
+
+        Returns:
+            The message the refusal carried.
+
+        """
+        from pcapkit.utilities.exceptions import EnumValueError
+
+        members = dict(registry.__members__)  # type: ignore[attr-defined]
+        with self.assertRaises(ValueError) as caught:
+            registry.register(port, f'pypcapkit-1792-probe-{next(_SERIAL)}')  # type: ignore[attr-defined]
+        self.assertIsInstance(caught.exception, ValueError)
+        self.assertIs(type(caught.exception), EnumValueError)
+        self.assertEqual(dict(registry.__members__), members)  # type: ignore[attr-defined]
+        self.assertFalse(registry.__registry__.getlist(port))  # type: ignore[attr-defined]
+        return str(caught.exception)
+
+    def test_register_refuses_an_out_of_range_port_as_get_does(self) -> None:
+        for registry in _registries():
+            for port in OUT_OF_RANGE:
+                with self.subTest(registry=registry.__name__, port=port):
+                    message = self.assertRefusedWithoutMinting(registry, port)
+                    with self.assertRaises(ValueError) as lookup:
+                        registry.get(port)  # type: ignore[attr-defined]
+                    self.assertEqual(message, str(lookup.exception))
+                    self.assertEqual(message, f'{port!r} is not a valid {registry.__name__}')
+
+    def test_register_refuses_a_non_int_port_as_the_constructor_does(self) -> None:
+        for registry in _registries():
+            for port in NOT_AN_INT:
+                with self.subTest(registry=registry.__name__, port=port):
+                    message = self.assertRefusedWithoutMinting(registry, port)
+                    with self.assertRaises(ValueError) as built:
+                        registry(port)
+                    self.assertEqual(message, str(built.exception))
+
+    def test_get_still_raises_after_a_refused_register(self) -> None:
+        from pcapkit.const.reg.apptype import AppType
+
+        for registry in _registries():
+            for port in OUT_OF_RANGE:
+                with self.subTest(registry=registry.__name__, port=port):
+                    with self.assertRaises(ValueError):
+                        registry.register(port, f'pypcapkit-1792-probe-{next(_SERIAL)}')  # type: ignore[attr-defined]
+                    with self.assertRaises(ValueError):
+                        registry.get(port)  # type: ignore[attr-defined]
+                    with self.assertRaises(ValueError):
+                        AppType.get(port, proto=registry.__transport__)  # type: ignore[attr-defined]
+
+    def test_register_still_mints_the_top_port_of_the_range(self) -> None:
+        from pcapkit.const.reg.apptype import AppType
+
+        port = 65535
+        for registry in _registries():
+            with self.subTest(registry=registry.__name__):
+                self.assertFalse(registry.__registry__.getlist(port))  # type: ignore[attr-defined]
+                member = registry.register(port, 'pypcapkit-1792-top')  # type: ignore[attr-defined]
+                self.assertEqual(member.port, port)
+                self.assertIs(member.proto, registry.__transport__)  # type: ignore[attr-defined]
+                self.assertIs(registry.get(port), member)  # type: ignore[attr-defined]
+                self.assertIs(AppType.get(port, proto=registry.__transport__), member)  # type: ignore[attr-defined]
+
+    def test_validate_value_agrees_with_missing_on_every_probe(self) -> None:
+        from pcapkit.utilities.exceptions import EnumValueError
+
+        for registry in _registries():
+            for port in (0, 65535, *OUT_OF_RANGE, *NOT_AN_INT):
+                with self.subTest(registry=registry.__name__, port=port):
+                    try:
+                        registry._missing_(port)  # type: ignore[attr-defined]
+                    except ValueError as error:
+                        with self.assertRaises(EnumValueError) as caught:
+                            registry._validate_value(port)  # type: ignore[attr-defined]
+                        self.assertEqual(str(caught.exception), str(error))
+                    else:
+                        self.assertIsNone(registry._validate_value(port))  # type: ignore[attr-defined]
+
+
+if __name__ == '__main__':
+    unittest.main()

tests/project/test_rfc_anchor_fragments.py

@@ -41,7 +41,7 @@
 test exists to catch, the same mistake as the allowlist above one layer down.
 The generators that produce them,
 :file:`pcapkit/vendor/tcp/mp_tcp_option.py:59` and
-:file:`pcapkit/vendor/reg/apptype/apptype.py:1169` (both parsing IANA's
+:file:`pcapkit/vendor/reg/apptype/apptype.py:1200` (both parsing IANA's
 ``RFC8684, Section 3.1`` with the same ``re.fullmatch(r'RFC(?P<rfc>\\d+)(, Section
 (?P<sec>.*?))?', ...)``, which discards the literal word ``Section`` and
 keeps only the number), are fixed to emit ``#section-{sec}``, and the ten