diff --git a/docs/source/pcapkit/const/reg.rst b/docs/source/pcapkit/const/reg.rst index 16ff8cfee1..95b3cd2933 100644 --- a/docs/source/pcapkit/const/reg.rst +++ b/docs/source/pcapkit/const/reg.rst @@ -109,7 +109,7 @@ registry is one enumeration per transport protocol sharing a memberless base. :members: :undoc-members: :show-inheritance: - :special-members: __new__, __repr__, __str__, __int__, __eq__, __registry__, __canonical__ + :special-members: __new__, __repr__, __str__, __int__, __eq__, __hash__, __registry__, __canonical__ .. module:: pcapkit.const.reg.apptype.tcp diff --git a/docs/source/pcapkit/foundation/reassembly/tcp.rst b/docs/source/pcapkit/foundation/reassembly/tcp.rst index 875045cd60..c29db428d2 100644 --- a/docs/source/pcapkit/foundation/reassembly/tcp.rst +++ b/docs/source/pcapkit/foundation/reassembly/tcp.rst @@ -14,6 +14,7 @@ which reconstructs fragmented TCP packets back to origin. .. automethod:: reassembly .. automethod:: submit + .. automethod:: _make_ports .. autoattribute:: __protocol_name__ .. autoattribute:: __protocol_type__ diff --git a/pcapkit/const/reg/apptype/apptype.py b/pcapkit/const/reg/apptype/apptype.py index 31fab56809..13c8c3b1e3 100644 --- a/pcapkit/const/reg/apptype/apptype.py +++ b/pcapkit/const/reg/apptype/apptype.py @@ -2497,6 +2497,17 @@ def __ne__(self, other: 'Any') -> 'bool': return self.port != other def __hash__(self) -> 'int': + """Hash the member by its port number, as :meth:`__eq__` compares it. + + Keeping the two consistent is what lets a member and the bare + :obj:`int` port it carries be the same key of an :obj:`int`-keyed + mapping -- which is how :meth:`Transport.analyze + ` reads + :attr:`Transport.__proto__ + ` with + either one. + + """ return hash(self.port) @classmethod @@ -2941,6 +2952,22 @@ def _unregistered_member(cls, value: 'int', name: 'str', exactly the one this registry must not remember past the call that built it. + The member is made reducible through + :meth:`~pcapkit.corekit.enum.EnumRegistry._make_reducible` before it is + returned, so that it survives :mod:`pickle` and :mod:`copy`. Without + that, ``Enum.__reduce_ex__`` would reduce it to a lookup of its own + crafted value -- ``TCP('unknown [54321 - tcp]')`` -- which + :meth:`_missing_` rejects as not a port, so the member pickled fine and + then raised :exc:`ValueError` on read-back. That bit reassembly, whose + :class:`~pcapkit.foundation.reassembly.data.tcp.Datagram` holds these + members in an unresolved + :class:`~pcapkit.foundation.reassembly.data.data.Deferred`: measured + before the fix, 59,413 of the 65,536 possible ports -- every + ``unknown`` and every IANA-span label alike, not just the truly + unregistered ones -- failed to round-trip. Rebuilding re-enters this + method, so an unpickled member is unregistered exactly as the original + was and neither lookup table nor :data:`__registry__` grows. + Args: value: Port number. name: Bare label for the unregistered member -- what @@ -2956,7 +2983,9 @@ def _unregistered_member(cls, value: 'int', name: 'str', obj.svc = name obj.port = value obj.proto = proto - return obj + # NOTE: reduce to this method's own (value, name, proto), not to the + # crafted ``temp`` the base was handed -- see _make_reducible's note. + return cls._make_reducible(obj, value, name, proto) @classmethod def _missing_(cls, value: 'int') -> 'Optional[AppType]': diff --git a/pcapkit/corekit/enum.py b/pcapkit/corekit/enum.py index 3503ad05e4..05176a4826 100644 --- a/pcapkit/corekit/enum.py +++ b/pcapkit/corekit/enum.py @@ -96,6 +96,7 @@ an unknown value does not mint one (:issue:`775`). """ +import functools from typing import TYPE_CHECKING from aenum import extend_enum @@ -104,7 +105,7 @@ from pcapkit.utilities.exceptions import BaseError, CaughtByCaller, EnumKeyError, EnumValueError if TYPE_CHECKING: - from typing import Any + from typing import Any, Callable, Type from typing_extensions import Self @@ -709,3 +710,97 @@ def _unregistered_member(cls, value: 'Any', name: 'str') -> 'Self': obj._name_ = name # pylint: disable=protected-access obj._value_ = value # pylint: disable=protected-access return obj + + @classmethod + def _make_reducible(cls, obj: 'Any', *args: 'Any') -> 'Any': + """Make an unregistered member survive :mod:`pickle` and :mod:`copy`. + + ``Enum.__reduce_ex__`` reduces a member to ``(cls, (value,))``, i.e. to + a *value*-keyed lookup -- the one table an unregistered member is + deliberately absent from. So a member :meth:`_unregistered_member` + built pickles happily and then raises :exc:`ValueError` on read-back, + where the failure is a long way from its cause. + + This replaces that reduction with one through + :func:`_rebuild_unregistered_member`, which re-enters + ``cls._unregistered_member(*args)``. Rebuilding therefore takes exactly + the path that built the original and never ``cls.__new__``, so an + unpickled member is unregistered just as the original was and no + lookup table grows. The rebuilt member is itself reducible, since the + method it comes back through installs this again. + + The override goes on the *instance* rather than on the type, which both + :mod:`pickle` and :mod:`copy` honour: each fetches ``__reduce_ex__`` + with :func:`getattr` on the object. :func:`functools.partial` rather + than a closure keeps the member itself out of its own reducer, so being + reducible does not put it in a reference cycle. + + Note: + This is opt-in per registry rather than done by + :meth:`_unregistered_member` itself, because the arguments to + rebuild *from* are the overriding method's own, not this base + method's. :class:`~pcapkit.const.reg.apptype.apptype.AppType`, + for one, hands its base the crafted ``'svc [port - proto]'`` + string while its own entry point takes ``(port, name, proto)``; + reducing to the former would rebuild a member whose value had been + formatted twice. A registry whose ``_missing_`` calls + :meth:`_unregistered_member` directly is unaffected and keeps + reducing the way ``Enum`` does. + + Args: + obj: The unregistered member to install the reducer on. + *args: The arguments ``cls._unregistered_member`` should be called + with to rebuild an equivalent member. + + Returns: + ``obj``, for use as a ``return`` expression. + + """ + obj.__reduce_ex__ = functools.partial(_reduce_unregistered_member, cls, args) + return obj + + +def _rebuild_unregistered_member(namespace: 'Type[EnumRegistry]', + args: 'tuple[Any, ...]') -> 'Any': + """Rebuild the member :meth:`EnumRegistry._make_reducible` reduced. + + This is what :mod:`pickle` and :mod:`copy` reconstruct through, in place of + the value lookup ``Enum.__reduce_ex__`` would have reduced the member to. + It is a module-level function rather than a method so that every + :mod:`pickle` protocol can name it: protocols below 4 cannot reference a + callable nested inside a class. + + Args: + namespace: The registry class to rebuild the member as an instance of. + args: The arguments to ``namespace._unregistered_member``. + + Returns: + A member equal to the original and, like it, absent from every one of + ``namespace``'s lookup tables. + + """ + return namespace._unregistered_member(*args) # pylint: disable=protected-access + + +def _reduce_unregistered_member( # pylint: disable=unused-argument + namespace: 'Type[EnumRegistry]', args: 'tuple[Any, ...]', + protocol: 'int') -> 'tuple[Callable[..., Any], tuple[Any, ...]]': + """The ``__reduce_ex__`` :meth:`EnumRegistry._make_reducible` installs. + + Bound to its first two arguments with :func:`functools.partial`, so that + the reducer holds the ingredients of the member rather than the member + itself. + + Args: + namespace: The registry class the member is an instance of. + args: The arguments to rebuild the member from. + protocol: The :mod:`pickle` protocol version, as passed by + :mod:`pickle` and :mod:`copy`. Unused -- the reduction is the same + for every protocol -- but accepted because both call this with it. + + Returns: + The callable and arguments :mod:`pickle` and :mod:`copy` should rebuild + the member through. + + """ + return (_rebuild_unregistered_member, (namespace, args)) diff --git a/pcapkit/foundation/reassembly/data/data.py b/pcapkit/foundation/reassembly/data/data.py index 5f6d9180c6..b7b0d28a6e 100644 --- a/pcapkit/foundation/reassembly/data/data.py +++ b/pcapkit/foundation/reassembly/data/data.py @@ -12,13 +12,14 @@ if TYPE_CHECKING: from typing import Callable, Optional + from pcapkit.const.reg.apptype import TCP as AppType_TCP from pcapkit.const.reg.transtype import TransType from pcapkit.foundation.reassembly.data.ip import Datagram as IP_Datagram from pcapkit.foundation.reassembly.data.tcp import Datagram as TCP_Datagram from pcapkit.protocols.protocol import ProtocolBase # registry key an analyser dispatches a reassembled payload on; see ``Deferred`` -_KT = TypeVar('_KT', 'TransType', 'tuple[int, int]') +_KT = TypeVar('_KT', 'TransType', 'tuple[AppType_TCP, AppType_TCP]') class Completion(EnumLookup, StrEnum): @@ -148,23 +149,37 @@ class Deferred(Generic[_KT]): :meth:`Protocol.analyze `, which looks the next layer up by protocol registry number. * :meth:`TCP.submit ` builds a - ``Deferred[tuple[int, int]]``. It passes a ``(source, destination)`` port - pair, because :meth:`Transport.analyze + ``Deferred[tuple[AppType_TCP, AppType_TCP]]``. It passes a + ``(source, destination)`` port pair, because :meth:`Transport.analyze ` overrides that parameter to one: a reassembled transport-layer payload carries no protocol number, so its application protocol is inferred from the ports. - Those are the two cases the type parameter is constrained to. The pairing - the regression actually produced -- a port pair handed to an analyser - declared over :class:`~pcapkit.const.reg.transtype.TransType` -- is - rejected, which is the direction a real mismatch takes and the one + Those are the two cases the type parameter is constrained to. Both are + *named registry values* rather than one named value and one pair of bare + port numbers: a port is a member of + :class:`~pcapkit.const.reg.apptype.tcp.TCP` here -- the per-transport + registry itself, not the base + :class:`~pcapkit.const.reg.apptype.apptype.AppType`, since that is what + :meth:`TCP._make_ports ` + really answers with -- so what the deferred analysis dispatches on is of a + piece with what the eager path dispatches on at + :attr:`TCP.src `, whichever + reassembler built it. (Of a piece, not identical: for an *unassigned* port + the two paths build equal members by different routes, which + ``TCP.submit``'s own docstring records.) + + The pairing the regression actually produced -- a port pair handed to an + analyser declared over :class:`~pcapkit.const.reg.transtype.TransType` -- + is rejected, which is the direction a real mismatch takes and the one :issue:`2417` was about. Rejecting the *reverse* pairing is not guaranteed by mypy alone, for a reason outside this class: |aenum|_ ships no type stubs, so under ``ignore_missing_imports`` the ``aenum.IntEnum`` base of :class:`~pcapkit.const.reg.transtype.TransType` resolves to ``Any``. A - ``TransType`` is therefore accepted where a ``tuple[int, int]`` is + ``TransType`` is therefore accepted where a + ``tuple[AppType_TCP, AppType_TCP]`` is expected, and a ``Deferred[TransType]`` built over the port-pair analyser type-checks clean. That is a gap in the dependency's type coverage rather than in the constraint, which would bite both ways were ``TransType`` fully diff --git a/pcapkit/foundation/reassembly/data/tcp.py b/pcapkit/foundation/reassembly/data/tcp.py index 82ace04fa4..886ce25277 100644 --- a/pcapkit/foundation/reassembly/data/tcp.py +++ b/pcapkit/foundation/reassembly/data/tcp.py @@ -19,6 +19,7 @@ from typing_extensions import TypeAlias + from pcapkit.const.reg.apptype import TCP as AppType_TCP from pcapkit.protocols.protocol import ProtocolBase _AT = TypeVar('_AT', 'IPv4Address', 'IPv6Address') @@ -84,7 +85,26 @@ def __init__(self, bufid: 'BufferID', dsn: 'int', ack: 'int', num: 'int', syn: ' @info_final class DatagramID(Info, Generic[_AT]): - """Data model for :term:`TCP ` original packet identifier.""" + """Data model for :term:`TCP ` original packet identifier. + + Note: + The ports here stay bare :obj:`int` port numbers, where the pair + :meth:`TCP.submit ` hands + :class:`~pcapkit.foundation.reassembly.data.data.Deferred` is one of + :class:`~pcapkit.const.reg.apptype.tcp.TCP` members + (:issue:`2417`). The two are not the same thing to name: this is the + flow's identity, split out of :data:`BufferID`, which every + :mod:`pcapkit.toolkit` adapter builds from a plain port number -- the + native ones by reading :attr:`AppType.port + ` off the parsed header, + the third-party ones from whatever their own engine exposes -- and which + is then used as a :class:`dict` key and dumped as part of the report. + The deferred pair is a *dispatch* key for + :meth:`Transport.analyze + `, which is + what gains from carrying the service name. + + """ #: Source address. src: 'tuple[_AT, int]' @@ -150,7 +170,7 @@ class Datagram(DeferredPacket, Info, Generic[_AT]): # :class:`~pcapkit.foundation.reassembly.data.ip.Datagram`, which applies # here identically: ``strict=False`` reports an incomplete payload buffer as # one contiguous ``bytes`` and analyses it. - def __init__(self, completed: 'Completion', id: 'DatagramID[_AT]', index: 'tuple[int, ...]', header: 'bytes', payload: 'bytes | tuple[bytes, ...]', packet: 'Optional[ProtocolBase | Deferred[tuple[int, int]]]', conflict: 'tuple[tuple[int, int], ...]') -> 'None': ... # pylint: disable=unused-argument,super-init-not-called,multiple-statements,line-too-long,redefined-builtin + def __init__(self, completed: 'Completion', id: 'DatagramID[_AT]', index: 'tuple[int, ...]', header: 'bytes', payload: 'bytes | tuple[bytes, ...]', packet: 'Optional[ProtocolBase | Deferred[tuple[AppType_TCP, AppType_TCP]]]', conflict: 'tuple[tuple[int, int], ...]') -> 'None': ... # pylint: disable=unused-argument,super-init-not-called,multiple-statements,line-too-long,redefined-builtin @info_final diff --git a/pcapkit/foundation/reassembly/tcp.py b/pcapkit/foundation/reassembly/tcp.py index f8eba1a4b3..7bd8ed2e55 100644 --- a/pcapkit/foundation/reassembly/tcp.py +++ b/pcapkit/foundation/reassembly/tcp.py @@ -12,6 +12,7 @@ import math from typing import TYPE_CHECKING +from pcapkit.const.reg.apptype import TransportProtocol as Enum_TransportProtocol from pcapkit.foundation.reassembly.data.data import Completion, Deferred from pcapkit.foundation.reassembly.data.tcp import (HOLE_OPEN_BOUND, Buffer, BufferID, Datagram, DatagramID, Fragment, HoleDescriptor, Packet) @@ -21,6 +22,8 @@ if TYPE_CHECKING: from typing import Type + from pcapkit.const.reg.apptype import TCP as AppType_TCP + __all__ = ['TCP'] @@ -653,6 +656,85 @@ def _merge_overlap(gap: 'list[tuple[int, int]]', old: 'bytes', new: 'bytes', index = stop return bytes(merged), conflicts + @staticmethod + def _make_ports(bufid: 'BufferID') -> 'tuple[AppType_TCP, AppType_TCP]': + """The flow's ``(source, destination)`` ports, as application types. + + Arguments: + bufid: :term:`buffer ` identifier, whose positions + ``1`` and ``3`` are the source and destination ports. + + Returns: + The pair :meth:`submit` hands + :class:`~pcapkit.foundation.reassembly.data.data.Deferred`, i.e. what + :meth:`Transport.analyze + ` dispatches + the reassembled payload on (:issue:`2417`). Typed as the + *per-transport* registry :class:`~pcapkit.const.reg.apptype.tcp.TCP` + rather than as the base + :class:`~pcapkit.const.reg.apptype.apptype.AppType`, because that is + what it really is: ``proto=tcp`` sends + :meth:`AppType.get ` + through :meth:`AppType._dispatch + ` to the TCP + registry, and ``type(...) is TCP`` holds of the member it answers + with -- measured, for a registered port and an unassigned one alike. + :meth:`Transport.analyze + ` keeps the + base in its own signature, since it is the one method + :class:`~pcapkit.protocols.transport.udp.UDP` and + :class:`~pcapkit.protocols.transport.sctp.SCTP` share, and each of + them would name a different registry. + + Important: + Every port a 16-bit header field can hold resolves to a member, so + this raises for nothing that can come off the wire: + :meth:`AppType.get ` + answers an assignment with its canonical service, a port inside a + documented IANA span with that span's label (``reserved`` for ``0``), + and one in no span at all with an unregistered ``unknown`` member -- + never :obj:`None`, and without growing the registry (:issue:`860`). + It rejects a port outside ``0``-``65535`` and a non-:obj:`int`. + + A :obj:`bool` is rejected by :meth:`Transport._make_port + ` rather + than by ``get``, which is the reason to go through it here and not + call ``get`` directly: :obj:`bool` is an :obj:`int` subclass, so + ``AppType.get(True, proto=tcp)`` resolves ``True`` as port ``1`` and + answers ```` -- measured -- where ``_make_port`` + raises, which is what closed :issue:`2287`. Calling ``get`` straight + would reopen it. None of the three can come out of a TCP header + either way; what ``_make_port`` also buys is idempotence, so a + ``bufid`` already holding a member passes through untouched. + + Note: + The resolution is reached through + :class:`~pcapkit.protocols.transport.tcp.TCP` rather than through + :attr:`self.protocol + `, + which is what :meth:`submit` reads the analyser itself off. That is + deliberate: ``__protocol_type__`` is an extension point, and an + analyser plugged into it need only define ``analyze`` -- it is not + required to be a + :class:`~pcapkit.protocols.transport.transport.Transport` subclass + and so need not have ``_make_port`` at all, which reaching it + through ``self.protocol`` would turn into an + :exc:`AttributeError`. What a TCP port *means* belongs to the TCP + protocol either way, not to whatever consumes the payload, which is + also why the transport below is named outright rather than read off + the analyser. + + """ + # NOTE: ``_make_port`` is protected, and reached from outside its own + # class as :meth:`SCTP._decode_next_layer + # ` reaches + # ``ProtocolBase._decode_next_layer``. Duplicating it here instead would + # put a second port normaliser beside the maker path's, free to drift + # from it; it is idempotent, so a ``bufid`` that already holds a member + # passes through unchanged. + return (TCP_Protocol._make_port(bufid[1], Enum_TransportProtocol.tcp), # pylint: disable=protected-access + TCP_Protocol._make_port(bufid[3], Enum_TransportProtocol.tcp)) # pylint: disable=protected-access + def submit(self, buf: 'Buffer', *, bufid: 'BufferID', # type: ignore[override] # pylint: disable=arguments-differ timeout: 'bool' = False) -> 'list[Datagram]': """Submit reassembled payload. @@ -667,6 +749,34 @@ def submit(self, buf: 'Buffer', *, bufid: 'BufferID', # type: ignore[override] Returns: Reassembled :term:`packets `. + Important: + The ``(source, destination)`` pair handed to + :class:`~pcapkit.foundation.reassembly.data.data.Deferred` is a pair + of :class:`~pcapkit.const.reg.apptype.tcp.TCP` members rather than + of the bare port numbers :data:`BufferID` holds (:issue:`2417`), so + a deferred analysis dispatches on the named service rather than on a + number. The resolution is :meth:`Transport._make_port + `, so it + is the one the maker path uses and cannot drift from it. See + :meth:`_make_ports` for why it is reached through + :class:`~pcapkit.protocols.transport.tcp.TCP` rather than through + :attr:`self.protocol + `. + + For a *registered* port that is the very member + :attr:`TCP.src ` reads off + a parsed header. For an unassigned one the two are **equal but not + identical**, and not even equally named: the parse path builds its + member through :meth:`EnumField._unregistered_member + `, + whose ``.name`` is ``''``, where ``get`` builds its own, + named ``'unknown'`` -- measured on port ``50000``. Both are real + members of the same registry carrying the same :attr:`AppType.port + `, which is what + every reader of this pair goes on to use, so the distinction costs + nothing here; it is recorded because "the same member the header + carries" would be an overstatement of it. + """ datagram = [] # type: list[Datagram] # reassembled datagram @@ -751,7 +861,8 @@ def submit(self, buf: 'Buffer', *, bufid: 'BufferID', # type: ignore[override] index=tuple(buffer.ind), header=buf.hdr, payload=bytes(payload), - packet=Deferred(self.protocol.analyze, (bufid[1], bufid[3]), bytes(payload)), + packet=Deferred(self.protocol.analyze, + self._make_ports(bufid), bytes(payload)), conflict=conflict, ) datagram.append(packet) diff --git a/pcapkit/protocols/transport/transport.py b/pcapkit/protocols/transport/transport.py index efb96647e1..85de8c222a 100644 --- a/pcapkit/protocols/transport/transport.py +++ b/pcapkit/protocols/transport/transport.py @@ -132,11 +132,15 @@ def register(cls, code: 'int', protocol: 'ModuleDescriptor[ProtocolBase] | Type[ cls.__proto__[code] = entry @classmethod - def analyze(cls, ports: 'tuple[int, int]', payload: 'bytes', **kwargs: 'Any') -> 'ProtocolBase': # type: ignore[override] # pylint: disable=arguments-renamed + def analyze(cls, ports: 'tuple[Enum_AppType, Enum_AppType]', payload: 'bytes', **kwargs: 'Any') -> 'ProtocolBase': # type: ignore[override] # pylint: disable=arguments-renamed """Analyse packet payload. Args: - ports: Source & destination port numbers. + ports: Source & destination ports, as the application types + :meth:`_make_port` resolves them to -- what + :attr:`TCP.src ` and + :attr:`TCP.dst ` read + off a parsed header, resolved the same way. payload: Packet payload. **kwargs: Arbitrary keyword arguments. @@ -144,9 +148,60 @@ def analyze(cls, ports: 'tuple[int, int]', payload: 'bytes', **kwargs: 'Any') -> Parsed payload as a :class:`~pcapkit.protocols.protocol.Protocol` instance. + Important: + The annotation names the **base** registry rather than one + transport's own, because this is the single method + :class:`~pcapkit.protocols.transport.tcp.TCP`, + :class:`~pcapkit.protocols.transport.udp.UDP` and + :class:`~pcapkit.protocols.transport.sctp.SCTP` share, and each + resolves its ports in a different one -- + :class:`~pcapkit.const.reg.apptype.tcp.TCP` for the first, and so + on. A caller that knows which transport it has gets the narrower + type from whatever handed it the pair, e.g. + :meth:`TCP._make_ports + `. + + A bare :obj:`int` port still resolves, so the tightened annotation + is a declaration of what callers should pass rather than a new + rejection -- the method predates 1.5.0, and a member and the port it + carries are the same key of an :obj:`int`-keyed mapping anyway: + :meth:`AppType.__hash__ + ` is its port's + and :meth:`AppType.__eq__ + ` compares against + it. So the port chosen below is tested against :attr:`__proto__` + exactly as it arrived, and the chosen one is unwrapped to the + :obj:`int` key the registry declares only when it is a member -- + through :meth:`AppType.__int__ + `, i.e. + :attr:`AppType.port + `. Reading + ``.port`` unconditionally would have made the :obj:`int` case an + :exc:`AttributeError`, and unwrapping unconditionally would have + changed what anything *else* does here: :func:`int` of :obj:`None` + raises where the lookup used to fall back to + :class:`~pcapkit.protocols.misc.raw.Raw`, and :func:`int` of + ``'80'`` would dispatch a :class:`str` nobody registered. + + Nothing is written back on a miss either way, so a payload on an + unregistered port does not grow a process-wide registry -- see + :meth:`ProtocolBase._lookup_registry + `. + + .. versionchanged:: 1.5.0 + + ``ports`` is a pair of + :class:`~pcapkit.const.reg.apptype.apptype.AppType` members rather + than of bare :obj:`int` port numbers, so that the one caller that + reaches this method -- :meth:`TCP.submit + `, through + :class:`~pcapkit.foundation.reassembly.data.data.Deferred` -- hands + over the same named ports a parsed header carries (:issue:`2417`). + """ + port = ports[0] if ports[0] in cls.__proto__ else ports[1] protocol = cls._lookup_next_layer( - cls.__proto__, ports[0] if ports[0] in cls.__proto__ else ports[1]) + cls.__proto__, int(port) if isinstance(port, Enum_AppType) else port) payload_io = io.BytesIO(payload) try: diff --git a/pcapkit/vendor/reg/apptype/apptype.py b/pcapkit/vendor/reg/apptype/apptype.py index 3fe2b99f1e..2d6083567a 100644 --- a/pcapkit/vendor/reg/apptype/apptype.py +++ b/pcapkit/vendor/reg/apptype/apptype.py @@ -451,6 +451,17 @@ def __ne__(self, other: 'Any') -> 'bool': return self.port != other def __hash__(self) -> 'int': + """Hash the member by its port number, as :meth:`__eq__` compares it. + + Keeping the two consistent is what lets a member and the bare + :obj:`int` port it carries be the same key of an :obj:`int`-keyed + mapping -- which is how :meth:`Transport.analyze + ` reads + :attr:`Transport.__proto__ + ` with + either one. + + """ return hash(self.port) @classmethod @@ -895,6 +906,22 @@ def _unregistered_member(cls, value: 'int', name: 'str', exactly the one this registry must not remember past the call that built it. + The member is made reducible through + :meth:`~pcapkit.corekit.enum.EnumRegistry._make_reducible` before it is + returned, so that it survives :mod:`pickle` and :mod:`copy`. Without + that, ``Enum.__reduce_ex__`` would reduce it to a lookup of its own + crafted value -- ``TCP('unknown [54321 - tcp]')`` -- which + :meth:`_missing_` rejects as not a port, so the member pickled fine and + then raised :exc:`ValueError` on read-back. That bit reassembly, whose + :class:`~pcapkit.foundation.reassembly.data.tcp.Datagram` holds these + members in an unresolved + :class:`~pcapkit.foundation.reassembly.data.data.Deferred`: measured + before the fix, 59,413 of the 65,536 possible ports -- every + ``unknown`` and every IANA-span label alike, not just the truly + unregistered ones -- failed to round-trip. Rebuilding re-enters this + method, so an unpickled member is unregistered exactly as the original + was and neither lookup table nor :data:`__registry__` grows. + Args: value: Port number. name: Bare label for the unregistered member -- what @@ -910,7 +937,9 @@ def _unregistered_member(cls, value: 'int', name: 'str', obj.svc = name obj.port = value obj.proto = proto - return obj + # NOTE: reduce to this method's own (value, name, proto), not to the + # crafted ``temp`` the base was handed -- see _make_reducible's note. + return cls._make_reducible(obj, value, name, proto) @classmethod def _missing_(cls, value: 'int') -> 'Optional[{NAME}]': diff --git a/tests/const/test_const_apptype_unregistered_pickle_2417_unit.py b/tests/const/test_const_apptype_unregistered_pickle_2417_unit.py new file mode 100644 index 0000000000..3a88ff08d0 --- /dev/null +++ b/tests/const/test_const_apptype_unregistered_pickle_2417_unit.py @@ -0,0 +1,234 @@ +# -*- coding: utf-8 -*- +"""An unregistered :class:`AppType` member survives :mod:`pickle` and :mod:`copy`. + +GitHub issue :issue:`2417`. ``AppType.get`` answers a port it holds no +assignment for with a member built by +:meth:`AppType._unregistered_member +`, deliberately +absent from every one of the registry's lookup tables so that a lookup does not +grow it (:issue:`860`). ``Enum.__reduce_ex__`` reduces a member to +``(cls, (value,))`` -- a *value*-keyed lookup, i.e. the one table such a member +is missing from -- so these pickled fine and then raised on read-back:: + + ValueError: 'unknown [54321 - tcp]' is not a valid TCP + +That became reachable from reassembly once +:meth:`TCP.submit ` began +dispatching on ``AppType`` members rather than bare port numbers: the +:class:`~pcapkit.foundation.reassembly.data.tcp.Datagram` it produces holds the +pair in an unresolved +:class:`~pcapkit.foundation.reassembly.data.data.Deferred`, so pickling an +unresolved datagram pickled the members. Measured before the fix, **59,413 of +the 65,536** possible TCP ports failed to round-trip -- not merely the truly +unregistered ``unknown`` ones but every IANA-span label too (``unassigned``, +``reserved``, ``x11``, ``swx`` and the rest), which is 90.7% of the port space. + +:meth:`EnumRegistry._make_reducible +` fixes it the way +:meth:`EnumField._unregistered_member +` already fixed +the same class of problem for the *parse* path (:issue:`575`, :issue:`1774`): a +per-instance ``__reduce_ex__`` that rebuilds through +``_unregistered_member`` instead of through a value lookup. Rebuilding +therefore cannot register anything either, which these also pin -- a fix that +made the member picklable by minting it would have been a worse bug than the +one it cured. + +""" +from __future__ import annotations + +import copy +import pickle +import unittest +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from typing import Any + +#: Test tier (#1958): exercises pickling and copying, whose protocols differ by +#: interpreter. +__tier__ = 'ALL' + +#: One port per label ``AppType.get`` answers a miss with, measured over the +#: whole port space on every transport. Every one of these failed to round-trip +#: before the fix. +MISS_PORTS = ( + (0, 'a reserved span, and the bottom of the range'), + (51, 'no row and no span: get\'s own "unknown" fallback'), + (225, 'a reserved span naming no transport'), + (272, 'an unassigned span naming no transport'), + (3322, 'the "active-net" span'), + (3367, 'the "satvid-datalnk" span'), + (4200, 'the "vrml-multi-use" span'), + (6000, 'the "x11" span'), + (6665, 'the "ircu" span, which is TCP-only'), + (7300, 'the "swx" span'), + (27000, 'the "flex-lm" span'), + (54321, 'an ephemeral port, the common case in a real capture'), + (65535, 'the top of the range'), +) + +#: Ports IANA assigns, which resolve to a real registered member instead. These +#: never went through ``_unregistered_member`` and so were never broken; they +#: are here because the fix must not have changed them. +HIT_PORTS = (80, 443, 22) + + +def _protos() -> 'tuple[Any, ...]': + from pcapkit.const.reg.apptype import TransportProtocol + + return (TransportProtocol.tcp, TransportProtocol.udp, + TransportProtocol.sctp, TransportProtocol.dccp) + + +def _tables(owner: 'Any') -> 'tuple[Any, ...]': + """Every table a member could leak into, as a comparable snapshot.""" + return (dict(owner.__members__), + dict(owner._value2member_map_), # pylint: disable=protected-access + [(port, list(owner.__registry__.getlist(port))) for port in owner.__registry__]) + + +class UnregisteredAppTypePickleTests(unittest.TestCase): + """The round trip, and that it registers nothing.""" + + def test_every_port_in_the_space_round_trips(self) -> None: + """All 65,536 ports, the measurement the fix is stated against. + + A sample would have passed before the fix too -- 6,123 of these ports + already round-tripped, being IANA assignments -- so the sweep is the + whole range rather than a selection from it. + + """ + from pcapkit.const.reg.apptype import AppType, TransportProtocol + + owner = AppType.__registries__[TransportProtocol.tcp] + failures = [] + for port in range(65536): + member = AppType.get(port, proto=TransportProtocol.tcp) + try: + restored = pickle.loads(pickle.dumps(member)) + except Exception as exc: # pylint: disable=broad-except + failures.append((port, member.name, type(exc).__name__)) + continue + if not (isinstance(restored, owner) + and (restored.port, restored.svc, restored.proto, restored.name) + == (member.port, member.svc, member.proto, member.name)): + failures.append((port, member.name, 'mismatch')) + self.assertEqual(failures, [], f'{len(failures)} of 65536 ports did not round-trip') + + def test_every_miss_label_round_trips_on_every_protocol(self) -> None: + """Each shape of miss, on every transport and every pickle protocol.""" + from pcapkit.const.reg.apptype import AppType + + for proto in _protos(): + owner = AppType.__registries__[proto] + for port, why in MISS_PORTS: + member = AppType.get(port, proto=proto) + for protocol in range(pickle.HIGHEST_PROTOCOL + 1): + with self.subTest(proto=proto.name, port=port, why=why, + protocol=protocol): + restored = pickle.loads(pickle.dumps(member, protocol)) + + self.assertIsInstance(restored, owner) + self.assertEqual(restored, member) + self.assertEqual( + (restored.port, restored.svc, restored.proto, restored.name), + (member.port, member.svc, member.proto, member.name)) + + def test_round_tripping_grows_no_table(self) -> None: + """The registry must not grow -- not on the way out, nor on read-back. + + A reducer that rebuilt through ``cls(value)`` or ``cls.register`` would + make the member picklable by *minting* it, trading a read-back failure + for a process-wide registry that grows once per ephemeral port seen. + + """ + from pcapkit.const.reg.apptype import AppType + + for proto in _protos(): + owner = AppType.__registries__[proto] + with self.subTest(proto=proto.name): + before = _tables(owner) + + for port, _ in MISS_PORTS: + member = AppType.get(port, proto=proto) + for protocol in range(pickle.HIGHEST_PROTOCOL + 1): + pickle.loads(pickle.dumps(member, protocol)) + copy.deepcopy(member) + + self.assertEqual(_tables(owner), before) + + def test_a_restored_member_stays_absent_from_the_lookup_tables(self) -> None: + """The rebuilt member is unregistered exactly as the original was.""" + from pcapkit.const.reg.apptype import AppType, TransportProtocol + + owner = AppType.__registries__[TransportProtocol.tcp] + member = AppType.get(54321, proto=TransportProtocol.tcp) + restored = pickle.loads(pickle.dumps(member)) + + for label, obj in (('original', member), ('restored', restored)): + with self.subTest(member=label): + self.assertNotIn( + obj._value_, owner._value2member_map_) # pylint: disable=protected-access + self.assertNotIn(obj.name, owner.__members__) + + def test_a_restored_member_is_itself_reducible(self) -> None: + """Rebuilding re-enters ``_unregistered_member``, so the reducer is + installed again and a restored member pickles as the original did. + + Without that, a member would survive one round trip and fail the + second, which is worse than failing the first: a datagram read back + from a dump could not be written out again. + + """ + from pcapkit.const.reg.apptype import AppType, TransportProtocol + + member = AppType.get(54321, proto=TransportProtocol.tcp) + once = pickle.loads(pickle.dumps(member)) + twice = pickle.loads(pickle.dumps(once)) + + self.assertEqual(twice, member) + self.assertEqual((twice.port, twice.svc, twice.proto, twice.name), + (member.port, member.svc, member.proto, member.name)) + + def test_copy_and_deepcopy_preserve_the_member(self) -> None: + """:mod:`copy` reduces through the same hook, so it is pinned too. + + On 3.11+ ``Enum.__copy__`` short-circuits ahead of any reduction and + both return the member itself; where it does not exist they return an + equal rebuilt one. Either is the right answer for an immutable value, + so this asserts equality rather than identity. + + """ + from pcapkit.const.reg.apptype import AppType + + for proto in _protos(): + member = AppType.get(54321, proto=proto) + for fn in (copy.copy, copy.deepcopy): + with self.subTest(proto=proto.name, fn=fn.__name__): + clone = fn(member) + + self.assertEqual(clone, member) + self.assertEqual((clone.port, clone.svc, clone.proto, clone.name), + (member.port, member.svc, member.proto, member.name)) + + def test_a_registered_member_still_reduces_to_the_registry_singleton(self) -> None: + """The fix touches the unregistered path only. + + A real assignment never goes through ``_unregistered_member``, so it + keeps ``Enum``'s own reduction and unpickles to the *identical* shared + member. Pinning identity here is what would catch the reducer being + installed on every member rather than on the unregistered ones. + + """ + from pcapkit.const.reg.apptype import AppType, TransportProtocol + + for port in HIT_PORTS: + member = AppType.get(port, proto=TransportProtocol.tcp) + for protocol in range(pickle.HIGHEST_PROTOCOL + 1): + with self.subTest(port=port, protocol=protocol): + self.assertIs(pickle.loads(pickle.dumps(member, protocol)), member) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/corekit/test_fields_numbers_port_enum_field_peek_1774_unit.py b/tests/corekit/test_fields_numbers_port_enum_field_peek_1774_unit.py index 49b4493e3d..b3706eff59 100644 --- a/tests/corekit/test_fields_numbers_port_enum_field_peek_1774_unit.py +++ b/tests/corekit/test_fields_numbers_port_enum_field_peek_1774_unit.py @@ -6,9 +6,14 @@ miss builds its member through ``EnumField._unregistered_member``. That peek was written to stop ``get`` minting a member per ephemeral port. ``get`` does not mint (#860), so the growth it guarded against cannot happen; but the member -``get`` builds on a miss is not the one a parse needs. These tests pin both -halves, so that the peek's documented reason fails here the day it stops being -true, and pin that a composite ``proto`` resolves a miss as ``get`` does. +``get`` builds on a miss is not the one a parse needs -- it is named ``unknown`` +where a parse wants ````. These tests pin both halves, so that the +peek's documented reason fails here the day it stops being true, and pin that a +composite ``proto`` resolves a miss as ``get`` does. + +The naming is the *only* half left. ``get``'s member was also unpicklable until +#2417, which is a difference these tests used to assert; that was a defect in +its own right, fixed there, and is now asserted the other way round. """ import pickle @@ -91,18 +96,41 @@ def test_a_miss_through_the_field_pickles_and_renders_as_unassigned(self) -> Non ('unknown', PORT, proto)) def test_gets_own_member_for_a_miss_is_not_that_member(self) -> None: - """Why the field cannot just call ``get``: its miss member has no reducer - and another name. If this starts failing, ``get``'s member has become - what a parse needs, and the peek can go -- see #1774.""" + """Why the field cannot just call ``get``: its miss member has another + name. If this starts failing, ``get``'s member has become what a parse + needs, and the peek can go -- see #1774. + + ``get``'s member used to differ in a second way, which this test also + pinned: it carried no reducer, so pickling it raised + ``ValueError: 'unknown [50000 - tcp]' is not a valid TCP`` on read-back. + That was a defect rather than a distinction -- reassembly dispatches on + these members, so an unresolved + :class:`~pcapkit.foundation.reassembly.data.data.Deferred` could not be + pickled for 90.7% of the port space -- and :issue:`2417` fixed it by + giving :meth:`AppType._unregistered_member + ` the + same per-instance ``__reduce_ex__`` the field's own path already had. + Both members now round-trip, so the ``name`` is the only difference + left, and it is the one that still justifies the peek. + + """ from pcapkit.const.reg.apptype import AppType for proto in self._protos(): with self.subTest(proto=proto.name): member = AppType.get(PORT, proto=proto) + parsed = self._field(proto).unpack(PORT.to_bytes(2, 'big'), {}) self.assertEqual(member.name, 'unknown') - with self.assertRaises(ValueError): - pickle.loads(pickle.dumps(member)) + self.assertEqual(parsed.name, '') + self.assertNotEqual(member.name, parsed.name) + + # ... and the difference really is only the name now: both + # survive the round trip, and compare equal to each other. + restored = pickle.loads(pickle.dumps(member)) + self.assertEqual(restored, member) + self.assertEqual(restored.name, 'unknown') + self.assertEqual(member, parsed) def test_a_composite_proto_resolves_a_miss_as_get_does(self) -> None: """``tcp | udp`` is a bare :class:`int` (``3``, which is SCTP's value). diff --git a/tests/foundation/reassembly/test_datagram_deferred_pickle_2417_runtime.py b/tests/foundation/reassembly/test_datagram_deferred_pickle_2417_runtime.py new file mode 100644 index 0000000000..867b4686c9 --- /dev/null +++ b/tests/foundation/reassembly/test_datagram_deferred_pickle_2417_runtime.py @@ -0,0 +1,76 @@ +# -*- coding: utf-8 -*- +"""The deferred-pickle fix of :issue:`2417`, end to end over the real captures. + +:mod:`tests.foundation.reassembly.test_datagram_deferred_pickle_2417_unit` +synthesises a single TCP datagram directly, on an unassigned and an assigned +flow. This reassembles every TCP flow out of :data:`CAPTURES` instead, with +each datagram's ``packet`` left deferred, and pins the same round trip -- which +is how the bug was originally found: a datagram is only worth pickling *while* +its payload is unresolved, so the failure sat exactly where nothing else would +catch it. + +The module reads generated captures, so it belongs to the fixture-dependent +tier. + +""" +from __future__ import annotations + +import pickle +import unittest +from typing import TYPE_CHECKING + +from tests._tiers import SAMPLE_ROOT + +if TYPE_CHECKING: + from typing import Any + +#: Test tier (#1958): exercises pickling, whose protocols differ by interpreter. +__tier__ = 'ALL' + +#: The sample captures carrying TCP flows, i.e. the ones reassembly has anything +#: to do. Generated by ``examples/generators/make_samples.py``. +CAPTURES = ('http.pcap', 'stream.pcap', 'tcp.pcap', 'http6.cap') + + +def _held(datagram: 'Any') -> 'Any': + """``datagram``'s unresolved ``packet`` placeholder, without resolving it.""" + key = datagram.__map__.get('packet', 'packet') + return datagram.__dict__[key] + + +class SampleCaptureDatagramPickleTests(unittest.TestCase): + """The same thing over the real captures, which is how the bug was found.""" + + @staticmethod + def _deferred_datagrams(path: 'Any') -> 'list[Any]': + """Every TCP datagram ``path`` reassembles, packets left unresolved.""" + import pcapkit + + extraction = pcapkit.extract(fin=str(path), nofile=True, store=True, + tcp=True, reassembly=True) + return [datagram for datagram in extraction.reassembly.tcp + if _held(datagram) is not None] + + def test_every_capture_with_tcp_flows_round_trips(self) -> None: + """Each of the named samples, every datagram, packets still deferred.""" + for name in CAPTURES: + path = SAMPLE_ROOT / name + with self.subTest(capture=name): + self.assertTrue(path.is_file(), + f'{path} is missing; run examples/generators/make_samples.py') + + datagrams = self._deferred_datagrams(path) + self.assertNotEqual(datagrams, [], + f'{name} reassembled no deferred TCP datagram, ' + 'so it pins nothing') + + for index, datagram in enumerate(datagrams): + restored = pickle.loads(pickle.dumps(datagram)) + + self.assertEqual(_held(restored).proto, _held(datagram).proto, + f'{name} datagram {index} lost its ports') + self.assertEqual(bytes(restored.payload), bytes(datagram.payload)) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/foundation/reassembly/test_datagram_deferred_pickle_2417_unit.py b/tests/foundation/reassembly/test_datagram_deferred_pickle_2417_unit.py new file mode 100644 index 0000000000..0495e4b67f --- /dev/null +++ b/tests/foundation/reassembly/test_datagram_deferred_pickle_2417_unit.py @@ -0,0 +1,148 @@ +# -*- coding: utf-8 -*- +"""A reassembled TCP datagram pickles while its ``packet`` is still deferred. + +GitHub issue :issue:`2417`. Dispatching on +:class:`~pcapkit.const.reg.apptype.apptype.AppType` members rather than bare +port numbers put those members inside the +:class:`~pcapkit.foundation.reassembly.data.data.Deferred` that +:meth:`TCP.submit ` stores as a +:class:`~pcapkit.foundation.reassembly.data.tcp.Datagram`'s ``packet``. For a +port the registry holds no assignment for, such a member was absent from the +value table ``Enum.__reduce_ex__`` reduces to, so the datagram pickled and then +raised on read-back:: + + ValueError: 'unknown [50000 - tcp]' is not a valid TCP + +which is 90.7% of the port space and therefore most real captures -- client +sockets are ephemeral. :meth:`EnumRegistry._make_reducible +` fixes the member itself; +:mod:`tests.const.test_const_apptype_unregistered_pickle_2417_unit` pins that +over the whole port space. These synthesise the datagram directly, on both an +unassigned and an assigned flow, rather than reading a capture -- +:mod:`tests.foundation.reassembly.test_datagram_deferred_pickle_2417_runtime` +pins the same thing end to end over the sample captures, which is how the bug +was originally found: a datagram is only worth pickling *while* its payload is +unresolved, so the failure sat exactly where nothing else would catch it. + +Pickled before :attr:`~...Datagram.packet` is read, deliberately: reading it +resolves the :class:`Deferred` through +:meth:`DeferredPacket.__analyse__ ` +and replaces the placeholder, so a datagram whose packet has been touched no +longer holds the members this is about. + +""" +from __future__ import annotations + +import pickle +import unittest +from ipaddress import ip_address +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from typing import Any + +#: Test tier (#1958): exercises pickling, whose protocols differ by interpreter. +__tier__ = 'ALL' + +#: Pickle protocols a whole :class:`Deferred` can go through. +#: +#: Protocols ``0`` and ``1`` are excluded, and not because of anything +#: :issue:`2417` changed: :class:`~pcapkit.foundation.reassembly.data.data.Deferred` +#: declares ``__slots__`` and no ``__getstate__``, which those two protocols +#: cannot serialise at all -- ``TypeError: a class that defines __slots__ +#: without defining __getstate__ cannot be pickled``, raised from +#: :mod:`copyreg`. Measured on CPython 3.14.10 to be independent of the ports: +#: it reproduces for an assigned flow, for an unassigned one, and for a bare +#: ``Deferred(..., 42, b'x')`` holding a plain :obj:`int`, while ``2``-``5`` +#: pass in every one of those cases. That is a separate limitation of the class, +#: not of the members it holds, which is why it is scoped out here rather than +#: worked around; the member-level round trip is pinned on *every* protocol from +#: ``0`` up in :mod:`tests.const.test_const_apptype_unregistered_pickle_2417_unit`. +PROTOCOLS = tuple(range(2, pickle.HIGHEST_PROTOCOL + 1)) + +#: A flow on two unassigned ports, which is what used to fail. +UNASSIGNED_BUFID = (ip_address('192.0.2.1'), 50000, ip_address('198.51.100.2'), 65535) + +#: The same shape on two IANA assignments, which always worked. +ASSIGNED_BUFID = (ip_address('192.0.2.1'), 12345, ip_address('198.51.100.2'), 443) + + +def _held(datagram: 'Any') -> 'Any': + """``datagram``'s unresolved ``packet`` placeholder, without resolving it.""" + key = datagram.__map__.get('packet', 'packet') + return datagram.__dict__[key] + + +def _datagram(bufid: 'Any') -> 'Any': + """One complete TCP datagram under ``bufid``, its ``packet`` still deferred. + + A single segment carrying FIN, which closes the stream and so submits it + complete -- the partial branch of ``submit`` reports ``packet=None`` and + builds no :class:`Deferred` at all. + + """ + from pcapkit.foundation.reassembly.data.tcp import Packet + from pcapkit.foundation.reassembly.tcp import TCP + + reasm = TCP() + reasm(Packet(bufid, 1, 500, 1, False, True, False, 5, 1, 5, + b'', bytearray(b'hello'), 1000.0)) + datagram, = reasm.datagram + return datagram + + +class DeferredDatagramPickleTests(unittest.TestCase): + """A synthesised datagram, on assigned and unassigned ports alike.""" + + def test_an_unassigned_flow_round_trips(self) -> None: + """The case that failed: both ports resolve to unregistered members.""" + from pcapkit.const.reg.apptype.tcp import TCP as AppType_TCP + + datagram = _datagram(UNASSIGNED_BUFID) + ports = _held(datagram).proto + self.assertEqual([port.name for port in ports], ['unknown', 'unknown']) + + for protocol in PROTOCOLS: + with self.subTest(protocol=protocol): + restored = pickle.loads(pickle.dumps(datagram, protocol)) + restored_ports = _held(restored).proto + + self.assertEqual(restored_ports, ports) + for port in restored_ports: + self.assertIsInstance(port, AppType_TCP) + self.assertEqual([port.port for port in restored_ports], + [UNASSIGNED_BUFID[1], UNASSIGNED_BUFID[3]]) + self.assertEqual(bytes(restored.payload), bytes(datagram.payload)) + + def test_an_assigned_flow_round_trips(self) -> None: + """The control: registered members were never broken, and still are not.""" + datagram = _datagram(ASSIGNED_BUFID) + ports = _held(datagram).proto + + restored_ports = _held(pickle.loads(pickle.dumps(datagram))).proto + + self.assertEqual(restored_ports, ports) + self.assertEqual([port.svc for port in restored_ports], ['italk', 'https']) + + def test_the_packet_still_resolves_after_a_round_trip(self) -> None: + """A restored datagram is still usable, not merely loadable. + + The members come back unregistered, so this also pins that + :meth:`Transport.analyze + ` can still key + :attr:`TCP.__proto__ + ` with one -- + which is what :meth:`AppType.__hash__ + ` being its port's + buys. + + """ + from pcapkit.protocols.protocol import ProtocolBase + + restored = pickle.loads(pickle.dumps(_datagram(UNASSIGNED_BUFID))) + + self.assertIsInstance(restored.packet, ProtocolBase) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/foundation/reassembly/test_deferred_proto_type_2417_unit.py b/tests/foundation/reassembly/test_deferred_proto_type_2417_unit.py index 6dfd335ea5..91c1882109 100644 --- a/tests/foundation/reassembly/test_deferred_proto_type_2417_unit.py +++ b/tests/foundation/reassembly/test_deferred_proto_type_2417_unit.py @@ -31,27 +31,47 @@ parameter. It is also why these tests read the declaration at runtime instead of resting on mypy. -The runtime value was always correct, so there is no behaviour to pin. What -these tests pin is the declared type against the values the reassemblers really -hand it. Two of them fail on anything but the generic: +The port pair itself then moved: both constraints are *named registry values*, +so ``TCP.submit`` resolves each port through :meth:`Transport._make_port +` and the constraint +is ``tuple[AppType_TCP, AppType_TCP]`` -- the *per-transport* registry, which +is what the resolution really answers with -- rather than ``tuple[int, int]``. +A deferred analysis now dispatches on a member of the registry +:attr:`TCP.src ` reads from: the very +same member for a registered port, an equal one reached by a different route +for an unassigned one (see :meth:`ResolvedPortTests.test_an_unassigned_port_is_equal_to_the_parse_paths_own`), +and :meth:`Transport.analyze +` declares that pair. +Unlike the generic itself, this half *is* a runtime change -- +:class:`~pcapkit.const.reg.apptype.apptype.AppType` is a +:class:`~enum.StrEnum`, so it is no :obj:`int` at all and +``isinstance(port, int)`` is now :data:`False` -- which is what +:class:`ResolvedPortTests` pins. + +Three cases fail on anything but the generic over named values: :meth:`DeclaredProtoTypeTests.test_declared_type_admits_what_each_reassembler_passes` requires each value to be admitted by *exactly one* constraint and the two by -different ones, and +different ones, :meth:`DeclaredProtoTypeTests.test_the_analyser_takes_the_same_type_variable_as_proto` -requires the analyser to take the variable ``proto`` names. Against the -regression's single class, and against the union that first widened it, both -report that ``proto`` is annotated with a type rather than a type parameter. +requires the analyser to take the variable ``proto`` names, and +:meth:`ResolvedPortTests.test_submit_hands_over_resolved_application_types` +requires the values themselves to be members. Against the regression's single +class, and against the union that first widened it, the first two report that +``proto`` is annotated with a type rather than a type parameter; against the +bare port pair, the first fails because ``tuple[AppType_TCP, AppType_TCP]`` +does not admit it and the third because an :obj:`int` is not a member. The "different ones" half is what the parameter buys over a union: a union admits a port pair and a protocol type on *either* call, so it cannot tell a -``Deferred[TransType]`` from a ``Deferred[tuple[int, int]]`` and leaves each -analyser's first parameter unconstrained. Pinning the annotation's text instead -would pass for a union that happened to name the right classes while still -excluding one of them. - -Both cases build their packets in memory, read no capture, and never *call* the -held analyser -- only :attr:`Deferred.proto` is read, so nothing here depends on -a parser. Classes are imported inside each test, after +``Deferred[TransType]`` from a ``Deferred[tuple[AppType_TCP, AppType_TCP]]`` and leaves +each analyser's first parameter unconstrained. Pinning the annotation's text +instead would pass for a union that happened to name the right classes while +still excluding one of them. + +Every case builds its packets in memory and reads no capture. Those in +:class:`DeclaredProtoTypeTests` never *call* the held analyser either -- only +:attr:`Deferred.proto` is read, so nothing there depends on a parser. Classes +are imported inside each test, after :func:`~tests._support.reimport_once_per_class`. """ @@ -64,14 +84,22 @@ from tests._support import reimport_once_per_class -#: Test tier (#1958): reads an annotation and two in-memory reassemblies, so the -#: same text and the same values on every leg; nothing version-specific to sweep. +#: Test tier (#1958): reads an annotation and a handful of in-memory +#: reassemblies, so the same text and the same values on every leg; nothing +#: version-specific to sweep. __tier__ = 'LATEST' #: TCP ``BUFID``: source address, source port, destination address, destination -#: port. ``TCP.submit`` passes positions 1 and 3 of this to ``Deferred``. +#: port. ``TCP.submit`` passes positions 1 and 3 of this to ``Deferred``. Both +#: ports carry an IANA assignment -- ``italk`` and ``https`` -- so the resolved +#: members are named ones; :data:`UNASSIGNED_BUFID` covers the other case. TCP_BUFID = (ip_address('192.0.2.1'), 12345, ip_address('198.51.100.2'), 443) +#: As :data:`TCP_BUFID`, but on ports IANA assigns no service to and lists in no +#: span of its own, which resolve to unregistered ``unknown`` members rather +#: than raising or coming back as :data:`None`. +UNASSIGNED_BUFID = (ip_address('192.0.2.1'), 50000, ip_address('198.51.100.2'), 65535) + def admits(declared: 'object', value: 'object') -> 'bool': """Whether the type ``declared`` accepts ``value``. @@ -124,6 +152,31 @@ def held(datagram: 'object') -> 'object': return datagram.__dict__[key] +def tcp_proto(bufid=TCP_BUFID): + """The ``proto`` ``TCP.submit`` hands :class:`Deferred` for ``bufid``. + + One segment with a FIN, which closes the stream and so submits it complete: + the partial branch of ``submit`` reports ``packet=None`` and builds no + :class:`~pcapkit.foundation.reassembly.data.data.Deferred` to read. + + Args: + bufid: The buffer identifier to reassemble under, i.e. where the ports + come from. + + Returns: + The placeholder's ``proto``, unresolved. + + """ + from pcapkit.foundation.reassembly.data.tcp import Packet + from pcapkit.foundation.reassembly.tcp import TCP + + reasm = TCP() + reasm(Packet(bufid, 1, 500, 1, False, True, False, 5, 1, 5, + b'', bytearray(b'hello'), 1000.0)) + datagram, = reasm.datagram + return held(datagram).proto + + class AdmitsTests(unittest.TestCase): """:func:`admits` is what the test below leans on, so it is pinned too.""" @@ -176,22 +229,25 @@ def _declared_proto_types(cls): """What that type variable is constrained to, each one evaluated. A constraint is written as a string literal, since - :class:`~pcapkit.const.reg.transtype.TransType` is imported under - ``TYPE_CHECKING`` and nothing at runtime has it in scope; the namespace - is rebuilt here rather than asking :func:`typing.get_type_hints`, which - raises :exc:`NameError` on it. :class:`~typing.TypeVar` keeps such a - constraint as the string itself from Python 3.14 and as a - :class:`typing.ForwardRef` before it, so both spellings are unwrapped. + :class:`~pcapkit.const.reg.transtype.TransType` and + :class:`~pcapkit.const.reg.apptype.tcp.TCP` are both imported + under ``TYPE_CHECKING`` and nothing at runtime has either in scope; the + namespace is rebuilt here rather than asking + :func:`typing.get_type_hints`, which raises :exc:`NameError` on them. + :class:`~typing.TypeVar` keeps such a constraint as the string itself + from Python 3.14 and as a :class:`typing.ForwardRef` before it, so both + spellings are unwrapped. Returns: The admissible types, in declaration order. """ + from pcapkit.const.reg.apptype import TCP as AppType_TCP from pcapkit.const.reg.transtype import TransType return tuple( eval(getattr(constraint, '__forward_arg__', constraint), # pylint: disable=eval-used - {'TransType': TransType}) + {'TransType': TransType, 'AppType_TCP': AppType_TCP}) for constraint in cls._proto_type_variable().__constraints__ ) @@ -207,16 +263,9 @@ def _ip_proto(self): datagram, = reasm.datagram return held(datagram).proto - def _tcp_proto(self): + def _tcp_proto(self, bufid=TCP_BUFID): """The ``proto`` ``TCP.submit`` hands to :class:`Deferred`.""" - from pcapkit.foundation.reassembly.data.tcp import Packet - from pcapkit.foundation.reassembly.tcp import TCP - - reasm = TCP() - reasm(Packet(TCP_BUFID, 1, 500, 1, False, True, False, 5, 1, 5, - b'', bytearray(b'hello'), 1000.0)) - datagram, = reasm.datagram - return held(datagram).proto + return tcp_proto(bufid) def test_ip_passes_the_payload_protocol_type(self) -> None: """IP reassembly reads it from the datagram's **Protocol** field.""" @@ -225,7 +274,16 @@ def test_ip_passes_the_payload_protocol_type(self) -> None: self.assertIs(self._ip_proto(), TransType.UDP) def test_tcp_passes_a_port_pair(self) -> None: - """A reassembled stream carries no protocol number, so it keys on ports.""" + """A reassembled stream carries no protocol number, so it keys on ports. + + The pair compares equal to the bare port numbers it was resolved from -- + :meth:`AppType.__eq__ ` + compares against :attr:`AppType.port + ` -- which is exactly why + an equality assertion cannot tell the two apart and + :class:`ResolvedPortTests` checks the members themselves. + + """ self.assertEqual(self._tcp_proto(), (TCP_BUFID[1], TCP_BUFID[3])) def test_the_analyser_takes_the_same_type_variable_as_proto(self) -> None: @@ -287,5 +345,196 @@ def test_the_held_analyser_is_not_called_to_learn_this(self) -> None: self.assertIsInstance(held(datagram), Deferred) +class ResolvedPortTests(unittest.TestCase): + """The ports themselves, not the declaration: members rather than numbers. + + This is the half of :issue:`2417` that runs. The declaration above would be + satisfied by a ``tuple[AppType_TCP, AppType_TCP]`` constraint over a ``submit`` that + still passed bare numbers -- :func:`admits` would simply report that nothing + admits the pair -- so what pins the resolution is reading the values. + + """ + + def setUp(self) -> None: + reimport_once_per_class(self) + + def test_submit_hands_over_resolved_application_types(self) -> None: + """Each port is the member its registry holds, carrying TCP's transport. + + Named outright rather than compared against a second + :meth:`AppType.get ` call, + which would pass against a ``submit`` that resolved the wrong transport's + registry as readily as the right one. + + """ + from pcapkit.const.reg.apptype import AppType, TransportProtocol + + src, dst = tcp_proto() + for port, member, service in ((TCP_BUFID[1], src, 'italk'), + (TCP_BUFID[3], dst, 'https')): + with self.subTest(port=port): + self.assertIsInstance(member, AppType) + self.assertEqual(member.port, port) + self.assertEqual(member.svc, service) + self.assertIs(member.proto, TransportProtocol.tcp) + + def test_each_port_is_a_member_of_the_per_transport_tcp_registry(self) -> None: + """Not merely *an* ``AppType`` -- the TCP partition, exactly. + + ``proto=tcp`` sends :meth:`AppType.get + ` through + :meth:`AppType._dispatch + ` to + :class:`~pcapkit.const.reg.apptype.tcp.TCP`, so ``type(...) is TCP`` + rather than merely ``isinstance(..., AppType)``. That is what lets the + annotations name the narrow registry, and it is checked with ``is`` + because an ``isinstance`` against the base would also pass for a member + of the UDP or SCTP partition -- which is the mistake the narrow + annotation exists to catch. + + """ + from pcapkit.const.reg.apptype import TCP as AppType_TCP + + for bufid in (TCP_BUFID, UNASSIGNED_BUFID): + for member in tcp_proto(bufid): + with self.subTest(port=member.port): + self.assertIs(type(member), AppType_TCP) + + def test_an_unassigned_port_is_equal_to_the_parse_paths_own(self) -> None: + """Equal and of one registry, but not the same object, and named apart. + + The claim worth stating precisely rather than overstating: for a + *registered* port this resolution and the parse path's own give the very + same member, but for an unassigned one they build equal members by + different routes -- the parse path through + :meth:`EnumField._unregistered_member + `, named + ``''``, and this one through ``get``'s own, named + ``'unknown'``. Both carry the same :attr:`AppType.port + `, which is what every + reader of the pair goes on to use. + + """ + from pcapkit.const.reg.apptype import TransportProtocol + from pcapkit.protocols.schema.transport.tcp import TCP as Schema_TCP + + field = Schema_TCP.__fields__['srcport'] + + registered, _ = tcp_proto() + self.assertIs(field.post_process(TCP_BUFID[1], None), registered) + self.assertIs(registered.proto, TransportProtocol.tcp) + + unassigned, _ = tcp_proto(UNASSIGNED_BUFID) + parsed = field.post_process(UNASSIGNED_BUFID[1], None) + self.assertEqual(parsed, unassigned) + self.assertIsNot(parsed, unassigned) + self.assertEqual(parsed.port, unassigned.port) + self.assertEqual((parsed.name, unassigned.name), ('', 'unknown')) + + def test_a_resolved_port_is_no_longer_an_int(self) -> None: + """The runtime change, stated as the thing a caller could notice. + + :class:`~pcapkit.const.reg.apptype.apptype.AppType` derives from + :class:`str`, so the pair is outside ``tuple[int, int]`` at runtime as + well as to a type checker, and that is what makes the old constraint + wrong rather than merely loose. It still *compares* equal to the number, + which is why nothing downstream of ``submit`` had to change. + + """ + src, _ = tcp_proto() + self.assertNotIsInstance(src, int) + self.assertEqual(src, TCP_BUFID[1]) + self.assertEqual(int(src), TCP_BUFID[1]) + self.assertEqual(hash(src), hash(TCP_BUFID[1])) + + def test_an_unassigned_port_resolves_rather_than_raising(self) -> None: + """Every port a 16-bit field can hold has to come back as *something*. + + :meth:`AppType.get ` + answers a port in no IANA span with an unregistered ``unknown`` member + and mints nothing into the registry (:issue:`860`), so resolving in + ``submit`` cannot fail on a capture of traffic on an ephemeral port -- + which is most captures, and would otherwise have made this change a new + way for reassembly to raise. + + """ + from pcapkit.const.reg.apptype import AppType + + src, dst = tcp_proto(UNASSIGNED_BUFID) + for port, member in ((UNASSIGNED_BUFID[1], src), (UNASSIGNED_BUFID[3], dst)): + with self.subTest(port=port): + self.assertIsInstance(member, AppType) + self.assertEqual(member.svc, 'unknown') + self.assertEqual(member.port, port) + + def test_resolving_grows_no_registry(self) -> None: + """Neither the application-type registry nor the next-layer one.""" + from pcapkit.const.reg.apptype import TCP as AppType_TCP + from pcapkit.protocols.transport.tcp import TCP as TCP_Protocol + + before_apptype = len(AppType_TCP.__registry__) + before_proto = sorted(TCP_Protocol.__proto__) + + tcp_proto(UNASSIGNED_BUFID) + + self.assertEqual(len(AppType_TCP.__registry__), before_apptype) + self.assertEqual(sorted(TCP_Protocol.__proto__), before_proto) + + def test_the_pair_is_what_the_analyser_dispatches_on(self) -> None: + """End to end: the resolved pair still selects the registered next layer. + + ``Transport.analyze`` keys :attr:`TCP.__proto__ + ` on the port, and that + mapping is keyed by number, so the members have to reach it as the + numbers they carry. Port 80 is bound to the version-dispatching HTTP + class; the unassigned pair falls through to the registry's own fallback. + + """ + from pcapkit.protocols.application.http import HTTP + from pcapkit.protocols.misc.raw import Raw + from pcapkit.protocols.transport.tcp import TCP as TCP_Protocol + + http_bufid = (ip_address('192.0.2.1'), 54321, ip_address('198.51.100.2'), 80) + self.assertIsInstance( + TCP_Protocol.analyze(tcp_proto(http_bufid), b'GET / HTTP/1.1\r\n\r\n'), HTTP) + self.assertIsInstance( + TCP_Protocol.analyze(tcp_proto(UNASSIGNED_BUFID), b'not a protocol'), Raw) + + def test_a_custom_analyser_need_not_be_a_transport(self) -> None: + """``__protocol_type__`` is an extension point, so ``_make_port`` is not read off it. + + The analyser plugged in here defines ``analyze`` and nothing else, which + is all :attr:`ReassemblyBase.protocol + ` + promises. Resolving the ports through it instead of through + :class:`~pcapkit.protocols.transport.tcp.TCP` would raise + :exc:`AttributeError` here, and it is the reassembly tests' own idiom. + + """ + from pcapkit.const.reg.apptype import AppType + from pcapkit.foundation.reassembly.data.tcp import Packet + from pcapkit.foundation.reassembly.tcp import TCP + + class Analyzer: # pylint: disable=too-few-public-methods + """Everything ``__protocol_type__`` is required to be.""" + + @classmethod + def analyze(cls, ports, payload): + return (ports, payload) + + class TestTCP(TCP): + __protocol_type__ = Analyzer + + reasm = TestTCP() + reasm(Packet(TCP_BUFID, 1, 500, 1, False, True, False, 5, 1, 5, + b'', bytearray(b'hello'), 1000.0)) + datagram, = reasm.datagram + ports, payload = datagram.packet + self.assertEqual(payload, b'hello') + self.assertEqual(ports, (TCP_BUFID[1], TCP_BUFID[3])) + for member in ports: + self.assertIsInstance(member, AppType) + + if __name__ == '__main__': unittest.main() diff --git a/tests/project/test_docs_internal_refs_1940_unit.py b/tests/project/test_docs_internal_refs_1940_unit.py index 0d263b35f6..4401e3cbc2 100644 --- a/tests/project/test_docs_internal_refs_1940_unit.py +++ b/tests/project/test_docs_internal_refs_1940_unit.py @@ -61,10 +61,12 @@ HELPERS = ( ('pcapkit/const/ftp.rst', 'pcapkit.const.ftp.command.Command.__new__'), ('pcapkit/const/reg.rst', 'pcapkit.const.reg.apptype.apptype.AppType.__eq__'), + ('pcapkit/const/reg.rst', 'pcapkit.const.reg.apptype.apptype.AppType.__hash__'), ('pcapkit/dumpkit/pcap.rst', 'pcapkit.dumpkit.pcap.PCAPIO.__init__'), ('pcapkit/foundation/extraction.rst', 'pcapkit.foundation.extraction.Extractor._note_eof_progress'), ('pcapkit/foundation/extraction.rst', 'pcapkit.foundation.extraction.Extractor._owns_input'), ('pcapkit/foundation/extraction.rst', 'pcapkit.foundation.extraction.Extractor._register_internal_engine'), + ('pcapkit/foundation/reassembly/tcp.rst', 'pcapkit.foundation.reassembly.tcp.TCP._make_ports'), ('pcapkit/protocols/application/http.rst', 'pcapkit.protocols.application.http.HTTP.from_data'), ('pcapkit/protocols/application/httpv2.rst', 'pcapkit.protocols.application.httpv2.HTTP.unpack'), ('pcapkit/protocols/application/ngap.rst', 'pcapkit.protocols.application.ngap._convert'),