test(tcp): reach the MP_JOIN dispatchers through TCP(), not a hand-written _flags (#603)

CHANGELOG.md

@@ -64,6 +64,8 @@ This is the resolution of #548, which reported `TransType.L2TP` (115) as registe
 - **Fixed** -- `httpv1`'s `_RE_METHOD` was unanchored and `re.match` anchors only at the start, so it prefix-matched, and the request-line reader then passed the whole `para1` to `Method.get` rather than the captured `method` group. Together those meant `b'Get'` matched on the single character `G`, satisfied the guard that decides a start-line is a request, and handed the entire mixed-case token to a lookup that raised on it. Fixing either half alone still gives a wrong answer -- normalising the lookup would parse `b'Get'` as `GET` off a one-character match, and passing the group would parse it as a method named `G`. The pattern is now anchored at both ends and the captured group is what is looked up, so a token that is not a method is a malformed request line rather than a mis-parsed one. Method tokens are case-sensitive per [RFC 9110 Section 9.1](https://datatracker.ietf.org/doc/html/rfc9110#section-9.1), so no `re.I` was added: `GET` parses, `Get` and `get` are rejected (#583).
 - **Fixed** -- `_RE_STATUS` in the same reader carried the same unanchored prefix defect, found by auditing `_RE_METHOD`'s siblings, and it escaped as the wrong exception type. That pattern is only a guard -- the value is taken from `int(para2)` on the raw token -- so a prefix match let a malformed status past the guard and then out of `int()` uncaught, where `_read_http_header` documents `ProtocolError`. Measured: a status of `200x` raised `ValueError: invalid literal for int() with base 10: b'200x'`, and one of `2000` raised `ValueError: 2000 is not a valid StatusCode`; both are now `ProtocolError`. [RFC 9112 Section 4](https://datatracker.ietf.org/doc/html/rfc9112#section-4) gives `status-code = 3DIGIT`, exactly three, so the anchor is what the grammar already said -- the production lives in HTTP/1.1 because `status-code` is part of its `status-line`, while [RFC 9110 Section 15](https://datatracker.ietf.org/doc/html/rfc9110#section-15) covers the code semantics and the IANA registry rather than the syntax. `_RE_VERSION` was audited at the same time and is safe as it stands, because both of its call sites read the captured group rather than the raw token (#583).
 - **Fixed** -- `get()`'s documented `default` was ignored on the integer path throughout the generated `pcapkit.const` tree, because `get` delegated the lookup to the enum call and `_missing_` has no access to the caller's `default` -- so `Hardware.get(99999, 0)` raised `ValueError: 99999 is not a valid Hardware` instead of returning the fallback it was handed. The integer path now consults `default` before letting the lookup error escape. `-1`, the placeholder the generated signature already carried, is what separates "no default was supplied" from "a default was supplied and should be used", so a caller that asked for no fallback still gets the error rather than a silent substitution. The sweep #584 asked for puts the scope at 110 of the 118 integer registries, not the three the issue named; the two carrying a bespoke integer fallback of their own, `pcapng` `OptionType` and `reg` `AppType`, are deliberately left alone, since neither drops a default by raising. Not reachable from wire data -- every value a wire field can carry already resolves -- so this is a contract fix rather than a parse fix. Applied to the nine vendor templates as well as the 113 generated modules, and a new test renders the shared template and compares it against the module generated from it, so a regeneration cannot quietly undo it (#584).
+- **Changed** -- `tests/protocols/transport/test_tcp_udp_unit.py` now reaches the MP_JOIN dispatchers through `TCP()` itself, instead of assigning a Python `set` to `_flags` on a bare `TCP.__new__(TCP)`. A `set` answers the membership tests `_make_mptcp_join` and `_read_mptcp_join` use, so every flag branch ran and both TCP modules read 100% statement and branch coverage -- while the attribute had neither the `aenum.IntFlag` type production assigns nor the ordering that governs when it exists at all, which is how #587 stayed invisible behind that number and how the `cast('Enum_Flags', 0)` no-op behind it went unnoticed too. Measured on the rewrite, against the 17 tests of that file: revert #587's hoist and two of them fail with `AttributeError: 'TCP' object has no attribute '_flags'` where all 17 passed before; restore the `cast` and two fail with `TypeError: argument of type 'int' is not a container or iterable`, again where all 17 passed. The library is unchanged and the file's tests still pass, so the coverage numbers do not move -- the point is what the same numbers are now worth (#603).
+- **Fixed** -- documentation. `mptcp_dss_ack_selector`'s note said a corrected field-width lambda "would not have worked" and that fixing it belonged to `pcapkit.corekit.fields.numbers`, which is exactly where #598 then fixed it; the same paragraph sat in `test_tcp_mptcp_length_arithmetic_unit.py`'s module docstring, whose other stale claim was that MP_JOIN "cannot be built through the public `TCP()` constructor at all", true only until #587. A callable-length `NumberField` packs and unpacks both DSS widths now, and wire *absence* was never the obstacle either: `MPTCPDSS.ssn`, `dl_len` and `checksum` have always been `ConditionalField` on the sibling `M` flag, so the class already relied on that wrapper to keep a field off the wire. The `SwitchField` form is kept for the narrower reason the note now gives -- `ConditionalField`'s `length` forwards to the wrapped field without consulting the condition, so it is safe here only because `Schema.pack` and `Schema.unpack` special-case that wrapper by name, whereas a `SwitchField` always resolves to a concrete field. Replacing it would be a behaviour change and is not made (#603).
 
 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

@@ -814,6 +814,37 @@ pull requests between #326 and #509.
   well as the 113 generated modules, and a new test renders the shared template
   and compares it against the module generated from it, so a regeneration
   cannot quietly undo it (#584).
+* **Changed** -- ``tests/protocols/transport/test_tcp_udp_unit.py`` now reaches the
+  MP_JOIN dispatchers through ``TCP()`` itself, instead of assigning a Python
+  ``set`` to ``_flags`` on a bare ``TCP.__new__(TCP)``. A ``set`` answers the
+  membership tests ``_make_mptcp_join`` and ``_read_mptcp_join`` use, so every flag
+  branch ran and both TCP modules read 100% statement and branch coverage -- while
+  the attribute had neither the ``aenum.IntFlag`` type production assigns nor the
+  ordering that governs when it exists at all, which is how #587 stayed invisible
+  behind that number and how the ``cast('Enum_Flags', 0)`` no-op behind it went
+  unnoticed too. Measured on the rewrite, against the 17 tests of that file: revert
+  #587's hoist and two of them fail with
+  ``AttributeError: 'TCP' object has no attribute '_flags'``
+  where all 17 passed before; restore the ``cast`` and two fail with
+  ``TypeError: argument of type 'int' is not a container or iterable``, again
+  where all 17 passed. The library is unchanged and the file's tests still pass, so
+  the coverage numbers do not move -- the point is what the same numbers are now
+  worth (#603).
+* **Fixed** -- documentation. ``mptcp_dss_ack_selector``'s note said a corrected
+  field-width lambda "would not have worked" and that fixing it belonged to
+  ``pcapkit.corekit.fields.numbers``, which is exactly where #598 then fixed it; the
+  same paragraph sat in ``test_tcp_mptcp_length_arithmetic_unit.py``'s module
+  docstring, whose other stale claim was that MP_JOIN "cannot be built through the
+  public ``TCP()`` constructor at all", true only until #587. A callable-length
+  ``NumberField`` packs and unpacks both DSS widths now, and wire *absence* was
+  never the obstacle either: ``MPTCPDSS.ssn``, ``dl_len`` and ``checksum`` have
+  always been ``ConditionalField`` on the sibling ``M`` flag, so the class already
+  relied on that wrapper to keep a field off the wire. The ``SwitchField`` form is
+  kept for the narrower reason the note now gives -- ``ConditionalField``'s
+  ``length`` forwards to the wrapped field without consulting the condition, so it
+  is safe here only because ``Schema.pack`` and ``Schema.unpack`` special-case that
+  wrapper by name, whereas a ``SwitchField`` always resolves to a concrete field.
+  Replacing it would be a behaviour change and is not made (#603).
 
 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

@@ -256,7 +256,7 @@ def mptcp_dss_ack_selector(pkt: 'dict[str, Any]') -> 'Field':
         This is a :class:`~pcapkit.corekit.fields.misc.SwitchField` selector
         rather than a :class:`~pcapkit.corekit.fields.misc.ConditionalField`
         wrapping ``NumberField(length=lambda pkt: ...)``, which is what it was
-        until #576, for two independent reasons.
+        until #576.
 
         The width lambda read ``8 if pkt['flags']['a'] else 0`` -- **0**, not 4 --
         so an unextended Data ACK packed no octets at all while the ``length``
@@ -265,20 +265,44 @@ def mptcp_dss_ack_selector(pkt: 'dict[str, Any]') -> 'Field':
         declared, and the ``ack`` value the caller supplied was simply not
         present.
 
-        Correcting the lambda to ``8 if ... else 4`` would not have worked,
-        because :class:`~pcapkit.corekit.fields.numbers.NumberField` cannot pack
-        a callable length at all: it calls ``build_template`` once at
-        ``__init__`` with the placeholder length ``-1``, which latches
-        ``_need_process = True``, and nothing clears that flag when
-        ``__call__`` later resolves the real length and rebuilds the template as
-        ``>I``/``>Q``. ``pre_process`` then hands :func:`struct.pack` bytes for
-        an integer template and it raises ``struct.error: required argument is
-        not an integer``. Measured on the 8-octet form, which the old lambda did
-        reach: ``_make_mptcp_dss(DSS, ack=1 << 40)`` raised exactly that. Fixing
-        that belongs to :mod:`pcapkit.corekit.fields.numbers`; selecting between
-        two fields that each fix ``__template__`` at class level sidesteps it
-        entirely and is the pattern :func:`mptcp_add_address_selector` already
-        uses here.
+        Correcting the lambda to ``8 if ... else 4`` would not have worked *at the
+        time*, because :class:`~pcapkit.corekit.fields.numbers.NumberField` could
+        not pack a callable length at all: it called ``build_template`` once at
+        ``__init__`` with the placeholder length ``-1``, which latched
+        ``_need_process = True``, and nothing cleared that flag when ``__call__``
+        later resolved the real length and rebuilt the template as ``>I``/``>Q``.
+        ``pre_process`` then handed :func:`struct.pack` bytes for an integer
+        template and it raised ``struct.error: required argument is not an
+        integer``. Measured on the 8-octet form, which the old lambda did reach:
+        ``_make_mptcp_dss(DSS, ack=1 << 40)`` raised exactly that.
+
+        That half is now history: **#598 fixed it**, in
+        :mod:`pcapkit.corekit.fields.numbers` where this note used to say the fix
+        belonged, by recomputing ``_need_process`` from the width actually in
+        force instead of once from the placeholder. A callable-length
+        ``NumberField`` packs and unpacks both the 4- and the 8-octet form today,
+        so ``ConditionalField(NumberField(length=...), lambda pkt:
+        pkt['flags']['A'])`` would express this field correctly. Nor was wire
+        *absence* ever the obstacle: :attr:`MPTCPDSS.ssn`, :attr:`MPTCPDSS.dl_len`
+        and :attr:`MPTCPDSS.checksum` are each a ``ConditionalField`` on the
+        sibling ``M`` flag, so this very class already leans on that wrapper to
+        keep a field off the wire.
+
+        The ``SwitchField`` form is kept anyway, for a narrower reason about
+        composition rather than about absence. A
+        :class:`~pcapkit.corekit.fields.misc.ConditionalField`'s ``length``
+        forwards to the wrapped field unconditionally, never consulting the
+        condition, so reading it while the condition is false -- the wrapped field
+        then still unresolved, at its ``-1`` placeholder -- raises
+        ``struct.error: bad char in struct format``. Nothing here meets that only
+        because :class:`Schema
+        <pcapkit.protocols.schema.schema.Schema>`'s ``pack`` and ``unpack``
+        special-case ``ConditionalField`` by name and skip the wrapped field
+        outright before any ``length`` is read. A ``SwitchField`` needs no such
+        special case: its selector always hands back an already-concrete field,
+        :class:`~pcapkit.corekit.fields.misc.NoValueField` included, so its
+        ``length`` is safe wherever it is read. Swapping the two would be a
+        behaviour change, not a tidy-up, and #603 does not make it.
 
     """
     if not pkt['flags']['A']:
@@ -308,8 +332,10 @@ def mptcp_dss_dsn_selector(pkt: 'dict[str, Any]') -> 'Field':
     Note:
         Identical in shape to :func:`mptcp_dss_ack_selector`, and it replaces the
         identical defect: ``NumberField(length=lambda pkt: 8 if pkt['flags']['m']
-        else 0, ...)``. See that function's note for why the ``0`` was wrong and
-        why a corrected lambda would not have packed either. C.f. #576.
+        else 0, ...)``. See that function's note for why the ``0`` was wrong, why a
+        corrected lambda would not have packed either *at the time*, and why the
+        ``SwitchField`` form is kept now that #598 has made a callable length work.
+        C.f. #576, #598.
 
     """
     if not pkt['flags']['M']:
@@ -881,7 +907,7 @@ class MPTCPDSS(MPTCP, code=Enum_MPTCPOption.DSS):
     #: 4 octets when ``A`` is set, 8 when ``a`` is set as well, absent otherwise --
     #: :rfc:`8684` section 3.3 figure 9. Both the presence test and the width live
     #: in :func:`mptcp_dss_ack_selector`, whose note records what this field
-    #: declared until #576 and why a narrower fix would not have packed.
+    #: declared until #576 and why the switch form is kept.
     ack: 'int' = SwitchField(
         selector=mptcp_dss_ack_selector,
     )

tests/protocols/transport/test_tcp_mptcp_length_arithmetic_unit.py

@@ -79,19 +79,25 @@
 the generator's own override arguments: 12 octets packed against a declared 20.
 
 A second defect sat behind the first: correcting the lambda to ``8 if ... else 4`` would still
-not have packed, because :class:`~pcapkit.corekit.fields.numbers.NumberField` cannot pack a
-callable length at all -- it calls ``build_template`` once at ``__init__`` with the placeholder
-length ``-1``, latching ``_need_process = True``, and nothing clears that when ``__call__``
-later resolves the real length and rebuilds the template as ``>I``/``>Q``. Measured on the
-8-octet form, which the old lambda did reach: ``_make_mptcp_dss(DSS, ack=1 << 40)`` raised
-``struct.error: required argument is not an integer``. That belongs to
-:mod:`pcapkit.corekit.fields.numbers` and is not fixed here; the schema instead selects between
+not have packed *at the time*, because :class:`~pcapkit.corekit.fields.numbers.NumberField`
+could not pack a callable length at all -- it called ``build_template`` once at ``__init__``
+with the placeholder length ``-1``, latching ``_need_process = True``, and nothing cleared that
+when ``__call__`` later resolved the real length and rebuilt the template as ``>I``/``>Q``.
+Measured on the 8-octet form, which the old lambda did reach: ``_make_mptcp_dss(DSS, ack=1 <<
+40)`` raised ``struct.error: required argument is not an integer``. #576 left that to
+:mod:`pcapkit.corekit.fields.numbers`, and **#598 has since fixed it** by recomputing
+``_need_process`` from the width in force rather than once from the placeholder, so a callable
+length packs and unpacks both widths today. The schema keeps selecting between
 :class:`~pcapkit.corekit.fields.numbers.UInt32Field` and
 :class:`~pcapkit.corekit.fields.numbers.UInt64Field`, which each fix ``__template__`` at class
 level, through a :class:`~pcapkit.corekit.fields.misc.SwitchField` -- the pattern
 :func:`~pcapkit.protocols.schema.transport.tcp.mptcp_add_address_selector` already uses in that
-module. :class:`TCPMPTCPDSSExtendedFieldsUnitTests` covers the 8-octet forms that could not be
-packed before at all.
+module -- for the narrower reason recorded in
+:func:`~pcapkit.protocols.schema.transport.tcp.mptcp_dss_ack_selector`'s own note, which is
+about :class:`~pcapkit.corekit.fields.misc.ConditionalField`'s condition-blind ``length`` and
+not about wire absence. Swapping the two would be a behaviour change and is not made here.
+:class:`TCPMPTCPDSSExtendedFieldsUnitTests` covers the 8-octet forms that could not be packed
+before at all.
 
 MP_JOIN-SYN is not a defect either
 -----------------------------------
@@ -119,13 +125,21 @@
 the blind spot :mod:`tests.protocols.test_option_roundtrip_unit`'s own docstring names ("a
 defect can leave the cycle closed").
 
-MP_JOIN cannot be built through the public ``TCP()`` constructor at all -- ``_make_mptcp_join``
-dispatches on ``self._flags``, which ``TCP._make`` assigns *after* it has already built the
-options, so construction raises ``AttributeError: 'TCP' object has no attribute '_flags'``.
-That is a separate, already-recorded gap (``tcp-mptcp/MP_JOIN`` in
-:data:`tests.protocols.test_option_roundtrip_unit.EXPECTED_FAILURES`, with exactly that
-diagnosis) and is not touched here, so the MP_JOIN classes drive the makers directly and reach
-the readers by parsing bytes, where ``_flags`` *is* set.
+When this module was written MP_JOIN could not be built through the public ``TCP()``
+constructor at all -- ``_make_mptcp_join`` dispatches on ``self._flags``, which ``TCP.make``
+assigned *after* it had already built the options, so construction raised ``AttributeError:
+'TCP' object has no attribute '_flags'``. That was a separate, already-recorded gap
+(``tcp-mptcp/MP_JOIN`` in :data:`tests.protocols.test_option_roundtrip_unit.EXPECTED_FAILURES`,
+with exactly that diagnosis) and was left alone here, which is why the MP_JOIN classes drive
+the makers directly and reach the readers by parsing bytes, where ``_flags`` *is* set.
+
+**#587 has since hoisted that assignment above the option build**, so the constructor route is
+open now and its ``EXPECTED_FAILURES`` entry is gone;
+:mod:`tests.protocols.transport.test_tcp_mptcp_join_flag_ordering_unit` covers all three
+layouts through ``TCP()`` proper. The classes here are still written against the makers, which
+is what keeps them a check on the *length arithmetic* of each form rather than on the dispatch,
+so they are left as they are -- but the reason is now choice rather than impossibility. C.f.
+#587, #603.
 
 """
 from __future__ import annotations