From 0475f20445f1f097bd3a3897491b5093d88c1f78 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Mon, 5 Oct 2026 07:37:22 -0400 Subject: [PATCH] fix(protocols): raise RegistryError for a non-class in seven register guards (#1026) The `register` classmethods in `pcapkit/protocols/` document `RegistryError` for a bad argument but leaked a bare `TypeError` for a non-class, because the only check was `issubclass`, which rejects a non-class itself. Closes #1026. * Seven sites gain an explicit `isinstance(..., type)` test ahead of their `issubclass`, after the `ModuleDescriptor` unwrap where one applies: `ProtocolBase.register` (`protocol.py:769`), `Frame.register` (`frame.py:152`), `PCAPNG.register` (`pcapng.py:920`), `SCTP.register` (`sctp.py:629`), `Link.register` (`link.py:144`), `Internet.register` (`internet.py:164`) and the `Transport` guard reached through `TCP.register` and `UDP.register` (`transport.py:115`). * No guard's target class changes, so a wrong *class* raises `RegistryError` exactly as before. `Transport.register` itself is unaffected: its `UnsupportedCall` is gated on `cls is Transport`, so it still short-circuits for the abstract class at `transport.py:110`. That gate is also why the guard below it was reachable at all -- `TCP.register` and `UDP.register` are the only callers that get past it, and both leaked. Measured per site, reading the raise location off `traceback.extract_tb(e.__traceback__)[-1]` rather than off the message, because a plain-`type` metaclass and an `ABCMeta`-derived one produce the identical `issubclass() arg 1 must be a class` text from different frames: ProtocolBase.register -> RegistryError | protocol.py:769 Frame.register -> RegistryError | frame.py:152 PCAPNG.register -> RegistryError | pcapng.py:920 SCTP.register -> RegistryError | sctp.py:629 Link.register -> RegistryError | link.py:144 Internet.register -> RegistryError | internet.py:164 TCP.register -> RegistryError | transport.py:115 UDP.register -> RegistryError | transport.py:115 Transport.register -> UnsupportedCall | transport.py:110 Not breaking for exception handling: `RegistryError` subclasses `TypeError` through `BaseError`, so `except TypeError` still catches it. Two side effects do come with it, because `BaseError` is loud by default -- the non-class path now emits one `CRITICAL` log record and, outside devmode, installs `sys.excepthook` and `threading.excepthook`, neither of which the bare `TypeError` from `abc` did. Measured on both trees. That is already the wrong-class path's behaviour, so this makes the two consistent. New test pins every site in 6 methods carrying 72 subtests. Against main's guards 40 subtests fail and 32 pass; with the subtest-free `Transport` method that is 33 checks passing either way. Those are genuine guards rather than filler: a wrong class still raising (8), a descriptor naming a wrong class still falling through to the subclass check (8), a valid class and a valid descriptor still registering (16, the only thing proving the new isinstance test rejects nothing it should accept), and `Transport` still short-circuiting (1). tests/protocols/test_register_class_guard_unit.py + protocol base, registry and code-registration: 49 passed, 83 subtests. Dispatch: 24 passed, 87 subtests. tests/protocols/transport + link: 191 passed, 152 subtests. tests/project: 268 passed, 1 skipped, 864 subtests. --- CHANGELOG.md | 1 + docs/source/changelog/1.5.0.rst | 22 ++++ .../contributing/conventions/process.rst | 2 +- pcapkit/protocols/internet/internet.py | 4 +- pcapkit/protocols/link/link.py | 4 +- pcapkit/protocols/misc/pcap/frame.py | 4 +- pcapkit/protocols/misc/pcapng.py | 4 +- pcapkit/protocols/protocol.py | 4 +- pcapkit/protocols/transport/sctp.py | 4 +- pcapkit/protocols/transport/transport.py | 4 +- .../test_register_class_guard_unit.py | 121 ++++++++++++++++++ 11 files changed, 166 insertions(+), 8 deletions(-) create mode 100644 tests/protocols/test_register_class_guard_unit.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 96bcc9c472..8241b1de35 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -171,6 +171,7 @@ This resolves [#548](https://github.com/JarryShaw/PyPCAPKit/issues/548), which r - `httpv2._guess_version` tried to parse a stream as HTTP/2 and read failure as "not HTTP/2", rather than identifying the version first, so a genuine connection preface followed by a real `SETTINGS` frame raised `ProtocolError: unknown HTTP version`. It now identifies before it parses: a preface is recognised as HTTP/2 outright, skipped rather than fed to `httpv2`, and counted as header so `info` stays byte-identical to reading the following frame alone (otherwise the injected `packet=self.packet.payload`, sliced from octet 9 of a buffer whose frame starts at 24, reported preface remnants as payload). A preface with no frame behind it raises `ProtocolError: HTTP/2: connection preface with no frame`; a bare mid-stream `SETTINGS` frame, an `Upgrade: h2c` request and non-HTTP input are unchanged, the first two left undecidable on purpose (`Upgrade: h2c` needs per-connection state a single payload cannot carry, and a mid-stream frame has no safe heuristic, since a `type <= 9` guess misfires on binary HTTP/1 bodies). Protochain over all 23 sample captures (1,604 frames, 231 HTTP-bearing) is byte-identical to the unfixed tree ([#800](https://github.com/JarryShaw/PyPCAPKit/issues/800)). - `TCP.__proto__` bound `httpv1.HTTP` directly for ports 80 and 8080, so a segment on either port was HTTP/1 by assertion of the port alone; both versions share those ports, so the payload has to decide. Repointed to the generic `pcapkit.protocols.application.http.HTTP` proxy, whose identification became positive only once [#800](https://github.com/JarryShaw/PyPCAPKit/issues/800) and [#814](https://github.com/JarryShaw/PyPCAPKit/pull/814) landed, which is why this waited. `udp.py` already bound the proxy for both ports; this removes the asymmetry its docstring and `docs/source/pep.rst` documented as an open request (prose-only on that side). Protochain over all 23 captures (1,604 frames) is *not* byte-identical, and that is the fix: all 231 HTTP/1.1 frames keep their chain, and nine frames in `options-transport.pcap` change `Ethernet:IPv4:TCP:Raw` to `Ethernet:IPv4:TCP:HTTP/2` -- genuine HTTP/2 frames this library's own `httpv2.HTTP.make` built, previously refused by the HTTP/1 parser. `_guess_version`'s entry count over the corpus goes 0 to 252 (231 HTTP/1.1, 9 HTTP/2, 12 fall-throughs that stay `Raw`). Not labelled breaking, but not free: TCP:80/8080 traffic that is neither valid HTTP/1 nor preface-carrying is now exposed to the fall-through arm, where the direct `httpv1` binding left it `Raw` regardless; a caller depending on that is the one who would notice ([#682](https://github.com/JarryShaw/PyPCAPKit/issues/682)). - **a breaking change to** 8 more bespoke registries: step 2 of [#860](https://github.com/JarryShaw/PyPCAPKit/issues/860), PR 1 of 2 (`AppType` is PR 2, [#874](https://github.com/JarryShaw/PyPCAPKit/pull/874) below), bringing `StatusCode`, `ReturnCode`, `ResponseKind`, `GroupingInformation`, `OptionType`, `FEATCode`, `Command` and `Method` onto `EnumRegistry` and applying [#775](https://github.com/JarryShaw/PyPCAPKit/issues/775)'s mint/unmint ruling to their `get()` as well as `_missing_`. The first five convert 12 unambiguous placeholder branches (`Unassigned`, `Unknown`, `opt_unknown`) to `_unregistered_member`; the three with a custom `__new__` (`StatusCode`, `ReturnCode`, `OptionType`) get an override reconstructing the attributes the base's generic helper would leave unset, and `StatusCode`/`ReturnCode`'s hand-written `get()`, still on the retired `default == -1` convention, is replaced by the base's (no caller relied on the old form). `OptionType` keeps its own `get()` for its multi-namespace dispatch, but a round-2 review found it still minted on both its int/namespace and `str` paths, and the live pcapng parse path (`PCAPNG._make_pcapng_options`) calls it with wire bytes, so parsing an undeclared option code still registered a permanent member; both paths now build an unregistered member. `FEATCode`, `Command` and `Method` mint the literal wire value as its own name rather than any placeholder; per the owner's ruling (*"get will not have sufficient information to create new ones"*: `Command` needs `feat`/`desc`/`type`/`conf` and `Method` needs `safe`/`idempotent`, which a bare wire string lacks), both `_missing_` and each class's own `get()` (a second, independent mint site) now build an unregistered member. `FEATCode`'s crawler now declares all 15 real FEAT-code values from the live IANA table instead of minting 10 of them as a side effect of evaluating `Command`'s rows at import time, the import-time-mutation shape [#861](https://github.com/JarryShaw/PyPCAPKit/pull/861) removed from `FilterType`; pinned count-agnostically so a future IANA update cannot fail a correct regeneration. Untouched, per rulings: `CommandType` stays `IntFlag` (real `A|P` composites in the generated data), and `TransportProtocol`'s `auto()` renumbering and `AppType` are PR 2's. `pcapkit/protocols/schema/misc/pcapng.py`'s `OptionEnumField.post_process` docstring is corrected: it still bypasses `OptionType.get()` directly, now for `pickle` round-trip safety ([#860](https://github.com/JarryShaw/PyPCAPKit/issues/860) already stops the minting that used to be the reason), since neither of `OptionType`'s two paths gets both pickling and correct rendering right at once ([#869](https://github.com/JarryShaw/PyPCAPKit/pull/869)). +- `register` on `ProtocolBase`, `Frame`, `PCAPNG`, `SCTP`, `Link`, `Internet` and the concrete `Transport` subclasses (`TCP`, `UDP`) now raises `RegistryError` for a non-class argument, where it raised a bare `TypeError` ("issubclass() arg 1 must be a class") from inside `abc`. Each guard gains an explicit `isinstance(protocol, type)` test ahead of its `issubclass`, after the `ModuleDescriptor` is unwrapped, so a descriptor naming a non-class attribute is rejected too. All seven `Raises:` clauses already promised `RegistryError` if `protocol` "is not a ... subclass", which a non-class satisfied in spirit but not in fact; they now read "is not a class, or not a ... subclass". No guard's target class changes, and a wrong class still raises `RegistryError` as before. `Transport.register` on `Transport` itself still raises `UnsupportedCall` first; only its subclasses reach the guard. Not breaking for exception handling: `RegistryError` subclasses `TypeError` through `BaseError`, so `except TypeError` still catches it, and the type and message text are the only things a handler can key on that change. Two observable side effects do come with it, because `BaseError` is loud by default: the non-class path now emits pcapkit's one `CRITICAL` log record, and outside devmode it installs `sys.excepthook` and `threading.excepthook`, neither of which the bare `TypeError` from `abc` did. That is the wrong-class path's existing behaviour, so the fix makes the two consistent rather than introducing something new. Follows the same fix for the foundation registrars ([#1021](https://github.com/JarryShaw/PyPCAPKit/issues/1021)) ([#1026](https://github.com/JarryShaw/PyPCAPKit/issues/1026)). ### pcapkit.toolkit diff --git a/docs/source/changelog/1.5.0.rst b/docs/source/changelog/1.5.0.rst index 9b625a04ba..a30d282021 100644 --- a/docs/source/changelog/1.5.0.rst +++ b/docs/source/changelog/1.5.0.rst @@ -2052,6 +2052,28 @@ Fixed already stops the minting that used to be the reason), since neither of ``OptionType``'s two paths gets both pickling and correct rendering right at once (:pr:`869`). +* ``register`` on ``ProtocolBase``, ``Frame``, ``PCAPNG``, ``SCTP``, ``Link``, + ``Internet`` and the concrete ``Transport`` subclasses (``TCP``, ``UDP``) now raises + ``RegistryError`` for a non-class argument, where it raised a bare ``TypeError`` + ("issubclass() arg 1 must be a class") from inside ``abc``. Each guard gains an + explicit ``isinstance(protocol, type)`` test ahead of its ``issubclass``, after the + ``ModuleDescriptor`` is unwrapped, so a descriptor naming a non-class attribute is + rejected too. All seven ``Raises:`` clauses already promised ``RegistryError`` if + ``protocol`` "is not a ... subclass", which a non-class satisfied in spirit but not + in fact; they now read "is not a class, or not a ... subclass". No guard's target + class changes, and a wrong class still raises ``RegistryError`` as before. + ``Transport.register`` on ``Transport`` itself still raises ``UnsupportedCall`` + first; only its subclasses reach the guard. Not breaking for exception handling: + ``RegistryError`` subclasses ``TypeError`` through ``BaseError``, so + ``except TypeError`` still catches it, and the type and message text are the only + things a handler can key on that change. Two observable side effects do come with + it, because ``BaseError`` is loud by default: the non-class path now emits + pcapkit's one ``CRITICAL`` log record, and outside devmode it installs + ``sys.excepthook`` and ``threading.excepthook``, neither of which the bare + ``TypeError`` from ``abc`` did. That is the wrong-class path's existing behaviour, + so the fix makes the two consistent rather than introducing something new. + Follows the same fix for the foundation registrars (:issue:`1021`) + (:issue:`1026`). pcapkit.toolkit --------------- diff --git a/docs/source/contributing/conventions/process.rst b/docs/source/contributing/conventions/process.rst index 32eb9444a5..83a1eff846 100644 --- a/docs/source/contributing/conventions/process.rst +++ b/docs/source/contributing/conventions/process.rst @@ -101,7 +101,7 @@ commands down instead of a figure that will be stale by the next merge: The grouping scheme was settled on :issue:`918`: **a section per top-level module, with** ``Added``/``Changed``/``Fixed`` **nested inside each** -- module granularity, not per-file and not per-subpackage. The file carries **9** module-level sections holding -158 entries, and no entry carries an inline kind label:: +159 entries, and no entry carries an inline kind label:: $ grep -cE '^\* \*\*(Added|Changed|Fixed)\*\*' docs/source/changelog/1.5.0.rst 0 diff --git a/pcapkit/protocols/internet/internet.py b/pcapkit/protocols/internet/internet.py index 514afca934..7240e5bd35 100644 --- a/pcapkit/protocols/internet/internet.py +++ b/pcapkit/protocols/internet/internet.py @@ -146,7 +146,7 @@ def register(cls, code: 'Enum_TransType', protocol: 'ModuleDescriptor[ProtocolBa Raises: pcapkit.utilities.exceptions.RegistryError: If ``protocol`` is not a - :class:`~pcapkit.protocols.protocol.Protocol` subclass. + class, or not a :class:`~pcapkit.protocols.protocol.Protocol` subclass. Warns: pcapkit.utilities.warnings.RegistryWarning: If this transport-layer @@ -160,6 +160,8 @@ def register(cls, code: 'Enum_TransType', protocol: 'ModuleDescriptor[ProtocolBa """ if isinstance(protocol, ModuleDescriptor): protocol = protocol.klass + if not isinstance(protocol, type): + raise RegistryError(f'protocol must be a class, not {protocol!r}') if not issubclass(protocol, ProtocolBase): raise RegistryError(f'protocol must be a Protocol subclass, not {protocol!r}') incumbent = cls.__proto__.get(code) diff --git a/pcapkit/protocols/link/link.py b/pcapkit/protocols/link/link.py index 2e761824d0..af9544ef7c 100644 --- a/pcapkit/protocols/link/link.py +++ b/pcapkit/protocols/link/link.py @@ -126,7 +126,7 @@ def register(cls, code: 'Enum_EtherType', protocol: 'ModuleDescriptor[ProtocolBa Raises: pcapkit.utilities.exceptions.RegistryError: If ``protocol`` is not a - :class:`~pcapkit.protocols.protocol.Protocol` subclass. + class, or not a :class:`~pcapkit.protocols.protocol.Protocol` subclass. Warns: pcapkit.utilities.warnings.RegistryWarning: If this EtherType is @@ -140,6 +140,8 @@ def register(cls, code: 'Enum_EtherType', protocol: 'ModuleDescriptor[ProtocolBa """ if isinstance(protocol, ModuleDescriptor): protocol = protocol.klass + if not isinstance(protocol, type): + raise RegistryError(f'protocol must be a class, not {protocol!r}') if not issubclass(protocol, ProtocolBase): raise RegistryError(f'protocol must be a Protocol subclass, not {protocol!r}') incumbent = cls.__proto__.get(code) diff --git a/pcapkit/protocols/misc/pcap/frame.py b/pcapkit/protocols/misc/pcap/frame.py index a391b386cc..5bb0c5c6cc 100644 --- a/pcapkit/protocols/misc/pcap/frame.py +++ b/pcapkit/protocols/misc/pcap/frame.py @@ -134,7 +134,7 @@ def register(cls, code: 'Enum_LinkType', protocol: 'ModuleDescriptor[ProtocolBas Raises: pcapkit.utilities.exceptions.RegistryError: If ``protocol`` is not a - :class:`~pcapkit.protocols.protocol.Protocol` subclass. + class, or not a :class:`~pcapkit.protocols.protocol.Protocol` subclass. Warns: pcapkit.utilities.warnings.RegistryWarning: If this link type is @@ -148,6 +148,8 @@ def register(cls, code: 'Enum_LinkType', protocol: 'ModuleDescriptor[ProtocolBas """ if isinstance(protocol, ModuleDescriptor): protocol = protocol.klass + if not isinstance(protocol, type): + raise RegistryError(f'protocol must be a class, not {protocol!r}') if not issubclass(protocol, ProtocolBase): raise RegistryError(f'protocol must be a Protocol subclass, not {protocol!r}') incumbent = cls.__proto__.get(code) diff --git a/pcapkit/protocols/misc/pcapng.py b/pcapkit/protocols/misc/pcapng.py index cb34659eea..331d1b60a7 100644 --- a/pcapkit/protocols/misc/pcapng.py +++ b/pcapkit/protocols/misc/pcapng.py @@ -901,7 +901,7 @@ def register(cls, code: 'Enum_LinkType', protocol: 'ModuleDescriptor[ProtocolBas Raises: pcapkit.utilities.exceptions.RegistryError: If ``protocol`` is not a - :class:`~pcapkit.protocols.protocol.Protocol` subclass. + class, or not a :class:`~pcapkit.protocols.protocol.Protocol` subclass. Warns: pcapkit.utilities.warnings.RegistryWarning: If this link type is @@ -916,6 +916,8 @@ def register(cls, code: 'Enum_LinkType', protocol: 'ModuleDescriptor[ProtocolBas """ if isinstance(protocol, ModuleDescriptor): protocol = protocol.klass + if not isinstance(protocol, type): + raise RegistryError(f'protocol must be a class, not {protocol!r}') if not issubclass(protocol, ProtocolBase): raise RegistryError(f'protocol must be a Protocol subclass, not {protocol!r}') incumbent = cls.__proto__.get(code) diff --git a/pcapkit/protocols/protocol.py b/pcapkit/protocols/protocol.py index af40bb74b8..890b7a31a4 100644 --- a/pcapkit/protocols/protocol.py +++ b/pcapkit/protocols/protocol.py @@ -738,7 +738,7 @@ def register(cls, code: 'int', protocol: 'ModuleDescriptor | Type[ProtocolBase]' Raises: pcapkit.utilities.exceptions.RegistryError: If ``protocol`` is not a - :class:`~pcapkit.protocols.protocol.ProtocolBase` subclass. + class, or not a :class:`~pcapkit.protocols.protocol.ProtocolBase` subclass. Warns: pcapkit.utilities.warnings.RegistryWarning: If ``code`` is already @@ -765,6 +765,8 @@ def register(cls, code: 'int', protocol: 'ModuleDescriptor | Type[ProtocolBase]' """ if isinstance(protocol, ModuleDescriptor): protocol = protocol.klass + if not isinstance(protocol, type): + raise RegistryError(f'protocol must be a class, not {protocol!r}') if not issubclass(protocol, ProtocolBase): raise RegistryError(f'protocol must be a Protocol subclass, not {protocol!r}') incumbent = cls.__proto__.get(code) diff --git a/pcapkit/protocols/transport/sctp.py b/pcapkit/protocols/transport/sctp.py index b60ace5aab..c337c74a71 100644 --- a/pcapkit/protocols/transport/sctp.py +++ b/pcapkit/protocols/transport/sctp.py @@ -611,7 +611,7 @@ def register(cls, code: 'Enum_PayloadProtocolIdentifier | int', protocol: 'Modul Raises: pcapkit.utilities.exceptions.RegistryError: If ``protocol`` is not a - :class:`~pcapkit.protocols.protocol.ProtocolBase` subclass. + class, or not a :class:`~pcapkit.protocols.protocol.ProtocolBase` subclass. Warns: pcapkit.utilities.warnings.RegistryWarning: If this PPID is already @@ -625,6 +625,8 @@ def register(cls, code: 'Enum_PayloadProtocolIdentifier | int', protocol: 'Modul """ if isinstance(protocol, ModuleDescriptor): protocol = protocol.klass + if not isinstance(protocol, type): + raise RegistryError(f'protocol must be a class, not {protocol!r}') if not issubclass(protocol, ProtocolBase): raise RegistryError(f'protocol must be a Protocol subclass, not {protocol!r}') incumbent = cls.__proto__.get(code) diff --git a/pcapkit/protocols/transport/transport.py b/pcapkit/protocols/transport/transport.py index 11353f6d07..d94d69efdb 100644 --- a/pcapkit/protocols/transport/transport.py +++ b/pcapkit/protocols/transport/transport.py @@ -88,7 +88,7 @@ def register(cls, code: 'int', protocol: 'ModuleDescriptor[ProtocolBase] | Type[ pcapkit.utilities.exceptions.UnsupportedCall: If called on :class:`Transport` itself. pcapkit.utilities.exceptions.RegistryError: If ``protocol`` is not a - :class:`~pcapkit.protocols.protocol.Protocol` subclass. + class, or not a :class:`~pcapkit.protocols.protocol.Protocol` subclass. Warns: pcapkit.utilities.warnings.RegistryWarning: If this port is already @@ -111,6 +111,8 @@ def register(cls, code: 'int', protocol: 'ModuleDescriptor[ProtocolBase] | Type[ if isinstance(protocol, ModuleDescriptor): protocol = protocol.klass + if not isinstance(protocol, type): + raise RegistryError(f'protocol must be a class, not {protocol!r}') if not issubclass(protocol, ProtocolBase): raise RegistryError(f'protocol must be a Protocol subclass, not {protocol!r}') incumbent = cls.__proto__.get(code) diff --git a/tests/protocols/test_register_class_guard_unit.py b/tests/protocols/test_register_class_guard_unit.py new file mode 100644 index 0000000000..6d85c2a0b9 --- /dev/null +++ b/tests/protocols/test_register_class_guard_unit.py @@ -0,0 +1,121 @@ +# -*- coding: utf-8 -*- +"""GitHub issue #1026: the ``register`` classmethods reject a non-class themselves. + +Each ``register`` classmethod guarded with a bare ``issubclass(protocol, +ProtocolBase)``. A non-class argument is refused by ``issubclass`` itself +before the guard's own ``raise`` is reached, so the caller got ``TypeError: +issubclass() arg 1 must be a class`` leaked out of ``abc`` rather than +:class:`~pcapkit.utilities.exceptions.RegistryError`. Each now tests +``isinstance(protocol, type)`` first, after the +:class:`~pcapkit.corekit.module.ModuleDescriptor` is unwrapped, so a descriptor +naming a non-class attribute is rejected too. + +Every registry is process-wide, so each call runs under +:func:`unittest.mock.patch.dict`; the refusals under test write nothing, and the +patch keeps a regression that *did* write from leaking into other tests. + +""" +from __future__ import annotations + +import importlib +import importlib.util +import unittest +import warnings +from unittest import mock + +RUNTIME_DEPS = ('tbtrim', 'aenum', 'chardet', 'dictdumper') +HAS_RUNTIME = all(importlib.util.find_spec(name) is not None for name in RUNTIME_DEPS) + +#: ``(label, module, class, code)``; the code is any key the site accepts. +SITES = ( + ('ProtocolBase', 'pcapkit.protocols.protocol', 'ProtocolBase', 1), + ('Frame', 'pcapkit.protocols.misc.pcap.frame', 'Frame', 1), + ('PCAPNG', 'pcapkit.protocols.misc.pcapng', 'PCAPNG', 1), + ('SCTP', 'pcapkit.protocols.transport.sctp', 'SCTP', 1), + ('Link', 'pcapkit.protocols.link.link', 'Link', 1), + ('Internet', 'pcapkit.protocols.internet.internet', 'Internet', 1), + # ``Transport.register`` raises ``UnsupportedCall`` only for ``Transport`` + # itself; a concrete subclass reaches the guard. + ('TCP', 'pcapkit.protocols.transport.tcp', 'TCP', 1), + ('UDP', 'pcapkit.protocols.transport.udp', 'UDP', 1), +) + + +@unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed') +class RegisterClassGuardTests(unittest.TestCase): + def _sites(self): + for label, module, name, code in SITES: + yield label, getattr(importlib.import_module(module), name), code + + def test_non_class_raises_registry_error_from_the_guard(self) -> None: + from pcapkit.utilities.exceptions import RegistryError + + for label, klass, code in self._sites(): + for bad in (1, "Raw", None, [int]): + with self.subTest(site=label, bad=repr(bad)): + with mock.patch.dict(klass.__proto__): + with self.assertRaises(RegistryError) as caught: + klass.register(code, bad) # type: ignore[arg-type] + self.assertIn('must be a class', str(caught.exception)) + # ``RegistryError`` is a ``TypeError``, so callers catching + # the old exception keep working. + self.assertIsInstance(caught.exception, TypeError) + + def test_wrong_class_still_raises_registry_error(self) -> None: + from pcapkit.utilities.exceptions import RegistryError + + for label, klass, code in self._sites(): + with self.subTest(site=label): + with mock.patch.dict(klass.__proto__): + with self.assertRaises(RegistryError) as caught: + klass.register(code, dict) # type: ignore[arg-type] + self.assertIn('Protocol subclass', str(caught.exception)) + + def test_descriptor_naming_a_non_class_raises_registry_error(self) -> None: + from pcapkit.corekit.module import ModuleDescriptor + from pcapkit.utilities.exceptions import RegistryError + + # ``pcapkit.protocols.protocol.__name__`` is a ``str``. + descriptor = ModuleDescriptor('pcapkit.protocols.protocol', '__name__') + for label, klass, code in self._sites(): + with self.subTest(site=label): + with mock.patch.dict(klass.__proto__): + with self.assertRaises(RegistryError) as caught: + klass.register(code, descriptor) # type: ignore[arg-type] + self.assertIn('must be a class', str(caught.exception)) + + def test_descriptor_naming_a_wrong_class_still_raises_registry_error(self) -> None: + from pcapkit.corekit.module import ModuleDescriptor + from pcapkit.utilities.exceptions import RegistryError + + descriptor = ModuleDescriptor('collections', 'OrderedDict') + for label, klass, code in self._sites(): + with self.subTest(site=label): + with mock.patch.dict(klass.__proto__): + with self.assertRaises(RegistryError) as caught: + klass.register(code, descriptor) # type: ignore[arg-type] + self.assertIn('Protocol subclass', str(caught.exception)) + + def test_valid_class_and_descriptor_still_register(self) -> None: + from pcapkit.corekit.module import ModuleDescriptor + from pcapkit.protocols.misc.raw import Raw + + for label, klass, code in self._sites(): + for arg in (Raw, ModuleDescriptor('pcapkit.protocols.misc.raw', 'Raw')): + with self.subTest(site=label, arg=type(arg).__name__): + with mock.patch.dict(klass.__proto__), warnings.catch_warnings(): + # Overwriting the built-in entry is expected here. + warnings.simplefilter('ignore') + klass.register(code, arg) + self.assertIs(klass.__proto__[code], Raw) + + def test_transport_itself_is_still_unsupported(self) -> None: + from pcapkit.protocols.transport.transport import Transport + from pcapkit.utilities.exceptions import UnsupportedCall + + with self.assertRaises(UnsupportedCall): + Transport.register(1, 1) # type: ignore[arg-type] + + +if __name__ == '__main__': + unittest.main()