diff --git a/docs/source/pcapkit/protocols/internet/internet.rst b/docs/source/pcapkit/protocols/internet/internet.rst index c49cbf69d6..4861661ff5 100644 --- a/docs/source/pcapkit/protocols/internet/internet.rst +++ b/docs/source/pcapkit/protocols/internet/internet.rst @@ -28,6 +28,8 @@ which is a base class for internet layer protocols, e.g. :class:`~pcapkit.protoc :no-value: .. autoattribute:: __proto__ :no-value: + .. autoattribute:: __proto_key__ + :no-value: .. autoattribute:: _exthdr_depth .. autodata:: pcapkit.protocols.internet.internet._exthdr_rebuilt diff --git a/docs/source/pcapkit/protocols/link/link.rst b/docs/source/pcapkit/protocols/link/link.rst index 879eed0b4e..203f94efdd 100644 --- a/docs/source/pcapkit/protocols/link/link.rst +++ b/docs/source/pcapkit/protocols/link/link.rst @@ -22,3 +22,5 @@ and :class:`~pcapkit.protocols.link.vlan.VLAN`. .. autoattribute:: __layer__ .. autoattribute:: __proto__ :no-value: + .. autoattribute:: __proto_key__ + :no-value: diff --git a/docs/source/pcapkit/protocols/misc/pcap.rst b/docs/source/pcapkit/protocols/misc/pcap.rst index 67e3f32d57..57041c2f99 100644 --- a/docs/source/pcapkit/protocols/misc/pcap.rst +++ b/docs/source/pcapkit/protocols/misc/pcap.rst @@ -143,6 +143,8 @@ whose structure is described as below: .. autoattribute:: __proto__ :no-value: + .. autoattribute:: __proto_key__ + :no-value: .. automethod:: __post_init__ .. automethod:: __index__ diff --git a/docs/source/pcapkit/protocols/misc/pcapng.rst b/docs/source/pcapkit/protocols/misc/pcapng.rst index cd0bac3eea..9d33b67751 100644 --- a/docs/source/pcapkit/protocols/misc/pcapng.rst +++ b/docs/source/pcapkit/protocols/misc/pcapng.rst @@ -194,6 +194,8 @@ which implements an extractor for PCAP-NG file format [*]_, and the .. autoattribute:: __proto__ :no-value: + .. autoattribute:: __proto_key__ + :no-value: .. autoattribute:: __block__ :no-value: .. autoattribute:: __option__ diff --git a/docs/source/pcapkit/protocols/transport/sctp.rst b/docs/source/pcapkit/protocols/transport/sctp.rst index a394c1611b..4bce600335 100644 --- a/docs/source/pcapkit/protocols/transport/sctp.rst +++ b/docs/source/pcapkit/protocols/transport/sctp.rst @@ -139,6 +139,8 @@ Octets Bits Name Description .. autoattribute:: __proto__ :no-value: + .. autoattribute:: __proto_key__ + :no-value: .. autoattribute:: __chunk__ :no-value: .. autoattribute:: __parameter__ diff --git a/docs/source/pcapkit/protocols/transport/tcp.rst b/docs/source/pcapkit/protocols/transport/tcp.rst index 50aab04830..08694b8c2b 100644 --- a/docs/source/pcapkit/protocols/transport/tcp.rst +++ b/docs/source/pcapkit/protocols/transport/tcp.rst @@ -131,6 +131,8 @@ Octets Bits Name Description .. autoattribute:: __proto__ :no-value: + .. autoattribute:: __proto_key__ + :no-value: .. autoattribute:: __option__ :no-value: .. autoattribute:: __mp_option__ diff --git a/docs/source/pcapkit/protocols/transport/udp.rst b/docs/source/pcapkit/protocols/transport/udp.rst index 17986ccf63..2f7777ff8f 100644 --- a/docs/source/pcapkit/protocols/transport/udp.rst +++ b/docs/source/pcapkit/protocols/transport/udp.rst @@ -33,6 +33,8 @@ Octets Bits Name Description .. autoattribute:: __proto__ :no-value: + .. autoattribute:: __proto_key__ + :no-value: .. automethod:: __index__ diff --git a/pcapkit/foundation/registry/protocols.py b/pcapkit/foundation/registry/protocols.py index fed3cc25ab..a910e76193 100644 --- a/pcapkit/foundation/registry/protocols.py +++ b/pcapkit/foundation/registry/protocols.py @@ -423,25 +423,6 @@ def _register_name(protocol: 'Type[ProtocolBase]', *targets: 'tuple[type, Any]') Enum_LinkType: (Frame, PCAPNG), } -#: Key type of each ``__proto__`` dispatch registry a ``code=`` target can name. -#: -#: Maps the class owning the registry to the enum its typed registrar checks -#: ``code`` against, and whether a bare :class:`int` is a valid key there too -- -#: the same :func:`_check_code` arguments that registrar passes. A destination is -#: looked up by the class it inherits ``__proto__`` from, so a subclass sharing -#: :class:`Link`'s registry is checked as :class:`Link` is, while one keeping a -#: registry of its own, such as :class:`~pcapkit.protocols.link.loopback.Loopback`, -#: names no entry here and is not checked (:issue:`2268`). -_DESTINATION_KINDS: 'dict[Type[ProtocolBase], tuple[Type[EnumRegistry], bool]]' = { - Link: (Enum_EtherType, False), # type: ignore[type-abstract] - Internet: (Enum_TransType, False), # type: ignore[type-abstract] - Frame: (Enum_LinkType, False), - PCAPNG: (Enum_LinkType, False), - TCP: (Enum_AppType, True), - UDP: (Enum_AppType, True), - SCTP: (Enum_PayloadProtocolIdentifier, True), -} - def _iter_code_targets(code: 'Any') -> 'Iterator[tuple[Type[ProtocolBase], Any]]': """Flatten a ``code=`` argument into ``(destination, key)`` pairs. @@ -549,18 +530,34 @@ def register_protocol_code(protocol: 'Type[ProtocolBase]', code: 'Any') -> 'None an enum member's type names no known destination registry -- inference refuses rather than guesses; or if a key is not of the type its destination's registry is keyed by, as its typed registrar - would reject it, e.g. ``{Frame: TransType.ICMP}``. Every target is - checked before any is registered, so a rejected ``code`` leaves - every registry as it was (:issue:`2268`). + would reject it, e.g. ``{Frame: TransType.ICMP}``. What a registry is + keyed by is read from the + :attr:`__proto_key__ ` + declared by the class owning it, so a destination sharing another's + registry is checked as its owner is, and one whose owner declares no + pairing is not checked. Every target is checked before any is + registered, so a rejected ``code`` leaves every registry as it was + (:issue:`2268`). """ targets = list(_iter_code_targets(code)) for destination, key in targets: owner = next((klass for klass in getattr(destination, '__mro__', ()) if '__proto__' in vars(klass)), None) - if owner in _DESTINATION_KINDS: - kind, number = _DESTINATION_KINDS[owner] - _check_code(key, kind, number=number) + if owner is None: + continue + + # NOTE: the pairing is read out of the owner's own ``vars()`` rather than + # with ``getattr``, which walks the MRO and would hand a class keeping a + # registry of its own its parent's pairing -- + # :attr:`Loopback.__proto__ ` + # is keyed by address family rather than by EtherType, so inheriting + # :class:`Link`'s would reject every key it does accept. A class is + # checked against the registry it owns, or not at all. + pairing = vars(owner).get('__proto_key__') + if pairing is not None: + kind, number = pairing + _check_code(key, kind, owner, number=number) for destination, key in targets: destination.register(key, protocol) @@ -568,7 +565,8 @@ def register_protocol_code(protocol: 'Type[ProtocolBase]', code: 'Any') -> 'None destination.__name__, key) -def _check_code(code: 'Any', kind: 'type', *, number: 'bool' = False) -> 'None': +def _check_code(code: 'Any', kind: 'type', target: 'Type[ProtocolBase]', *, + number: 'bool' = False) -> 'None': """Reject a registration key that is not a member of the registrar's enum. Each registrar typed on one enum calls this before touching any registry, @@ -582,6 +580,10 @@ def _check_code(code: 'Any', kind: 'type', *, number: 'bool' = False) -> 'None': Args: code: Registration key handed to the registrar. kind: Enum the registrar is typed on. + target: Protocol class whose registry ``code`` is destined for, named in + the error message so a rejection says *whose* registry rejected the + key rather than only what it wanted -- the registrars share one + message, and several of them are typed on the same enum. number: Whether a bare :class:`int` is a valid key too, as for the registrars keyed on a port number or a payload protocol identifier. @@ -600,8 +602,9 @@ def _check_code(code: 'Any', kind: 'type', *, number: 'bool' = False) -> 'None': return expected = f'an int or a member of {kind.__name__}' if number else f'a member of {kind.__name__}' if member: - raise RegistryError(f'code must be {expected}, not a member of {type(code).__name__}: {code!r}') - raise RegistryError(f'code must be {expected}, not {code!r}') + raise RegistryError(f'{target.__name__}: code must be {expected}, not a member of ' + f'{type(code).__name__}: {code!r}') + raise RegistryError(f'{target.__name__}: code must be {expected}, not {code!r}') ############################################################################### @@ -646,7 +649,10 @@ def register_linktype(code: 'LinkType', module: 'str | ModuleDescriptor[Protocol subclass. """ - _check_code(code, Enum_LinkType) + # NOTE: this writes into both Frame's and PCAP-NG's registries, which are + # keyed alike, so the key check is the same for either and Frame names it. + # :func:`register_pcap` and :func:`register_pcapng` name one each. + _check_code(code, Enum_LinkType, Frame) if isinstance(module, str): module = cast('ModuleDescriptor[ProtocolBase]', ModuleDescriptor(module, class_)) @@ -691,7 +697,7 @@ def register_pcap(code: 'LinkType', module: 'str | ModuleDescriptor[ProtocolBase subclass. """ - _check_code(code, Enum_LinkType) + _check_code(code, Enum_LinkType, Frame) if isinstance(module, str): module = cast('ModuleDescriptor[ProtocolBase]', ModuleDescriptor(module, class_)) @@ -735,7 +741,7 @@ def register_pcapng(code: 'LinkType', module: 'str | ModuleDescriptor[ProtocolBa subclass. """ - _check_code(code, Enum_LinkType) + _check_code(code, Enum_LinkType, PCAPNG) if isinstance(module, str): module = cast('ModuleDescriptor[ProtocolBase]', ModuleDescriptor(module, class_)) @@ -784,7 +790,7 @@ def register_ethertype(code: 'EtherType', module: 'str | ModuleDescriptor[Protoc subclass. """ - _check_code(code, Enum_EtherType) + _check_code(code, Enum_EtherType, Link) # type: ignore[type-abstract] if isinstance(module, str): module = cast('ModuleDescriptor[ProtocolBase]', ModuleDescriptor(module, class_)) @@ -833,7 +839,7 @@ def register_transtype(code: 'TransType', module: 'str | ModuleDescriptor[Protoc subclass. """ - _check_code(code, Enum_TransType) + _check_code(code, Enum_TransType, Internet) # type: ignore[type-abstract] if isinstance(module, str): module = cast('ModuleDescriptor[ProtocolBase]', ModuleDescriptor(module, class_)) @@ -870,7 +876,7 @@ def register_ipv4_option(code: 'IPv4_OptionNumber', meth: 'str | tuple[IPv4_Opti is a name with no matching ``_read_opt_*`` method. """ - _check_code(code, IPv4_OptionNumber) + _check_code(code, IPv4_OptionNumber, IPv4) if isinstance(meth, str) and not hasattr(IPv4, f'_read_opt_{meth}'): raise RegistryError('method must be a valid IPv4 option parser function') @@ -904,7 +910,7 @@ def register_hip_parameter(code: 'HIP_Parameter', meth: 'str | tuple[HIP_Paramet is a name with no matching ``_read_param_*`` method. """ - _check_code(code, HIP_Parameter) + _check_code(code, HIP_Parameter, HIP) if isinstance(meth, str) and not hasattr(HIP, f'_read_param_{meth}'): raise RegistryError('method must be a valid HIP parameter parser function') @@ -938,7 +944,7 @@ def register_hopopt_option(code: 'IPv6_Option', meth: 'str | tuple[HOPOPT_Option is a name with no matching ``_read_opt_*`` method. """ - _check_code(code, IPv6_Option) + _check_code(code, IPv6_Option, HOPOPT) if isinstance(meth, str) and not hasattr(HOPOPT, f'_read_opt_{meth}'): raise RegistryError('method must be a valid HOPOPT option parser function') @@ -972,7 +978,7 @@ def register_ipv6_opts_option(code: 'IPv6_Option', meth: 'str | tuple[IPv6_Opts_ is a name with no matching ``_read_opt_*`` method. """ - _check_code(code, IPv6_Option) + _check_code(code, IPv6_Option, IPv6_Opts) if isinstance(meth, str) and not hasattr(IPv6_Opts, f'_read_opt_{meth}'): raise RegistryError('method must be a valid IPv6-Opts option parser function') @@ -1006,7 +1012,7 @@ def register_ipv6_route_routing(code: 'IPv6_Routing', meth: 'str | tuple[IPv6_Ro is a name with no matching ``_read_data_type_*`` method. """ - _check_code(code, IPv6_Routing) + _check_code(code, IPv6_Routing, IPv6_Route) if isinstance(meth, str) and not hasattr(IPv6_Route, f'_read_data_type_{meth}'): raise RegistryError('method must be a valid IPv6-Route routing data parser function') @@ -1040,7 +1046,7 @@ def register_mh_message(code: 'MH_Packet', meth: 'str | tuple[MH_PacketParser, M is a name with no matching ``_read_msg_*`` method. """ - _check_code(code, MH_Packet) + _check_code(code, MH_Packet, MH) if isinstance(meth, str) and not hasattr(MH, f'_read_msg_{meth}'): raise RegistryError('method must be a valid MH message type parser function') @@ -1074,7 +1080,7 @@ def register_mh_option(code: 'MH_Option', meth: 'str | tuple[MH_OptionParser, MH is a name with no matching ``_read_opt_*`` method. """ - _check_code(code, MH_Option) + _check_code(code, MH_Option, MH) if isinstance(meth, str) and not hasattr(MH, f'_read_opt_{meth}'): raise RegistryError('method must be a valid MH option parser function') @@ -1113,7 +1119,7 @@ def register_mh_extension(code: 'MH_CGAExtension', meth: 'str | tuple[MH_Extensi that take one (:issue:`516`). """ - _check_code(code, MH_CGAExtension) + _check_code(code, MH_CGAExtension, MH) if isinstance(meth, str) and not hasattr(MH, f'_read_ext_{meth}'): raise RegistryError('method must be a valid MH CGA extension function') @@ -1227,7 +1233,12 @@ def register_apptype(code: 'int | Enum_AppType', module: 'str | ModuleDescriptor protocol per argument, with no alias (:issue:`806`). """ - _check_code(code, Enum_AppType, number=True) + # NOTE: which of TCP's and UDP's registries this writes into depends on + # ``*transport``, which is resolved below -- after this check, so a bad + # ``code`` is still reported before a bad transport protocol. The two are + # keyed alike, so the check itself is the same for either and TCP names it. + # :func:`register_tcp` and :func:`register_udp` name one each. + _check_code(code, Enum_AppType, TCP, number=True) # NOTE: ``class_`` sits at the sibling position -- positional, ahead of # ``*transport`` -- per maintainer ruling, so a third positional that is @@ -1356,7 +1367,7 @@ def register_tcp(code: 'int | Enum_AppType', module: 'str | ModuleDescriptor[Pro member of another enum. """ - _check_code(code, Enum_AppType, number=True) + _check_code(code, Enum_AppType, TCP, number=True) if isinstance(code, Enum_AppType): code = code.port if isinstance(module, str): @@ -1395,7 +1406,7 @@ def register_tcp_option(code: 'TCP_Option', meth: 'str | tuple[TCP_OptionParser, is a name with no matching ``_read_mode_*`` method. """ - _check_code(code, TCP_Option) + _check_code(code, TCP_Option, TCP) if isinstance(meth, str) and not hasattr(TCP, f'_read_mode_{meth}'): raise RegistryError('method must be a TCP option parser function') @@ -1429,7 +1440,7 @@ def register_tcp_mp_option(code: 'TCP_MPTCPOption', meth: 'str | tuple[TCP_MPOpt is a name with no matching ``_read_mptcp_*`` method. """ - _check_code(code, TCP_MPTCPOption) + _check_code(code, TCP_MPTCPOption, TCP) if isinstance(meth, str) and not hasattr(TCP, f'_read_mptcp_{meth}'): raise RegistryError('method must be a MPTCP option parser function') @@ -1470,7 +1481,7 @@ def register_udp(code: 'int | Enum_AppType', module: 'str | ModuleDescriptor[Pro member of another enum. """ - _check_code(code, Enum_AppType, number=True) + _check_code(code, Enum_AppType, UDP, number=True) if isinstance(code, Enum_AppType): code = code.port if isinstance(module, str): @@ -1524,7 +1535,7 @@ def register_sctp(code: 'int | SCTP_PayloadProtocolIdentifier', module: 'str | M :class:`~pcapkit.const.reg.apptype.sctp.SCTP` port enum. """ - _check_code(code, Enum_PayloadProtocolIdentifier, number=True) + _check_code(code, Enum_PayloadProtocolIdentifier, SCTP, number=True) if isinstance(module, str): module = cast('ModuleDescriptor[ProtocolBase]', ModuleDescriptor(module, class_)) @@ -1566,7 +1577,7 @@ def register_http_frame(code: 'HTTP_Frame', meth: 'str | tuple[HTTP_FrameParser, is a name with no matching ``_read_http_*`` method. """ - _check_code(code, HTTP_Frame) + _check_code(code, HTTP_Frame, HTTPv2) if isinstance(meth, str) and not hasattr(HTTPv2, f'_read_http_{meth}'): raise RegistryError('method must be a frame parser function') @@ -1605,7 +1616,7 @@ def register_pcapng_block(code: 'PCAPNG_BlockType', meth: 'str | tuple[PCAPNG_Bl is a name with no matching ``_read_block_*`` method. """ - _check_code(code, PCAPNG_BlockType) + _check_code(code, PCAPNG_BlockType, PCAPNG) if isinstance(meth, str) and not hasattr(PCAPNG, f'_read_block_{meth}'): raise RegistryError('method must be a block parser function') @@ -1639,7 +1650,7 @@ def register_pcapng_option(code: 'PCAPNG_OptionType', meth: 'str | tuple[PCAPNG_ is a name with no matching ``_read_option_*`` method. """ - _check_code(code, PCAPNG_OptionType) + _check_code(code, PCAPNG_OptionType, PCAPNG) if isinstance(meth, str) and not hasattr(PCAPNG, f'_read_option_{meth}'): raise RegistryError('method must be a option parser function') @@ -1673,7 +1684,7 @@ def register_pcapng_record(code: 'PCAPNG_RecordType', meth: 'str | tuple[PCAPNG_ is a name with no matching ``_read_record_*`` method. """ - _check_code(code, PCAPNG_RecordType) + _check_code(code, PCAPNG_RecordType, PCAPNG) if isinstance(meth, str) and not hasattr(PCAPNG, f'_read_record_{meth}'): raise RegistryError('method must be a name resolution record parser function') @@ -1707,7 +1718,7 @@ def register_pcapng_secrets(code: 'PCAPNG_SecretsType', meth: 'str | tuple[PCAPN is a name with no matching ``_read_secrets_*`` method. """ - _check_code(code, PCAPNG_SecretsType) + _check_code(code, PCAPNG_SecretsType, PCAPNG) if isinstance(meth, str) and not hasattr(PCAPNG, f'_read_secrets_{meth}'): raise RegistryError('method must be a decryption secrets parser function') diff --git a/pcapkit/protocols/internet/internet.py b/pcapkit/protocols/internet/internet.py index 2498f0f979..a64b5c769e 100644 --- a/pcapkit/protocols/internet/internet.py +++ b/pcapkit/protocols/internet/internet.py @@ -28,10 +28,11 @@ from pcapkit.utilities.warnings import ProtocolWarning, RegistryWarning, warn if TYPE_CHECKING: - from typing import IO, Any, Optional, Type + from typing import IO, Any, ClassVar, Optional, Type from typing_extensions import Literal, Self + from pcapkit.corekit.enum import EnumRegistry from pcapkit.protocols.data.data import Data __all__ = ['Internet'] @@ -138,6 +139,18 @@ class Internet(ProtocolBase[_PT, _ST], Generic[_PT, _ST]): # pylint: disable=ab }, ) + #: What :attr:`__proto__` is keyed by: the enum a key must be a member of, + #: and whether a bare :class:`int` is a valid key there too. Declared by the + #: class *owning* the registry, and read from it by + #: :func:`~pcapkit.foundation.registry.protocols.register_protocol_code` so a + #: ``code=`` key is checked exactly as + #: :func:`~pcapkit.foundation.registry.protocols.register_transtype` checks + #: its own -- a subclass sharing this registry is therefore checked as + #: :class:`Internet` is, while :class:`~pcapkit.protocols.internet.ipx.IPX`, + #: which keeps one of its own, declares its own pairing or none at all + #: (:issue:`2268`). + __proto_key__: 'ClassVar[tuple[Type[EnumRegistry], bool]]' = (Enum_TransType, False) + #: int: How many IPv6 extension headers in a row the next layer follows: #: this layer's place in such a chain, or how many headers #: :class:`~pcapkit.protocols.internet.ipv6.IPv6` walked, else ``0``, see diff --git a/pcapkit/protocols/link/link.py b/pcapkit/protocols/link/link.py index ce41a97434..cc4d148afe 100644 --- a/pcapkit/protocols/link/link.py +++ b/pcapkit/protocols/link/link.py @@ -25,10 +25,12 @@ from pcapkit.utilities.warnings import RegistryWarning, warn if TYPE_CHECKING: - from typing import IO, Any, DefaultDict, Optional, Type + from typing import IO, Any, ClassVar, DefaultDict, Optional, Type from typing_extensions import Literal, Self + from pcapkit.corekit.enum import EnumRegistry + __all__ = ['Link'] @@ -99,6 +101,18 @@ class Link(ProtocolBase[_PT, _ST], Generic[_PT, _ST]): # pylint: disable=abstra }, ) # type: DefaultDict[int | Enum_EtherType, ModuleDescriptor[ProtocolBase] | Type[ProtocolBase]] + #: What :attr:`__proto__` is keyed by: the enum a key must be a member of, + #: and whether a bare :class:`int` is a valid key there too. Declared by the + #: class *owning* the registry, and read from it by + #: :func:`~pcapkit.foundation.registry.protocols.register_protocol_code` so a + #: ``code=`` key is checked exactly as + #: :func:`~pcapkit.foundation.registry.protocols.register_ethertype` checks + #: its own -- :class:`~pcapkit.protocols.link.ethernet.Ethernet`, which shares + #: this registry, is therefore checked as :class:`Link` is, while + #: :class:`~pcapkit.protocols.link.loopback.Loopback`, which keeps one of its + #: own, declares its own pairing or none at all (:issue:`2268`). + __proto_key__: 'ClassVar[tuple[Type[EnumRegistry], bool]]' = (Enum_EtherType, False) + ########################################################################## # Properties. ########################################################################## diff --git a/pcapkit/protocols/misc/pcap/frame.py b/pcapkit/protocols/misc/pcap/frame.py index 9bc7dd6c33..11028754f5 100644 --- a/pcapkit/protocols/misc/pcap/frame.py +++ b/pcapkit/protocols/misc/pcap/frame.py @@ -55,10 +55,11 @@ if TYPE_CHECKING: from datetime import datetime as dt_type from decimal import Decimal - from typing import IO, Any, DefaultDict, Optional, Type + from typing import IO, Any, ClassVar, DefaultDict, Optional, Type from typing_extensions import Literal + from pcapkit.corekit.enum import EnumRegistry from pcapkit.protocols.data.misc.pcap.header import Header as Data_Header from pcapkit.protocols.schema.schema import Schema @@ -115,6 +116,17 @@ class Frame(ProtocolBase[Data_Frame, Schema_Frame], }, ) # type: DefaultDict[Enum_LinkType | int, ModuleDescriptor[ProtocolBase] | Type[ProtocolBase]] + #: What :attr:`__proto__` is keyed by: the enum a key must be a member of, + #: and whether a bare :class:`int` is a valid key there too. Declared by the + #: class *owning* the registry, and read from it by + #: :func:`~pcapkit.foundation.registry.protocols.register_protocol_code` so a + #: ``code=`` key is checked exactly as + #: :func:`~pcapkit.foundation.registry.protocols.register_pcap` checks its own -- + #: :class:`~pcapkit.protocols.misc.pcapng.PCAPNG` keeps a registry of its own, + #: keyed by the same enum, and so declares the same pairing rather than + #: sharing this one (:issue:`2268`). + __proto_key__: 'ClassVar[tuple[Type[EnumRegistry], bool]]' = (Enum_LinkType, False) + ########################################################################## # Properties. ########################################################################## diff --git a/pcapkit/protocols/misc/pcapng.py b/pcapkit/protocols/misc/pcapng.py index 2abd0b3210..8bfbd05500 100644 --- a/pcapkit/protocols/misc/pcapng.py +++ b/pcapkit/protocols/misc/pcapng.py @@ -225,13 +225,14 @@ from decimal import Decimal from enum import IntEnum as StdlibEnum from ipaddress import IPv4Address, IPv4Interface, IPv6Address, IPv6Interface - from typing import (IO, Any, Callable, Counter, DefaultDict, Optional, Tuple, Type, TypeVar, - Union) + from typing import (IO, Any, Callable, ClassVar, Counter, DefaultDict, Optional, Tuple, Type, + TypeVar, Union) from aenum import IntEnum as AenumEnum from mypy_extensions import DefaultArg, KwArg, NamedArg from typing_extensions import Literal + from pcapkit.corekit.enum import EnumRegistry from pcapkit.foundation.engines.pcapng import Context from pcapkit.protocols.schema.misc.pcapng import Option as Schema_Option from pcapkit.protocols.schema.schema import Schema @@ -751,6 +752,17 @@ class PCAPNG(ProtocolBase[Data_PCAPNG, Schema_PCAPNG], }, ) # type: DefaultDict[Enum_LinkType | int, ModuleDescriptor[ProtocolBase] | Type[ProtocolBase]] + #: What :attr:`__proto__` is keyed by: the enum a key must be a member of, + #: and whether a bare :class:`int` is a valid key there too. Declared by the + #: class *owning* the registry, and read from it by + #: :func:`~pcapkit.foundation.registry.protocols.register_protocol_code` so a + #: ``code=`` key is checked exactly as + #: :func:`~pcapkit.foundation.registry.protocols.register_pcapng` checks its own -- + #: :class:`~pcapkit.protocols.misc.pcap.frame.Frame` keeps a registry of its own, + #: keyed by the same enum, and so declares the same pairing rather than + #: sharing this one (:issue:`2268`). + __proto_key__: 'ClassVar[tuple[Type[EnumRegistry], bool]]' = (Enum_LinkType, False) + #: Block type to method mapping. A :obj:`str` value names the #: ``_read_block_${name}`` and ``_make_block_${name}`` methods, which fall #: back to ``_read_block_unknown`` and ``_make_block_unknown`` when not diff --git a/pcapkit/protocols/transport/sctp.py b/pcapkit/protocols/transport/sctp.py index 9a5b66dec1..8a7ee9f53c 100644 --- a/pcapkit/protocols/transport/sctp.py +++ b/pcapkit/protocols/transport/sctp.py @@ -161,12 +161,13 @@ if TYPE_CHECKING: from ipaddress import IPv4Address, IPv6Address - from typing import Any, Callable, DefaultDict, Optional, Sequence, Type, TypeVar + from typing import Any, Callable, ClassVar, DefaultDict, Optional, Sequence, Type, TypeVar from mypy_extensions import DefaultArg, KwArg, NamedArg from typing_extensions import Literal from pcapkit.const.reg.apptype import AppType as Enum_AppType + from pcapkit.corekit.enum import EnumRegistry from pcapkit.protocols.data.data import Data from pcapkit.protocols.data.transport.sctp import Chunk as Data_Chunk from pcapkit.protocols.data.transport.sctp import ErrorCause as Data_ErrorCause @@ -471,6 +472,16 @@ class SCTP(Transport[Data_SCTP, Schema_SCTP], }, ) # type: DefaultDict[int, ModuleDescriptor[ProtocolBase] | Type[ProtocolBase]] + #: What :attr:`__proto__` is keyed by: the enum a key must be a member of, + #: and whether a bare :class:`int` is a valid key there too. Declared by the + #: class *owning* the registry, and read from it by + #: :func:`~pcapkit.foundation.registry.protocols.register_protocol_code` so a + #: ``code=`` key is checked exactly as + #: :func:`~pcapkit.foundation.registry.protocols.register_sctp` checks its own + #: -- a bare :class:`int` is accepted because the registry is keyed by + #: payload protocol identifier, which is a number in its own right (:issue:`2268`). + __proto_key__: 'ClassVar[tuple[Type[EnumRegistry], bool]]' = (Enum_PayloadProtocolIdentifier, True) + #: Chunk type to method mapping, see :meth:`_read_sctp_chunks` and #: :meth:`_make_sctp_chunks`. A :class:`str` value names the methods #: ``_read_chunk_${name}`` and ``_make_chunk_${name}``, falling back to diff --git a/pcapkit/protocols/transport/tcp.py b/pcapkit/protocols/transport/tcp.py index 51bf127972..7c9ade29a9 100644 --- a/pcapkit/protocols/transport/tcp.py +++ b/pcapkit/protocols/transport/tcp.py @@ -43,6 +43,7 @@ import math from typing import TYPE_CHECKING, cast +from pcapkit.const.reg.apptype import AppType as Enum_AppType from pcapkit.const.reg.apptype import TransportProtocol as Enum_TransportProtocol from pcapkit.const.reg.transtype import TransType from pcapkit.const.tcp.checksum import Checksum as Enum_Checksum @@ -141,13 +142,13 @@ from datetime import timedelta from enum import IntEnum as StdlibEnum from ipaddress import IPv4Address, IPv6Address - from typing import Any, Callable, DefaultDict, Optional, Type + from typing import Any, Callable, ClassVar, DefaultDict, Optional, Type from aenum import IntEnum as AenumEnum from mypy_extensions import DefaultArg, KwArg, NamedArg from typing_extensions import Literal - from pcapkit.const.reg.apptype import AppType as Enum_AppType + from pcapkit.corekit.enum import EnumRegistry from pcapkit.protocols.data.transport.tcp import MPTCP as Data_MPTCP from pcapkit.protocols.data.transport.tcp import MPTCPJoin as Data_MPTCPJoin from pcapkit.protocols.data.transport.tcp import Option as Data_Option @@ -369,6 +370,17 @@ class TCP(Transport[Data_TCP, Schema_TCP], }, ) + #: What :attr:`__proto__` is keyed by: the enum a key must be a member of, + #: and whether a bare :class:`int` is a valid key there too. Declared by the + #: class *owning* the registry, and read from it by + #: :func:`~pcapkit.foundation.registry.protocols.register_protocol_code` so a + #: ``code=`` key is checked exactly as + #: :func:`~pcapkit.foundation.registry.protocols.register_tcp` checks its own + #: -- a bare :class:`int` is accepted because the registry is keyed by + #: port number, and an :class:`~pcapkit.const.reg.apptype.apptype.AppType` + #: member carries one (:issue:`2268`). + __proto_key__: 'ClassVar[tuple[Type[EnumRegistry], bool]]' = (Enum_AppType, True) + #: Option code to method mapping, see :meth:`_read_tcp_options` and #: :meth:`_make_tcp_options`. A :class:`str` value names the methods #: ``_read_mode_${name}`` and ``_make_mode_${name}``, falling back to diff --git a/pcapkit/protocols/transport/udp.py b/pcapkit/protocols/transport/udp.py index 6dd61ff53d..224d1c8ecf 100644 --- a/pcapkit/protocols/transport/udp.py +++ b/pcapkit/protocols/transport/udp.py @@ -25,6 +25,7 @@ import collections from typing import TYPE_CHECKING +from pcapkit.const.reg.apptype import AppType as Enum_AppType from pcapkit.const.reg.apptype import TransportProtocol as Enum_TransportProtocol from pcapkit.const.reg.transtype import TransType as Enum_TransType from pcapkit.corekit.module import ModuleDescriptor @@ -33,11 +34,11 @@ from pcapkit.protocols.transport.transport import Transport if TYPE_CHECKING: - from typing import Any, Optional + from typing import Any, ClassVar, Optional, Type from typing_extensions import Literal - from pcapkit.const.reg.apptype import AppType as Enum_AppType + from pcapkit.corekit.enum import EnumRegistry from pcapkit.protocols.protocol import ProtocolBase from pcapkit.protocols.schema.schema import Schema @@ -116,6 +117,17 @@ class UDP(Transport[Data_UDP, Schema_UDP], }, ) + #: What :attr:`__proto__` is keyed by: the enum a key must be a member of, + #: and whether a bare :class:`int` is a valid key there too. Declared by the + #: class *owning* the registry, and read from it by + #: :func:`~pcapkit.foundation.registry.protocols.register_protocol_code` so a + #: ``code=`` key is checked exactly as + #: :func:`~pcapkit.foundation.registry.protocols.register_udp` checks its own + #: -- a bare :class:`int` is accepted because the registry is keyed by + #: port number, and an :class:`~pcapkit.const.reg.apptype.apptype.AppType` + #: member carries one (:issue:`2268`). + __proto_key__: 'ClassVar[tuple[Type[EnumRegistry], bool]]' = (Enum_AppType, True) + ########################################################################## # Properties. ########################################################################## diff --git a/tests/foundation/registry/test_registrar_code_check_unit.py b/tests/foundation/registry/test_registrar_code_check_unit.py index 16d064f429..a3e60c6bdd 100644 --- a/tests/foundation/registry/test_registrar_code_check_unit.py +++ b/tests/foundation/registry/test_registrar_code_check_unit.py @@ -236,13 +236,51 @@ def test_number_registrars_reject_a_bool(self) -> None: """A :class:`bool` is an :class:`int` subclass, but never a port or PPID (#2269).""" from pcapkit.foundation.registry import protocols as registrars from pcapkit.protocols.misc.raw import Raw + from pcapkit.protocols.transport.tcp import TCP from pcapkit.utilities.exceptions import RegistryError for name, _, targets, extra in NUMBER_REGISTRARS: self._assert_rejected_unchanged(name, tuple(map(resolve, targets)), Raw, *extra, codes=(True, False)) - with self.assertRaisesRegex(RegistryError, r'^code must be an int or a member of AppType, not True$'): - registrars._check_code(True, registrars.Enum_AppType, number=True) + with self.assertRaisesRegex(RegistryError, r'^TCP: code must be an int or a member of AppType, not True$'): + registrars._check_code(True, registrars.Enum_AppType, TCP, number=True) + + def test_every_rejection_names_the_destination_registry(self) -> None: + """The message says *whose* registry rejected the key (#2268). + + Each registrar hands + :func:`~pcapkit.foundation.registry.protocols._check_code` the protocol + class whose registry the key is destined for, and the shared message + leads with its name -- several registrars are typed on the same enum, so + the enum alone does not say which one refused. The class expected here is + the first one listed above for that registrar, which for the two + registrars writing into a pair of registries keyed alike + (``register_linktype``, ``register_apptype``) is the one the check names. + """ + from pcapkit.const.reg.linktype import LinkType + from pcapkit.foundation.registry import protocols as registrars + from pcapkit.protocols.misc.raw import Raw + from pcapkit.utilities.exceptions import RegistryError + + def parser(*args: 'object', **kwargs: 'object') -> None: ... + + cases = [] # type: list[tuple[str, str, object, tuple[object, ...], dict[str, object]]] + for name, proto, schema in METHOD_REGISTRARS: + cases.append((name, resolve(proto).__name__, self._wrong_code(name), + ((parser, parser),), {'schema': resolve(schema)})) + for name, targets in MODULE_REGISTRARS: + cases.append((name, resolve(targets[0]).__name__, self._wrong_code(name), (Raw,), {})) + for name, number, targets, extra in NUMBER_REGISTRARS: + # these four take a plain int, so only a member of another enum is wrong + cases.append((name, resolve(targets[0]).__name__, LinkType(number), + (Raw, *extra), {})) + + for name, expected, code, args, kwargs in cases: + with self.subTest(registrar=name): + with warnings.catch_warnings(): + warnings.simplefilter('ignore') + with self.assertRaisesRegex(RegistryError, rf'^{expected}: code must be '): + getattr(registrars, name)(code, *args, **kwargs) if __name__ == '__main__': diff --git a/tests/protocols/test_protocol_code_registration_unit.py b/tests/protocols/test_protocol_code_registration_unit.py index 0341327308..cc757f6904 100644 --- a/tests/protocols/test_protocol_code_registration_unit.py +++ b/tests/protocols/test_protocol_code_registration_unit.py @@ -525,36 +525,37 @@ class FakeProtocol: reg_loopback.assert_called_once_with(2, FakeProtocol) def test_every_destination_kind_row_is_enforced(self) -> None: - """Every row of ``_DESTINATION_KINDS`` is exercised, both cells of it. + """Every ``__proto_key__`` declaration is exercised, both cells of it. - The table is what the check is made of, so a row dropped from it, or a - ``kind``/``number`` cell changed in it, silently stops checking a - destination -- the registrars it stands in for are not called here, so - nothing else would notice. Each row is pinned by behaviour rather than - by comparing the table with a copy of itself: + The declarations are what the check is made of, so one dropped from a + class, or a ``kind``/``number`` cell changed in it, silently stops + checking a destination -- the registrars they stand in for are not + called here, so nothing else would notice. Each is pinned by behaviour + rather than by comparing the declaration with a copy of itself: * ``kind`` -- a member of *another* registry's enum must be rejected, and must leave every registry as it was. That is the #2224 confusion - the check exists for, so a row that stops being checked, or one + the check exists for, so a class that stops being checked, or one checked against the wrong enum, fails here. * ``number`` -- a bare :class:`int` is accepted where the registrar accepts one (a port, a payload protocol identifier) and rejected where it does not, which is the only observable difference the flag - makes. The :class:`int` given to a rejecting row is the *value* of a + makes. The :class:`int` given to a rejecting class is the *value* of a member its registry already ships an entry at, so the check is pinned as being on the type and the entry at risk is a real one. - The ``kind`` identity is asserted as well, since a row retyped to an - enum unrelated to every key used here would reject both keys for the - wrong reason. + The declared pairing is asserted as well, since one retyped to an enum + unrelated to every key used here would reject both keys for the wrong + reason. It is read out of the class's own :func:`vars`, which is where + :func:`~pcapkit.foundation.registry.protocols.register_protocol_code` + reads it: an inherited one would not be its own declaration. """ from pcapkit.const.reg.apptype import AppType from pcapkit.const.reg.ethertype import EtherType from pcapkit.const.reg.linktype import LinkType from pcapkit.const.reg.transtype import TransType from pcapkit.const.sctp.payload_protocol_identifier import PayloadProtocolIdentifier - from pcapkit.foundation.registry.protocols import (_DESTINATION_KINDS, - register_protocol_code) + from pcapkit.foundation.registry.protocols import register_protocol_code from pcapkit.protocols.internet.internet import Internet from pcapkit.protocols.link.link import Link from pcapkit.protocols.misc.pcap.frame import Frame @@ -581,7 +582,7 @@ def test_every_destination_kind_row_is_enforced(self) -> None: for destination, (kind, number, member, raw) in expected.items(): with self.subTest(destination=destination.__name__): - self.assertNotIsInstance(member, kind) # the test data, not the table + self.assertNotIsInstance(member, kind) # the test data, not the declaration self._assert_rejected_unchanged( lambda dest=destination, key=member: register_protocol_code(Raw, {dest: key})) @@ -594,11 +595,86 @@ def test_every_destination_kind_row_is_enforced(self) -> None: self._assert_rejected_unchanged( lambda dest=destination, key=raw: register_protocol_code(Raw, {dest: key})) - self.assertIs(_DESTINATION_KINDS.get(destination, (None, None))[0], kind, - f'{destination.__name__} is not checked against {kind.__name__}') + self.assertEqual(vars(destination).get('__proto_key__'), (kind, number), + f'{destination.__name__} does not declare ' + f'({kind.__name__}, {number})') + + self.assertEqual(self._declaring_classes(), set(expected), + '__proto_key__ is no longer declared by the classes cased here') + + @staticmethod + def _declaring_classes() -> 'set[type]': + """Every ``ProtocolBase`` descendant declaring a ``__proto_key__`` of its own.""" + import pcapkit # noqa: F401 # pylint: disable=unused-import + from pcapkit.protocols.protocol import ProtocolBase + + found = set() + pending = [ProtocolBase] + while pending: + klass = pending.pop() + if '__proto_key__' in vars(klass): + found.add(klass) + pending.extend(klass.__subclasses__()) + return found + + def test_a_registry_of_its_own_does_not_inherit_its_parent_pairing(self) -> None: + """``Loopback.__proto__`` is keyed by address family, ``IPX.__proto__`` by packet type. + + The pairing is read from the owning class's own :func:`vars`, not with + :func:`getattr`, which walks the MRO: :class:`~pcapkit.protocols.link.\ +loopback.Loopback` subclasses :class:`~pcapkit.protocols.link.link.Link` and + :class:`~pcapkit.protocols.internet.ipx.IPX` subclasses + :class:`~pcapkit.protocols.internet.internet.Internet`, so each + *inherits* the attribute while owning a registry keyed by something + else entirely. A key is therefore registered here rather than only the + absent declaration asserted: checking address family ``2`` against + :class:`~pcapkit.const.reg.ethertype.EtherType`, or an + :class:`~pcapkit.const.ipx.packet.Packet` member against + :class:`~pcapkit.const.reg.transtype.TransType`, rejects every key + these registries do accept, and an assertion on + :func:`vars` alone passes either way. + """ + from pcapkit.const.ipx.packet import Packet + from pcapkit.foundation.registry.protocols import register_protocol_code + from pcapkit.protocols.internet.internet import Internet + from pcapkit.protocols.internet.ipx import IPX + from pcapkit.protocols.link.link import Link + from pcapkit.protocols.link.loopback import Loopback - self.assertEqual(set(_DESTINATION_KINDS), set(expected), - '_DESTINATION_KINDS no longer lists the destinations cased here') + cases = ( + (Loopback, Link, 2), # an address family, not an EtherType + (IPX, Internet, 4), # a bare packet type, not an IP protocol number + (IPX, Internet, Packet.PEP), # the same, as a member of its own enum + ) + for destination, parent, key in cases: + with self.subTest(destination=destination.__name__, key=key): + self.assertIn('__proto__', vars(destination)) # it owns a registry + self.assertNotIn('__proto_key__', vars(destination)) # declares no pairing + # inherited, not declared -- which is what makes the ``vars()`` read load-bearing + self.assertEqual(destination.__proto_key__, parent.__proto_key__) + + class FakeProtocol: + pass + + with mock.patch.object(destination, 'register') as register: + register_protocol_code(FakeProtocol, {destination: key}) + register.assert_called_once_with(key, FakeProtocol) + + def test_a_rejection_names_the_destination_registry(self) -> None: + """The message says *whose* registry rejected the key, not only what it wanted.""" + from pcapkit.const.reg.linktype import LinkType + from pcapkit.const.reg.transtype import TransType + from pcapkit.foundation.registry.protocols import register_protocol_code + from pcapkit.protocols.misc.pcap.frame import Frame + from pcapkit.protocols.misc.raw import Raw + from pcapkit.protocols.transport.udp import UDP + from pcapkit.utilities.exceptions import RegistryError + + for destination, key in ((Frame, TransType.ICMP), (UDP, LinkType(53))): + with self.subTest(destination=destination.__name__): + with self.assertRaisesRegex(RegistryError, + rf'^{destination.__name__}: code must be '): + register_protocol_code(Raw, {destination: key}) if __name__ == '__main__':