From 817128d73e1a95412aee396067d54e96fd3cc1c4 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Tue, 27 Oct 2026 13:51:54 -0400 Subject: [PATCH] fix(tcp): correct MPTCP unknown-subtype length and drop an undeclared parse-only attribute (#2272, #2293) #2293: _MPTCP.post_process set ret.option = Enum_Option.Multipath_TCP on the subtype schema. MPTCP declares no option field, the declared kind field already holds Multipath_TCP, and no _make_mptcp_* maker mirrors it -- so a parsed schema carried a key a built one did not, though both packed identical octets. Drop the assignment; length and subtype stay, since those really are set here and nowhere else on the unpacking path. #2272: an unknown MPTCP subtype with data=b'' declared Length 2 but MPTCPUnknown always packs 3 header octets (kind, length, and the subtype octet whose low nibble takes data[0]) plus data[1:]. The two agree for any non-empty data and disagree only at data=b''. Size the length from what the schema actually writes: 3 + len(data[1:]). Tests: option-attribute parity across MP_CAPABLE, MP_FASTCLOSE and the MPTCPUnknown fallback; declared-vs-wire length across payload widths, the 3-octet minimum, and a real parse-back through the public TCP() constructor. test_tcp_udp_unit.py's post_process assertion is updated to require option's absence rather than its value. --- pcapkit/protocols/schema/transport/tcp.py | 11 +- pcapkit/protocols/transport/tcp.py | 14 +- .../test_tcp_mptcp_option_attr_2293_unit.py | 261 ++++++++++++++++++ ...test_tcp_mptcp_unknown_length_2272_unit.py | 203 ++++++++++++++ .../protocols/transport/test_tcp_udp_unit.py | 9 +- 5 files changed, 495 insertions(+), 3 deletions(-) create mode 100644 tests/protocols/transport/test_tcp_mptcp_option_attr_2293_unit.py create mode 100644 tests/protocols/transport/test_tcp_mptcp_unknown_length_2272_unit.py diff --git a/pcapkit/protocols/schema/transport/tcp.py b/pcapkit/protocols/schema/transport/tcp.py index 1f6311504d..9cb7f26b8c 100644 --- a/pcapkit/protocols/schema/transport/tcp.py +++ b/pcapkit/protocols/schema/transport/tcp.py @@ -730,7 +730,16 @@ def post_process(self, packet: 'dict[str, Any]') -> 'MPTCP': """ ret = self.data - ret.option = Enum_Option.Multipath_TCP + # NOTE: Only attributes that the construction path sets too are assigned + # here, so that a schema parsed off the wire carries exactly the same + # attributes as one built in memory by ``TCP._make_mode_mp`` (#2293). An + # earlier revision also set ``ret.option = Enum_Option.Multipath_TCP``, + # which no maker mirrored and nothing ever read: ``MPTCP`` declares no + # ``option`` field, and its real ``kind`` field already carries + # ``Multipath_TCP``. ``subtype`` is the one attribute that genuinely has + # to be assigned on both paths -- see the NOTE on :class:`MPTCP` for why + # it cannot be a field -- and ``TCP._make_mode_mp`` makes the matching + # assignment for construction. ret.length = self.test['length'] ret.subtype = Enum_MPTCPOption.get(packet['test']['subtype']) diff --git a/pcapkit/protocols/transport/tcp.py b/pcapkit/protocols/transport/tcp.py index 51bf127972..fc45e378ef 100644 --- a/pcapkit/protocols/transport/tcp.py +++ b/pcapkit/protocols/transport/tcp.py @@ -2883,9 +2883,21 @@ def _make_mptcp_unknown(self, subtype: 'Enum_MPTCPOption', opt: 'Optional[Data_M if opt is not None: data = opt.data + # NOTE: The length is sized from what the schema actually writes, not + # from ``data``'s own width (#2272). ``Schema_MPTCPUnknown`` always packs + # three octets of header -- ``kind``, ``length`` and the subtype/flags + # octet whose low nibble takes ``data[0]`` -- and then ``data[1:]`` as + # its body, so the option is never shorter than 3 octets -- the smallest + # Multipath TCP option :rfc:`8684` section 3 permits. The former + # ``2 + len(data)`` declared 2 for ``data=b''`` while still packing those + # three octets, and the segment could not be read back: + # ``MPTCPUnknown.data`` sizes itself ``pkt['length'] - 3``, which went + # negative and raised ``ProtocolError: Field data resolved to a negative + # length``. For every non-empty ``data`` the two expressions agree, so + # this only changes the empty case. return Schema_MPTCPUnknown( kind=cast('Enum_Option', Enum_Option.Multipath_TCP), - length=2 + len(data), + length=3 + len(data[1:]), test={ 'subtype': subtype.value, 'data': data[0] & 0x0F if data else 0, diff --git a/tests/protocols/transport/test_tcp_mptcp_option_attr_2293_unit.py b/tests/protocols/transport/test_tcp_mptcp_option_attr_2293_unit.py new file mode 100644 index 0000000000..cff1f0d952 --- /dev/null +++ b/tests/protocols/transport/test_tcp_mptcp_option_attr_2293_unit.py @@ -0,0 +1,261 @@ +# -*- coding: utf-8 -*- +"""A parsed Multipath TCP schema carries the same attributes as a built one, ``option`` included. + +GitHub issue #2293. :meth:`_MPTCP.post_process +` set ``ret.option = +Enum_Option.Multipath_TCP`` on the subtype schema it returns. No ``_make_mptcp_*`` maker, and +not :meth:`TCP._make_mode_mp ` either, made +the matching assignment -- so the attribute existed on a schema unpacked from wire bytes and +not on the same schema built in memory. + +Why the assignment had nothing to be symmetric about +---------------------------------------------------- + +``option`` was not a field. :class:`~pcapkit.protocols.schema.transport.tcp.MPTCP` declares +exactly ``kind`` and ``length`` (#541) and every concrete subtype adds its own on top; none of +them declares ``option``, under :data:`~typing.TYPE_CHECKING` or otherwise. So the assignment +created an attribute out of nothing at unpack time, and nothing in ``pcapkit`` read it back: +``kind``, a real :class:`~pcapkit.corekit.fields.enums.EnumField`, already carries +:attr:`~pcapkit.const.tcp.option.Option.Multipath_TCP`, and it is ``kind`` that the schema +packs. + +That is the difference from ``subtype``, the other attribute ``post_process`` assigns, which +*is* needed on both paths and stays. ``subtype`` cannot be a field -- every subtype schema +already packs those 4 bits inside its own ``test`` +:class:`~pcapkit.corekit.fields.strings.BitField`, so a second field for them would either +double-pack or need a derive-don't-pack field this library has no equivalent of -- and every +``_read_mptcp_*`` reader reads it, which is why ``_make_mode_mp`` sets it for construction +(#566, #579, pinned in +:mod:`tests.protocols.transport.test_tcp_mptcp_subtype_unit`). ``option`` had neither +property: no reader wanted it and no maker set it. Dropping it is therefore the symmetric +fix, where adding a mirror assignment to ``_make_mode_mp`` would have propagated a value +with no purpose to the construction path as well. + +Why the defect is invisible to a round trip +------------------------------------------- + +Both schemas pack to the same octets, because ``option`` is not among the fields ``pack`` +walks. Only the attribute *sets* differ, so a byte-level round trip stays closed over the +defect -- the blind spot +:mod:`tests.protocols.test_option_roundtrip_unit`'s docstring names. The assertion here is +accordingly on ``set(schema.to_dict())``, the attributes each path leaves behind, rather than +on packed bytes; the packed bytes are asserted equal as well, to pin that dropping the +assignment moved nothing on the wire. + +Coverage +-------- + +Three subtypes, each reached by parsing hand-built spec-correct octets and then rebuilt from +the resulting data model through ``_make_mode_mp``: MP_CAPABLE and MP_FASTCLOSE for the +registered makers, and an unknown subtype for the ``MPTCPUnknown`` fallback, since +``post_process`` runs once per option regardless of which subtype schema it dispatched to. + +MP_JOIN is deliberately absent. ``_make_mptcp_join`` picks between the three layouts of +:rfc:`8684` section 3.2 by reading ``self._flags``, which a bare ``TCP.__new__(TCP)`` has +not got, so reaching it would mean constructing a whole protocol instance and would test the +flag dispatch rather than the attribute symmetry this module is about -- that dispatch is +covered in :mod:`tests.protocols.transport.test_tcp_mptcp_join_flag_ordering_unit`. + +:meth:`~TCPMPTCPOptionAttributeUnitTests.test_option_is_not_a_declared_field` is the positive +half: it derives from the schema classes themselves that ``option`` never was a field, so the +case for deleting the assignment rather than mirroring it does not rest on this docstring. + +Every case builds its own octets in memory and reads no capture under +:file:`examples/captures/`, so this belongs to the unit tier. + +""" +from __future__ import annotations + +import unittest + +#: Hand-built, spec-correct options, each starting at its ``kind`` octet and already a +#: multiple of 4 octets long, keyed by the subtype they carry. +#: +#: * ``MP_CAPABLE`` -- :rfc:`8684` section 3.1 figure 4, the 12-octet form: ``1e`` kind, ``0c`` +#: length, ``01`` the subtype nibble ``0`` beside version ``1``, ``81`` the ``A``..``H`` +#: flags, then the sender's key. +#: * ``MP_FASTCLOSE`` -- :rfc:`8684` section 3.5 figure 14: subtype nibble ``7`` with its 12 +#: reserved bits as ``0x70 0x00``, then the receiver's key. +#: * ``unknown`` -- the 3-octet minimum of section 3 with subtype nibble ``0xf`` +#: (``Reserved_for_Private_Use``) and a data nibble of ``5``, padded with one +#: End-of-Option-List octet. +SPEC_OPTIONS = { + 'MP_CAPABLE': bytes([0x1E, 0x0C, 0x01, 0x81]) + b'\xAA' * 8, + 'MP_FASTCLOSE': bytes([0x1E, 0x0C, 0x70, 0x00]) + b'\xCC' * 8, + 'unknown': bytes([0x1E, 0x03, 0xF5, 0x00]), +} + + +def build_tcp_segment(option_octets: 'bytes') -> 'bytes': + """Pack a whole SYN segment carrying ``option_octets`` verbatim as its only option. + + Goes through :class:`~pcapkit.protocols.schema.transport.tcp.TCP` (the schema, not the + protocol) directly, so no ``_make_mptcp_*`` maker runs and the option bytes reach the + wire exactly as given -- which is what makes the parsed side of each comparison below an + independent starting point rather than something a maker produced. + + Args: + option_octets: The whole option, header octets included, already padded to a multiple + of 4 octets. + + Returns: + The packed TCP segment. + + Note: + ``pcapkit`` is imported here, inside the function, rather than at module level, for + the reason + :func:`tests.protocols.transport.test_tcp_mptcp_subtype_unit.build_mptcp_option`'s own + docstring records: another module in this suite pops ``pcapkit``'s submodules out of + :data:`sys.modules`, so a name bound at collection time can end up pointing at a + schema class built from a stale ``Schema`` base. + + """ + from pcapkit.protocols.schema.transport.tcp import TCP as Schema_TCP + + if len(option_octets) % 4: + raise ValueError('option_octets must already be a multiple of 4 octets long') + + schema = Schema_TCP( + srcport=50000, dstport=80, seq=1, ack=0, + offset={'offset': 5 + len(option_octets) // 4, 'reserved': 0, 'ns': 0}, + flags={'cwr': 0, 'ece': 0, 'urg': 0, 'ack': 0, 'psh': 0, 'rst': 0, 'syn': 1, 'fin': 0}, + window=8192, checksum=b'\x00\x00', urgent=0, + options=option_octets, + payload=b'', + ) + return schema.pack() + + +def parse_and_rebuild(option_octets: 'bytes') -> 'tuple[object, object]': + """Parse one MPTCP option off the wire, then rebuild it from the data model that produced. + + Args: + option_octets: The whole option, as :func:`build_tcp_segment` wants it. + + Returns: + The schema :meth:`_MPTCP.post_process + ` returned for the parsed + option, and the schema :meth:`TCP._make_mode_mp + ` builds from the parsed option's + data model -- the two paths #2293 is about, starting from the same option. + + """ + from pcapkit.const.tcp.option import Option as Enum_Option + from pcapkit.protocols.transport.tcp import TCP + + raw = build_tcp_segment(option_octets) + protocol = TCP(raw, len(raw)) + + parsed = protocol.schema.options[0] + built = TCP.__new__(TCP)._make_mode_mp( # pylint: disable=protected-access + Enum_Option.Multipath_TCP, protocol.info.options[Enum_Option.Multipath_TCP]) + return parsed, built + + +class TCPMPTCPOptionAttributeUnitTests(unittest.TestCase): + """Parsing and building an MPTCP option leave the same attributes on the schema.""" + + def test_parsed_and_built_schemas_carry_the_same_attributes(self) -> None: + """``set(schema.to_dict())`` agrees between the parse path and the build path. + + The assertion #2293 reports: on ``main`` the parsed side carries a surplus + ``option``, so this reads ``{'option'}`` rather than the empty set. + + """ + for name, octets in SPEC_OPTIONS.items(): + with self.subTest(subtype=name): + parsed, built = parse_and_rebuild(octets) + + self.assertEqual(set(parsed.to_dict()), set(built.to_dict()), + 'a parsed schema and a built schema must have the same keys') + + def test_parsed_schema_has_no_option_attribute(self) -> None: + """The surplus attribute itself is gone, named rather than inferred from a set diff. + + Pinned separately so a regression says *which* attribute came back, and so that + making ``option`` symmetric by also setting it on the construction path -- which + would satisfy the test above -- does not pass here. + + """ + for name, octets in SPEC_OPTIONS.items(): + with self.subTest(subtype=name): + parsed, built = parse_and_rebuild(octets) + + self.assertFalse(hasattr(parsed, 'option')) + self.assertFalse(hasattr(built, 'option')) + + def test_kind_still_carries_multipath_tcp(self) -> None: + """Dropping ``option`` loses no value: ``kind`` is where the option number lives. + + ``kind`` is a real field on :class:`~pcapkit.protocols.schema.transport.tcp.MPTCP`, so + it is both set on every path and actually packed, which is the whole reason ``option`` + was redundant rather than merely asymmetric. + + """ + from pcapkit.const.tcp.option import Option as Enum_Option + + for name, octets in SPEC_OPTIONS.items(): + with self.subTest(subtype=name): + parsed, built = parse_and_rebuild(octets) + + self.assertEqual(parsed.kind, Enum_Option.Multipath_TCP) + self.assertEqual(built.kind, Enum_Option.Multipath_TCP) + + def test_subtype_is_still_set_on_both_paths(self) -> None: + """``subtype``, the assignment that stays, is unaffected. + + ``post_process`` assigns both ``option`` and ``subtype``; only the first was surplus + (#566, #579). Guards against a fix that reached for the whole block. + + """ + from pcapkit.const.tcp.mp_tcp_option import MPTCPOption as Enum_MPTCPOption + + expected = { + 'MP_CAPABLE': Enum_MPTCPOption.MP_CAPABLE, + 'MP_FASTCLOSE': Enum_MPTCPOption.MP_FASTCLOSE, + 'unknown': Enum_MPTCPOption.Reserved_for_Private_Use, + } + for name, octets in SPEC_OPTIONS.items(): + with self.subTest(subtype=name): + parsed, built = parse_and_rebuild(octets) + + self.assertEqual(parsed.subtype, expected[name]) + self.assertEqual(built.subtype, expected[name]) + + def test_packed_octets_are_unchanged(self) -> None: + """Both schemas still pack the octets they were parsed from. + + ``option`` was never packed, so removing it must move nothing on the wire. This is + also what makes the attribute-set assertion above necessary: a byte-level round trip + passes either way. + + """ + for name, octets in SPEC_OPTIONS.items(): + with self.subTest(subtype=name): + parsed, built = parse_and_rebuild(octets) + expected = octets.rstrip(b'\x00') if name == 'unknown' else octets + + self.assertEqual(parsed.pack(), expected) + self.assertEqual(built.pack(), expected) + + def test_option_is_not_a_declared_field(self) -> None: + """No MPTCP schema declares an ``option`` field, so nothing could have depended on it. + + Derived from the schema classes the library ships rather than from a list written + here, so the claim is rechecked whenever a subtype gains a field. + + """ + from pcapkit.protocols.schema.transport.tcp import MPTCP + + schemas = [MPTCP, *MPTCP.__enum__.values(), + MPTCP.__enum__.default_factory()] # the ``MPTCPUnknown`` fallback + self.assertGreater(len(schemas), 1, 'the subtype registry must not be empty') + + for schema in schemas: + with self.subTest(schema=schema.__name__): + self.assertNotIn('option', schema.__fields__) + self.assertIn('kind', schema.__fields__) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/protocols/transport/test_tcp_mptcp_unknown_length_2272_unit.py b/tests/protocols/transport/test_tcp_mptcp_unknown_length_2272_unit.py new file mode 100644 index 0000000000..66efa52b4d --- /dev/null +++ b/tests/protocols/transport/test_tcp_mptcp_unknown_length_2272_unit.py @@ -0,0 +1,203 @@ +# -*- coding: utf-8 -*- +"""An unknown Multipath TCP subtype declares the length it actually packs, 3 octets at least. + +GitHub issue #2272. ``TCP._make_mptcp_unknown`` sized the option as ``2 + len(data)``, +which is one octet short whenever ``data`` is empty, and only then. + +Where the octet goes missing +---------------------------- + +:class:`~pcapkit.protocols.schema.transport.tcp.MPTCPUnknown` packs three octets of header +unconditionally -- ``kind`` and ``length``, both declared on +:class:`~pcapkit.protocols.schema.transport.tcp.MPTCP` (#541), and its own ``test`` +:class:`~pcapkit.corekit.fields.strings.BitField`, whose high nibble is the subtype and whose +low nibble takes ``data[0]`` -- and then ``data[1:]`` as the body. So the first octet of +``data`` is folded into the header's low nibble rather than appended, and the option occupies +``3 + len(data[1:])`` octets: never fewer than 3, which is the smallest Multipath TCP option +:rfc:`8684` section 3 permits (``Kind``, ``Length``, ``Subtype``). + +``2 + len(data)`` agrees with that for every non-empty ``data`` -- at one octet both read 3, +at two octets both read 4 -- and disagrees only at ``data=b''``, where it declares 2 while +the schema still writes the three header octets. Hence a defect that a one-octet payload, the +maker's own default, cannot see. + +The mismatch is not inert, because the parse side derives the body width from that same +declared length: ``MPTCPUnknown.data`` is a +:class:`~pcapkit.corekit.fields.strings.BytesField` sized ``pkt['length'] - 3``. At a declared +2 that is ``-1``, and reading the segment back warns ``SchemaWarning: packet length < 0: -1`` +and then raises ``ProtocolError: Field data resolved to a negative length; template='-1s'``. +The option is therefore unreadable rather than merely mislabelled, which is why +:meth:`TCPMPTCPUnknownLengthUnitTests.test_empty_data_round_trips_through_tcp` asserts a +parse rather than only an octet count. + +Coverage +-------- + +:meth:`~TCPMPTCPUnknownLengthUnitTests.test_declared_length_matches_wire_length` is the +invariant the defect broke, asserted across payload widths rather than at ``b''`` alone, so a +fix that trades the empty case for a wide one is caught too. The remaining cases pin the +parts that must *not* move: the exact minimum octets, the non-empty widths that were already +correct, and the maker's own default. + +``data=b''`` and ``data=b'\\x00'`` pack identically, to ``1e03f0``, and both read back as +``data=b'\\x00'``: the low nibble of the header octet is always written and always read, so +an absent first octet and a zero first octet are the same three octets on the wire. That is a +property of :rfc:`8684`'s layout, not of this fix -- +:meth:`~TCPMPTCPUnknownLengthUnitTests.test_empty_and_zero_data_pack_alike` records it so a +reader does not mistake the asymmetry for the defect, and so a later change that tried to +distinguish them would have to argue with a test. + +Every case builds its own octets in memory and reads no capture under +:file:`examples/captures/`, so this belongs to the unit tier. + +""" +from __future__ import annotations + +import unittest +import warnings + +#: The smallest Multipath TCP option :rfc:`8684` section 3 permits: ``Kind`` ``0x1e``, +#: ``Length`` ``3``, then the subtype nibble ``0xf`` +#: (:attr:`~pcapkit.const.tcp.mp_tcp_option.MPTCPOption.Reserved_for_Private_Use`) beside an +#: all-zero data nibble. This is what ``data=b''`` has to produce. +MINIMUM_UNKNOWN_OPTION = bytes([0x1E, 0x03, 0xF0]) + +#: Payload widths and the octets the maker must produce for each, derived by adding up what +#: :class:`~pcapkit.protocols.schema.transport.tcp.MPTCPUnknown` packs: ``kind``, ``length``, +#: the subtype/data octet, then ``data[1:]``. Only the first entry changed with #2272; the +#: rest are pinned so the fix is shown to be confined to the empty case. +UNKNOWN_PAYLOAD_CASES = ( + (b'', bytes([0x1E, 0x03, 0xF0])), + (b'\x05', bytes([0x1E, 0x03, 0xF5])), + (b'\x05\x06', bytes([0x1E, 0x04, 0xF5, 0x06])), + (b'\x05\x06\x07', bytes([0x1E, 0x05, 0xF5, 0x06, 0x07])), +) + +#: Header fields shared by every constructed TCP segment in this module, matching +#: :data:`tests.protocols.transport.test_tcp_mptcp_subtype_unit.TCP_BASE`. +TCP_BASE = { + 'srcport': 50000, 'dstport': 80, 'seq_no': 1, 'ack_no': 0, + 'ns': False, 'cwr': False, 'ece': False, 'urg': False, 'ack': False, + 'psh': False, 'rst': False, 'syn': True, 'fin': False, + 'window': 8192, 'checksum': b'\x00\x00', 'urgent': 0, + 'payload': b'', +} + + +def make_unknown(**kwargs: 'object') -> 'bytes': + """Build an unknown-subtype MPTCP option through its maker and pack it. + + Args: + **kwargs: forwarded to + :meth:`~pcapkit.protocols.transport.tcp.TCP._make_mptcp_unknown` as the option's + own arguments. Passing none exercises the maker's default payload. + + Returns: + The packed option bytes. + + Note: + ``pcapkit`` is imported here, inside the function, rather than at module level, for + the reason + :func:`tests.protocols.transport.test_tcp_mptcp_subtype_unit.build_mptcp_option`'s own + docstring records: another module in this suite pops ``pcapkit``'s submodules out of + :data:`sys.modules`, so a name bound at collection time can end up pointing at a + schema class built from a stale ``Schema`` base. + + """ + from pcapkit.const.tcp.mp_tcp_option import MPTCPOption as Enum_MPTCPOption + from pcapkit.protocols.transport.tcp import TCP + + tcp = TCP.__new__(TCP) + schema = tcp._make_mptcp_unknown( # pylint: disable=protected-access + Enum_MPTCPOption.Reserved_for_Private_Use, **kwargs) # type: ignore[arg-type] + return schema.pack() + + +class TCPMPTCPUnknownLengthUnitTests(unittest.TestCase): + """``_make_mptcp_unknown`` writes a ``Length`` octet that counts the octets it packs.""" + + def test_declared_length_matches_wire_length(self) -> None: + """Every payload width declares its own octet count. + + The invariant #2272 broke, and the one assertion here that fails on ``main``: at + ``data=b''`` the option is 3 octets long and its ``Length`` octet reads 2. + + """ + for data, expected in UNKNOWN_PAYLOAD_CASES: + with self.subTest(data=data): + packed = make_unknown(data=data) + + self.assertEqual(len(packed), len(expected), + 'the option must occupy the octets the schema packs') + self.assertEqual(packed[1], len(packed), + 'the declared length octet must match the wire length') + + def test_packs_the_expected_octets(self) -> None: + """Each payload width packs byte-exactly what RFC 8684's layout gives. + + A declared length and a packed length can agree while both are wrong, so the octets + themselves are pinned as well as their count. + + """ + for data, expected in UNKNOWN_PAYLOAD_CASES: + with self.subTest(data=data): + self.assertEqual(make_unknown(data=data), expected) + + def test_empty_data_packs_the_minimum_option(self) -> None: + """``data=b''`` gives the 3-octet minimum of :rfc:`8684` section 3, ``1e03f0``.""" + self.assertEqual(make_unknown(data=b''), MINIMUM_UNKNOWN_OPTION) + + def test_default_payload_is_unchanged(self) -> None: + """The maker's own ``data=b'\\x00'`` default still packs ``1e03f0``. + + ``2 + len(data)`` and ``3 + len(data[1:])`` agree at one octet, so the default -- + which is what every caller who names no payload gets -- must come through #2272 + untouched. + + """ + self.assertEqual(make_unknown(), MINIMUM_UNKNOWN_OPTION) + + def test_empty_and_zero_data_pack_alike(self) -> None: + """``b''`` and ``b'\\x00'`` are the same three octets on the wire. + + Not a defect: the subtype octet's low nibble is written either way, so there is no + encoding that distinguishes them. Recorded so the surviving asymmetry is not mistaken + for the one #2272 fixed. + + """ + self.assertEqual(make_unknown(data=b''), make_unknown(data=b'\x00')) + + def test_empty_data_round_trips_through_tcp(self) -> None: + """A segment built with ``data=b''`` parses back, rather than raising. + + On ``main`` this is where the mismatch bites: ``MPTCPUnknown.data`` sizes itself + ``pkt['length'] - 3``, so a declared 2 warns ``SchemaWarning: packet length < 0: -1`` + and raises ``ProtocolError: Field data resolved to a negative length; + template='-1s'``. Asserted through the public constructor and a real parse, so a + pack-side fix that left the option unreadable would still fail. + + """ + from pcapkit.const.tcp.mp_tcp_option import MPTCPOption as Enum_MPTCPOption + from pcapkit.const.tcp.option import Option as Enum_Option + from pcapkit.protocols.transport.tcp import TCP + + tcp = TCP(options=[(Enum_Option.Multipath_TCP, { + 'subtype': Enum_MPTCPOption.Reserved_for_Private_Use, + 'data': b'', + })], **TCP_BASE) # type: ignore[arg-type] + raw = tcp.data + + # the option, then the one End-of-Option-List octet that pads it to a 4-octet word + self.assertEqual(raw[20:], MINIMUM_UNKNOWN_OPTION + b'\x00') + + with warnings.catch_warnings(): + warnings.simplefilter('error') + data = TCP(raw, len(raw)).info.options[Enum_Option.Multipath_TCP] + + self.assertEqual(data.subtype, Enum_MPTCPOption.Reserved_for_Private_Use) + self.assertEqual(data.length, 3) + self.assertEqual(data.data, b'\x00') + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/protocols/transport/test_tcp_udp_unit.py b/tests/protocols/transport/test_tcp_udp_unit.py index c0956960c5..7afe004994 100644 --- a/tests/protocols/transport/test_tcp_udp_unit.py +++ b/tests/protocols/transport/test_tcp_udp_unit.py @@ -1239,6 +1239,7 @@ def test_transport_schema_helpers_cover_selectors_and_port_fields(self) -> None: tcp_schema.mptcp_add_address_selector({'length': 9}) inner = tcp_schema.MPTCPUnknown( + kind=Option.Multipath_TCP, test={'subtype': MPTCPOption.Reserved_for_Private_Use.value, 'data': 1}, data=b'x', ) @@ -1254,7 +1255,13 @@ def test_transport_schema_helpers_cover_selectors_and_port_fields(self) -> None: {'test': {'subtype': MPTCPOption.Reserved_for_Private_Use.value}}, ) self.assertIs(processed, inner) - self.assertEqual(processed.option, Option.Multipath_TCP) + # ``post_process`` assigns only the attributes the construction path assigns too + # (#2293). ``option`` is no longer among them: it was never a declared field and + # nothing read it, and ``kind`` -- a real field, so set on both paths and the one + # actually packed -- already carries the option number. ``kind`` is passed to the + # schema above rather than asserted off ``post_process``, which does not set it. + self.assertFalse(hasattr(processed, 'option')) + self.assertEqual(processed.kind, Option.Multipath_TCP) self.assertEqual(processed.length, 3) self.assertEqual(processed.subtype, MPTCPOption.Reserved_for_Private_Use)