From c6c370fdadcb396280e38699b557eba7c30edac6 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Mon, 19 Oct 2026 20:22:45 -0400 Subject: [PATCH] fix(reassembly): bound TCP holes with a platform-independent open bound (#1959) - Add `HOLE_OPEN_BOUND` (`2 ** 63 - 1`) to `pcapkit.foundation.reassembly.data.tcp` and use it, and its negation, for the open ends of the hole descriptor list instead of `sys.maxsize`, which is `2 ** 31 - 1` on GraalPy and 32-bit builds and so falls inside the TCP sequence space - Document the constant on `HoleDescriptor` and the TCP reassembly docs page - Pin the existing hole-list tests to the constant, and add regression tests that patch `sys.maxsize` to `2 ** 31 - 1` with sequence numbers above `2 ** 31` Closes #1959 --- .../pcapkit/foundation/reassembly/tcp.rst | 5 +- pcapkit/foundation/reassembly/data/tcp.py | 21 +++++- pcapkit/foundation/reassembly/tcp.py | 13 ++-- tests/foundation/reassembly/test_tcp.py | 66 +++++++++++++++++-- 4 files changed, 88 insertions(+), 17 deletions(-) diff --git a/docs/source/pcapkit/foundation/reassembly/tcp.rst b/docs/source/pcapkit/foundation/reassembly/tcp.rst index 83b5ec2eba..ccb8859b5b 100644 --- a/docs/source/pcapkit/foundation/reassembly/tcp.rst +++ b/docs/source/pcapkit/foundation/reassembly/tcp.rst @@ -81,7 +81,8 @@ Algorithm ISN <- PSN; put data from fragment into data buffer with BUFID [from octet fragment.first to octet fragment.last]; - HDL <- [one hole from PSN + fragment.len to infinity]; + HDL <- [one hole from PSN + fragment.len to infinity, + i.e. HOLE_OPEN_BOUND]; } ELSE { put data from fragment into data buffer with BUFID [from octet fragment.first to octet fragment.last]; @@ -360,6 +361,8 @@ Data Models :members: :show-inheritance: +.. autodata:: pcapkit.foundation.reassembly.data.tcp.HOLE_OPEN_BOUND + .. autoclass:: pcapkit.foundation.reassembly.data.tcp.Fragment :members: :show-inheritance: diff --git a/pcapkit/foundation/reassembly/data/tcp.py b/pcapkit/foundation/reassembly/data/tcp.py index 104f402ea9..b7dccc133c 100644 --- a/pcapkit/foundation/reassembly/data/tcp.py +++ b/pcapkit/foundation/reassembly/data/tcp.py @@ -10,7 +10,7 @@ __all__ = [ 'Packet', 'DatagramID', 'Datagram', 'HoleDescriptor', - 'Fragment', 'Buffer', 'BufferID', + 'Fragment', 'Buffer', 'BufferID', 'HOLE_OPEN_BOUND', ] if TYPE_CHECKING: @@ -26,6 +26,20 @@ #: Buffer ID. BufferID: 'TypeAlias' = Tuple[_AT, int, _AT, int] +#: Open bound of the :class:`HoleDescriptor` list. A hole whose ``last`` is +#: this value is still open above, and one whose ``first`` is its negation +#: stands for whatever preceded a capture that missed the handshake. +#: +#: It is ``2 ** 63 - 1`` on every platform, not :data:`sys.maxsize`, which is +#: ``2 ** 31 - 1`` on GraalPy and 32-bit builds and so falls inside the +#: ``2 ** 32`` sequence space. The unwrapped sequence line reaches it only +#: after ``2 ** 31`` wraps. It stays an :class:`int`, like the descriptor's +#: own fields, and in both signs fits a standard JSON number and a +#: :mod:`plistlib` integer, which :data:`math.inf` does not. +#: +#: .. versionadded:: 1.5.0 +HOLE_OPEN_BOUND = (1 << 63) - 1 + @info_final class Packet(Info): @@ -147,8 +161,9 @@ class HoleDescriptor(Info): :attr:`Fragment.raw`: the descriptor list is kept once per buffer ID, across every acknowledgement number's payload buffer, and describes the stream as a whole. It decides when a stream whose FIN has been seen is - whole; a hole whose ``first`` is ``-sys.maxsize`` stands for whatever - preceded a capture that missed the handshake, and does not count. + whole. A hole whose ``first`` is ``-HOLE_OPEN_BOUND`` stands for whatever + preceded a capture that missed the handshake, and does not count; one + whose ``last`` is :data:`HOLE_OPEN_BOUND` is still open above. .. versionadded:: 1.5.0 diff --git a/pcapkit/foundation/reassembly/tcp.py b/pcapkit/foundation/reassembly/tcp.py index 23351b0718..64533a0138 100644 --- a/pcapkit/foundation/reassembly/tcp.py +++ b/pcapkit/foundation/reassembly/tcp.py @@ -10,12 +10,11 @@ """ import math -import sys from typing import TYPE_CHECKING from pcapkit.foundation.reassembly.data.data import Completion, Deferred -from pcapkit.foundation.reassembly.data.tcp import (Buffer, BufferID, Datagram, DatagramID, - Fragment, HoleDescriptor, Packet) +from pcapkit.foundation.reassembly.data.tcp import (HOLE_OPEN_BOUND, Buffer, BufferID, Datagram, + DatagramID, Fragment, HoleDescriptor, Packet) from pcapkit.foundation.reassembly.reassembly import ReassemblyBase from pcapkit.protocols.transport.tcp import TCP as TCP_Protocol @@ -237,7 +236,7 @@ def reassembly(self, info: 'Packet') -> 'None': # everything from the octet after this segment onwards is # still missing first=PSN + info.len, - last=sys.maxsize, + last=HOLE_OPEN_BOUND, ), ] if not ANCHORED: @@ -246,7 +245,7 @@ def reassembly(self, info: 'Packet') -> 'None': # marks this one hole as "before the capture began" rather than # as data missing from the stream; it is what lets a segment # arriving *below* this one leave a real hole between the two. - hdl.insert(0, HoleDescriptor(first=-sys.maxsize, last=PSN - 1)) + hdl.insert(0, HoleDescriptor(first=-HOLE_OPEN_BOUND, last=PSN - 1)) self._buffer[BUFID] = Buffer( hdl=hdl, hdr=HDR, @@ -351,7 +350,7 @@ def _anchored(buf: 'Buffer') -> 'bool': with the sentinel hole that a buffer begun without a SYN carries. """ - return not (buf.hdl and buf.hdl[0].first == -sys.maxsize) + return not (buf.hdl and buf.hdl[0].first == -HOLE_OPEN_BOUND) def _stream_whole(self, BUFID: 'BufferID') -> 'bool': """Whether a stream has seen its FIN and every octet below it. @@ -366,7 +365,7 @@ def _stream_whole(self, BUFID: 'BufferID') -> 'bool': """ for hole in self._buffer[BUFID].hdl: - if hole.first != -sys.maxsize or hole.last == sys.maxsize: + if hole.first != -HOLE_OPEN_BOUND or hole.last == HOLE_OPEN_BOUND: return False return True diff --git a/tests/foundation/reassembly/test_tcp.py b/tests/foundation/reassembly/test_tcp.py index 3e340b8d56..c93889cbc8 100644 --- a/tests/foundation/reassembly/test_tcp.py +++ b/tests/foundation/reassembly/test_tcp.py @@ -101,7 +101,8 @@ class TestTCP(TCP): self.assertEqual(reasm._buffer[self._bufid()].hdr, b'new-header') def test_fragment_merging_covers_gaps_overlaps_new_acks_and_holes(self) -> None: - from pcapkit.foundation.reassembly.data.tcp import Buffer, Fragment, HoleDescriptor + from pcapkit.foundation.reassembly.data.tcp import (HOLE_OPEN_BOUND, Buffer, Fragment, + HoleDescriptor) from pcapkit.foundation.reassembly.tcp import TCP class Analyzer: @@ -115,7 +116,7 @@ class TestTCP(TCP): reasm = TestTCP() bufid = self._bufid() reasm._buffer[bufid] = Buffer( - [HoleDescriptor(0, 4), HoleDescriptor(20, 30), HoleDescriptor(40, sys.maxsize)], + [HoleDescriptor(0, 4), HoleDescriptor(20, 30), HoleDescriptor(40, HOLE_OPEN_BOUND)], b'', { 500: Fragment([1], 10, 10, bytearray(b'0123456789'), [], []), @@ -146,14 +147,14 @@ class TestTCP(TCP): bytearray(b'abcdefghij0123456789AP\x00\x00\x00after-gap')) self.assertEqual(reasm._buffer[bufid].ack[500].conflict, [(15, 19), (10, 12)]) self.assertEqual([(hole.first, hole.last) for hole in reasm._buffer[bufid].hdl], - [(0, 4), (25, 30), (40, sys.maxsize)]) + [(0, 4), (25, 30), (40, HOLE_OPEN_BOUND)]) reasm(self._packet(num=5, dsn=0, ack=501, payload=b'alt', first=50, last=52)) self.assertEqual(reasm._buffer[bufid].ack[501].raw, bytearray(b'alt')) before_gap = TestTCP() before_gap._buffer[bufid] = Buffer( - [HoleDescriptor(50, sys.maxsize)], + [HoleDescriptor(50, HOLE_OPEN_BOUND)], b'', {500: Fragment([1], 10, 5, bytearray(b'world'), [], [])}, 1000.0, @@ -518,12 +519,14 @@ def test_hole_descriptor_bounds_are_absolute_sequence_numbers(self) -> None: sequence numbers, so one descriptor held one of each. """ + from pcapkit.foundation.reassembly.data.tcp import HOLE_OPEN_BOUND + packets = self._stream(segments=[(0, b'A' * 10), (20, b'C' * 10)], teardown=None) reasm = self._run(packets) buffer = reasm._buffer[self._bufid()] self.assertEqual([(hole.first, hole.last) for hole in buffer.hdl], - [(ISN + 11, ISN + 20), (ISN + 31, sys.maxsize)]) + [(ISN + 11, ISN + 20), (ISN + 31, HOLE_OPEN_BOUND)]) # the SYN acknowledges nothing, so it gets a payload buffer of its own # and the data lands in the one keyed by the data segments' ACK @@ -548,6 +551,8 @@ def test_payload_free_segments_leave_the_hole_list_alone(self) -> None: long-lived connection, for no change in what is missing. """ + from pcapkit.foundation.reassembly.data.tcp import HOLE_OPEN_BOUND + base = ISN + 1 reasm = self._run([ self._segment(num=1, seq=ISN, syn=True, ack=0), @@ -556,7 +561,7 @@ def test_payload_free_segments_leave_the_hole_list_alone(self) -> None: ]) before = [(hole.first, hole.last) for hole in reasm._buffer[self._bufid()].hdl] - self.assertEqual(before, [(base + 10, base + 19), (base + 30, sys.maxsize)]) + self.assertEqual(before, [(base + 10, base + 19), (base + 30, HOLE_OPEN_BOUND)]) # one past the data, one *inside* the hole, and one repeated for (num, seq) in ((4, base + 30), (5, base + 15), (6, base + 30)): @@ -566,6 +571,55 @@ def test_payload_free_segments_leave_the_hole_list_alone(self) -> None: self.assertEqual(after, before) self.assertEqual(len(reasm._buffer[self._bufid()].ack[1000].raw), 30) + ########################################################################## + # The open bound of the hole list does not depend on the platform. + ########################################################################## + + def test_open_hole_bound_is_not_sys_maxsize(self) -> None: + """Holes stay right above ``2 ** 31`` when :data:`sys.maxsize` is ``2 ** 31 - 1``. + + That is :data:`sys.maxsize` on GraalPy and on 32-bit builds, and it lies + inside the ``2 ** 32`` sequence space. Used as the open bound, it left + the open-ended hole with ``last`` below its own ``first`` once sequence + numbers passed ``2 ** 31`` (#1959). + + """ + from unittest import mock + + base = ISN + 1 + self.assertGreater(base, 2 ** 31) + with mock.patch.object(sys, 'maxsize', 2 ** 31 - 1): + reasm = self._run(self._stream(segments=[(0, b'A' * 10), (20, b'C' * 10)], + teardown=None)) + holes = [(hole.first, hole.last) for hole in reasm._buffer[self._bufid()].hdl] + + self.assertEqual(len(holes), 2, holes) + self.assertEqual(holes[0], (base + 10, base + 19)) + self.assertEqual(holes[1][0], base + 30) + self.assertGreater(holes[1][1], 0xFFFFFFFF) + + def test_fin_completes_a_stream_above_2_31_whatever_sys_maxsize(self) -> None: + """A whole stream is submitted at its FIN, with or without its handshake. + + With :data:`sys.maxsize` at ``2 ** 31 - 1`` the open-ended hole above a + stream past ``2 ** 31`` survived the FIN, so the stream never counted as + whole and was held until the capture ended (#1959). + + """ + from unittest import mock + + from pcapkit.foundation.reassembly.data.data import Completion + + for syn in (True, False): + with self.subTest(syn=syn), mock.patch.object(sys, 'maxsize', 2 ** 31 - 1): + reasm = self._run(self._stream(segments=[(0, b'A' * 10), (10, b'B' * 10)], + syn=syn)) + + self.assertNotIn(self._bufid(), reasm._buffer) + datagram, = reasm._dtgram + self.assertIs(datagram.completed, Completion.COMPLETE) + self.assertEqual(datagram.payload, b'A' * 10 + b'B' * 10) + # The end-to-end case on a generated capture lives in # tests/foundation/reassembly/test_tcp_runtime.py, since the unit-test # workflow runs without the generated fixtures.