fix(tcp): set MPTCP.subtype on construction, fix MP_CAPABLE's length/rkey (#566, #567)

CHANGELOG.md

@@ -41,6 +41,7 @@ The largest release since 1.0, and the first recorded here as it happened rather
 - **Fixed** -- `TCP._make_mptcp_addaddr` could not build an `ADD_ADDR` option end to end: its `kind=`/`length=` arguments were rejected with `UnknownFieldWarning` and silently dropped, and `.pack()` then raised `KeyError: 'length'` from `port`'s own condition, `pkt['length'] in (10, 22)`. The cause was one layer up -- `MPTCP`, the base class every Multipath TCP subtype schema inherits, declared `kind` and `length` only under `typing.TYPE_CHECKING` rather than as real fields, unlike `Option`, which every non-Multipath TCP option schema inherits instead. That silently dropped `kind=`/`length=` for every `_make_mptcp_*` constructor, not only `ADD_ADDR`'s, so `MPTCP` now declares both for real, the same way `Option` already did (#541). The same missing fields broke parsing too: with no `kind`/`length` fields ahead of it, a Multipath TCP subtype schema's own leading field read the `kind` octet itself rather than the octet meant for it, an off-by-two in field alignment rather than a wire-format change -- a correct sender's octets were always right, only this library's reading of them was shifted. Spec-correct `ADD_ADDR` and `MP_PRIO` options failed to parse with `FieldError: TCP: [OptNo 30] 3 invalid IP version` and `KeyError: 'length'` respectively; both parse correctly now.
 - **Fixed** -- which exception a malformed TCP SACK option raised depended on unrelated process state: a clean interpreter raised `ProtocolError` as documented, but a process that had already popped `pcapkit.corekit.fields.misc` from `sys.modules` -- which the `#439` ABC-cache regression tests do in every case's `setUp`/`tearDown` -- raised `FieldValueError` instead, from a different layer entirely, before the documented check was even reached (#525). The cause was `ListField.unpack` resolving `SchemaField` through a function-local import re-run on every call; a module popped and reimported mid-process comes back as a second, distinct class, so `isinstance` against it silently misclassified the field and billed each item by its declared length instead of by what it actually consumed. **Any caller relying on the previously-observed** `FieldValueError` **for this case now gets** `ProtocolError` **instead, deterministically**, matching the method's own docstring. Fixed by importing at module level instead.
 - **Fixed** -- two dropped-keyword/wrong-cast defects flagged in review during this release and never filed until now: HIP's `_make_param_encrypted` passed `cipher=` to a schema with no such field, so the value was silently dropped and an AES-cipher `ENCRYPTED` parameter built through `make` packed without its IV; and IPv6-Route's `RPL.post_process`, which runs on every `Schema.pack` and not only after a parse, assumed `self.addresses` was still the concatenated `bytes` a parse leaves it as, and raised slicing the `list[bytes]` a `make`-built multi-address header actually holds there (#556).
+- **Fixed** -- two more defects #541 exposed rather than caused, both since it let construction reach code that had never run before. `MPTCP.subtype` was still `typing.TYPE_CHECKING`-only, an annotation rather than a field, so `TCP(options=[(Enum_Option.Multipath_TCP, ...)])` raised `AttributeError: ... has no attribute 'subtype'` for every subtype but `MP_JOIN`: the convenience constructor builds a schema in memory and reads it straight back through `_read_mptcp_*` with no byte round trip, so `_MPTCP.post_process` -- the only code that ever set `subtype` -- never ran. Fixed on the construction path (`TCP._make_mode_mp`) rather than by adding a third real field the way `kind`/`length` got in #541: unlike those two, `subtype` is already packed as 4 bits of each subtype's own `test` bitfield, and a second, independent field for the same bits would either double-encode them or need a "derive, don't pack" field kind this library does not have (#566). Separately, `_make_mptcp_capable` wrote `length=20 if rkey is None else 32` where [RFC 8684](https://datatracker.ietf.org/doc/html/rfc8684) section 3.1 gives 12 and 20, and `MPTCPCapable.rkey`'s own condition (`pkt['length'] != 32`) dropped the receiver's key for exactly the length the maker used to mean "key present" -- so a spec-correct, key-absent MP_CAPABLE could not be built at all, and a key-present one silently lost its key on the wire. Both, and the matching guard in `_read_mptcp_capable`, now agree on 12/20. **This changes MP_CAPABLE's packed output**: a 20-octet, key-present option built or parsed under the old code becomes 12 octets with no key, or 20 octets with the key actually present, depending on which the caller meant (#567).
 
 Preceded by `1.5.0a1` (2026-09-15), `1.5.0b1` and `1.5.0b2` (both 2026-09-18) and `1.5.0b3` (2026-09-19), all published as prereleases and so resolved only by `pip install --pre`. `1.5.0b1` half-shipped: the tag, the GitHub release and the Conda deployments landed, but PyPI rejected the wheel because `twine check` found a Sphinx-only `:mod:` role in `README.rst`, which `pyproject.toml` declares as the dynamic long description. `1.5.0b2` is what reshipped it -- the release workflow is version-driven, so an existing version cannot republish -- and `1.5.0b3` followed the CI change that stops a TestPyPI outage from costing a release its wheels (#497, #498).
 

docs/source/changelog/1.5.0.rst

@@ -389,6 +389,29 @@ pull requests between #326 and #509.
   still the concatenated ``bytes`` a parse leaves it as, and raised slicing the
   ``list[bytes]`` a ``make``-built multi-address header actually holds there
   (#556).
+* **Fixed** -- two more defects #541 exposed rather than caused, both since it let
+  construction reach code that had never run before. ``MPTCP.subtype`` was still
+  ``typing.TYPE_CHECKING``-only, an annotation rather than a field, so
+  ``TCP(options=[(Enum_Option.Multipath_TCP, ...)])`` raised
+  ``AttributeError: ... has no attribute 'subtype'`` for every subtype but
+  ``MP_JOIN``: the convenience constructor builds a schema in memory and reads it
+  straight back through ``_read_mptcp_*`` with no byte round trip, so
+  ``_MPTCP.post_process`` -- the only code that ever set ``subtype`` -- never ran.
+  Fixed on the construction path (``TCP._make_mode_mp``) rather than by adding a
+  third real field the way ``kind``/``length`` got in #541: unlike those two,
+  ``subtype`` is already packed as 4 bits of each subtype's own ``test``
+  bitfield, and a second, independent field for the same bits would either
+  double-encode them or need a "derive, don't pack" field kind this library does
+  not have (#566). Separately, ``_make_mptcp_capable`` wrote
+  ``length=20 if rkey is None else 32`` where :rfc:`8684` section 3.1 gives 12 and
+  20, and ``MPTCPCapable.rkey``'s own condition (``pkt['length'] != 32``) dropped
+  the receiver's key for exactly the length the maker used to mean "key present"
+  -- so a spec-correct, key-absent MP_CAPABLE could not be built at all, and a
+  key-present one silently lost its key on the wire. Both, and the matching guard
+  in ``_read_mptcp_capable``, now agree on 12/20. **This changes MP_CAPABLE's
+  packed output**: a 20-octet, key-present option built or parsed under the old
+  code becomes 12 octets with no key, or 20 octets with the key actually present,
+  depending on which the caller meant (#567).
 
 Preceded by ``1.5.0a1`` (2026-09-15), ``1.5.0b1`` and ``1.5.0b2`` (both
 2026-09-18) and ``1.5.0b3`` (2026-09-19), all published as prereleases and so

pcapkit/protocols/schema/transport/tcp.py

@@ -645,6 +645,23 @@ class MPTCP(EnumSchema[Enum_MPTCPOption]):
     #: MPTCP length.
     length: 'int' = UInt8Field()
 
+    # NOTE: ``subtype`` stays an annotation rather than becoming a third real
+    # field alongside ``kind``/``length`` above -- deliberately, and the
+    # difference from those two is why. ``kind`` and ``length`` are each the
+    # *sole* source of their own octet: nothing else in the schema packs them,
+    # so declaring them as real fields was the only way to get them onto the
+    # wire at all. ``subtype`` is not like that: every concrete subclass
+    # already encodes it as 4 bits of its own ``test`` :class:`BitField` (e.g.
+    # ``MPTCPCapable.test['subtype']``), which is what actually gets packed.
+    # A real ``Field`` for ``subtype`` on top of that would either pack the
+    # same 4 bits twice under two names, or need a "derive, don't pack" kind
+    # of field that this library's :mod:`~pcapkit.corekit.fields` does not
+    # have. So this attribute is populated by the *construction* path instead
+    # -- :meth:`~pcapkit.protocols.transport.tcp.TCP._make_mode_mp` sets it
+    # right after building the subtype-specific schema, mirroring exactly what
+    # :meth:`_MPTCP.post_process` already does for real unpacking. C.f. #566,
+    # the third and last ``TYPE_CHECKING``-only attribute this class had; the
+    # other two (``kind``, ``length``) were fixed in #541.
     if TYPE_CHECKING:
         #: MPTCP subtype.
         subtype: 'Enum_MPTCPOption'
@@ -684,9 +701,14 @@ class MPTCPCapable(MPTCP, code=Enum_MPTCPOption.MP_CAPABLE):
     #: Option sender's key.
     skey: 'int' = UInt64Field()
     #: Option receiver's key.
+    #:
+    #: :rfc:`8684` section 3.1 gives MP_CAPABLE as 12 octets without this key
+    #: and 20 octets with it, so the field is present only for the latter --
+    #: not, as it read until #567, for every length *except* 32, which is not
+    #: an MP_CAPABLE length either RFC form uses.
     rkey: 'int' = ConditionalField(
         UInt64Field(),
-        lambda pkt: pkt['length'] != 32,
+        lambda pkt: pkt['length'] == 20,
     )
 
     if TYPE_CHECKING:

pcapkit/protocols/transport/tcp.py

@@ -1483,10 +1483,15 @@ def _read_mptcp_capable(self, schema: 'Schema_MPTCPCapable', *, options: 'Option
             Parsed option data.
 
         Raises:
-            ProtocolError: If length is **NOT** ``20`` or ``32``.
+            ProtocolError: If length is **NOT** ``12`` or ``20``.
 
         """
-        if schema.length not in (20, 32):
+        # NOTE: :rfc:`8684` section 3.1 gives MP_CAPABLE as 12 octets without
+        # the receiver's key and 20 octets with it -- this guard, and the
+        # ``rkey=`` below, read ``(20, 32)``/``32`` until #567, which is what
+        # made a spec-correct 12-octet MP_CAPABLE unparseable and read a
+        # spec-correct 20-octet one (with the key) as having none.
+        if schema.length not in (12, 20):
             raise ProtocolError(f'{self.alias}: [OptNo {schema.kind}] invalid format')
 
         data = Data_MPTCPCapable(
@@ -1500,7 +1505,7 @@ def _read_mptcp_capable(self, schema: 'Schema_MPTCPCapable', *, options: 'Option
                 hsa=bool(schema.flags['hsa']),
             ),
             skey=schema.skey,
-            rkey=schema.rkey if schema.length == 32 else None,
+            rkey=schema.rkey if schema.length == 20 else None,
         )
         return data
 
@@ -2601,6 +2606,15 @@ def _make_mode_mp(self, code: 'Enum_Option', opt: 'Optional[Data_MPTCP]' = None,
             meth = name[1]
 
         schema = meth(subtype_val, opt, **kwargs)
+        # NOTE: ``Schema_MPTCP.subtype`` is not a packable field (see the
+        # comment on :class:`~pcapkit.protocols.schema.transport.tcp.MPTCP`),
+        # so nothing above set it -- ``subtype`` only ever went into ``test``,
+        # the bitfield each concrete maker actually packs. Real unpacking gets
+        # it from :meth:`~pcapkit.protocols.schema.transport.tcp._MPTCP.post_process`;
+        # this is that same assignment for the construction path, so a schema
+        # built via ``TCP(options=[(Enum_Option.Multipath_TCP, ...)])`` has
+        # ``.subtype`` set exactly as one built by parsing bytes does. C.f. #566.
+        schema.subtype = subtype_val
         return schema
 
     def _make_mptcp_unknown(self, subtype: 'Enum_MPTCPOption', opt: 'Optional[Data_MPTCPUnknown]' = None, *,
@@ -2666,7 +2680,12 @@ def _make_mptcp_capable(self, subtype: 'Enum_MPTCPOption', opt: 'Optional[Data_M
 
         return Schema_MPTCPCapable(
             kind=cast('Enum_Option', Enum_Option.Multipath_TCP),
-            length=20 if rkey is None else 32,
+            # NOTE: :rfc:`8684` section 3.1 gives MP_CAPABLE as 12 octets
+            # without the receiver's key and 20 octets with it. This read
+            # ``20 if rkey is None else 32`` until #567 -- both branches
+            # wrong, and the no-key branch writing the value that RFC 8684
+            # assigns to the *other* case.
+            length=12 if rkey is None else 20,
             test={
                 'subtype': subtype.value,
                 'version': version,

tests/protocols/test_option_roundtrip_unit.py

@@ -146,53 +146,32 @@ class Gap(NamedTuple):
         'negative below 40 kbps, and the option it emits at rate=0 does not '
         'parse back'),
 
-    # #541 declared real ``kind``/``length`` fields on ``MPTCP``, which is what
-    # gets these seven far enough to construct and pack -- they used to fail
-    # here with ``KeyError: 'length'`` (four of them) or
-    # ``AttributeError: ... no attribute 'kind'`` (three), and #541's fix closed
-    # both. The next field the construction path never sets is ``subtype``:
-    # ``MPTCP.subtype`` is still ``TYPE_CHECKING``-only, an annotation rather
-    # than a field, and the only thing that ever sets it is
-    # ``_MPTCP.post_process``, which runs on a real byte-level unpack -- not on
-    # the schema a ``_make_mptcp_*`` maker returns in memory, which is what
-    # ``TCP``'s convenience constructor (``TCP(options=[(code, kwargs)])``)
-    # reads straight back through ``_read_mptcp_*`` with no round trip in
-    # between. Filed as #566, which also has the fix for the pattern itself
-    # (whether ``subtype`` should become a real field the way ``kind``/
-    # ``length`` did, or be set some other way) rather than a per-case patch
-    # here.
-    'tcp-mptcp/MP_CAPABLE': Gap(
-        'CONSTRUCT', "'MPTCPCapable' object has no attribute 'subtype'",
-        'pcapkit/protocols/schema/transport/tcp.py:650 -- MPTCP.subtype is '
-        'TYPE_CHECKING-only, set only by _MPTCP.post_process (#566); and '
-        "separately, pcapkit/protocols/transport/tcp.py:2669's "
-        '`length=20 if rkey is None else 32` is RFC 8684 section 3.1\'s 12/20 '
-        'swapped, so once #566 is fixed this case would still pack the wrong '
-        'length rather than round-trip (#567)'),
-    'tcp-mptcp/ADD_ADDR': Gap(
-        'CONSTRUCT', "'MPTCPAddAddress' object has no attribute 'subtype'",
-        'pcapkit/protocols/schema/transport/tcp.py:650 -- MPTCP.subtype is '
-        'TYPE_CHECKING-only, set only by _MPTCP.post_process (#566)'),
-    'tcp-mptcp/REMOVE_ADDR': Gap(
-        'CONSTRUCT', "'MPTCPRemoveAddress' object has no attribute 'subtype'",
-        'pcapkit/protocols/schema/transport/tcp.py:650 -- MPTCP.subtype is '
-        'TYPE_CHECKING-only, set only by _MPTCP.post_process (#566)'),
-    'tcp-mptcp/MP_PRIO': Gap(
-        'CONSTRUCT', "'MPTCPPriority' object has no attribute 'subtype'",
-        'pcapkit/protocols/schema/transport/tcp.py:650 -- MPTCP.subtype is '
-        'TYPE_CHECKING-only, set only by _MPTCP.post_process (#566)'),
-    'tcp-mptcp/DSS': Gap(
-        'CONSTRUCT', "'MPTCPDSS' object has no attribute 'subtype'",
-        'pcapkit/protocols/schema/transport/tcp.py:650 -- MPTCP.subtype is '
-        'TYPE_CHECKING-only, set only by _MPTCP.post_process (#566)'),
-    'tcp-mptcp/MP_FAIL': Gap(
-        'CONSTRUCT', "'MPTCPFallback' object has no attribute 'subtype'",
-        'pcapkit/protocols/schema/transport/tcp.py:650 -- MPTCP.subtype is '
-        'TYPE_CHECKING-only, set only by _MPTCP.post_process (#566)'),
+    # #541 declared real ``kind``/``length`` fields on ``MPTCP``, which got six
+    # of these seven far enough to construct and pack, and #566/#567 closed
+    # the rest of the chain: #566 gave ``MPTCP.subtype`` the same treatment
+    # (it was ``TYPE_CHECKING``-only, an annotation rather than a field, and
+    # the only thing that ever set it was ``_MPTCP.post_process``, which runs
+    # on a real byte-level unpack -- not on the schema a ``_make_mptcp_*``
+    # maker returns in memory, which is what ``TCP``'s convenience
+    # constructor, ``TCP(options=[(code, kwargs)])``, reads straight back
+    # through ``_read_mptcp_*`` with no round trip in between); #567 fixed
+    # MP_CAPABLE's own ``length``/``rkey`` arithmetic on top of that. Six of
+    # the seven (MP_CAPABLE, ADD_ADDR, REMOVE_ADDR, MP_PRIO, DSS, MP_FAIL) now
+    # read ``'OK'`` and so have no entry below any more.
+    #
+    # MP_FASTCLOSE does not: fixing ``subtype`` got it *past* the
+    # ``AttributeError`` this table used to record and into a second,
+    # unrelated defect that #566/#567 do not touch and #576 tracks -- see that
+    # entry for the detail.
     'tcp-mptcp/MP_FASTCLOSE': Gap(
-        'CONSTRUCT', "'MPTCPFastclose' object has no attribute 'subtype'",
-        'pcapkit/protocols/schema/transport/tcp.py:650 -- MPTCP.subtype is '
-        'TYPE_CHECKING-only, set only by _MPTCP.post_process (#566)'),
+        'CONSTRUCT', 'TCP: [OptNo 30] invalid format',
+        'pcapkit/protocols/transport/tcp.py:1893 -- _read_mptcp_fastclose '
+        'requires schema.length == 16, which agrees with neither the maker '
+        '(_make_mptcp_fastclose at pcapkit/protocols/transport/tcp.py:3054, '
+        'length=12, which is the RFC 8684 section 3.7 value) nor the schema '
+        '(MPTCPFastclose.test at '
+        'pcapkit/protocols/schema/transport/tcp.py:907, which packs an '
+        '11-octet option against the declared 12) (#576)'),
 
     # ``_make_mptcp_join`` branches on ``self._flags``, which only the parse
     # path ever sets, so the constructor cannot be called at all.