fix(tcp): resolve the connection flags before building the options (#587)

CHANGELOG.md

@@ -51,6 +51,8 @@ The largest release since 1.0, and the first recorded here as it happened rather
 This is the resolution of #548, which reported `TransType.L2TP` (115) as registered nowhere and proposed binding `L2TPv2` there. That binding is wrong rather than merely awkward: [RFC 3931](https://datatracker.ietf.org/doc/html/rfc3931) §4.1.1 gives 115 to *L2TPv3 over IP*, whose session header is "free of any restrictions imposed by coexistence with L2TPv2 and L2F" and carries **no version nibble at all**, so a v2 parser cannot even detect that the datagram is not its own. Measured, it produced `version=4`, `tunnelid=0x5678` and `sessionid=0xff03` from the top half of a Session ID and two octets of the PPP frame behind it. 115 is a missing *class*, not a missing registration, and stays unbound until an `L2TPv3` class exists; no dissector was invented here to fill it. The reasoning is now recorded in `pcapkit.protocols.link.l2tp` rather than only in a test, and `register_protocol_code`'s worked example -- which named `L2TPv2` at 115 -- names `L2TPv3` instead (#548).
 - **Added** -- `tests/protocols/test_dispatch_reachability_unit.py`, the coverage #548 asked for: every `ProtocolBase` descendant whose `__index__` returns an enum member is checked to be reachable under that code in the registry its enum *type* designates, read from the same `_CODE_DESTINATIONS` table backing `code=` so the two cannot drift. Where `test_dispatch_registry_unit.py` walks the 38 entries that exist and checks each parses, this walks the classes and catches one nothing registered at all -- the shape in which `OSPF` once shipped reachable from no table. 23 claims verified, no gaps; a companion case injects a gap and confirms the audit reports it, so the guard cannot rot into a permanently green no-op (#548).
 - **Fixed** -- both halves of what the `Changelog drift` gate told an author, in `util/changelog_md.py`. `ResidualMarkupError` named the entry file but numbered its lines against the *converted* body, which rule 6 joins onto one line per block: measured on this entry, the body is 55 lines against the file's 580, so a reported number could not reach most of the file at all, and four roles written on two source lines were all reported as `line 46`. The conversion now carries a source map -- which line of the entry each stretch of output came from -- so every complaint cites a line of the file the message names, one complaint per construct rather than one per joined line, in the entry's own order (#588). Rule 2 separately accepted only the bare-number spelling of Sphinx's `:rfc:` role, so a citation of [RFC 6554 Section 3](https://datatracker.ietf.org/doc/html/rfc6554#section-3) fell past the rule that exists for it and was reported as a role the rules do not cover; both spellings now convert, with the anchor carried into the link target and the link text taken from Sphinx's own so that the Markdown and the rendered history say the same thing about the same page (#592). Regenerating `CHANGELOG.md` is byte-identical over all 37 committed entries, so the fix changes what the gate *says* and nothing about what it emits.
+- **Fixed** -- building any `MP_JOIN` option raised `AttributeError: 'TCP' object has no attribute '_flags'`. `TCP.make` constructed the options *before* it assigned the `self._flags` that the option makers read, and `_make_mptcp_join` branches on that attribute to choose between the three layouts [RFC 8684](https://datatracker.ietf.org/doc/html/rfc8684) section 3.2 gives for MP_JOIN -- figure 5 for SYN at 12 octets, figure 6 for SYN/ACK at 16, figure 7 for ACK at 24. The parse path was never affected: `read` assigns the flags before it parses the options, so the identical branches in `_read_mptcp_join` always had them. Fixed by hoisting the flag resolution above the `_make_tcp_options` call, leaving only the header's data-offset computation -- which genuinely needs the options' total length -- after it. **Not** fixed by giving `_flags` a zero default, which would have been worse: `Protocol.pack` is public and calls `make`, so an instance that had already parsed a segment always had the attribute set, and it silently built the option for the segment it had *read* rather than the one it was asked to write. Measured pre-fix, a parsed MP_JOIN-SYN instance asked to pack an MP_JOIN-ACK segment emitted an ACK header carrying figure 5's 12-octet SYN option, with the caller's 20-octet HMAC -- the whole authentication payload of figure 7's form -- replaced by an all-zero phantom token and nonce, and nothing raised. The crash was the benign symptom; a default value fixes only that and leaves the silent corruption. Hoisting also made one branch reachable for the first time, an MP_JOIN asked for on a segment with neither SYN nor ACK set: the accumulator was seeded with `cast('Enum_Flags', 0)`, and `typing.cast` being a runtime no-op, `self._flags` stayed a plain `int` on which the first membership test raised `TypeError` rather than the `ProtocolError` the method documents. It is now seeded with `Enum_Flags(0)`, a real flagless member that compares equal to `0` and ORs identically. The read path's own seed is deliberately unchanged -- a flagless MP_JOIN is rejected by `mptcp_data_selector` before `_read_mptcp_join` runs, so it cannot reach those branches (#587).
+- **Added** -- `tests/protocols/transport/test_tcp_mptcp_join_flag_ordering_unit.py`, covering all three MP_JOIN layouts through the *public* constructor, the construct-pack-parse cycle for each, the stale-flags case that rules out a zero-valued default, the statement order itself, and controls that the parse path and the flag-independent options are unaffected. The gap it closes is why 100% statement and branch coverage of the two changed modules coexisted with a completely broken public path: the pre-existing cases reach `_make_mptcp_join` by assigning a Python `set` to `_flags` on a bare `TCP.__new__(TCP)`, which executes every branch while bypassing both the ordering and the accumulator's type. The now-stale `tcp-mptcp/MP_JOIN` entry is deleted from `EXPECTED_FAILURES`, and the MP_JOIN exclusion in `test_tcp_mptcp_subtype_unit.py` is lifted (#587).
 
 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

@@ -584,6 +584,46 @@ pull requests between #326 and #509.
   rendered history say the same thing about the same page (#592). Regenerating
   ``CHANGELOG.md`` is byte-identical over all 37 committed entries, so the fix
   changes what the gate *says* and nothing about what it emits.
+* **Fixed** -- building any ``MP_JOIN`` option raised
+  ``AttributeError: 'TCP' object has no attribute '_flags'``. ``TCP.make``
+  constructed the options *before* it assigned the ``self._flags`` that the option
+  makers read, and ``_make_mptcp_join`` branches on that attribute to choose
+  between the three layouts :rfc:`8684` section 3.2 gives for MP_JOIN -- figure 5
+  for SYN at 12 octets, figure 6 for SYN/ACK at 16, figure 7 for ACK at 24. The
+  parse path was never affected: ``read`` assigns the flags before it parses the
+  options, so the identical branches in ``_read_mptcp_join`` always had them. Fixed
+  by hoisting the flag resolution above the ``_make_tcp_options`` call, leaving only
+  the header's data-offset computation -- which genuinely needs the options' total
+  length -- after it. **Not** fixed by giving ``_flags`` a zero default, which would
+  have been worse: ``Protocol.pack`` is public and calls ``make``, so an instance
+  that had already parsed a segment always had the attribute set, and it silently
+  built the option for the segment it had *read* rather than the one it was asked to
+  write. Measured pre-fix, a parsed MP_JOIN-SYN instance asked to pack an
+  MP_JOIN-ACK segment emitted an ACK header carrying figure 5's 12-octet SYN option,
+  with the caller's 20-octet HMAC -- the whole authentication payload of figure 7's
+  form -- replaced by an all-zero phantom token and nonce, and nothing raised. The
+  crash was the benign symptom; a default value fixes only that and leaves the
+  silent corruption. Hoisting also made one branch reachable for the first time, an
+  MP_JOIN asked for on a segment with neither SYN nor ACK set: the accumulator was
+  seeded with ``cast('Enum_Flags', 0)``, and ``typing.cast`` being a runtime no-op,
+  ``self._flags`` stayed a plain ``int`` on which the first membership test raised
+  ``TypeError`` rather than the ``ProtocolError`` the method documents. It is now
+  seeded with ``Enum_Flags(0)``, a real flagless member that compares equal to ``0``
+  and ORs identically. The read path's own seed is deliberately unchanged -- a
+  flagless MP_JOIN is rejected by ``mptcp_data_selector`` before
+  ``_read_mptcp_join`` runs, so it cannot reach those branches (#587).
+* **Added** -- ``tests/protocols/transport/test_tcp_mptcp_join_flag_ordering_unit.py``,
+  covering all three MP_JOIN layouts through the *public* constructor, the
+  construct-pack-parse cycle for each, the stale-flags case that rules out a
+  zero-valued default, the statement order itself, and controls that the parse path
+  and the flag-independent options are unaffected. The gap it closes is why 100%
+  statement and branch coverage of the two changed modules coexisted with a
+  completely broken public path: the pre-existing cases reach ``_make_mptcp_join``
+  by assigning a Python ``set`` to ``_flags`` on a bare ``TCP.__new__(TCP)``, which
+  executes every branch while bypassing both the ordering and the accumulator's
+  type. The now-stale ``tcp-mptcp/MP_JOIN`` entry is deleted from
+  ``EXPECTED_FAILURES``, and the MP_JOIN exclusion in
+  ``test_tcp_mptcp_subtype_unit.py`` is lifted (#587).
 
 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/transport/tcp.py

@@ -543,12 +543,22 @@ def make(self,
             Constructed packet data.
 
         """
-        if options is not None:
-            options_value, total_length = self._make_tcp_options(options)
-        else:
-            options_value, total_length = [], 0
-
-        offset = math.ceil((20 + total_length) / 4)
+        # NOTE: the connection control flags are resolved *before* the options are
+        # built, because option makers reached from ``_make_tcp_options`` read
+        # :attr:`self._flags` -- ``_make_mptcp_join`` branches on it to pick between
+        # the three MP_JOIN layouts of :rfc:`8684` section 3.2 (figure 5 for SYN,
+        # figure 6 for SYN/ACK, figure 7 for ACK). This block used to sit *after* the
+        # ``_make_tcp_options`` call below, so on a fresh instance MP_JOIN construction
+        # died with ``AttributeError: 'TCP' object has no attribute '_flags'``, and on
+        # an instance that had already parsed a segment it silently built the option
+        # for *that* segment's flags instead: measured pre-fix, a parsed MP_JOIN-SYN
+        # instance asked to ``pack`` an MP_JOIN-ACK segment emitted an ACK header
+        # carrying the 12-octet SYN option, dropping the caller's 20-octet HMAC. That
+        # second outcome is why initialising ``_flags`` to zero is not the fix -- it
+        # would leave the stale read intact and turn the fresh case into a spurious
+        # ``invalid flags combination``. Nothing between here and the old assignment
+        # site reads ``self._flags`` or depends on the option build, so hoisting the
+        # whole block is behaviour-preserving for every other option. C.f. #587.
         flags = {
             'cwr': int(cwr),
             'ece': int(ece),
@@ -560,12 +570,34 @@ def make(self,
             'fin': int(fin),
         }  # type: Schema_Flags
 
-        _flag = cast('Enum_Flags', 0)
+        # NOTE: ``Enum_Flags(0)``, not ``cast('Enum_Flags', 0)``.
+        # :func:`typing.cast` is a runtime no-op, so the accumulator used to stay the
+        # plain :class:`int` ``0`` whenever no flag was set, and ``Enum_Flags.SYN in
+        # self._flags`` then raised ``TypeError: argument of type 'int' is not a
+        # container or iterable`` instead of reaching ``_make_mptcp_join``'s own
+        # ``ProtocolError: ... invalid flags combination``. That branch was unreachable
+        # before the hoist above -- construction died on the missing attribute first --
+        # so this keeps the newly reachable no-SYN-no-ACK case raising the library's
+        # documented error rather than a bare Python one. :class:`Enum_Flags` is an
+        # :class:`aenum.IntFlag`, so ``Enum_Flags(0)`` is a valid flagless member that
+        # still compares equal to ``0`` and still ORs as before. The read path keeps its
+        # ``cast`` at the top of ``read``: it cannot reach these branches, because
+        # ``mptcp_data_selector`` rejects a flagless MP_JOIN before ``_read_mptcp_join``
+        # runs, and changing it would alter the ``connection`` value reported for every
+        # flagless parsed segment. C.f. #587.
+        _flag = Enum_Flags(0)
         for key, val in flags.items():
             if val == 1:
                 _flag |= Enum_Flags.get(key.upper())
         self._flags = _flag
 
+        if options is not None:
+            options_value, total_length = self._make_tcp_options(options)
+        else:
+            options_value, total_length = [], 0
+
+        offset = math.ceil((20 + total_length) / 4)
+
         return Schema_TCP(
             srcport=self._make_port(srcport, Enum_TransportProtocol.tcp),
             dstport=self._make_port(dstport, Enum_TransportProtocol.tcp),

tests/protocols/test_option_roundtrip_unit.py

@@ -173,15 +173,25 @@ class Gap(NamedTuple):
     # :mod:`tests.protocols.transport.test_tcp_mptcp_length_arithmetic_unit`
     # against RFC 8684 rather than against the cycle.
 
-    # ``_make_mptcp_join`` branches on ``self._flags``, which only the parse
-    # path ever sets, so the constructor cannot be called at all. Independent of
-    # #576 and left in place: ``TCP._make`` assigns ``_flags`` *after* it has
-    # already built the options, so this is a statement-ordering defect rather
-    # than a length one.
-    'tcp-mptcp/MP_JOIN': Gap(
-        'CONSTRUCT', "no attribute '_flags'",
-        'pcapkit/protocols/transport/tcp.py:2675 -- _make_mptcp_join reads '
-        'self._flags, which exists only while parsing'),
+    # MP_JOIN was the last of the eight MPTCP subtypes with a registered maker to
+    # keep an entry here, recording ``CONSTRUCT`` with ``"no attribute '_flags'"``
+    # against ``_make_mptcp_join``. Its entry is gone with #587, which was a
+    # statement-ordering defect rather than a length one and so deliberately
+    # outlived #576/#585: ``_make_mptcp_join`` branches on ``self._flags`` to
+    # choose between the three MP_JOIN layouts of RFC 8684 section 3.2, and
+    # ``TCP.make`` assigned that attribute *after* it had already built the
+    # options. #587 hoists the flag resolution above the ``_make_tcp_options``
+    # call, leaving only the ``offset`` computation -- which genuinely needs the
+    # options' ``total_length`` -- after it.
+    #
+    # The cycle closes for whichever layout this suite's flags select, the SYN
+    # form of figure 5, since ``examples.generators.options``' ``TCP_BASE`` sets
+    # ``syn`` and leaves ``ack`` clear. The other two forms, and the silent
+    # wrong-layout outcome that made a zero-valued default the wrong fix, are
+    # covered per layout in
+    # :mod:`tests.protocols.transport.test_tcp_mptcp_join_flag_ordering_unit`
+    # -- this suite can only ever exercise one MP_JOIN form, there being one
+    # case per registry code.
 
     # -- IPv4, whose option padding is now fixed ------------------------------