fix(reassembly): the deferred TCP analyser dispatches on AppType, not bare ports (#2421)

Follow-up to #2418, which made `Deferred` generic over what its analyser
dispatches on but left the TCP side of that constraint as `tuple[int, int]`.
The owner's note on it: we are still using int tuples for transport ports, so
move to AppType tuples.

* `TCP.submit` resolves `bufid[1]` / `bufid[3]` through `Transport._make_port`
  before building the `Deferred`, in a new `TCP._make_ports` helper. A deferred
  analysis now dispatches on a named service rather than on a number.
* `_KT`'s second constraint is `tuple[AppType_TCP, AppType_TCP]` -- the
  per-transport registry `pcapkit.const.reg.apptype.tcp.TCP`, which is what the
  resolution really answers with (`type(...) is TCP`, measured), not the base
  `AppType`. `Transport.analyze` keeps the base, being the one method TCP, UDP
  and SCTP share, and says so.
* `Transport.analyze` tests the chosen port against `__proto__` exactly as it
  arrived and unwraps it to the registry's `int` key only when it is a member,
  so a bare `int` caller keeps working -- the method is in v1.4.1 -- and
  `(None, None)` or `(54321, '80')` still fall back to `Raw` as on mainline
  rather than raising or dispatching.
* `_make_ports` reaches `_make_port` through `TCP` rather than through
  `self.protocol`: `__protocol_type__` is an extension point whose analyser need
  only define `analyze`, so `self.protocol._make_port` would be an
  `AttributeError` for one that is not a `Transport`. Going through `_make_port`
  rather than calling `AppType.get` directly is what keeps #2287's bool
  rejection: `AppType.get(True, proto=tcp)` resolves `True` as port 1 and
  answers `<TCP.tcpmux: 1 [tcp]>`, measured.
* `DatagramID`'s ports stay bare `int`s, and its docstring now says why: it is
  the flow identity split out of `BufferID`, a dict key every toolkit adapter
  builds from a plain port number and which is dumped as part of the report.
* Docs entries for `TCP._make_ports` and for `AppType.__hash__`, both now named
  in public prose (#1940).

All 65,536 ports resolve to a member -- an assignment to its canonical service,
an IANA span to that span's label, anything else to an unregistered `unknown` --
none raise and the registry does not grow, so this adds no way for reassembly to
raise. For a registered port the member is the one `TCP.src` reads off a parsed
header; for an unassigned one the two are equal but not identical and named
apart, which the docstrings record rather than overstate. UDP and SCTP need no
change: neither has a reassembler, so `Transport.analyze` has no caller but
`TCP.submit`.

Make an unregistered member survive pickling
--------------------------------------------

Dispatching on members rather than numbers put them inside the `Deferred` that
`TCP.submit` stores as a `Datagram`'s `packet`, and that made an unresolved
datagram unpicklable for most ports. `AppType.get` answers a port it holds no
assignment for through `AppType._unregistered_member`, which builds a member
deliberately absent from every one of the registry's lookup tables so the
lookup does not grow it (#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 it pickled fine and then raised on read-back:

    ValueError: 'unknown [50000 - tcp]' is not a valid TCP

Measured before the fix: 59,413 of the 65,536 possible TCP ports failed to
round-trip, 90.7% of the space, and not merely the truly unregistered `unknown`
ones but every IANA-span label too. All four sample captures with TCP flows
failed, every datagram, because a client socket is ephemeral.

* `EnumRegistry._make_reducible` installs a per-instance `__reduce_ex__` that
  rebuilds through `cls._unregistered_member(*args)` instead of through a value
  lookup, with module-level `_rebuild_unregistered_member` /
  `_reduce_unregistered_member` so that pickle protocols below 4 can name the
  callable. This is the technique `EnumField._unregistered_member` already uses
  for the same class of problem on the *parse* path (#575, #1774).
* `AppType._unregistered_member` opts in, reducing to its own
  `(value, name, proto)` rather than to the crafted `'svc [port - proto]'`
  string it hands its base -- reducing to the latter would rebuild a member
  whose value had been formatted twice. It is opt-in per registry for exactly
  that reason; a registry whose `_missing_` calls the base method directly is
  unchanged.
* Rebuilding re-enters `_unregistered_member` and never `cls.__new__`, so an
  unpickled member is unregistered just as the original was and no table grows
  -- `__members__`, `_value2member_map_` and `__registry__` all measured
  unchanged before and after, on every transport. A restored member is itself
  reducible, so a datagram read back from a dump can be written out again.

After: all 65,536 ports round-trip on every transport (262,144 members), on
every pickle protocol 0-5, under `copy.copy`/`copy.deepcopy`, and all four
named captures load back -- 231 deferred datagrams across `http.pcap`,
`stream.pcap`, `tcp.pcap` and `http6.cap`. A registered member still unpickles
to the identical shared singleton, which pins that the reducer went on the
unregistered path only.

* `tests/const/test_const_apptype_unregistered_pickle_2417_unit.py` pins the
  member-level round trip over the whole port space, every protocol, and the
  no-growth property.
* `tests/foundation/reassembly/test_datagram_deferred_pickle_2417_unit.py` pins
  the symptom on a synthesised datagram, unassigned and assigned alike.
  `test_datagram_deferred_pickle_2417_runtime.py` pins the same thing end to
  end over the four sample captures, split into its own fixture-dependent
  module because CI's unit-tests leg never generates them -- the unit file
  reading a generated capture directly is what broke that leg. Both scope to
  pickle protocols 2-5: `Deferred` declares `__slots__` and no `__getstate__`,
  which protocols 0 and 1 cannot serialise at all. That is pre-existing and
  independent of these members -- it reproduces for an assigned flow and for a
  bare `Deferred(..., 42, b'x')` holding a plain `int` -- so it is recorded
  there rather than worked around, and wants its own issue.
* The #1774 canary asserted that `get`'s miss member was *not* picklable, which
  was a defect rather than a distinction. It now asserts the round trip works
  and that the member's `name` is the only remaining difference from what a
  parse returns -- which is what still justifies the peek. `AppType.__hash__`
  gained the docstring its new `#1940` entry requires.

mypy over `pcapkit/` is byte-identical to mainline (98 errors in 32 files, same
set); `pylint` with the Makefile flags reports the same message set and counts
over the touched files as the pre-review revision; `isort -l100 -ppcapkit` is
clean. Tests: 876 tests / 25,888 subtests over `foundation/reassembly`,
`protocols/transport`, the `const` registries, the field-pickling suites, the
`#1940` docs guard and the docstring contract, with 4 skips that predate this
branch (the local venv has sphinx-autodoc-typehints 3.13.6, not the 3.17.2
`docs/requirements.txt` pins). Reassembly over the sample captures resolves the
same next layer as before, port for port.

Author: JarryShaw

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
 

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__

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
+        <pcapkit.protocols.transport.transport.Transport.analyze>` reads
+        :attr:`Transport.__proto__
+        <pcapkit.protocols.transport.transport.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]':

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))

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 <pcapkit.protocols.protocol.Protocol.analyze>`,
       which looks the next layer up by protocol registry number.
     * :meth:`TCP.submit <pcapkit.foundation.reassembly.tcp.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
       <pcapkit.protocols.transport.transport.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 <pcapkit.foundation.reassembly.tcp.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 <pcapkit.protocols.transport.tcp.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

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 <reasm.tcp.datagram>` original packet identifier."""
+    """Data model for :term:`TCP <reasm.tcp.datagram>` original packet identifier.
+
+    Note:
+        The ports here stay bare :obj:`int` port numbers, where the pair
+        :meth:`TCP.submit <pcapkit.foundation.reassembly.tcp.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
+        <pcapkit.const.reg.apptype.apptype.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
+        <pcapkit.protocols.transport.transport.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