diff --git a/docs/source/contributing/conventions/index.rst b/docs/source/contributing/conventions/index.rst index 59e492102b..2b41814f99 100644 --- a/docs/source/contributing/conventions/index.rst +++ b/docs/source/contributing/conventions/index.rst @@ -13,7 +13,8 @@ House Conventions * :ref:`mint-criterion` -- :mod:`pcapkit.const`, which is where the settled questions have mostly arisen; * :ref:`sentinel-convention` -- :mod:`pcapkit.corekit`; - * :ref:`registry-protocol` -- :mod:`pcapkit.const` again; + * :ref:`registry-protocol` -- :mod:`pcapkit.const` again, and the registration + wrappers of :mod:`pcapkit.foundation.registry`; * :ref:`extension-header-subclassing` -- a protocol class hierarchy; * :ref:`protocol-family-grouping` -- how a protocol family's base and members spell :meth:`~pcapkit.protocols.protocol.Protocol.id`; diff --git a/docs/source/contributing/conventions/registry-protocol.rst b/docs/source/contributing/conventions/registry-protocol.rst index c9df762763..950a4b6885 100644 --- a/docs/source/contributing/conventions/registry-protocol.rst +++ b/docs/source/contributing/conventions/registry-protocol.rst @@ -642,3 +642,84 @@ reasoning. :mod:`pcapkit.vendor.default`'s template, never in the generated file alone -- the next regeneration would discard it. +Registration Wrappers +~~~~~~~~~~~~~~~~~~~~~ + +The owner ruled on :issue:`2511` that every class-level registry map a protocol +dispatches through -- ``__proto__``, ``__option__``, ``__chunk__`` and the like -- has a +public wrapper in :mod:`pcapkit.foundation.registry`, carrying the validation the +established ones carry. A map's registration comes in two parts: + +1. **A** ``register`` **or** ``register_`` **classmethod** on the class owning + the map -- ``register`` for ``__proto__``, ``register_`` for ``____`` + -- which writes it and warns when it displaces an entry. An inherited one counts + only when it is typed and validated for exactly that map's keys, which the owner + chose over an override on every class (:issue:`2511`). So + :class:`~pcapkit.protocols.transport.tcp.TCP` and + :class:`~pcapkit.protocols.transport.udp.UDP` keep + :meth:`Transport.register `, + typed for the port their maps are keyed by, while + :class:`~pcapkit.protocols.internet.ipx.IPX` and + :class:`~pcapkit.protocols.link.loopback.Loopback` override their parents' + :meth:`Internet.register ` + and :meth:`Link.register `: those are + typed for :class:`~pcapkit.const.reg.transtype.TransType` and + :class:`~pcapkit.const.reg.ethertype.EtherType`, not for the IPX packet type and + address family these two maps are keyed by. +2. **A** ``register_`` **function** in + :mod:`pcapkit.foundation.registry.protocols`, tagged ``# NOTE: `` on the + line above its ``def``. It calls the classmethod and logs, and a bad argument + writes nothing: ``_check_code`` checks the code against the map's enum, or, with + ``kind=None``, as a bare :class:`int` where no enum keys the map; a parser named + by string has to exist as a ``_read_*`` method; and a ``__proto__`` entry has to + be a :class:`~pcapkit.protocols.protocol.ProtocolBase` subclass or a descriptor + naming one. A wrapper writing a ``__proto__`` map also records the class's name + with ``_register_name``, and one whose entries have a schema registry takes a + keyword-only ``schema``. + +**A** ``__proto__`` **wrapper is named after its keys.** One taking only an enum's +members takes the enum's name, as +:func:`~pcapkit.foundation.registry.protocols.register_ethertype` does; one taking +bare :class:`int` keys, with or without an enum beside them, takes the owning +protocol's name, as :func:`~pcapkit.foundation.registry.protocols.register_tcp` and +:func:`~pcapkit.foundation.registry.protocols.register_loopback` do. These depart +from it: + +* :func:`~pcapkit.foundation.registry.protocols.register_pcap` and + :func:`~pcapkit.foundation.registry.protocols.register_pcapng` take only + :class:`~pcapkit.const.reg.linktype.LinkType` members, but + :func:`~pcapkit.foundation.registry.protocols.register_linktype` holds that name + for writing both their maps at once, so each takes its capture format's name. +* :func:`~pcapkit.foundation.registry.protocols.register_apptype` takes bare + :class:`int` keys, but writes :class:`~pcapkit.protocols.transport.tcp.TCP`'s map, + :class:`~pcapkit.protocols.transport.udp.UDP`'s or both, by transport, so no one + owner's name fits and it takes + :class:`~pcapkit.const.reg.apptype.apptype.AppType`'s. + +:func:`~pcapkit.foundation.registry.protocols.register_protocol` and +:func:`~pcapkit.foundation.registry.protocols.register_protocol_code` are outside the +rule: the first writes :data:`pcapkit.protocols.__proto__`, keyed by class name, and +the second whichever maps its ``code`` names. + +:func:`~pcapkit.foundation.registry.protocols.register_sctp_chunk` is the shape for a +parser map: + +.. code-block:: python + + # NOTE: pcapkit.protocols.transport.sctp.SCTP.__chunk__ + def register_sctp_chunk(code, meth, *, schema=None): + _check_code(code, SCTP_Chunk, SCTP) + if isinstance(meth, str) and not hasattr(SCTP, f'_read_chunk_{meth}'): + raise RegistryError('method must be a valid SCTP chunk parser function') + + SCTP.register_chunk(code, meth) + if schema is not None: + Schema_SCTP_Chunk.register(code, schema) + logger.debug('registered SCTP chunk parser: %s', code.name) + +:file:`tests/foundation/registry/test_registry_wrapper_coverage_2511_unit.py` walks +every protocol class for such a map, and fails on one that no wrapper's tag names +unless its ``EXCUSED`` table gives the reason. It fails as well on a map whose owner +defines no ``register*`` classmethod of its own, unless its ``INHERITED`` table names +the one typed for that map's keys. + diff --git a/docs/source/ext.rst b/docs/source/ext.rst index d4ca3b733e..c36a972333 100644 --- a/docs/source/ext.rst +++ b/docs/source/ext.rst @@ -194,6 +194,12 @@ The registry function per protocol type: | | | :func:`~pcapkit.foundation.registry.protocols.register_sctp` | | +-------------------+-------------------------------------------------------------------+----------------------------------------------------------------+--------------------+ +A protocol carried over IPX is keyed by IPX Packet Type rather than by IP protocol +number, so it registers through +:func:`~pcapkit.foundation.registry.protocols.register_ipx`; one carried over BSD +loopback is keyed by address family rather than by EtherType, and registers through +:func:`~pcapkit.foundation.registry.protocols.register_loopback`. + Samples ~~~~~~~ @@ -384,19 +390,19 @@ writes them: + + +-----------------------------------------------------------------------------------------------+----------------------------------------------------------------------------+ | | | :attr:`MH.__extension__ ` | :func:`~pcapkit.foundation.registry.protocols.register_mh_extension` | + +------------------------------------------------------------+-----------------------------------------------------------------------------------------------+----------------------------------------------------------------------------+ -| | :class:`~pcapkit.protocols.internet.icmpv4.ICMPv4` | :attr:`ICMPv4.__type__ ` | :meth:`~pcapkit.protocols.internet.icmpv4.ICMPv4.register_type` | +| | :class:`~pcapkit.protocols.internet.icmpv4.ICMPv4` | :attr:`ICMPv4.__type__ ` | :func:`~pcapkit.foundation.registry.protocols.register_icmp_type` | + +------------------------------------------------------------+-----------------------------------------------------------------------------------------------+----------------------------------------------------------------------------+ -| | :class:`~pcapkit.protocols.internet.icmpv6.ICMPv6` | :attr:`ICMPv6.__type__ ` | :meth:`~pcapkit.protocols.internet.icmpv6.ICMPv6.register_type` | +| | :class:`~pcapkit.protocols.internet.icmpv6.ICMPv6` | :attr:`ICMPv6.__type__ ` | :func:`~pcapkit.foundation.registry.protocols.register_icmp_type` | +-------------------+------------------------------------------------------------+-----------------------------------------------------------------------------------------------+----------------------------------------------------------------------------+ | | | :attr:`TCP.__option__ ` | :func:`~pcapkit.foundation.registry.protocols.register_tcp_option` | + Transport Layer + :class:`~pcapkit.protocols.transport.tcp.TCP` +-----------------------------------------------------------------------------------------------+----------------------------------------------------------------------------+ | | | :attr:`TCP.__mp_option__ ` | :func:`~pcapkit.foundation.registry.protocols.register_tcp_mp_option` | + +------------------------------------------------------------+-----------------------------------------------------------------------------------------------+----------------------------------------------------------------------------+ -| | | :attr:`SCTP.__chunk__ ` | :meth:`~pcapkit.protocols.transport.sctp.SCTP.register_chunk` | +| | | :attr:`SCTP.__chunk__ ` | :func:`~pcapkit.foundation.registry.protocols.register_sctp_chunk` | + + +-----------------------------------------------------------------------------------------------+----------------------------------------------------------------------------+ -| | :class:`~pcapkit.protocols.transport.sctp.SCTP` | :attr:`SCTP.__parameter__ ` | :meth:`~pcapkit.protocols.transport.sctp.SCTP.register_parameter` | +| | :class:`~pcapkit.protocols.transport.sctp.SCTP` | :attr:`SCTP.__parameter__ ` | :func:`~pcapkit.foundation.registry.protocols.register_sctp_parameter` | + + +-----------------------------------------------------------------------------------------------+----------------------------------------------------------------------------+ -| | | :attr:`SCTP.__cause__ ` | :meth:`~pcapkit.protocols.transport.sctp.SCTP.register_cause` | +| | | :attr:`SCTP.__cause__ ` | :func:`~pcapkit.foundation.registry.protocols.register_sctp_cause` | +-------------------+------------------------------------------------------------+-----------------------------------------------------------------------------------------------+----------------------------------------------------------------------------+ | Application Layer | :class:`~pcapkit.protocols.application.httpv2.HTTP` | :attr:`HTTP.__frame__ ` | :func:`~pcapkit.foundation.registry.protocols.register_http_frame` | +-------------------+------------------------------------------------------------+-----------------------------------------------------------------------------------------------+----------------------------------------------------------------------------+ diff --git a/docs/source/pcapkit/foundation/registry.rst b/docs/source/pcapkit/foundation/registry.rst index b37b6cec10..ac7cd1c2fc 100644 --- a/docs/source/pcapkit/foundation/registry.rst +++ b/docs/source/pcapkit/foundation/registry.rst @@ -83,11 +83,15 @@ Link Layer Registries .. autofunction:: pcapkit.foundation.registry.protocols.register_ethertype +.. autofunction:: pcapkit.foundation.registry.protocols.register_loopback + Internet Layer Registries ~~~~~~~~~~~~~~~~~~~~~~~~~ .. autofunction:: pcapkit.foundation.registry.protocols.register_transtype +.. autofunction:: pcapkit.foundation.registry.protocols.register_ipx + .. autofunction:: pcapkit.foundation.registry.protocols.register_ipv4_option .. autofunction:: pcapkit.foundation.registry.protocols.register_icmp_type @@ -121,6 +125,12 @@ Transport Layer Registries .. autofunction:: pcapkit.foundation.registry.protocols.register_sctp +.. autofunction:: pcapkit.foundation.registry.protocols.register_sctp_chunk + +.. autofunction:: pcapkit.foundation.registry.protocols.register_sctp_parameter + +.. autofunction:: pcapkit.foundation.registry.protocols.register_sctp_cause + Application Layer Registries ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/docs/source/pcapkit/protocols/internet/ipx.rst b/docs/source/pcapkit/protocols/internet/ipx.rst index c8f268526c..69f4975337 100644 --- a/docs/source/pcapkit/protocols/internet/ipx.rst +++ b/docs/source/pcapkit/protocols/internet/ipx.rst @@ -32,6 +32,7 @@ Octets Bits Name Description .. automethod:: read .. automethod:: make + .. automethod:: register .. automethod:: _make_data .. automethod:: _next_exthdr_depth @@ -40,6 +41,8 @@ Octets Bits Name Description .. autoattribute:: __proto__ :no-value: + .. autoattribute:: __proto_key__ + :no-value: Header Schemas -------------- diff --git a/docs/source/pcapkit/protocols/link/loopback.rst b/docs/source/pcapkit/protocols/link/loopback.rst index f31305f6fa..6f942eb81a 100644 --- a/docs/source/pcapkit/protocols/link/loopback.rst +++ b/docs/source/pcapkit/protocols/link/loopback.rst @@ -32,6 +32,7 @@ is kept as ``loopback.byteorder``. .. automethod:: read .. automethod:: make + .. automethod:: register .. automethod:: _make_data diff --git a/pcapkit/all.py b/pcapkit/all.py index 90b845478c..0e57f747e4 100644 --- a/pcapkit/all.py +++ b/pcapkit/all.py @@ -90,7 +90,9 @@ 'register_linktype', 'register_pcap', 'register_pcapng', 'register_ethertype', + 'register_loopback', 'register_transtype', + 'register_ipx', 'register_icmp_type', 'register_ipv4_option', 'register_hip_parameter', 'register_hopopt_option', 'register_ipv6_opts_option', 'register_ipv6_route_routing', @@ -98,6 +100,7 @@ 'register_apptype', 'register_tcp', 'register_udp', 'register_sctp', 'register_tcp_option', 'register_tcp_mp_option', + 'register_sctp_chunk', 'register_sctp_parameter', 'register_sctp_cause', 'register_http_frame', 'register_pcapng_block', 'register_pcapng_option', 'register_pcapng_secrets', 'register_pcapng_record', diff --git a/pcapkit/foundation/__init__.py b/pcapkit/foundation/__init__.py index aee568ac99..e1b8971e62 100644 --- a/pcapkit/foundation/__init__.py +++ b/pcapkit/foundation/__init__.py @@ -31,7 +31,9 @@ 'register_linktype', 'register_pcap', 'register_pcapng', 'register_ethertype', + 'register_loopback', 'register_transtype', + 'register_ipx', 'register_icmp_type', 'register_ipv4_option', 'register_hip_parameter', 'register_hopopt_option', 'register_ipv6_opts_option', 'register_ipv6_route_routing', @@ -39,6 +41,7 @@ 'register_apptype', 'register_tcp', 'register_udp', 'register_sctp', 'register_tcp_option', 'register_tcp_mp_option', + 'register_sctp_chunk', 'register_sctp_parameter', 'register_sctp_cause', 'register_http_frame', 'register_pcapng_block', 'register_pcapng_option', 'register_pcapng_secrets', 'register_pcapng_record', diff --git a/pcapkit/foundation/registry/__init__.py b/pcapkit/foundation/registry/__init__.py index 2c125930eb..845a09782c 100644 --- a/pcapkit/foundation/registry/__init__.py +++ b/pcapkit/foundation/registry/__init__.py @@ -31,8 +31,10 @@ 'register_pcap', 'register_pcapng', 'register_ethertype', + 'register_loopback', 'register_transtype', + 'register_ipx', 'register_icmp_type', 'register_ipv4_option', 'register_hip_parameter', 'register_hopopt_option', 'register_ipv6_opts_option', 'register_ipv6_route_routing', @@ -41,6 +43,7 @@ 'register_apptype', 'register_tcp', 'register_udp', 'register_sctp', 'register_tcp_option', 'register_tcp_mp_option', + 'register_sctp_chunk', 'register_sctp_parameter', 'register_sctp_cause', 'register_http_frame', diff --git a/pcapkit/foundation/registry/protocols.py b/pcapkit/foundation/registry/protocols.py index 3541cac5f6..71fbae9ba9 100644 --- a/pcapkit/foundation/registry/protocols.py +++ b/pcapkit/foundation/registry/protocols.py @@ -27,6 +27,7 @@ from pcapkit.const.ipv4.option_number import OptionNumber as IPv4_OptionNumber from pcapkit.const.ipv6.option import Option as IPv6_Option from pcapkit.const.ipv6.routing import Routing as IPv6_Routing +from pcapkit.const.ipx.packet import Packet as IPX_Packet from pcapkit.const.mh.cga_extension import CGAExtension as MH_CGAExtension from pcapkit.const.mh.option import Option as MH_Option from pcapkit.const.mh.packet import Packet as MH_Packet @@ -39,6 +40,9 @@ from pcapkit.const.reg.ethertype import EtherType as Enum_EtherType from pcapkit.const.reg.linktype import LinkType as Enum_LinkType from pcapkit.const.reg.transtype import TransType as Enum_TransType +from pcapkit.const.sctp.cause_code import CauseCode as SCTP_CauseCode +from pcapkit.const.sctp.chunk import Chunk as SCTP_Chunk +from pcapkit.const.sctp.parameter import Parameter as SCTP_Parameter from pcapkit.const.sctp.payload_protocol_identifier import \ PayloadProtocolIdentifier as Enum_PayloadProtocolIdentifier from pcapkit.const.tcp.mp_tcp_option import MPTCPOption as TCP_MPTCPOption @@ -55,8 +59,10 @@ from pcapkit.protocols.internet.ipv4 import IPv4 from pcapkit.protocols.internet.ipv6_opts import IPv6_Opts from pcapkit.protocols.internet.ipv6_route import IPv6_Route +from pcapkit.protocols.internet.ipx import IPX from pcapkit.protocols.internet.mh import MH from pcapkit.protocols.link.link import Link +from pcapkit.protocols.link.loopback import Loopback from pcapkit.protocols.misc.pcap.frame import Frame from pcapkit.protocols.misc.pcapng import PCAPNG from pcapkit.protocols.protocol import ProtocolBase @@ -75,6 +81,9 @@ from pcapkit.protocols.schema.misc.pcapng import \ NameResolutionRecord as Schema_PCAPNG_NameResolutionRecord from pcapkit.protocols.schema.misc.pcapng import Option as Schema_PCAPNG_Option +from pcapkit.protocols.schema.transport.sctp import Chunk as Schema_SCTP_Chunk +from pcapkit.protocols.schema.transport.sctp import ErrorCause as Schema_SCTP_ErrorCause +from pcapkit.protocols.schema.transport.sctp import Parameter as Schema_SCTP_Parameter from pcapkit.protocols.schema.transport.tcp import MPTCP as Schema_TCP_MPTCP from pcapkit.protocols.schema.transport.tcp import Option as Schema_TCP_Option from pcapkit.protocols.transport.sctp import SCTP @@ -120,6 +129,12 @@ from pcapkit.protocols.misc.pcapng import RecordParser as PCAPNG_RecordParser from pcapkit.protocols.misc.pcapng import SecretsConstructor as PCAPNG_SecretsConstructor from pcapkit.protocols.misc.pcapng import SecretsParser as PCAPNG_SecretsParser + from pcapkit.protocols.transport.sctp import CauseConstructor as SCTP_CauseConstructor + from pcapkit.protocols.transport.sctp import CauseParser as SCTP_CauseParser + from pcapkit.protocols.transport.sctp import ChunkConstructor as SCTP_ChunkConstructor + from pcapkit.protocols.transport.sctp import ChunkParser as SCTP_ChunkParser + from pcapkit.protocols.transport.sctp import ParameterConstructor as SCTP_ParameterConstructor + from pcapkit.protocols.transport.sctp import ParameterParser as SCTP_ParameterParser from pcapkit.protocols.transport.tcp import MPOptionConstructor as TCP_MPOptionConstructor from pcapkit.protocols.transport.tcp import MPOptionParser as TCP_MPOptionParser from pcapkit.protocols.transport.tcp import OptionConstructor as TCP_OptionConstructor @@ -133,8 +148,10 @@ 'register_pcap', 'register_pcapng', 'register_ethertype', + 'register_loopback', 'register_transtype', + 'register_ipx', 'register_icmp_type', 'register_ipv4_option', 'register_hip_parameter', 'register_hopopt_option', 'register_ipv6_opts_option', 'register_ipv6_route_routing', @@ -143,6 +160,7 @@ 'register_apptype', 'register_tcp', 'register_udp', 'register_sctp', 'register_tcp_option', 'register_tcp_mp_option', + 'register_sctp_chunk', 'register_sctp_parameter', 'register_sctp_cause', 'register_http_frame', @@ -569,7 +587,7 @@ def register_protocol_code(protocol: 'Type[ProtocolBase]', code: 'Any') -> 'None destination.__name__, key) -def _check_code(code: 'Any', kind: 'type', target: 'Type[ProtocolBase]', *, +def _check_code(code: 'Any', kind: 'Optional[type]', target: 'Type[ProtocolBase]', *, number: 'bool' = False) -> 'None': """Reject a registration key that is not a member of the enum its destination registry is keyed by. @@ -586,13 +604,16 @@ def _check_code(code: 'Any', kind: 'type', target: 'Type[ProtocolBase]', *, Args: code: Registration key handed to the registrar. - kind: Enum the registrar is typed on. + kind: Enum the registrar is typed on, or :data:`None` for a registry + keyed by a number no enum names, which takes a bare :class:`int` + only (:issue:`2511`). 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 -- its callers 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. + Implied when ``kind`` is :data:`None`. Raises: RegistryError: If ``code`` is not a member of ``kind`` and, with @@ -602,12 +623,18 @@ def _check_code(code: 'Any', kind: 'type', target: 'Type[ProtocolBase]', *, (:issue:`2269`). """ - if isinstance(code, kind): + if kind is not None and isinstance(code, kind): return + number = number or kind is None member = isinstance(code, (enum.Enum, aenum.Enum)) if number and isinstance(code, int) and not isinstance(code, bool) and not member: return - expected = f'an int or a member of {kind.__name__}' if number else f'a member of {kind.__name__}' + if kind is None: + expected = 'an int' + elif number: + expected = f'an int or a member of {kind.__name__}' + else: + expected = f'a member of {kind.__name__}' if member: raise RegistryError(f'{target.__name__}: code must be {expected}, not a member of ' f'{type(code).__name__}: {code!r}') @@ -810,6 +837,57 @@ def register_ethertype(code: 'EtherType', module: 'str | ModuleDescriptor[Protoc _register_name(module, (Link, code)) +@overload +def register_loopback(code: 'int', module: 'ModuleDescriptor[ProtocolBase] | Type[ProtocolBase]') -> 'None': ... +@overload +def register_loopback(code: 'int', module: 'str', class_: 'str') -> 'None': ... + + +# NOTE: pcapkit.protocols.link.loopback.Loopback.__proto__ +def register_loopback(code: 'int', module: 'str | ModuleDescriptor[ProtocolBase] | Type[ProtocolBase]', + class_: 'str | NullType' = NULL) -> 'None': + r"""Register a new protocol class. + + Notes: + The fully qualified class name of the new protocol class + should be ``{module}.{class_}``. + + The function will register the given protocol class to the + :data:`pcapkit.protocols.link.loopback.Loopback.__proto__` registry. + + Arguments: + code: BSD address family + module: module name or module descriptor or a + :class:`~pcapkit.protocols.protocol.ProtocolBase` subclass + class\_: class name + + Important: + Unlike :func:`register_ethertype`, ``code`` is an address family, + **not** an EtherType: :class:`~pcapkit.protocols.link.loopback.Loopback` + keeps a registry of its own. + + Raises: + RegistryError: If ``code`` is not an :class:`int` (a :class:`bool` is + not accepted, even though it is one), or is a member of an enum, or + ``module`` is not a :class:`~pcapkit.protocols.protocol.ProtocolBase` + subclass. + + .. versionadded:: 1.6.0 + + """ + _check_code(code, None, Loopback) + if isinstance(module, str): + module = cast('ModuleDescriptor[ProtocolBase]', ModuleDescriptor(module, class_)) + + Loopback.register(code, module) + logger.debug('registered loopback address family: %s', code) + + # register protocol to protocol registry + if isinstance(module, ModuleDescriptor): + module = module.klass + _register_name(module, (Loopback, code)) + + ############################################################################### # Internet Layer Registries ############################################################################### @@ -859,6 +937,58 @@ def register_transtype(code: 'TransType', module: 'str | ModuleDescriptor[Protoc _register_name(module, (Internet, code)) +@overload +def register_ipx(code: 'int | IPX_Packet', module: 'ModuleDescriptor[ProtocolBase] | Type[ProtocolBase]') -> 'None': ... +@overload +def register_ipx(code: 'int | IPX_Packet', module: 'str', class_: 'str') -> 'None': ... + + +# NOTE: pcapkit.protocols.internet.ipx.IPX.__proto__ +def register_ipx(code: 'int | IPX_Packet', module: 'str | ModuleDescriptor[ProtocolBase] | Type[ProtocolBase]', + class_: 'str | NullType' = NULL) -> 'None': + r"""Register a new protocol class. + + Notes: + The fully qualified class name of the new protocol class + should be ``{module}.{class_}``. + + The function will register the given protocol class to the + :data:`pcapkit.protocols.internet.ipx.IPX.__proto__` registry. + + Arguments: + code: IPX Packet Type, as in :class:`~pcapkit.const.ipx.packet.Packet` + module: module name or module descriptor or a + :class:`~pcapkit.protocols.protocol.ProtocolBase` subclass + class\_: class name + + Important: + Unlike :func:`register_transtype`, ``code`` is an IPX Packet Type, **not** + an IP protocol number: :class:`~pcapkit.protocols.internet.ipx.IPX` + keeps a registry of its own (:issue:`2169`). + + Raises: + RegistryError: If ``code`` is neither an :class:`int` (a :class:`bool` + is not accepted, even though it is one) nor a + :class:`~pcapkit.const.ipx.packet.Packet` member, e.g. a + :class:`~pcapkit.const.reg.transtype.TransType` member, or ``module`` + is not a :class:`~pcapkit.protocols.protocol.ProtocolBase` subclass. + + .. versionadded:: 1.6.0 + + """ + _check_code(code, IPX_Packet, IPX, number=True) + if isinstance(module, str): + module = cast('ModuleDescriptor[ProtocolBase]', ModuleDescriptor(module, class_)) + + IPX.register(code, module) + logger.debug('registered IPX packet type protocol: %s', code) + + # register protocol to protocol registry + if isinstance(module, ModuleDescriptor): + module = module.klass + _register_name(module, (IPX, code)) + + # NOTE: pcapkit.protocols.internet.ipv4.IPv4.__option__ def register_ipv4_option(code: 'IPv4_OptionNumber', meth: 'str | tuple[IPv4_OptionParser, IPv4_OptionConstructor]', *, schema: 'Optional[Type[Schema_IPv4_Option]]' = None) -> 'None': @@ -1589,6 +1719,115 @@ def register_sctp(code: 'int | SCTP_PayloadProtocolIdentifier', module: 'str | M _register_name(module, (SCTP, code)) +# NOTE: pcapkit.protocols.transport.sctp.SCTP.__chunk__ +def register_sctp_chunk(code: 'SCTP_Chunk', meth: 'str | tuple[SCTP_ChunkParser, SCTP_ChunkConstructor]', *, + schema: 'Optional[Type[Schema_SCTP_Chunk]]' = None) -> 'None': + """Register a chunk parser. + + The function will register the given chunk parser to the + :data:`pcapkit.protocols.transport.sctp.SCTP.__chunk__` registry. + + Args: + code: :class:`~pcapkit.protocols.transport.sctp.SCTP` chunk type as in + :class:`~pcapkit.const.sctp.chunk.Chunk`. + meth: Method name, or a ``(parser, constructor)`` pair of callables, to + parse and construct the chunk. A name names the + ``_read_chunk_*`` and ``_make_chunk_*`` methods, and has to name + an existing ``_read_chunk_*`` one. + schema: :class:`~pcapkit.protocols.schema.schema.Schema` class for the chunk. + It should be a subclass of :class:`pcapkit.protocols.schema.transport.sctp.Chunk`. + + Raises: + RegistryError: If ``code`` is not a :class:`~pcapkit.const.sctp.chunk.Chunk` + member, e.g. a member of another enum, or ``meth`` + is a name with no matching ``_read_chunk_*`` method. + + .. versionadded:: 1.6.0 + + """ + _check_code(code, SCTP_Chunk, SCTP) + if isinstance(meth, str) and not hasattr(SCTP, f'_read_chunk_{meth}'): + raise RegistryError('method must be a valid SCTP chunk parser function') + + SCTP.register_chunk(code, meth) + if schema is not None: + Schema_SCTP_Chunk.register(code, schema) + logger.debug('registered SCTP chunk parser: %s', code.name) + + +# NOTE: pcapkit.protocols.transport.sctp.SCTP.__parameter__ +def register_sctp_parameter(code: 'SCTP_Parameter', + meth: 'str | tuple[SCTP_ParameterParser, SCTP_ParameterConstructor]', *, + schema: 'Optional[Type[Schema_SCTP_Parameter]]' = None) -> 'None': + """Register a chunk parameter parser. + + The function will register the given chunk parameter parser to the + :data:`pcapkit.protocols.transport.sctp.SCTP.__parameter__` registry. + + Args: + code: :class:`~pcapkit.protocols.transport.sctp.SCTP` chunk parameter type + as in :class:`~pcapkit.const.sctp.parameter.Parameter`. + meth: Method name, or a ``(parser, constructor)`` pair of callables, to + parse and construct the parameter. A name names the + ``_read_param_*`` and ``_make_param_*`` methods, and has to name + an existing ``_read_param_*`` one. + schema: :class:`~pcapkit.protocols.schema.schema.Schema` class for the parameter. + It should be a subclass of :class:`pcapkit.protocols.schema.transport.sctp.Parameter`. + + Raises: + RegistryError: If ``code`` is not a :class:`~pcapkit.const.sctp.parameter.Parameter` + member, e.g. a member of another enum, or ``meth`` + is a name with no matching ``_read_param_*`` method. + + .. versionadded:: 1.6.0 + + """ + _check_code(code, SCTP_Parameter, SCTP) + if isinstance(meth, str) and not hasattr(SCTP, f'_read_param_{meth}'): + raise RegistryError('method must be a valid SCTP chunk parameter parser function') + + SCTP.register_parameter(code, meth) + if schema is not None: + Schema_SCTP_Parameter.register(code, schema) + logger.debug('registered SCTP chunk parameter parser: %s', code.name) + + +# NOTE: pcapkit.protocols.transport.sctp.SCTP.__cause__ +def register_sctp_cause(code: 'SCTP_CauseCode', meth: 'str | tuple[SCTP_CauseParser, SCTP_CauseConstructor]', *, + schema: 'Optional[Type[Schema_SCTP_ErrorCause]]' = None) -> 'None': + """Register an error cause parser. + + The function will register the given error cause parser to the + :data:`pcapkit.protocols.transport.sctp.SCTP.__cause__` registry. + + Args: + code: :class:`~pcapkit.protocols.transport.sctp.SCTP` error cause code as + in :class:`~pcapkit.const.sctp.cause_code.CauseCode`. + meth: Method name, or a ``(parser, constructor)`` pair of callables, to + parse and construct the cause. A name names the + ``_read_cause_*`` and ``_make_cause_*`` methods, and has to name + an existing ``_read_cause_*`` one. + schema: :class:`~pcapkit.protocols.schema.schema.Schema` class for the cause. + It should be a subclass of :class:`pcapkit.protocols.schema.transport.sctp.ErrorCause`. + + Raises: + RegistryError: If ``code`` is not a :class:`~pcapkit.const.sctp.cause_code.CauseCode` + member, e.g. a member of another enum, or ``meth`` + is a name with no matching ``_read_cause_*`` method. + + .. versionadded:: 1.6.0 + + """ + _check_code(code, SCTP_CauseCode, SCTP) + if isinstance(meth, str) and not hasattr(SCTP, f'_read_cause_{meth}'): + raise RegistryError('method must be a valid SCTP error cause parser function') + + SCTP.register_cause(code, meth) + if schema is not None: + Schema_SCTP_ErrorCause.register(code, schema) + logger.debug('registered SCTP error cause parser: %s', code.name) + + ############################################################################### # Application Layer Registries ############################################################################### diff --git a/pcapkit/protocols/internet/internet.py b/pcapkit/protocols/internet/internet.py index fa5029479b..441db3d529 100644 --- a/pcapkit/protocols/internet/internet.py +++ b/pcapkit/protocols/internet/internet.py @@ -158,9 +158,9 @@ class Internet(ProtocolBase[_PT, _ST], Generic[_PT, _ST]): # pylint: disable=ab #: ``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`). + #: :class:`Internet` is (:issue:`2268`), while + #: :class:`~pcapkit.protocols.internet.ipx.IPX`, which keeps one of its own, + #: declares its own pairing (:issue:`2511`). __proto_key__: 'ClassVar[tuple[Type[EnumRegistry], bool]]' = (Enum_TransType, False) #: int: How many IPv6 extension headers in a row the next layer follows: diff --git a/pcapkit/protocols/internet/ipx.py b/pcapkit/protocols/internet/ipx.py index 2801655ea0..42d0ed4e45 100644 --- a/pcapkit/protocols/internet/ipx.py +++ b/pcapkit/protocols/internet/ipx.py @@ -36,14 +36,16 @@ from pcapkit.protocols.data.internet.ipx import Address as Data_Address from pcapkit.protocols.internet.internet import Internet from pcapkit.protocols.schema.internet.ipx import IPX as Schema_IPX +from pcapkit.utilities.warnings import RegistryWarning, warn if TYPE_CHECKING: from enum import IntEnum as StdlibEnum - from typing import Any, DefaultDict, Optional, Type + from typing import Any, ClassVar, DefaultDict, Optional, Type from aenum import IntEnum as AenumEnum from typing_extensions import Literal + from pcapkit.corekit.enum import EnumRegistry from pcapkit.protocols.protocol import ProtocolBase from pcapkit.protocols.schema.schema import Schema @@ -83,11 +85,20 @@ class IPX(Internet[Data_IPX, Schema_IPX], #: instead of sharing :attr:`Internet.__proto__ #: ` (:issue:`2169`). #: No IPX-native protocol is shipped, so the payload is - #: :class:`~pcapkit.protocols.misc.raw.Raw` unless one is registered. + #: :class:`~pcapkit.protocols.misc.raw.Raw` unless one is registered with + #: :func:`~pcapkit.foundation.registry.protocols.register_ipx`. __proto__ = collections.defaultdict( lambda: ModuleDescriptor('pcapkit.protocols.misc.raw', 'Raw'), ) # type: DefaultDict[int, ModuleDescriptor[ProtocolBase] | Type[ProtocolBase]] + #: What :attr:`__proto__` is keyed by, declared as + #: :attr:`Internet.__proto_key__ ` + #: is, so a ``code=`` key is checked as + #: :func:`~pcapkit.foundation.registry.protocols.register_ipx` checks its own. + #: A bare :class:`int` is a valid key too, because the IPX Packet Type is a + #: number in its own right (:issue:`2268`, :issue:`2511`). + __proto_key__: 'ClassVar[tuple[Type[EnumRegistry], bool]]' = (Enum_Packet, True) + ########################################################################## # Properties. ########################################################################## @@ -212,6 +223,46 @@ def make(self, trailer=trailer, ) + @classmethod + def register(cls, code: 'Enum_Packet | int', protocol: 'ModuleDescriptor[ProtocolBase] | Type[ProtocolBase]') -> 'None': # pylint: disable=line-too-long + r"""Register a new protocol class for an IPX Packet Type. + + Notes: + The fully qualified class name of the new protocol class + should be ``{protocol.module}.{protocol.name}``. + + Arguments: + code: IPX Packet Type, as in :class:`~pcapkit.const.ipx.packet.Packet` + protocol: module descriptor or a + :class:`~pcapkit.protocols.protocol.ProtocolBase` subclass + + Important: + This overrides :meth:`Internet.register + ` because + :attr:`self.__proto__ ` is keyed by IPX Packet Type + rather than by IP protocol number (:issue:`2511`). + + Raises: + pcapkit.utilities.exceptions.RegistryError: If ``protocol`` is not a + class, or not a :class:`~pcapkit.protocols.protocol.ProtocolBase` subclass. + + Warns: + pcapkit.utilities.warnings.RegistryWarning: If this packet type is + already registered and the incumbent differs from the + replacement; the message names both. See :meth:`Protocol.register + ` for the guard, + and for the shipped descriptor a restore puts back (:issue:`1504`). + + """ + incumbent = cls.__proto__.get(code) + entry = cls._next_layer_entry(code, protocol) + if entry is None: + return + if incumbent is not None and incumbent is not entry: + warn(f'packet type {code} already registered, overwriting ' + f'{incumbent!r} with {entry!r}', RegistryWarning) + cls.__proto__[code] = entry + ########################################################################## # Data models. ########################################################################## diff --git a/pcapkit/protocols/link/loopback.py b/pcapkit/protocols/link/loopback.py index 73664d89b7..3623d5d887 100644 --- a/pcapkit/protocols/link/loopback.py +++ b/pcapkit/protocols/link/loopback.py @@ -32,6 +32,7 @@ from pcapkit.protocols.link.link import Link from pcapkit.protocols.schema.link.loopback import Loopback as Schema_Loopback from pcapkit.utilities.exceptions import ProtocolError +from pcapkit.utilities.warnings import RegistryWarning, warn if TYPE_CHECKING: from typing import Any, DefaultDict, Optional, Type @@ -124,7 +125,8 @@ class Loopback(Link[Data_Loopback, Schema_Loopback], #: `. Keyed by #: address family rather than by EtherType, so this class keeps a registry #: of its own instead of sharing :attr:`Link.__proto__ - #: `. + #: `; an entry is registered with + #: :func:`~pcapkit.foundation.registry.protocols.register_loopback`. __proto__ = collections.defaultdict( lambda: ModuleDescriptor('pcapkit.protocols.misc.raw', 'Raw'), { @@ -250,6 +252,46 @@ def make(self, payload=payload, ) + @classmethod + def register(cls, code: 'int', protocol: 'ModuleDescriptor[ProtocolBase] | Type[ProtocolBase]') -> 'None': + r"""Register a new protocol class for an address family. + + Notes: + The fully qualified class name of the new protocol class + should be ``{protocol.module}.{protocol.name}``. + + Arguments: + code: BSD address family + protocol: module descriptor or a + :class:`~pcapkit.protocols.protocol.ProtocolBase` subclass + + Important: + This overrides :meth:`Link.register + ` because + :attr:`self.__proto__ ` is keyed by address + family rather than by EtherType (:issue:`2511`). + + Raises: + pcapkit.utilities.exceptions.RegistryError: If ``protocol`` is not a + class, or not a :class:`~pcapkit.protocols.protocol.ProtocolBase` subclass. + + Warns: + pcapkit.utilities.warnings.RegistryWarning: If this address family is + already registered and the incumbent differs from the + replacement; the message names both. See :meth:`Protocol.register + ` for the guard, + and for the shipped descriptor a restore puts back (:issue:`1504`). + + """ + incumbent = cls.__proto__.get(code) + entry = cls._next_layer_entry(code, protocol) + if entry is None: + return + if incumbent is not None and incumbent is not entry: + warn(f'address family {code} already registered, overwriting ' + f'{incumbent!r} with {entry!r}', RegistryWarning) + cls.__proto__[code] = entry + ########################################################################## # Data models. ########################################################################## diff --git a/pcapkit/protocols/protocol.py b/pcapkit/protocols/protocol.py index 75b04a49f4..3afcebc89d 100644 --- a/pcapkit/protocols/protocol.py +++ b/pcapkit/protocols/protocol.py @@ -1093,13 +1093,9 @@ def _next_layer_entry(cls, code: 'Any', protocol: 'ModuleDescriptor[ProtocolBase ) -> 'Optional[ModuleDescriptor[ProtocolBase] | Type[ProtocolBase]]': """Work out what a next-layer ``register`` stores under ``code``. - :meth:`register` and the :class:`~pcapkit.protocols.link.link.Link`, - :class:`~pcapkit.protocols.internet.internet.Internet`, - :class:`~pcapkit.protocols.transport.transport.Transport`, - :class:`~pcapkit.protocols.transport.sctp.SCTP`, - :class:`~pcapkit.protocols.misc.pcap.frame.Frame` and - :class:`~pcapkit.protocols.misc.pcapng.PCAPNG` overrides share this; - each still warns and stores for itself. + :meth:`register` and every override of it, such as + :meth:`Link.register `, + share this; each still warns and stores for itself. Arguments: code: registry key diff --git a/tests/foundation/registry/test_protocols.py b/tests/foundation/registry/test_protocols.py index c9132109ac..b4ef103a8f 100644 --- a/tests/foundation/registry/test_protocols.py +++ b/tests/foundation/registry/test_protocols.py @@ -33,6 +33,9 @@ 'register_pcapng_option', 'register_pcapng_record', 'register_pcapng_secrets', + 'register_sctp_cause', + 'register_sctp_chunk', + 'register_sctp_parameter', 'register_tcp_mp_option', 'register_tcp_option', ) @@ -452,6 +455,7 @@ def test_top_level_link_internet_and_transport_protocol_wrappers(self) -> None: # names TCP alone (#806), which the notes below rely on. from pcapkit.const.reg.apptype import TCP as AppType_TCP from pcapkit.const.reg.apptype import UDP as AppType_UDP + from pcapkit.const.ipx.packet import Packet as IPXPacket from pcapkit.const.reg.apptype import TransportProtocol from pcapkit.const.reg.ethertype import EtherType from pcapkit.const.reg.linktype import LinkType @@ -470,7 +474,9 @@ def test_top_level_link_internet_and_transport_protocol_wrappers(self) -> None: (registry.register_pcapng, LinkType.ETHERNET, [(registry.PCAPNG, 'register')]), (registry.register_ethertype, EtherType.Internet_Protocol_version_4, [(registry.Link, 'register')]), + (registry.register_loopback, 2, [(registry.Loopback, 'register')]), (registry.register_transtype, TransType.TCP, [(registry.Internet, 'register')]), + (registry.register_ipx, IPXPacket.NCP, [(registry.IPX, 'register')]), ] for func, code, targets in class_routes: with self.subTest(func=f'{func.__name__}-class'): @@ -882,6 +888,9 @@ def test_option_like_registry_wrappers_validate_methods_and_register_schema(self from pcapkit.const.pcapng.option_type import OptionType from pcapkit.const.pcapng.record_type import RecordType from pcapkit.const.pcapng.secrets_type import SecretsType + from pcapkit.const.sctp.cause_code import CauseCode + from pcapkit.const.sctp.chunk import Chunk + from pcapkit.const.sctp.parameter import Parameter as SCTPParameter from pcapkit.const.tcp.mp_tcp_option import MPTCPOption from pcapkit.const.tcp.option import Option as TCPOption from pcapkit.foundation.registry import protocols as registry @@ -908,6 +917,12 @@ def test_option_like_registry_wrappers_validate_methods_and_register_schema(self registry.Schema_TCP_Option, 'register_option', TCPOption.No_Operation), (registry.register_tcp_mp_option, registry.TCP, '_read_mptcp_unit', registry.Schema_TCP_MPTCP, 'register_mp_option', MPTCPOption.MP_CAPABLE), + (registry.register_sctp_chunk, registry.SCTP, '_read_chunk_unit', + registry.Schema_SCTP_Chunk, 'register_chunk', Chunk.Payload_Data), + (registry.register_sctp_parameter, registry.SCTP, '_read_param_unit', + registry.Schema_SCTP_Parameter, 'register_parameter', SCTPParameter.Heartbeat_Info), + (registry.register_sctp_cause, registry.SCTP, '_read_cause_unit', + registry.Schema_SCTP_ErrorCause, 'register_cause', CauseCode.Stale_Cookie), (registry.register_http_frame, registry.HTTPv2, '_read_http_unit', registry.Schema_HTTP_FrameType, 'register_frame', HTTPFrame.DATA), (registry.register_pcapng_block, registry.PCAPNG, '_read_block_unit', diff --git a/tests/foundation/registry/test_registrar_code_check_unit.py b/tests/foundation/registry/test_registrar_code_check_unit.py index a3e60c6bdd..915fdfea34 100644 --- a/tests/foundation/registry/test_registrar_code_check_unit.py +++ b/tests/foundation/registry/test_registrar_code_check_unit.py @@ -12,11 +12,11 @@ method tables, the schema tables, :data:`pcapkit.protocols.__proto__` and the name leases -- calls the registrar with an otherwise valid argument list, and compares every registry with the snapshot. The registrars that take a plain -``int`` port or PPID (``register_tcp``, ``register_udp``, ``register_sctp`` and -``register_apptype``) are out of scope for those codes: an ``int`` is a valid -code there. +``int`` (``NUMBER_REGISTRARS``) accept the ``int``, but still reject +:obj:`None` and a :class:`str`. Those keyed by no enum at all +(``INT_ONLY_REGISTRARS``) take nothing but an ``int`` (#2511). -Every registrar, those four included, also rejects a member of an enum other +Every registrar, those included, also rejects a member of an enum other than the one it is typed on (#2224). The registry enums compare equal to their values, so such a member keys the entry of the registrar's own member of that value: without the check, ``register_linktype(TransType.ICMP, ...)`` replaces @@ -64,6 +64,12 @@ 'pcapkit.protocols.schema.transport.tcp:Option'), ('register_tcp_mp_option', 'pcapkit.protocols.transport.tcp:TCP', 'pcapkit.protocols.schema.transport.tcp:MPTCP'), + ('register_sctp_chunk', 'pcapkit.protocols.transport.sctp:SCTP', + 'pcapkit.protocols.schema.transport.sctp:Chunk'), + ('register_sctp_parameter', 'pcapkit.protocols.transport.sctp:SCTP', + 'pcapkit.protocols.schema.transport.sctp:Parameter'), + ('register_sctp_cause', 'pcapkit.protocols.transport.sctp:SCTP', + 'pcapkit.protocols.schema.transport.sctp:ErrorCause'), ('register_http_frame', 'pcapkit.protocols.application.httpv2:HTTP', 'pcapkit.protocols.schema.application.httpv2:FrameType'), ('register_pcapng_block', 'pcapkit.protocols.misc.pcapng:PCAPNG', @@ -103,6 +109,9 @@ 'register_mh_extension': 'pcapkit.const.mh.cga_extension:CGAExtension', 'register_tcp_option': 'pcapkit.const.tcp.option:Option', 'register_tcp_mp_option': 'pcapkit.const.tcp.mp_tcp_option:MPTCPOption', + 'register_sctp_chunk': 'pcapkit.const.sctp.chunk:Chunk', + 'register_sctp_parameter': 'pcapkit.const.sctp.parameter:Parameter', + 'register_sctp_cause': 'pcapkit.const.sctp.cause_code:CauseCode', 'register_http_frame': 'pcapkit.const.http.frame:Frame', 'register_pcapng_block': 'pcapkit.const.pcapng.block_type:BlockType', 'register_pcapng_option': 'pcapkit.const.pcapng.option_type:OptionType', @@ -115,9 +124,12 @@ 'register_transtype': 'pcapkit.const.reg.transtype:TransType', } -#: Registrars that also take a plain ``int``: the port or PPID used, the classes -#: whose ``__proto__`` they write, and any trailing arguments. +#: Registrars that also take a plain ``int``: the port, PPID, packet type or +#: address family used, the classes whose ``__proto__`` they write, and any +#: trailing arguments. NUMBER_REGISTRARS = ( + ('register_loopback', 2, ('pcapkit.protocols.link.loopback:Loopback',), ()), + ('register_ipx', 17, ('pcapkit.protocols.internet.ipx:IPX',), ()), ('register_tcp', 80, ('pcapkit.protocols.transport.tcp:TCP',), ()), ('register_udp', 53, ('pcapkit.protocols.transport.udp:UDP',), ()), ('register_sctp', 60, ('pcapkit.protocols.transport.sctp:SCTP',), ()), @@ -125,6 +137,10 @@ 'pcapkit.protocols.transport.udp:UDP'), ('tcp',)), ) +#: The registrars above whose registry no enum keys, so a plain ``int`` is the +#: only code they take (#2511). +INT_ONLY_REGISTRARS = ('register_loopback',) + def resolve(path: 'str') -> 'type': module, _, name = path.partition(':') @@ -220,6 +236,14 @@ def test_module_registrars_reject_a_member_of_another_enum(self) -> None: self._assert_rejected_unchanged(name, tuple(map(resolve, targets)), Raw, codes=(self._wrong_code(name),)) + def test_number_registrars_reject_none_and_a_str(self) -> None: + """Accepting an ``int`` lets in neither :obj:`None` nor its digits as a :class:`str` (#2511).""" + from pcapkit.protocols.misc.raw import Raw + + for name, number, targets, extra in NUMBER_REGISTRARS: + self._assert_rejected_unchanged(name, tuple(map(resolve, targets)), Raw, *extra, + codes=(None, str(number))) + def test_number_registrars_reject_a_member_of_another_enum(self) -> None: from pcapkit.const.reg.linktype import LinkType from pcapkit.protocols.misc.raw import Raw @@ -245,6 +269,29 @@ def test_number_registrars_reject_a_bool(self) -> None: 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_int_only_registrars_reject_anything_but_an_int(self) -> None: + """No enum keys Loopback's registry, so only a plain ``int`` is a code (#2511). + + ``_check_code`` with ``kind=None`` is the check, so its message keeps the + shape the enum-typed ones have. + """ + from pcapkit.const.reg.transtype import TransType + from pcapkit.foundation.registry import protocols as registrars + from pcapkit.protocols.link.loopback import Loopback + from pcapkit.protocols.misc.raw import Raw + from pcapkit.utilities.exceptions import RegistryError + + for name, _, targets, extra in NUMBER_REGISTRARS: + if name in INT_ONLY_REGISTRARS: + self._assert_rejected_unchanged(name, tuple(map(resolve, targets)), Raw, *extra, + codes=(None, '2', 2.0, TransType.IPv4)) + for code, message in ((None, 'None'), (True, 'True'), + (TransType.IPv4, 'a member of TransType: ')): + with self.subTest(code=code): + with self.assertRaisesRegex(RegistryError, rf'^Loopback: code must be an int, not {message}'): + registrars._check_code(code, None, Loopback) + registrars._check_code(2, None, Loopback) + def test_every_rejection_names_the_destination_registry(self) -> None: """The message says *whose* registry rejected the key (#2268). @@ -271,7 +318,7 @@ def parser(*args: 'object', **kwargs: 'object') -> None: ... 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 + # these take a plain int, so only a member of another enum is wrong cases.append((name, resolve(targets[0]).__name__, LinkType(number), (Raw, *extra), {})) diff --git a/tests/foundation/registry/test_registry_symmetry_unit.py b/tests/foundation/registry/test_registry_symmetry_unit.py index 35b5ade292..878d76db2a 100644 --- a/tests/foundation/registry/test_registry_symmetry_unit.py +++ b/tests/foundation/registry/test_registry_symmetry_unit.py @@ -35,10 +35,9 @@ here; nor does ``register_protocol_code``, which funnels into the same :meth:`ProtocolBase.register ` the ``register_tcp`` cases -reach. Class-level registrars with no foundation wrapper are out of scope: -``SCTP.register_chunk``, ``register_parameter`` and ``register_cause`` (covered -by :file:`tests/protocols/transport/test_sctp_unit.py`), ``Schema.register``, -the PCAP-NG ``Option.register`` and ``ESP.register``. +reach; nor does ``register_ipx``, whose registry ships no entry to restore. +Class-level registrars with no foundation wrapper are out of scope: +``Schema.register``, the PCAP-NG ``Option.register`` and ``ESP.register``. Every case restores the registries it touched, whatever its outcome. Classes are imported inside each test, after :func:`~tests._support.reimport_once_per_class`. @@ -77,6 +76,7 @@ def resolve(path: 'str') -> 'Any': 'Frame': 'pcapkit.protocols.misc.pcap.frame:Frame', 'PCAPNG': 'pcapkit.protocols.misc.pcapng:PCAPNG', 'Link': 'pcapkit.protocols.link.link:Link', + 'Loopback': 'pcapkit.protocols.link.loopback:Loopback', 'Internet': 'pcapkit.protocols.internet.internet:Internet', 'TCP': 'pcapkit.protocols.transport.tcp:TCP', 'UDP': 'pcapkit.protocols.transport.udp:UDP', @@ -128,6 +128,15 @@ class MethodRegistrar(NamedTuple): MethodRegistrar('register_tcp_mp_option', 'pcapkit.protocols.transport.tcp:TCP', '__mp_option__', 'pcapkit.protocols.schema.transport.tcp:MPTCP', ('pcapkit.const.tcp.mp_tcp_option:MPTCPOption.MP_JOIN',)), + MethodRegistrar('register_sctp_chunk', 'pcapkit.protocols.transport.sctp:SCTP', '__chunk__', + 'pcapkit.protocols.schema.transport.sctp:Chunk', + ('pcapkit.const.sctp.chunk:Chunk.Cookie_Acknowledgement',)), + MethodRegistrar('register_sctp_parameter', 'pcapkit.protocols.transport.sctp:SCTP', '__parameter__', + 'pcapkit.protocols.schema.transport.sctp:Parameter', + ('pcapkit.const.sctp.parameter:Parameter.Heartbeat_Info',)), + MethodRegistrar('register_sctp_cause', 'pcapkit.protocols.transport.sctp:SCTP', '__cause__', + 'pcapkit.protocols.schema.transport.sctp:ErrorCause', + ('pcapkit.const.sctp.cause_code:CauseCode.Stale_Cookie',)), MethodRegistrar('register_http_frame', 'pcapkit.protocols.application.httpv2:HTTP', '__frame__', 'pcapkit.protocols.schema.application.httpv2:FrameType', ('pcapkit.const.http.frame:Frame.HEADERS',)), @@ -173,6 +182,7 @@ class ModuleRegistrar(NamedTuple): ModuleRegistrar('register_ethertype', 'pcapkit.const.reg.ethertype:EtherType.Address_Resolution_Protocol', 'Link', 'pcapkit.protocols.link.link:Link'), + ModuleRegistrar('register_loopback', 2, 'Loopback', 'pcapkit.protocols.internet.internet:Internet'), ModuleRegistrar('register_transtype', 'pcapkit.const.reg.transtype:TransType.TCP', 'Internet', 'pcapkit.protocols.transport.transport:Transport'), ModuleRegistrar('register_tcp', 80, 'TCP', 'pcapkit.protocols.application.application:Application'), @@ -433,6 +443,7 @@ def test_every_public_registrar_is_covered_or_excused(self) -> None: 'register_reassembly_tcp_callback': 'append-only, nothing to undo with', 'register_traceflow_tcp_callback': 'append-only, nothing to undo with', 'register_icmp_type': 'no schema argument; override and restore are in test_icmp_type_registrar_unit', + 'register_ipx': 'IPX.__proto__ ships no entry, so an override has nothing to be restored to', } self.assertEqual(sorted(set(registry.__all__) - covered - set(excused)), []) self.assertEqual(sorted((covered | set(excused)) - set(registry.__all__)), []) diff --git a/tests/foundation/registry/test_registry_wrapper_coverage_2511_unit.py b/tests/foundation/registry/test_registry_wrapper_coverage_2511_unit.py new file mode 100644 index 0000000000..f3b927301d --- /dev/null +++ b/tests/foundation/registry/test_registry_wrapper_coverage_2511_unit.py @@ -0,0 +1,253 @@ +# -*- coding: utf-8 -*- +"""Every class-level registry map has its registration wrapper. C.f. #2511. + +The owner ruled that each dispatch map a protocol class keeps -- ``__proto__``, +``__option__``, ``__chunk__`` and the rest -- is written through a public +``register_*`` wrapper in :mod:`pcapkit.foundation.registry.protocols`, which +validates the code and the target before calling the owning class's +``register`` classmethod. Each wrapper names the map it writes in a +``# NOTE: `` line directly above its ``def``, and that tag is what this +module reads. + +The maps are found by walking every :class:`~pcapkit.protocols.protocol.ProtocolBase` +descendant for a dunder :class:`dict` in its own :func:`vars`, so a map added +tomorrow fails here until it has a wrapper or an entry in ``EXCUSED``. The +table is checked in both directions: an excused map that gains a wrapper, or +stops existing, fails too. + +The classmethod is the owning class's own -- ``register`` for ``__proto__``, +``register_`` for ``____`` -- unless ``INHERITED`` names the one it +inherits and why that one is typed for exactly the map's keys, the owner's +option 2 on #2511. That table is checked in both directions as well. + +The behaviour of the wrappers and classmethods #2511 added is pinned at the end: +each writes the owning class's map and warns in its own key's terms, and +``register_ipx`` and ``register_loopback`` dispatch a parse to what they +registered. + +""" + +import importlib +import inspect +import io +import pkgutil +import re +import unittest +import warnings +from unittest import mock + +from tests._support import reimport_once_per_class + +#: Test tier (#1958): a structural sweep over every protocol class. +__tier__ = 'LATEST' + +#: ``map path -> why it has no wrapper``. Each reason names where the gap is +#: owned, so that closing it deletes the entry here. +EXCUSED = { + 'pcapkit.protocols.protocol.ProtocolBase.__proto__': + 'the empty default a class without a map of its own inherits; nothing dispatches by it', + 'pcapkit.protocols.link.frad.FRAD.__proto__': 'wrapper and classmethod owned by #2510', + 'pcapkit.protocols.link.netlink.Netlink.__proto__': 'wrapper and classmethod owned by #2510', +} + +#: ``map path -> (the classmethod its owner inherits, why it fits the map's keys)``. +INHERITED = { + 'pcapkit.protocols.transport.tcp.TCP.__proto__': + ('pcapkit.protocols.transport.transport.Transport.register', 'typed for the port the map is keyed by'), + 'pcapkit.protocols.transport.udp.UDP.__proto__': + ('pcapkit.protocols.transport.transport.Transport.register', 'typed for the port the map is keyed by'), +} + +#: A wrapper's tag: one or more map paths on the comment line above its ``def``. +TAG = re.compile(r'^# NOTE: (?P\S.*)\ndef (?Pregister_\w+)\(', re.MULTILINE) + + +def _import_protocols() -> None: + """Import every shipped protocol module, so every class is a subclass by now.""" + import pcapkit.protocols as package + + for info in pkgutil.walk_packages(package.__path__, 'pcapkit.protocols.'): + if 'NotImplemented' not in info.name: + importlib.import_module(info.name) + + +def _maps() -> 'dict[str, type]': + """The class holding each dunder :class:`dict` in its own :func:`vars`, by path.""" + from pcapkit.protocols.protocol import ProtocolBase + + _import_protocols() + found = {} # type: dict[str, type] + seen = set() # type: set[type] + pending = [ProtocolBase] # type: list[type] + while pending: + klass = pending.pop() + if klass in seen: + continue + seen.add(klass) + pending.extend(klass.__subclasses__()) + for name, value in vars(klass).items(): + if (name.startswith('__') and name.endswith('__') and name != '__annotations__' + and isinstance(value, dict)): + found[f'{klass.__module__}.{klass.__qualname__}.{name}'] = klass + return found + + +def _classmethod_name(path: 'str') -> 'str': + """The ``register*`` classmethod writing the map at ``path``.""" + kind = path.rpartition('.')[2][2:-2] + return 'register' if kind == 'proto' else f'register_{kind}' + + +def _resolve(path: 'str') -> 'object': + """The object a dotted ``module.Class.attr`` path names.""" + parts = path.split('.') + for cut in range(len(parts) - 1, 0, -1): + try: + target = importlib.import_module('.'.join(parts[:cut])) + except ImportError: + continue + for attr in parts[cut:]: + target = vars(target)[attr] if isinstance(target, type) else getattr(target, attr) + return target + raise AssertionError(path) + + +def _tags() -> 'dict[str, list[str]]': + """``wrapper name -> the map paths its tag names``.""" + from pcapkit.foundation.registry import protocols + + source = inspect.getsource(protocols) + return {match['name']: [path.strip() for path in match['paths'].split(',')] + for match in TAG.finditer(source)} + + +class RegistryWrapperCoverageTests(unittest.TestCase): + """Each map is tagged by a public wrapper, or excused with a reason.""" + + def setUp(self) -> None: + reimport_once_per_class(self) + + def test_every_tag_names_a_real_map_of_a_public_wrapper(self) -> None: + from pcapkit.foundation.registry import protocols + + tags = _tags() + self.assertIn('register_sctp_chunk', tags) # the pattern still matches the module + for name, paths in tags.items(): + self.assertIn(name, protocols.__all__) + for path in paths: + with self.subTest(wrapper=name, map=path): + self.assertIsInstance(_resolve(path), dict) + + def test_every_map_has_a_wrapper_or_an_excuse(self) -> None: + tagged = {path for paths in _tags().values() for path in paths} + maps = _maps() + self.assertEqual(sorted(set(maps) - tagged - set(EXCUSED)), [], + 'a registry map has no register_* wrapper tagged "# NOTE: "') + self.assertEqual(sorted(set(EXCUSED) & tagged), [], 'an excused map has a wrapper now') + self.assertEqual(sorted(set(EXCUSED) - set(maps)), [], 'an excused map no longer exists') + + def test_every_map_owner_has_its_classmethod_or_inherits_a_typed_one(self) -> None: + maps = _maps() + self.assertIn('pcapkit.protocols.transport.tcp.TCP.__proto__', maps) # the walk still finds them + missing = [] + for path in sorted(set(maps) - set(EXCUSED)): + owner, name = maps[path], _classmethod_name(path) + with self.subTest(map=path): + if name in vars(owner): + self.assertNotIn(path, INHERITED, f'{owner.__name__} defines its own {name} now') + elif path in INHERITED: + inherited = _resolve(INHERITED[path][0]) + self.assertIs(getattr(owner, name).__func__, inherited.__func__) + else: + missing.append(f'{owner.__name__}.{name}') + self.assertEqual(missing, [], 'a map owner inherits a register* classmethod INHERITED does not name') + self.assertEqual(sorted(set(INHERITED) - set(maps)), [], 'an inherited map no longer exists') + self.assertEqual(sorted(set(INHERITED) & set(EXCUSED)), [], 'a map is both inherited and excused') + + def test_ipx_loopback_and_sctp_wrappers_write_the_owning_class_map(self) -> None: + from pcapkit.const.ipx.packet import Packet + from pcapkit.const.sctp.cause_code import CauseCode + from pcapkit.const.sctp.chunk import Chunk + from pcapkit.const.sctp.parameter import Parameter + from pcapkit.foundation.registry import protocols as registry + 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 + from pcapkit.protocols.transport.sctp import SCTP + from pcapkit.protocols.transport.udp import UDP + + # NCP is Packet Type 17, which is UDP's IP protocol number too. + cases = ((registry.register_ipx, IPX, Internet, Packet.NCP), + (registry.register_ipx, IPX, Internet, 4), + (registry.register_loopback, Loopback, Link, 2)) + for func, owner, parent, code in cases: + parent_before = dict(parent.__proto__) + with self.subTest(func=func.__name__, code=code), mock.patch.dict(owner.__proto__), \ + warnings.catch_warnings(): + warnings.simplefilter('ignore') + func(code, UDP) + self.assertIs(owner.__proto__[code], UDP) + self.assertEqual(dict(parent.__proto__), parent_before) + + cases = ( + (registry.register_sctp_chunk, Chunk.Cookie_Acknowledgement, '__chunk__', 'abort'), + (registry.register_sctp_parameter, Parameter.Heartbeat_Info, '__parameter__', 'cookie'), + (registry.register_sctp_cause, CauseCode.Stale_Cookie, '__cause__', 'no_user_data'), + ) + for func, code, table, meth in cases: + with self.subTest(func=func.__name__), mock.patch.dict(getattr(SCTP, table)), \ + warnings.catch_warnings(): + warnings.simplefilter('ignore') + func(code, meth) + self.assertEqual(getattr(SCTP, table)[code], meth) + + def test_ipx_and_loopback_classmethods_warn_in_their_own_key_terms(self) -> None: + """Each overrides its parent's, whose warning names an IP protocol or EtherType.""" + from pcapkit.const.ipx.packet import Packet + from pcapkit.protocols.internet.ipx import IPX + from pcapkit.protocols.link.loopback import Loopback + from pcapkit.protocols.misc.raw import Raw + from pcapkit.protocols.transport.udp import UDP + from pcapkit.utilities.warnings import RegistryWarning + + for owner, code, term in ((IPX, Packet.NCP, 'packet type'), (Loopback, 2, 'address family')): + with self.subTest(owner=owner.__name__), mock.patch.dict(owner.__proto__): + with warnings.catch_warnings(): + warnings.simplefilter('ignore') + owner.register(code, UDP) + with self.assertWarnsRegex(RegistryWarning, + '^' + re.escape(f'{term} {code} already registered, overwriting ')): + owner.register(code, Raw) + self.assertIs(owner.__proto__[code], Raw) + with warnings.catch_warnings(): + warnings.simplefilter('error') + owner.register(code, Raw) # handing back the stored entry is silent + self.assertIs(owner.__proto__[code], Raw) + + def test_register_ipx_and_register_loopback_dispatch_the_payload(self) -> None: + from pcapkit.const.ipx.packet import Packet + from pcapkit.foundation.registry import protocols as registry + from pcapkit.protocols import __proto__ as names + from pcapkit.protocols.internet.ipx import IPX + from pcapkit.protocols.link.loopback import Loopback + from pcapkit.protocols.transport.udp import UDP + + udp = bytes.fromhex('0035' '0035' '0008' '0000') + ipx = b'\xff\xff' + (30 + len(udp)).to_bytes(2, 'big') + b'\x00\x11' + bytes(24) + udp + # Address family 99, which nothing ships, written as LINKTYPE_NULL does. + loopback = (99).to_bytes(4, 'little') + udp + cases = ((registry.register_ipx, IPX, Packet.NCP, ipx), + (registry.register_loopback, Loopback, 99, loopback)) + for func, owner, code, packet in cases: + with self.subTest(func=func.__name__), mock.patch.dict(owner.__proto__), \ + mock.patch.dict(names), mock.patch.dict(registry._CODE_LEASES), \ + mock.patch.dict(registry._NAME_LEASES), warnings.catch_warnings(): + warnings.simplefilter('ignore') + func(code, 'pcapkit.protocols.transport.udp', 'UDP') + parsed = owner(io.BytesIO(packet), len(packet)) + self.assertIs(type(parsed.payload), UDP) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/protocols/test_protocol_code_registration_unit.py b/tests/protocols/test_protocol_code_registration_unit.py index b546611210..2ec9ec51f9 100644 --- a/tests/protocols/test_protocol_code_registration_unit.py +++ b/tests/protocols/test_protocol_code_registration_unit.py @@ -436,6 +436,7 @@ def setUp(self) -> None: def _assert_rejected_unchanged(self, call: 'Any') -> None: from pcapkit.protocols import __proto__ as protocol_registry from pcapkit.protocols.internet.internet import Internet + from pcapkit.protocols.internet.ipx import IPX from pcapkit.protocols.link.link import Link from pcapkit.protocols.misc.pcap.frame import Frame from pcapkit.protocols.misc.pcapng import PCAPNG @@ -445,7 +446,7 @@ def _assert_rejected_unchanged(self, call: 'Any') -> None: from pcapkit.utilities.exceptions import RegistryError tables = {klass.__name__: klass.__proto__ - for klass in (Link, Internet, Frame, PCAPNG, TCP, UDP, SCTP)} + for klass in (Link, Internet, IPX, Frame, PCAPNG, TCP, UDP, SCTP)} tables['pcapkit.protocols'] = protocol_registry before = {name: dict(table) for name, table in tables.items()} try: @@ -540,7 +541,8 @@ def test_every_destination_kind_row_is_enforced(self) -> None: 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 + accepts one (a port, a payload protocol identifier, an IPX Packet + Type) and rejected where it does not, which is the only observable difference the flag 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 @@ -552,6 +554,7 @@ def test_every_destination_kind_row_is_enforced(self) -> None: :func:`~pcapkit.foundation.registry.protocols.register_protocol_code` reads it: an inherited one would not be its own declaration. """ + from pcapkit.const.ipx.packet import Packet from pcapkit.const.reg.apptype import AppType from pcapkit.const.reg.ethertype import EtherType from pcapkit.const.reg.linktype import LinkType @@ -559,6 +562,7 @@ def test_every_destination_kind_row_is_enforced(self) -> None: from pcapkit.const.sctp.payload_protocol_identifier import PayloadProtocolIdentifier 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.misc.pcap.frame import Frame from pcapkit.protocols.misc.pcapng import PCAPNG @@ -573,6 +577,9 @@ def test_every_destination_kind_row_is_enforced(self) -> None: Link: (EtherType, False, TransType.ICMP, 0x0800), # 6 is TransType.TCP, where Internet ships TCP. Internet: (TransType, False, EtherType.XEROX_NS_IDP, 6), + # TransType.UDP is 17, Packet.NCP: IPX's registry of its own exists so + # that the one cannot key the other (#2169). + IPX: (Packet, True, TransType.UDP, 4), # The issue's own example: TransType.ICMP == LinkType.ETHERNET, where # both Frame and PCAPNG ship Ethernet. Frame: (LinkType, False, TransType.ICMP, 1), @@ -624,17 +631,19 @@ def test_a_registry_of_its_own_does_not_inherit_its_parent_pairing(self) -> None 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 +loopback.Loopback` subclasses :class:`~pcapkit.protocols.link.link.Link`, so it *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. + else entirely, and no enum names its keys for it to declare one of its + own. A key is therefore registered here rather than only the absent + declaration asserted: checking address family ``2`` against + :class:`~pcapkit.const.reg.ethertype.EtherType` rejects every key this + registry does accept, and an assertion on :func:`vars` alone passes + either way. + + :class:`~pcapkit.protocols.internet.ipx.IPX` declares a pairing of its + own instead (#2511), so a bare packet type and a member of its own enum + both reach it, where :class:`~pcapkit.protocols.internet.internet.Internet`'s + would refuse the first and misread the second as an IP protocol number. """ from pcapkit.const.ipx.packet import Packet from pcapkit.foundation.registry.protocols import register_protocol_code @@ -651,9 +660,12 @@ def test_a_registry_of_its_own_does_not_inherit_its_parent_pairing(self) -> None 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__) + if destination is IPX: + self.assertNotEqual(vars(destination)['__proto_key__'], parent.__proto_key__) + else: + 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