From c1268738fc43278f91bba2e6e6c4fa33c8c6d242 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Wed, 4 Nov 2026 12:39:23 -0500 Subject: [PATCH] fix(tcp): rebuild a TCP header with the End of Option List padding its parse read (#2769) TCP.make writes the octets after an End of Option List inside the options it builds, through _make_tcp_options, and sets no padding on the schema, so TCP.read, which took padding from that schema, read a made or rebuilt header's padding as empty while its octets were exact. TCP.read now slices the padding from the octets past the options it read, as IPv4.read does, so from_data over info or to_dict() reads what the parse read. Adds test_tcp_eool_padding_2769_unit (ALL tier): zero to three octets after EOOL, zero and non-zero, standalone and inside IPv4, from info and to_dict(), and a made header against the parse of its own octets. --- pcapkit/protocols/transport/tcp/protocol.py | 15 ++- .../test_tcp_eool_padding_2769_unit.py | 117 ++++++++++++++++++ 2 files changed, 128 insertions(+), 4 deletions(-) create mode 100644 tests/protocols/transport/test_tcp_eool_padding_2769_unit.py diff --git a/pcapkit/protocols/transport/tcp/protocol.py b/pcapkit/protocols/transport/tcp/protocol.py index 5068c54986..661586dcea 100644 --- a/pcapkit/protocols/transport/tcp/protocol.py +++ b/pcapkit/protocols/transport/tcp/protocol.py @@ -368,6 +368,10 @@ class TCP(Transport[Data_TCP, Schema_TCP], octets, which hold no kind, raise :exc:`~pcapkit.utilities.exceptions.ProtocolError` rather than an :exc:`IndexError` (:issue:`2708`). + * A header :meth:`make` builds, such as the one + :meth:`~pcapkit.protocols.protocol.Protocol.from_data` rebuilds, reads + the octets after its End of Option List as ``padding``, as a parse of + its octets does, rather than as empty (:issue:`2769`). .. versionremoved:: 1.6.0 @@ -621,11 +625,14 @@ def read(self, length: 'Optional[int]' = None, **kwargs: 'Any') -> 'Data_TCP': if _optl: # NOTE: the octets after an End of Option List option are kept as # read, so that :meth:`make` rebuilds the option area -- and the - # Data Offset computed from it -- at its original length. A schema - # built by hand rather than unpacked carries no ``padding``. + # Data Offset computed from it -- at its original length. They are + # sliced from the octets, as IPv4 does, since a schema :meth:`make` + # built holds them in ``options`` and no ``padding`` (#2769). + options = self._read_tcp_options(_optl) + _used = sum(opt.length for _, opt in options.items(multi=True)) tcp.__update__({ - 'options': self._read_tcp_options(_optl), - 'padding': schema.get('padding', b''), + 'options': options, + 'padding': self._data[TCP_MIN_HEADER_LEN + _used:tcp.hdr_len], }) return self._decode_next_layer(tcp, (tcp.srcport.port, tcp.dstport.port), length - tcp.hdr_len) diff --git a/tests/protocols/transport/test_tcp_eool_padding_2769_unit.py b/tests/protocols/transport/test_tcp_eool_padding_2769_unit.py new file mode 100644 index 0000000000..51da35c9b3 --- /dev/null +++ b/tests/protocols/transport/test_tcp_eool_padding_2769_unit.py @@ -0,0 +1,117 @@ +# -*- coding: utf-8 -*- +"""A rebuilt TCP header reads back the octets after its End of Option List. + +GitHub issue #2769: a parse keeps the octets after an End of Option List option +as ``padding``, and :meth:`TCP.make +` writes them back inside the +options, so the rebuilt octets were exact while its ``info.padding`` read as +empty: ``TCP.read`` took ``padding`` from the schema, which a made schema does +not set. It slices them from the octets instead, as ``IPv4.read`` does. Inside +IPv4 the rebuild parses its payload from the octets, so it read the padding +already; the cases there pin that it still does. + +Each option area below is one 32-bit word ending in zero to three octets after +the End of Option List (:rfc:`9293#section-3.1`), zeros and other octets alike, +and each segment carries a payload, so a padding read past the header shows. +Every case builds its own octets in memory and reads no capture. The classes are +imported inside each test, after :func:`~tests._support.reimport_once_per_class`. + +""" +# Deferring these is the design: each build runs after the per-class re-import. +# pylint: disable=import-outside-toplevel +from __future__ import annotations + +import unittest +import warnings + +from tests._support import reimport_once_per_class + +#: Test tier (#1958): hand-picked round-trip cases, not a sweep. +__tier__ = 'ALL' + +#: Ports 1234 and 80, sequence 1, acknowledgement 0; octet 12 follows. +HEAD = bytes.fromhex('04d2 0050 00000001 00000000') +#: SYN, window 1024, a zero checksum and urgent pointer. +TAIL = bytes.fromhex('02 0400 0000 0000') +#: Payload after the header. +PAYLOAD = b'PAY' +#: Octet values the padding is made of. +FILLS = (0x00, 0xa5) + + +def _cases() -> 'dict[str, tuple[bytes, bytes]]': + """Return ``name -> (segment, padding)`` for every padding length and fill.""" + cases = {} + for size in range(4): + for fill in FILLS: + padding = bytes([fill]) * size + # NOP options fill the word up to the End of Option List. + area = b'\x01' * (3 - size) + b'\x00' + padding + segment = HEAD + bytes([6 << 4]) + TAIL + area + PAYLOAD + cases[f'{size}-octet-{fill:02x}'] = (segment, padding) + return cases + + +class TestTCPEOOLPaddingRebuild(unittest.TestCase): + """Parse and rebuild agree on the octets after the End of Option List.""" + + def setUp(self) -> None: + reimport_once_per_class(self) + + def test_parse_reads_the_octets_after_eool(self) -> None: + """The specification side: Data Offset counts six words, the rest is padding.""" + from pcapkit.protocols.transport.tcp.protocol import TCP + + for name, (segment, padding) in _cases().items(): + with self.subTest(segment=name): + info = TCP(segment).info + self.assertEqual(info.hdr_len, 24) + self.assertEqual(info.padding, padding) + self.assertEqual(segment[24 - len(padding):24], padding) + + def test_standalone_rebuild_reads_the_padding(self) -> None: + """``from_data`` over ``info`` or ``to_dict()`` reads the padding its parse did.""" + from pcapkit.protocols.transport.tcp.protocol import TCP + + for name, (segment, padding) in _cases().items(): + parsed = TCP(segment) + for source in ('info', 'to_dict'): + with self.subTest(segment=name, source=source): + data = parsed.info if source == 'info' else parsed.info.to_dict() + rebuilt = TCP.from_data(data) + self.assertEqual(rebuilt.info.padding, padding) + self.assertEqual(rebuilt.data, segment) + + def test_rebuild_inside_ipv4_reads_the_padding(self) -> None: + """So does the TCP layer of an IPv4 packet rebuilt the same two ways.""" + from pcapkit.protocols.internet.ipv4.protocol import IPv4 + + for name, (segment, padding) in _cases().items(): + with warnings.catch_warnings(): + warnings.simplefilter('ignore') # the zero TCP checksum is not checked here + packet = IPv4(src='192.0.2.1', dst='192.0.2.2', protocol=6, payload=segment).data + parsed = IPv4(packet) + self.assertEqual(parsed.payload.info.padding, padding) + for source in ('info', 'to_dict'): + with self.subTest(segment=name, source=source): + data = parsed.info if source == 'info' else parsed.info.to_dict() + rebuilt = IPv4.from_data(data) + self.assertEqual(rebuilt.payload.info.padding, padding) + self.assertEqual(rebuilt.data, packet) + + def test_made_header_reads_as_its_octets_parse(self) -> None: + """``make`` with ``padding`` after a lone End of Option List, then alignment zeros.""" + from pcapkit.const.tcp.option import Option + from pcapkit.protocols.transport.tcp.protocol import TCP + + for size in range(4): + with self.subTest(size=size): + made = TCP(options=[(Option.End_of_Option_List, {})], padding=b'\xa5' * size) + expected = b'\xa5' * size + bytes(3 - size) + self.assertEqual(made.data[20:24], b'\x00' + expected) + self.assertEqual(made.info.padding, expected) + self.assertEqual(TCP(made.data).info.padding, expected) + + +if __name__ == '__main__': + unittest.main()