Skip to content

fix(protocols): restoring a registration puts back the shipped ModuleDescriptor (#1504) #1533

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JarryShaw merged 1 commit into from
Oct 9, 2026
Merged

fix(protocols): restoring a registration puts back the shipped ModuleDescriptor (#1504) #1533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 7 additions & 14 deletions pcapkit/protocols/internet/internet.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
from pcapkit.protocols.protocol import _PT, _ST, ProtocolBase
from pcapkit.protocols.schema.schema import keep_short_read, replay_short_read
from pcapkit.utilities.decorators import beholder
from pcapkit.utilities.exceptions import RegistryError
from pcapkit.utilities.warnings import RegistryWarning, warn

if TYPE_CHECKING:
Expand Down Expand Up @@ -156,24 +155,18 @@ def register(cls, code: 'Enum_TransType', protocol: 'ModuleDescriptor[ProtocolBa
replacement so a caller can tell *what* was lost. Fires only
when the incumbent differs from the replacement; see :meth:`ProtocolBase.register
<pcapkit.protocols.protocol.ProtocolBase.register>` for the
guard this shares with ``register_protocol``.
guard this shares with ``register_protocol``, and for the
shipped descriptor a restore puts back (:issue:`1504`).

"""
incumbent = cls.__proto__.get(code)
if incumbent is not None and (incumbent is protocol or (
isinstance(incumbent, ModuleDescriptor) and isinstance(protocol, ModuleDescriptor)
and incumbent == protocol)):
entry = cls._next_layer_entry(code, protocol)
if entry is None:
return
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}')
if incumbent is not None and incumbent is not protocol:
if incumbent is not None and incumbent is not entry:
warn(f'protocol {code} already registered, overwriting '
f'{incumbent!r} with {protocol!r}', RegistryWarning)
cls.__proto__[code] = protocol
f'{incumbent!r} with {entry!r}', RegistryWarning)
cls.__proto__[code] = entry

@classmethod
def from_data(cls, data: '_PT | dict[str, Any]', **kwargs: 'Any') -> 'Self':
Expand Down
21 changes: 7 additions & 14 deletions pcapkit/protocols/link/link.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
from pcapkit.corekit.module import ModuleDescriptor
from pcapkit.protocols.protocol import _PT, _ST, ProtocolBase
from pcapkit.protocols.schema.schema import keep_short_read, replay_short_read
from pcapkit.utilities.exceptions import RegistryError
from pcapkit.utilities.warnings import RegistryWarning, warn

if TYPE_CHECKING:
Expand Down Expand Up @@ -136,24 +135,18 @@ def register(cls, code: 'Enum_EtherType', protocol: 'ModuleDescriptor[ProtocolBa
when the incumbent differs from the replacement -- see
:meth:`ProtocolBase.register
<pcapkit.protocols.protocol.ProtocolBase.register>` for the
guard this shares with ``register_protocol``.
guard this shares with ``register_protocol``, and for the
shipped descriptor a restore puts back (:issue:`1504`).

"""
incumbent = cls.__proto__.get(code)
if incumbent is not None and (incumbent is protocol or (
isinstance(incumbent, ModuleDescriptor) and isinstance(protocol, ModuleDescriptor)
and incumbent == protocol)):
entry = cls._next_layer_entry(code, protocol)
if entry is None:
return
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}')
if incumbent is not None and incumbent is not protocol:
if incumbent is not None and incumbent is not entry:
warn(f'protocol {code} already registered, overwriting '
f'{incumbent!r} with {protocol!r}', RegistryWarning)
cls.__proto__[code] = protocol
f'{incumbent!r} with {entry!r}', RegistryWarning)
cls.__proto__[code] = entry

@classmethod
def from_data(cls, data: '_PT | dict[str, Any]', **kwargs: 'Any') -> 'Self':
Expand Down
22 changes: 8 additions & 14 deletions pcapkit/protocols/misc/pcap/frame.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
from pcapkit.protocols.protocol import ProtocolBase
from pcapkit.protocols.schema.misc.pcap.frame import Frame as Schema_Frame
from pcapkit.utilities.compat import localcontext
from pcapkit.utilities.exceptions import RegistryError, StreamEOFError, UnsupportedCall, stacklevel
from pcapkit.utilities.exceptions import StreamEOFError, UnsupportedCall, stacklevel
from pcapkit.utilities.warnings import ProtocolWarning, RegistryWarning, warn

if TYPE_CHECKING:
Expand Down Expand Up @@ -143,24 +143,18 @@ def register(cls, code: 'Enum_LinkType', protocol: 'ModuleDescriptor[ProtocolBas
Fires only when the incumbent differs from the replacement --
see :meth:`ProtocolBase.register
<pcapkit.protocols.protocol.ProtocolBase.register>` for the
guard this shares with ``register_protocol``.
guard this shares with ``register_protocol``, and for the
shipped descriptor a restore puts back (:issue:`1504`).

"""
incumbent = cls.__proto__.get(code)
if incumbent is not None and (incumbent is protocol or (
isinstance(incumbent, ModuleDescriptor) and isinstance(protocol, ModuleDescriptor)
and incumbent == protocol)):
entry = cls._next_layer_entry(code, protocol)
if entry is None:
return
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}')
if incumbent is not None and incumbent is not protocol:
if incumbent is not None and incumbent is not entry:
warn(f'protocol {code} already registered, overwriting '
f'{incumbent!r} with {protocol!r}', RegistryWarning)
cls.__proto__[code] = protocol
f'{incumbent!r} with {entry!r}', RegistryWarning)
cls.__proto__[code] = entry

def index(self, name: 'str | ProtocolBase | Type[ProtocolBase]') -> 'int':
"""Call :meth:`ProtoChain.index <pcapkit.corekit.protochain.ProtoChain.index>`.
Expand Down
79 changes: 74 additions & 5 deletions pcapkit/protocols/protocol.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import string
import struct
import textwrap
import types
import urllib.parse
from typing import TYPE_CHECKING, Any, Generic, Optional, Type, TypeVar, cast, overload

Expand Down Expand Up @@ -456,6 +457,13 @@ class ProtocolBase(Generic[_PT, _ST], metaclass=ProtocolMeta):
lambda: ModuleDescriptor('pcapkit.protocols.misc.raw', 'Raw'),
)

#: Read-only copy of :attr:`__proto__` as the class defining it shipped it,
#: taken by :meth:`__init_subclass__` before anything can register into it.
#: :meth:`_next_layer_entry` reads it to tell a descriptor handed back from
#: a new one without resolving it (:issue:`1504`).
_shipped_proto: 'types.MappingProxyType[Any, ModuleDescriptor[ProtocolBase] | Type[ProtocolBase]]' = \
types.MappingProxyType({})

#: Construction keywords this protocol consumes out of ``**kwargs`` instead
#: of declaring as a parameter, e.g. with ``kwargs.get('spam')`` in
#: :meth:`read` -- as :meth:`ESP.read <pcapkit.protocols.internet.esp.ESP.read>`
Expand Down Expand Up @@ -914,22 +922,73 @@ class object, or a :class:`~pcapkit.corekit.module.ModuleDescriptor`
deferred, and an incumbent left as an unresolved descriptor still
reports as different from the class it names.

A descriptor equal to the one ``code`` shipped with restores that
shipped object, unresolved, so undoing an override leaves the entry
as shipped (:issue:`1504`); see :meth:`_next_layer_entry`, which
every layer's ``register`` shares.

"""
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'protocol {code} already registered, overwriting '
f'{incumbent!r} with {entry!r}', RegistryWarning)
cls.__proto__[code] = entry

@classmethod
def _next_layer_entry(cls, code: 'Any', protocol: 'ModuleDescriptor[ProtocolBase] | Type[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` and
:class:`~pcapkit.protocols.misc.pcap.frame.Frame` overrides share this;
each still warns and stores for itself.

Arguments:
code: registry key
protocol: module descriptor or a :class:`ProtocolBase` subclass

Returns:
:obj:`None` if ``protocol`` is the entry stored under ``code``
already -- the same object, or a descriptor equal to a stored one
(:issue:`1364`). Otherwise the entry to store: if ``protocol`` is a
descriptor equal to the one ``code`` shipped with, per
:attr:`_shipped_proto`, the shipped object itself, unresolved;
else ``protocol`` resolved to its class.

Raises:
pcapkit.utilities.exceptions.RegistryError: If ``protocol`` is not a
class, or not a :class:`ProtocolBase` subclass.

Note:
The shipped descriptor is matched before it is resolved, so an
override undone by handing back the shipped entry leaves the
registry as it shipped: the entry is still a descriptor, nothing is
imported, and the class is looked up when a frame dispatches to it
(:issue:`1504`).

"""
incumbent = cls.__proto__.get(code)
if incumbent is not None and (incumbent is protocol or (
isinstance(incumbent, ModuleDescriptor) and isinstance(protocol, ModuleDescriptor)
and incumbent == protocol)):
return
return None
shipped = cls._shipped_proto.get(code)
if (isinstance(protocol, ModuleDescriptor) and isinstance(shipped, ModuleDescriptor)
and protocol == shipped):
return shipped
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}')
if incumbent is not None and incumbent is not protocol:
warn(f'protocol {code} already registered, overwriting '
f'{incumbent!r} with {protocol!r}', RegistryWarning)
cls.__proto__[code] = protocol
return protocol

@classmethod
def from_schema(cls, schema: '_ST | dict[str, Any]') -> 'Self':
Expand Down Expand Up @@ -1304,6 +1363,16 @@ def __init_subclass__(cls, /, schema: 'Optional[Type[_ST]]' = None,
cls.__schema__ = schema
cls.__data__ = data

# NOTE: a class defining its own ``__proto__`` ships what its body put
# there; copy it now, before anything can register into it, so that
# ``_next_layer_entry`` can restore a shipped descriptor without
# resolving it (:issue:`1504`). A class that inherits ``__proto__``
# inherits this copy with it.
if '__proto__' in cls.__dict__:
table = cls.__dict__['__proto__']
cls._shipped_proto = types.MappingProxyType(
dict(table) if isinstance(table, collections.abc.Mapping) else {})

if code is not None:
from pcapkit.foundation.registry.protocols import \
register_protocol_code # pylint: disable=import-outside-toplevel
Expand Down
22 changes: 8 additions & 14 deletions pcapkit/protocols/transport/sctp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@
from pcapkit.protocols.schema.transport.sctp import \
UserInitiatedAbortCause as Schema_UserInitiatedAbortCause
from pcapkit.protocols.transport.transport import Transport
from pcapkit.utilities.exceptions import ProtocolError, RegistryError
from pcapkit.utilities.exceptions import ProtocolError
from pcapkit.utilities.warnings import RegistryWarning, warn

if TYPE_CHECKING:
Expand Down Expand Up @@ -736,24 +736,18 @@ def register(cls, code: 'Enum_PayloadProtocolIdentifier | int', protocol: 'Modul
:class:`~pcapkit.corekit.module.ModuleDescriptor` incumbent
counts as different from the class it names; handing back the
stored entry, or a descriptor equal to it, is a silent no-op
that keeps it.
that keeps it, and handing back the descriptor a PPID shipped
with restores that object, unresolved (:issue:`1504`).

"""
incumbent = cls.__proto__.get(code)
if incumbent is not None and (incumbent is protocol or (
isinstance(incumbent, ModuleDescriptor) and isinstance(protocol, ModuleDescriptor)
and incumbent == protocol)):
entry = cls._next_layer_entry(code, protocol)
if entry is None:
return
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}')
if incumbent is not None and incumbent is not protocol:
if incumbent is not None and incumbent is not entry:
warn(f'payload protocol identifier {code} already registered, overwriting '
f'{incumbent!r} with {protocol!r}', RegistryWarning)
cls.__proto__[code] = protocol
f'{incumbent!r} with {entry!r}', RegistryWarning)
cls.__proto__[code] = entry

@classmethod
def register_chunk(cls, code: 'Enum_Chunk', meth: 'str | tuple[ChunkParser, ChunkConstructor]') -> 'None':
Expand Down
22 changes: 8 additions & 14 deletions pcapkit/protocols/transport/transport.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
from pcapkit.corekit.module import ModuleDescriptor
from pcapkit.protocols.protocol import _PT, _ST, ProtocolBase
from pcapkit.protocols.schema.schema import keep_short_read, replay_short_read
from pcapkit.utilities.exceptions import RegistryError, StructError, UnsupportedCall, stacklevel
from pcapkit.utilities.exceptions import StructError, UnsupportedCall, stacklevel
from pcapkit.utilities.logging import DEVMODE, get_logger
from pcapkit.utilities.warnings import RegistryWarning, warn

Expand Down Expand Up @@ -100,7 +100,8 @@ def register(cls, code: 'int', protocol: 'ModuleDescriptor[ProtocolBase] | Type[
different from the class it names. See
:meth:`ProtocolBase.register
<pcapkit.protocols.protocol.ProtocolBase.register>` for the
guard this shares with ``register_protocol``.
guard this shares with ``register_protocol``, and for the
shipped descriptor a restore puts back (:issue:`1504`).

Note:
:class:`~pcapkit.protocols.transport.tcp.TCP` and
Expand All @@ -114,20 +115,13 @@ def register(cls, code: 'int', protocol: 'ModuleDescriptor[ProtocolBase] | Type[
raise UnsupportedCall(f'{cls.__name__} is an abstract class')

incumbent = cls.__proto__.get(code)
if incumbent is not None and (incumbent is protocol or (
isinstance(incumbent, ModuleDescriptor) and isinstance(protocol, ModuleDescriptor)
and incumbent == protocol)):
entry = cls._next_layer_entry(code, protocol)
if entry is None:
return
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}')
if incumbent is not None and incumbent is not protocol:
if incumbent is not None and incumbent is not entry:
warn(f'port {code} already registered, overwriting '
f'{incumbent!r} with {protocol!r}', RegistryWarning)
cls.__proto__[code] = protocol
f'{incumbent!r} with {entry!r}', RegistryWarning)
cls.__proto__[code] = entry

@classmethod
def analyze(cls, ports: 'tuple[int, int]', payload: 'bytes', **kwargs: 'Any') -> 'ProtocolBase': # type: ignore[override] # pylint: disable=arguments-renamed
Expand Down
17 changes: 7 additions & 10 deletions tests/foundation/registry/test_registry_symmetry_unit.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,17 +405,14 @@ class TestRegistrySymmetry(harness.RoundTripBase):

STATUSES = ('RAISED', 'CHANGED')
KNOWN_FAILURES = (
Gap(1504, 'restoring a shipped dispatch entry over an override stores the class it names, '
'not the shipped ModuleDescriptor: each register resolves a descriptor unequal '
'to the incumbent (pcapkit/protocols/protocol.py:924, link/link.py:148, '
'internet/internet.py:168, transport/transport.py:122, misc/pcap/frame.py:155, '
'misc/pcapng.py:1027, transport/sctp.py:748), where the foundation registrars '
'keep the shipped object (#1363, #1364)',
'CHANGED', ("ModuleDescriptor(module='pcapkit.protocols.",),
Gap(1504, 'restoring a shipped PCAPNG.__proto__ entry over an override stores the class '
'it names, not the shipped ModuleDescriptor: PCAPNG.register resolves a '
'descriptor unequal to the incumbent (pcapkit/protocols/misc/pcapng.py:1027) '
'instead of sharing ProtocolBase._next_layer_entry, as the other layers do',
'CHANGED', ("PCAPNG.__proto__[<LinkType.ETHERNET: 1>] "
"ModuleDescriptor(module='pcapkit.protocols.",),
tuple(f'{name}/{variant}/dispatch'
for name in ('register_linktype', 'register_pcap', 'register_pcapng',
'register_ethertype', 'register_transtype', 'register_tcp',
'register_udp', 'register_apptype', 'register_sctp')
for name in ('register_linktype', 'register_pcapng')
for variant in ('override', 'override-same-name', 'restore-by-name'))),
Gap(1505, 'a protocol registrar adds the class it registers to pcapkit.protocols.__proto__ '
'by name (pcapkit/foundation/registry/protocols.py:235, reached from e.g. :416), '
Expand Down
Loading
Loading