From b9cb36549ecec316d9b940909e3e2640ad50671e Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Wed, 16 Sep 2026 17:06:14 -0400 Subject: [PATCH 1/8] reassembly: make the four IPv6 adapters agree, and stop advertising IPv6-Frag (#415) The four toolkit adapters disagreed about whether the 8-octet IPv6 Fragment header belongs to a reassembly packet's `ihl`, `header` and `tl`. On `examples/captures/ipv6.pcap` frame 13, `(ihl, len(header), tl)` read `(48, 48, 1496)` for `pcap` and `pcapng`, `(40, 40, 1488)` for `dpkt` and `(40, 40, 1496)` for `scapy` -- no two adapters agreeing on all three, and a three-way split on `tl` alone. RFC 8200 section 4.5 settles it: the Fragment header is not present in the reassembled packet, so none of the three may count it. All four now report `(40, 40, 1488)`. * `toolkit/pcap.py`, `toolkit/pcapng.py`: `ihl` and `header` were `ipv6_info.hdr_len` and the whole of `ipv6_info.fragment.header`, both of which count the Fragment header, because `ipv6.py` adds each extension header's length before the Fragment-header check breaks its loop. Subtract the Fragment header's own length back off, undoing exactly that addition. `hdr_len` keeps its documented meaning. * `toolkit/scapy.py`: `tl` was `len(ipv6)`, which counts the Fragment header while its `ihl` did not. The reassembly machinery writes each fragment's payload over the span `tl - ihl`, so that overstated it by 8 and the reassembled datagram came out 4786 octets instead of 4778 -- eight stray zeroes per fragment. Derive `tl` from the payload handed over instead, which is what makes the invariant structural rather than a coincidence. * `toolkit/dpkt.py`: already correct on all three fields; only its `header` comment, which called a `bytes` value a `bytearray`, needed fixing. The `# header length, only headers before IPv6-Frag` comment repeated in all four was already true here and in `scapy`, and is now true in the other two. * `reassembly/ip.py`, `reassembly/ipv6.py`: excluding the Fragment header's octets still left the field *pointing* at it, so every engine reassembled a datagram whose `header[6]` was 44. RFC 8200 section 4.5 also moves the Fragment header's Next Header value into the last header of the unfragmentable part, so `IP._rectify_header` is a hook the IPv6 subclass overrides to do that once for all four engines. It walks the extension header chain rather than assuming offset 6, since Hop-by-Hop, Routing and Destination Options headers may precede the Fragment header. Deliberately not changed: the reassembled header's Payload Length field still describes the first fragment. It cannot be computed from one fragment, and `len(payload)` already gives the datagram's length; the docs now say so. Tests: `tests/integration/test_reassembly_engine_parity.py` runs all four adapters over the same octets -- the frames of `ipv6.pcap`, rewrapped as PCAP-NG for the adapter with no fragmented sample of its own -- and asserts they agree; on the pristine tree it fails with the table above. `tests/foundation/reassembly/test_ipv6.py` covers the chain walk, including the Authentication Header's different length encoding. `tests/toolkit/test_pcap_unit.py` was pinning the old behaviour: it asserted `v6.header == b'V' * 48`, i.e. a header with the Fragment header still in it, for both the PCAP and PCAP-NG adapters. Updated to 40, with `ihl`, `tl` and the `tl - ihl == len(payload)` invariant pinned alongside; its IPv6 fake gained the `length` attribute the real `IPv6_Frag` has and its `raw_len` now agrees with its own payload. 822 passed, 17 skipped. mypy 124 errors either side, pylint message multiset identical (bar `cyclic-import`, which is non-deterministic run to run: 115 then 124 on two consecutive runs of the unchanged tree). --- .../pcapkit/foundation/reassembly/ip/ipv6.rst | 43 ++- pcapkit/foundation/reassembly/ip.py | 26 +- pcapkit/foundation/reassembly/ipv6.py | 93 ++++++ pcapkit/toolkit/dpkt.py | 2 +- pcapkit/toolkit/pcap.py | 23 +- pcapkit/toolkit/pcapng.py | 23 +- pcapkit/toolkit/scapy.py | 19 +- tests/foundation/reassembly/test_ipv6.py | 234 +++++++++++++ .../test_reassembly_engine_parity.py | 311 ++++++++++++++++++ tests/toolkit/test_pcap_unit.py | 27 +- 10 files changed, 767 insertions(+), 34 deletions(-) create mode 100644 tests/foundation/reassembly/test_ipv6.py create mode 100644 tests/integration/test_reassembly_engine_parity.py diff --git a/docs/source/pcapkit/foundation/reassembly/ip/ipv6.rst b/docs/source/pcapkit/foundation/reassembly/ip/ipv6.rst index d7e11ca34e..235bc3a53c 100644 --- a/docs/source/pcapkit/foundation/reassembly/ip/ipv6.rst +++ b/docs/source/pcapkit/foundation/reassembly/ip/ipv6.rst @@ -29,6 +29,9 @@ Terminology .. code-block:: python + hdr_len = ipv6_info.hdr_len - ipv6_frag.length + payload = bytearray(ipv6_info.fragment.payload) + packet_dict = dict( bufid = ( ipv6_info.src, # source IP address @@ -38,27 +41,24 @@ Terminology ), num = frame.info.number, # original packet range number fo = ipv6_frag_info.offset, # fragment offset, in octets - ihl = ipv6_info.hdr_len, # header length, IPv6-Frag included + ihl = hdr_len, # header length, only headers before IPv6-Frag mf = ipv6_frag_info.mf, # more fragment flag - tl = ipv6_info.hdr_len - + ipv6_info.raw_len, # total length, header includes + tl = hdr_len + len(payload), # total length, header includes header = ipv6_info.fragment - .header, # raw bytes type header, IPv6-Frag included - payload = bytearray( - ipv6_info.fragment - .payload), # raw bytearray type payload after IPv6-Frag + .header[:hdr_len], # raw bytes type header before IPv6-Frag + payload = payload, # raw bytearray type payload after IPv6-Frag ) - .. warning:: + .. note:: - ``ihl`` and ``header`` here **include** the 8-octet Fragment header, - because :attr:`IPv6.hdr_len ` - counts every extension header it has walked, the Fragment one included. - The ``dpkt`` and ``scapy`` adapters stop short of it and report 40 where - this one reports 48 for the same packet, so the value is not comparable - across engines -- and :rfc:`8200#section-4.5` says the Fragment header - is not present in a reassembled packet at all. Tracked as #415; expect - this line to change when that is fixed. + ``ihl``, ``header`` and ``tl`` all stop short of the 8-octet Fragment + header, because :rfc:`8200#section-4.5` says it is not present in the + reassembled packet. :attr:`IPv6.hdr_len ` + does count it -- it is a header length, and the Fragment header is one + of the extension headers it has walked -- so the adapters subtract it + back off. All four adapters (``pcap``, ``pcapng``, ``dpkt`` and + ``scapy``) agree on the three fields; they used to report three + different values for ``tl`` alone, which is what #415 was about. .. note:: @@ -104,6 +104,17 @@ Terminology | |--> 'packet' : (None) |--> (Info) data ... + .. note:: + + ``header`` is the fragment's unfragmentable part with one field + rewritten: the Next Header field of its last header carries the + Fragment header's Next Header value, as :rfc:`8200#section-4.5` + requires of a reassembled packet. Without that rewrite the datagram + would still advertise a Fragment header (``44``) on a datagram that is + no longer a fragment. The Payload Length field is *not* adjusted, so it + still describes the first fragment rather than the reassembled + datagram; use ``len(payload)`` instead. + reasm.ipv6.buffer Data structure for internal buffering when performing reassembly algorithms (:attr:`IPv6._buffer `) diff --git a/pcapkit/foundation/reassembly/ip.py b/pcapkit/foundation/reassembly/ip.py index da4925fe3b..996ee58593 100644 --- a/pcapkit/foundation/reassembly/ip.py +++ b/pcapkit/foundation/reassembly/ip.py @@ -52,6 +52,25 @@ class IP(Reassembly[Packet[_AT], Datagram[_AT], BufferID, Buffer[_AT]], Generic[ # Methods. ########################################################################## + def _rectify_header(self, header: 'bytes', proto: 'TransType') -> 'bytes': # pylint: disable=unused-argument + """Adapt a fragment's header into the reassembled datagram's header. + + The base implementation returns ``header`` unchanged, which is what IPv4 + wants: an IPv4 fragment's header needs nothing removed to describe the + datagram it belongs to. IPv6 overrides this, because + :rfc:`8200#section-4.5` drops the Fragment header from the reassembled + packet and hands its Next Header field to the header before it. + + Args: + header: Raw header octets of the fragment at fragment offset zero. + proto: Payload protocol type, i.e. ``bufid[3]``. + + Returns: + Header octets to keep for the reassembled datagram. + + """ + return header + def reassembly(self, info: 'Packet[_AT]') -> 'None': """Reassembly procedure. @@ -77,19 +96,22 @@ def reassembly(self, info: 'Packet[_AT]') -> 'None': ) return + # the header of the fragment at offset zero is the reassembled datagram's + header = b'' if FO else self._rectify_header(info.header, BUFID[3]) + # initialise buffer with BUFID if BUFID not in self._buffer: self._buffer[BUFID] = Buffer( TDL=-1, # Total Data Length RCVBT=bytearray(8191), # Fragment Received Bit Table index=[], # index record - header=b'' if FO else info.header, # header buffer + header=header, # header buffer datagram=bytearray(65535), # data buffer ) else: # put header into header buffer if not FO: # pylint: disable=else-if-used - self._buffer[BUFID].__update__(header=info.header) + self._buffer[BUFID].__update__(header=header) # append packet index self._buffer[BUFID].index.append(info.num) diff --git a/pcapkit/foundation/reassembly/ipv6.py b/pcapkit/foundation/reassembly/ipv6.py index c859eedfba..f054f97a3d 100644 --- a/pcapkit/foundation/reassembly/ipv6.py +++ b/pcapkit/foundation/reassembly/ipv6.py @@ -10,11 +10,66 @@ origin. Please refer to :doc:`ip` for more information. """ +from typing import TYPE_CHECKING + from pcapkit.foundation.reassembly.ip import IP from pcapkit.protocols.internet.ipv6 import IPv6 as IPv6_Protocol +if TYPE_CHECKING: + from pcapkit.const.reg.transtype import TransType + __all__ = ['IPv6'] +#: Length of the fixed IPv6 header, i.e. the offset of the first extension +#: header (:rfc:`8200#section-3`). +_IPV6_HDR_LEN = 40 + +#: Offset of the Next Header field within the fixed IPv6 header. +_IPV6_NEXT_HEADER = 6 + +#: Next Header value of the IPv6 Fragment header (:rfc:`8200#section-4.5`). +_NH_IPV6_FRAG = 44 + +#: Next Header value of the Authentication Header. It is the one extension +#: header that does not measure its length in 8-octet units +#: (:rfc:`4302#section-2.2`), so the header walk below has to special-case it. +_NH_AH = 51 + + +def _next_header_offset(header: 'bytes') -> 'int': + """Locate the Next Header field of a datagram's last header. + + :rfc:`8200#section-4.5` gives the Next Header field of the *last* header of + the unfragmentable part -- not necessarily the fixed IPv6 header's, since + Hop-by-Hop Options, Routing and Destination Options headers may precede the + Fragment header. Each of those starts with its own Next Header field, so the + answer is found by walking the chain to its end. + + Args: + header: Unfragmentable part of a datagram, i.e. every octet of the + fragment before its Fragment header. + + Returns: + Offset, within ``header``, of the Next Header field to rewrite. + + """ + offset = _IPV6_NEXT_HEADER + position = _IPV6_HDR_LEN + proto = header[offset] + + # NOTE: ``header[position]`` is the Next Header field of the extension + # header starting at ``position`` and ``header[position + 1]`` its length, + # but which of the two length encodings applies is decided by the *previous* + # header's Next Header value -- hence ``proto`` trailing one step behind. + while position + 1 < len(header): + offset = position + if proto == _NH_AH: + position += (header[position + 1] + 2) * 4 + else: + position += (header[position + 1] + 1) * 8 + proto = header[offset] + return offset + # BUG: It is supposed to be ``IP[IPv6Address]``. But somehow Python # thinks that ``IP`` should take 4 arguments as in its parent class @@ -48,3 +103,41 @@ class IPv6(IP): __protocol_name__ = 'IPv6' #: Protocol of current reassembly object. __protocol_type__ = IPv6_Protocol + + ########################################################################## + # Methods. + ########################################################################## + + def _rectify_header(self, header: 'bytes', proto: 'TransType') -> 'bytes': + """Remove the Fragment header from a datagram's header chain. + + :rfc:`8200#section-4.5` states that the Fragment header is not present in + the reassembled packet, and that the Next Header field of the last header + of the unfragmentable part comes from the Fragment header's. Left alone, + the reassembled datagram advertises a Fragment header on a datagram that + is by definition no longer a fragment, which is what every engine used to + report -- the toolkit adapters differ over whether the Fragment header's + *octets* belong to ``header``, but none of them rewrote the field that + points at it. + + The Fragment header's own octets are already excluded by the adapters, so + only the field pointing at it is left to fix. + + Args: + header: Raw header octets of the fragment at fragment offset zero. + proto: Payload protocol type, i.e. the Fragment header's Next Header + field, which is what the rewritten field must carry. + + Returns: + Header octets to keep for the reassembled datagram. + + """ + # a header too short to hold the fixed IPv6 header cannot be walked, and + # a chain not ending in the Fragment header has nothing to rewrite -- + # which also makes this idempotent + if len(header) < _IPV6_HDR_LEN: + return header + offset = _next_header_offset(header) + if header[offset] != _NH_IPV6_FRAG: + return header + return header[:offset] + bytes((int(proto),)) + header[offset + 1:] diff --git a/pcapkit/toolkit/dpkt.py b/pcapkit/toolkit/dpkt.py index 793f17b1cc..d0c5cfdbd1 100644 --- a/pcapkit/toolkit/dpkt.py +++ b/pcapkit/toolkit/dpkt.py @@ -215,7 +215,7 @@ def ipv6_reassembly(packet: 'Packet', *, count: 'int' = -1) -> 'IP_Packet[IPv6Ad ihl=hdr_len, # header length, only headers before IPv6-Frag mf=bool(ipv6_frag.m_flag), # more fragment flag tl=hdr_len + len(payload), # total length, header includes - header=ipv6.pack()[:hdr_len], # raw bytearray type header before IPv6-Frag + header=ipv6.pack()[:hdr_len], # raw bytes type header before IPv6-Frag payload=bytearray(payload), # raw bytearray type payload after IPv6-Frag ) return data diff --git a/pcapkit/toolkit/pcap.py b/pcapkit/toolkit/pcap.py index c3bacdf5cd..4e30a35677 100644 --- a/pcapkit/toolkit/pcap.py +++ b/pcapkit/toolkit/pcap.py @@ -104,6 +104,21 @@ def ipv6_reassembly(frame: 'Frame') -> 'IP_Packet[IPv6Address] | None': return None ipv6_frag_info = cast('IPv6_Frag', ipv6_frag).info + # NOTE: ``Data_IPv6.hdr_len`` counts the Fragment header, since + # :meth:`IPv6._decode_next_layer ` + # adds each extension header's length before the Fragment-header check + # breaks its loop. That is correct for a header length, but the + # reassembled packet carries no Fragment header at all + # (:rfc:`8200#section-4.5`), so the boundary wanted here is the one + # *before* it -- which is that same length less the Fragment header's, + # undoing the addition that included it. + hdr_len = ipv6_info.hdr_len - cast('IPv6_Frag', ipv6_frag).length + # NOTE: The reassembly machinery writes the payload into its datagram + # buffer over the span ``tl - ihl``, so ``tl`` is derived from the + # payload actually handed over rather than from ``raw_len``: the two + # agree, but only the former cannot drift out of step with it. + payload = bytearray(ipv6_info.fragment.payload) + data = IP_Packet( bufid=( ipv6_info.src, # source IP address @@ -113,11 +128,11 @@ def ipv6_reassembly(frame: 'Frame') -> 'IP_Packet[IPv6Address] | None': ), num=frame.info.number, # original packet range number fo=ipv6_frag_info.offset, # fragment offset - ihl=ipv6_info.hdr_len, # header length, only headers before IPv6-Frag + ihl=hdr_len, # header length, only headers before IPv6-Frag mf=ipv6_frag_info.mf, # more fragment flag - tl=ipv6_info.hdr_len + ipv6_info.raw_len, # total length, header includes - header=ipv6_info.fragment.header, # raw bytearray type header before IPv6-Frag - payload=bytearray(ipv6_info.fragment.payload), # raw bytearray type payload after IPv6-Frag + tl=hdr_len + len(payload), # total length, header includes + header=ipv6_info.fragment.header[:hdr_len], # raw bytes type header before IPv6-Frag + payload=payload, # raw bytearray type payload after IPv6-Frag ) return data return None diff --git a/pcapkit/toolkit/pcapng.py b/pcapkit/toolkit/pcapng.py index 04c2598f0a..9050e11eb1 100644 --- a/pcapkit/toolkit/pcapng.py +++ b/pcapkit/toolkit/pcapng.py @@ -107,6 +107,21 @@ def ipv6_reassembly(frame: 'PCAPNG') -> 'IP_Packet[IPv6Address] | None': return None ipv6_frag_info = cast('IPv6_Frag', ipv6_frag).info + # NOTE: ``Data_IPv6.hdr_len`` counts the Fragment header, since + # :meth:`IPv6._decode_next_layer ` + # adds each extension header's length before the Fragment-header check + # breaks its loop. That is correct for a header length, but the + # reassembled packet carries no Fragment header at all + # (:rfc:`8200#section-4.5`), so the boundary wanted here is the one + # *before* it -- which is that same length less the Fragment header's, + # undoing the addition that included it. + hdr_len = ipv6_info.hdr_len - cast('IPv6_Frag', ipv6_frag).length + # NOTE: The reassembly machinery writes the payload into its datagram + # buffer over the span ``tl - ihl``, so ``tl`` is derived from the + # payload actually handed over rather than from ``raw_len``: the two + # agree, but only the former cannot drift out of step with it. + payload = bytearray(ipv6_info.fragment.payload) + frame_info = cast('Packet', frame.info) data = IP_Packet( bufid=( @@ -117,11 +132,11 @@ def ipv6_reassembly(frame: 'PCAPNG') -> 'IP_Packet[IPv6Address] | None': ), num=frame_info.number, # original packet range number fo=ipv6_frag_info.offset, # fragment offset - ihl=ipv6_info.hdr_len, # header length, only headers before IPv6-Frag + ihl=hdr_len, # header length, only headers before IPv6-Frag mf=ipv6_frag_info.mf, # more fragment flag - tl=ipv6_info.hdr_len + ipv6_info.raw_len, # total length, header includes - header=ipv6_info.fragment.header, # raw bytearray type header before IPv6-Frag - payload=bytearray(ipv6_info.fragment.payload), # raw bytearray type payload after IPv6-Frag + tl=hdr_len + len(payload), # total length, header includes + header=ipv6_info.fragment.header[:hdr_len], # raw bytes type header before IPv6-Frag + payload=payload, # raw bytearray type payload after IPv6-Frag ) return data return None diff --git a/pcapkit/toolkit/scapy.py b/pcapkit/toolkit/scapy.py index 3d79007647..02b2dbb417 100644 --- a/pcapkit/toolkit/scapy.py +++ b/pcapkit/toolkit/scapy.py @@ -202,6 +202,13 @@ def ipv6_reassembly(packet: 'Packet', *, count: 'int' = -1) -> 'IP_Packet[IPv6Ad return None # dismiss not fragmented packet ipv6_frag = cast('IPv6ExtHdrFragment', ipv6['IPv6ExtHdrFragment']) + # NOTE: ``len()`` of a Scapy layer spans that layer and everything after + # it, so the difference is the unfragmentable part -- every octet before + # the Fragment header, which is the only part of the header the + # reassembled packet keeps (:rfc:`8200#section-4.5`). + hdr_len = len(ipv6) - len(ipv6_frag) + payload = bytearray(bytes(ipv6_frag.payload)) + data = IP_Packet( bufid=( cast('IPv6Address', @@ -216,11 +223,15 @@ def ipv6_reassembly(packet: 'Packet', *, count: 'int' = -1) -> 'IP_Packet[IPv6Ad # units (:rfc:`8200#section-4.5`), but the reassembly machinery indexes # the datagram buffer with ``fo``, so it must be scaled into octets. fo=ipv6_frag.offset * 8, # fragment offset - ihl=len(ipv6) - len(ipv6_frag), # header length, only headers before IPv6-Frag + ihl=hdr_len, # header length, only headers before IPv6-Frag mf=bool(ipv6_frag.m), # more fragment flag - tl=len(ipv6), # total length, header includes - header=bytes(ipv6)[:-len(ipv6_frag)], # raw bytes type header before IPv6-Frag - payload=bytearray(bytes(ipv6_frag.payload)), # raw bytearray type payload after IPv6-Frag + # NOTE: ``len(ipv6)`` counts the Fragment header, so it overstates + # this by 8 -- and the reassembly machinery writes the payload over + # the span ``tl - ihl``, so those 8 octets became 8 octets of stray + # zeroes in every reassembled datagram. + tl=hdr_len + len(payload), # total length, header includes + header=bytes(ipv6)[:hdr_len], # raw bytes type header before IPv6-Frag + payload=payload, # raw bytearray type payload after IPv6-Frag ) return data return None diff --git a/tests/foundation/reassembly/test_ipv6.py b/tests/foundation/reassembly/test_ipv6.py new file mode 100644 index 0000000000..a6a13504b1 --- /dev/null +++ b/tests/foundation/reassembly/test_ipv6.py @@ -0,0 +1,234 @@ +from __future__ import annotations + +import importlib.util +from ipaddress import ip_address +import struct +import unittest + +from tests._support import purge_modules + +RUNTIME_DEPS = ('tbtrim', 'aenum', 'chardet', 'dictdumper') +HAS_RUNTIME = all(importlib.util.find_spec(name) is not None for name in RUNTIME_DEPS) + +#: Next Header values used below (:rfc:`8200#section-4.1`). +NH_HOPOPT = 0 +NH_ROUTING = 43 +NH_IPV6_FRAG = 44 +NH_AH = 51 +NH_DSTOPT = 60 +NH_UDP = 17 + + +def ipv6_header(next_header: int, payload_len: int = 0) -> bytes: + """The 40 octet fixed IPv6 header, with ``next_header`` at offset 6.""" + return (struct.pack('>IHBB', 6 << 28, payload_len, next_header, 64) + + ip_address('2001:db8::1').packed + + ip_address('2001:db8::2').packed) + + +def ext_header(next_header: int, octets: int) -> bytes: + """A Hop-by-Hop/Routing/Destination Options style extension header. + + Args: + next_header: Value of the header's Next Header field. + octets: Total length of the header, which must be a positive multiple of + 8 -- the Hdr Ext Len field counts 8-octet units beyond the first + (:rfc:`8200#section-4.3`). + + Returns: + ``octets`` octets of extension header, padded with zeroes. + + """ + assert octets >= 8 and octets % 8 == 0 + return bytes((next_header, octets // 8 - 1)) + b'\x00' * (octets - 2) + + +def ah_header(next_header: int, octets: int) -> bytes: + """An Authentication Header, whose Payload Len counts 4-octet units less two. + + Args: + next_header: Value of the header's Next Header field. + octets: Total length of the header, a positive multiple of 4 + (:rfc:`4302#section-2.2`). + + Returns: + ``octets`` octets of Authentication Header, padded with zeroes. + + """ + assert octets >= 12 and octets % 4 == 0 + return bytes((next_header, octets // 4 - 2)) + b'\x00' * (octets - 2) + + +@unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed') +class NextHeaderOffsetTests(unittest.TestCase): + """Where the Next Header field of a datagram's last header lives. + + :rfc:`8200#section-4.5` hands the Fragment header's Next Header value to *the + last header of the unfragmentable part*, which is only the fixed IPv6 header + when nothing precedes the Fragment header. A Hop-by-Hop Options, Routing or + Destination Options header may, and then the field to rewrite is that + header's -- so the chain has to be walked rather than assumed. + + """ + + def setUp(self) -> None: + purge_modules(['pcapkit']) + + def test_a_bare_ipv6_header_answers_with_its_own_field(self) -> None: + from pcapkit.foundation.reassembly.ipv6 import _next_header_offset + + self.assertEqual(_next_header_offset(ipv6_header(NH_IPV6_FRAG)), 6) + + def test_one_extension_header_moves_the_answer_past_the_fixed_header(self) -> None: + from pcapkit.foundation.reassembly.ipv6 import _next_header_offset + + header = ipv6_header(NH_HOPOPT) + ext_header(NH_IPV6_FRAG, 8) + self.assertEqual(len(header), 48) + self.assertEqual(_next_header_offset(header), 40) + + def test_the_walk_follows_each_header_s_own_length(self) -> None: + from pcapkit.foundation.reassembly.ipv6 import _next_header_offset + + # a 24 octet Routing header, then an 8 octet Destination Options header + header = (ipv6_header(NH_ROUTING) + ext_header(NH_DSTOPT, 24) + + ext_header(NH_IPV6_FRAG, 8)) + self.assertEqual(len(header), 72) + self.assertEqual(_next_header_offset(header), 64) + + def test_the_authentication_header_uses_its_own_length_encoding(self) -> None: + """AH counts 4-octet units less two, not 8-octet units less one. + + This is the case that distinguishes the two encodings: read with the + 8-octet rule, a 24 octet AH advances 40 octets instead of 24, the walk + overshoots the end of the chain and stops on the wrong header -- so the + Fragment header the datagram advertises never gets rewritten at all. + + """ + from pcapkit.foundation.reassembly.ipv6 import _next_header_offset + + header = (ipv6_header(NH_AH) + ah_header(NH_DSTOPT, 24) + + ext_header(NH_IPV6_FRAG, 8)) + self.assertEqual(len(header), 72) + self.assertEqual(_next_header_offset(header), 64) + self.assertEqual(header[64], NH_IPV6_FRAG) + # what the 8-octet rule would have answered instead + self.assertNotEqual(_next_header_offset(header), 40) + + +@unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed') +class RectifyHeaderTests(unittest.TestCase): + """:meth:`IPv6._rectify_header`, which removes the Fragment header's trace. + + The adapters already stop ``header`` short of the Fragment header's octets; + what is left is the field pointing at it. Left alone, the reassembled + datagram advertises a Fragment header (``44``) on a datagram that is by + definition no longer a fragment (#415). + + """ + + def setUp(self) -> None: + purge_modules(['pcapkit']) + + def rectify(self, header: bytes) -> bytes: + from pcapkit.const.reg.transtype import TransType + from pcapkit.foundation.reassembly.ipv6 import IPv6 + + return IPv6()._rectify_header(header, TransType.UDP) + + def test_the_fixed_header_s_field_is_replaced_and_nothing_else_is(self) -> None: + header = ipv6_header(NH_IPV6_FRAG, payload_len=1456) + rectified = self.rectify(header) + + self.assertEqual(rectified[6], NH_UDP) + self.assertEqual(len(rectified), len(header)) + # every other octet is untouched, the Payload Length field included: it + # still describes the fragment, since the reassembled length is not + # knowable from one fragment + self.assertEqual(rectified[:6], header[:6]) + self.assertEqual(rectified[7:], header[7:]) + self.assertEqual(struct.unpack_from('>H', rectified, 4)[0], 1456) + + def test_an_extension_header_s_field_is_the_one_replaced(self) -> None: + header = ipv6_header(NH_HOPOPT) + ext_header(NH_IPV6_FRAG, 8) + rectified = self.rectify(header) + + # the last header of the unfragmentable part, not the fixed header + self.assertEqual(rectified[40], NH_UDP) + self.assertEqual(rectified[6], NH_HOPOPT) + + def test_a_chain_not_ending_in_a_fragment_header_is_returned_unchanged(self) -> None: + """Which is also what makes the rewrite idempotent.""" + header = ipv6_header(NH_UDP) + self.assertEqual(self.rectify(header), header) + self.assertEqual(self.rectify(self.rectify(ipv6_header(NH_IPV6_FRAG))), + self.rectify(ipv6_header(NH_IPV6_FRAG))) + + def test_a_header_too_short_to_walk_is_returned_unchanged(self) -> None: + """A truncated capture must not raise out of the reassembly path.""" + for header in (b'', b'\x60', ipv6_header(NH_IPV6_FRAG)[:39]): + with self.subTest(length=len(header)): + self.assertEqual(self.rectify(header), header) + + def test_ipv4_reassembly_leaves_the_header_alone(self) -> None: + """The base implementation is the identity, which is what IPv4 needs.""" + from pcapkit.const.reg.transtype import TransType + from pcapkit.foundation.reassembly.ipv4 import IPv4 + + header = bytes.fromhex('4500001c007b2000400600000000000000000000') + self.assertEqual(IPv4()._rectify_header(header, TransType.TCP), header) + + +@unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed') +class IPv6ReassemblyHeaderTests(unittest.TestCase): + """The rewrite reaching the datagram, through the reassembly machinery.""" + + def setUp(self) -> None: + purge_modules(['pcapkit']) + + def _packet(self, *, num: int, fo: int, mf: bool, payload: bytes, header: bytes): + from pcapkit.const.reg.transtype import TransType + from pcapkit.foundation.reassembly.data.ip import Packet + + src = ip_address('2001:db8::1') + dst = ip_address('2001:db8::2') + return Packet((src, dst, 4321, TransType.UDP), num, fo, len(header), mf, + len(header) + len(payload), header, bytearray(payload)) + + def test_the_reassembled_datagram_does_not_advertise_a_fragment_header(self) -> None: + from pcapkit.foundation.reassembly.ipv6 import IPv6 + + header = ipv6_header(NH_HOPOPT) + ext_header(NH_IPV6_FRAG, 8) + reasm = IPv6() + reasm(self._packet(num=1, fo=0, mf=True, payload=b'a' * 8, header=header)) + reasm(self._packet(num=2, fo=8, mf=False, payload=b'b' * 8, header=b'')) + + datagram, = reasm.datagram + self.assertTrue(datagram.completed) + self.assertEqual(datagram.payload, b'a' * 8 + b'b' * 8) + self.assertEqual(datagram.header[40], NH_UDP) + self.assertNotEqual(datagram.header[40], NH_IPV6_FRAG) + # only the first fragment carries a header, so the second must not have + # replaced it with its own empty one + self.assertEqual(len(datagram.header), len(header)) + + def test_a_later_first_fragment_updates_the_stored_header(self) -> None: + """The ``fo == 0`` fragment may arrive after the ones behind it. + + ``IP.reassembly`` writes the header into an existing buffer in that case, + which is a second call site for the rewrite -- one that a test feeding the + fragments in wire order never reaches. + + """ + from pcapkit.foundation.reassembly.ipv6 import IPv6 + + header = ipv6_header(NH_IPV6_FRAG) + reasm = IPv6() + reasm(self._packet(num=1, fo=8, mf=True, payload=b'b' * 8, header=b'')) + reasm(self._packet(num=2, fo=0, mf=True, payload=b'a' * 8, header=header)) + + buffer, = reasm._buffer.values() + self.assertEqual(buffer.header[6], NH_UDP) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/integration/test_reassembly_engine_parity.py b/tests/integration/test_reassembly_engine_parity.py new file mode 100644 index 0000000000..dff288c320 --- /dev/null +++ b/tests/integration/test_reassembly_engine_parity.py @@ -0,0 +1,311 @@ +# -*- coding: utf-8 -*- +"""The four toolkit adapters must describe an IPv6 fragment identically. + +Each of :mod:`pcapkit.toolkit.pcap`, :mod:`pcapkit.toolkit.pcapng`, +:mod:`pcapkit.toolkit.dpkt` and :mod:`pcapkit.toolkit.scapy` builds a +:term:`reasm.ipv6.packet` from whatever object model its engine hands it, and +each was only ever tested against itself. That is how #415 survived: for +``ipv6.pcap`` frame 13 the four reported ``ihl`` 48/48/40/40, ``len(header)`` +48/48/40/40 and ``tl`` 1496/1496/1488/1496 -- a *three*-way disagreement on +``tl``, with no two adapters agreeing on all three fields. + +So the assertions here are on agreement rather than on constants: every adapter +is run over the same octets and the results are compared with each other. The +expected values are pinned as well, because agreement alone would be satisfied by +all four being wrong in the same way. + +The PCAP-NG adapter has no fragmented sample capture of its own, so the frames of +``ipv6.pcap`` are rewrapped into a PCAP-NG file here. That is deliberate: reading +two different captures would compare two different datagrams and prove nothing +about the adapters. + +""" +from __future__ import annotations + +import struct +import unittest +from typing import TYPE_CHECKING + +from tests._support import sample_path +from tests.integration._helpers import HAS_DPKT, HAS_RUNTIME, HAS_SCAPY, EndToEndTestCase + +if TYPE_CHECKING: + from typing import Any + +#: Next Header value of the IPv6 Fragment header (:rfc:`8200#section-4.5`). +NH_IPV6_FRAG = 44 +#: Next Header value of UDP, which is what ``ipv6.pcap``'s fragments carry. +NH_UDP = 17 + +#: Length of the fixed IPv6 header, and of the unfragmentable part of every +#: fragment in ``ipv6.pcap`` -- none of them carries an extension header before +#: its Fragment header. +IPV6_HDR_LEN = 40 + +#: The four fragments of ``ipv6.pcap``, as ``(fragment offset, payload length)``. +#: Frames 13 to 16 are one 4778 octet UDP datagram. +FRAGMENTS = ((0, 1448), (1448, 1448), (2896, 1448), (4344, 434)) + +#: PCAP-NG block types used below (Section 4 of the PCAP-NG specification). +BLOCK_SECTION_HEADER = 0x0A0D0D0A +BLOCK_INTERFACE_DESCRIPTION = 0x00000001 +BLOCK_ENHANCED_PACKET = 0x00000006 + +#: Link layer type 1, i.e. ``LinkType.ETHERNET``. Spelled as a literal because +#: the capture below is assembled before :mod:`pcapkit` is imported. +LINKTYPE_ETHERNET = 1 + + +def pcap_frames(path: 'str') -> 'list[bytes]': + """Raw link-layer frames of a PCAP file, in capture order. + + Read by hand rather than through :mod:`pcapkit`, so that the octets handed to + the ``dpkt`` and ``scapy`` adapters are the file's own and not something a + pcapkit parse has already been over. + + Args: + path: Absolute path to a PCAP file, of either byte order. + + Returns: + One :obj:`bytes` per record. A trailing record whose declared length runs + past the end of the file is dropped: ``ipv6.pcap`` ends in a truncated + one, which is what the extractor reports as ``EOF reached``. + + """ + with open(path, 'rb') as file: + data = file.read() + + magic = data[:4] + if magic in (b'\xd4\xc3\xb2\xa1', b'\x4d\x3c\xb2\xa1'): + endian = '<' + elif magic in (b'\xa1\xb2\xc3\xd4', b'\xa1\xb2\x3c\x4d'): + endian = '>' + else: + raise AssertionError(f'not a PCAP file: {magic!r}') + + frames = [] # type: list[bytes] + offset = 24 # past the 24 octet global header + while offset + 16 <= len(data): + incl_len = struct.unpack_from(f'{endian}IIII', data, offset)[2] + offset += 16 + if offset + incl_len > len(data): + break + frames.append(data[offset:offset + incl_len]) + offset += incl_len + return frames + + +def write_pcapng(path: 'Any', frames: 'list[bytes]') -> 'str': + """Write ``frames`` into a minimal little-endian PCAP-NG capture. + + One Section Header Block, one Interface Description Block and one Enhanced + Packet Block per frame, none of them carrying options. + + Args: + path: Destination, as anything :func:`open` accepts. + frames: Raw link-layer frames, which become the packet data. + + Returns: + The destination as a :obj:`str`, ready to hand to ``extract(fin=...)``. + + """ + def block(block_type: 'int', body: 'bytes') -> 'bytes': + body += b'\x00' * (-len(body) % 4) + length = 12 + len(body) + return struct.pack(' 'dict[str, list]': + """Run each available adapter and collect its fragments in wire order. + + Returns: + Adapter name to the :term:`reasm.ipv6.packet` it built for each + fragment. Adapters whose engine is not installed are absent. + + """ + from pcapkit.toolkit import pcap as pcap_toolkit + from pcapkit.toolkit import pcapng as pcapng_toolkit + + frames = pcap_frames(sample_path('ipv6.pcap')) + packets = {} # type: dict[str, list] + + # the committed capture, through the default engine + extractor = self.extract(fin=sample_path('ipv6.pcap'), nofile=True, store=True, + ipv6=True, reassembly=True) + packets['pcap'] = [data for frame in extractor.frame + if (data := pcap_toolkit.ipv6_reassembly(frame)) is not None] + + # the same frames as PCAP-NG, so the two formats meet on the same octets + pcapng = write_pcapng(self.tmp_path / 'ipv6.pcapng', frames) + extractor = self.extract(fin=pcapng, nofile=True, store=True, + ipv6=True, reassembly=True) + packets['pcapng'] = [data for block in extractor.frame + if (data := pcapng_toolkit.ipv6_reassembly(block)) is not None] + + if HAS_DPKT: + import dpkt + + from pcapkit.toolkit import dpkt as dpkt_toolkit + packets['dpkt'] = [ + data for number, frame in enumerate(frames, start=1) + if (data := dpkt_toolkit.ipv6_reassembly( + dpkt.ethernet.Ethernet(frame), count=number)) is not None + ] + + if HAS_SCAPY: + from scapy.layers.l2 import Ether + + from pcapkit.toolkit import scapy as scapy_toolkit + packets['scapy'] = [ + data for number, frame in enumerate(frames, start=1) + if (data := scapy_toolkit.ipv6_reassembly( + Ether(frame), count=number)) is not None + ] + + return packets + + def test_every_adapter_finds_the_same_four_fragments(self) -> None: + packets = self.adapter_packets() + + # both format adapters are always available; the other two are gated + self.assertIn('pcap', packets) + self.assertIn('pcapng', packets) + for name, fragments in packets.items(): + with self.subTest(adapter=name): + self.assertEqual([(data.fo, len(data.payload)) for data in fragments], + list(FRAGMENTS)) + + def test_every_adapter_reports_the_same_ihl_header_and_tl(self) -> None: + """``ihl``, ``header`` and ``tl`` must not depend on the engine. + + The three fields of #415, asserted both against each other and against + the values :rfc:`8200#section-4.5` calls for: the Fragment header is not + present in the reassembled packet, so none of the three may count its 8 + octets. + + """ + packets = self.adapter_packets() + + for index, (offset, payload_len) in enumerate(FRAGMENTS): + described = {name: (data[index].ihl, data[index].header, data[index].tl) + for name, data in packets.items()} + + # a failure message naming the three numbers per adapter, since the + # raw ``header`` octets in ``described`` are unreadable in a diff + summary = {name: (ihl, len(header), tl) + for name, (ihl, header, tl) in described.items()} + + with self.subTest(fragment=offset): + # every adapter agrees ... + self.assertEqual(len(set(described.values())), 1, + f'adapters disagree on (ihl, len(header), tl): {summary}') + + # ... and what they agree on excludes the Fragment header + ihl, header, tl = next(iter(described.values())) + self.assertEqual(ihl, IPV6_HDR_LEN) + self.assertEqual(len(header), IPV6_HDR_LEN) + self.assertEqual(tl, IPV6_HDR_LEN + payload_len) + # the invariant the reassembly machinery relies on: it writes the + # payload into its datagram buffer over the span ``tl - ihl``, so + # a ``tl`` 8 octets too large leaves 8 stray zeroes per fragment + self.assertEqual(tl - ihl, payload_len) + + def test_the_fragment_header_octets_are_the_ones_left_out(self) -> None: + """The 40 octets kept are the IPv6 header, not an arbitrary prefix. + + A ``header`` of the right *length* could still be the wrong 40 octets, so + this pins what they are: the fixed IPv6 header, still pointing at the + Fragment header, which is what the fragment on the wire says. Rewriting + that field is the reassembler's job, not the adapter's -- see + :class:`IPv6DatagramHeaderTests`. + + """ + packets = self.adapter_packets() + + for name, fragments in packets.items(): + with self.subTest(adapter=name): + header = fragments[0].header + self.assertEqual(header[0] >> 4, 6) # IPv6 version + self.assertEqual(header[6], NH_IPV6_FRAG) # Next Header + # the Payload Length field spans the Fragment header and the + # fragment's payload, i.e. 8 octets more than ``tl - ihl`` + self.assertEqual(struct.unpack_from('>H', header, 4)[0], + 8 + FRAGMENTS[0][1]) + + +@unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed') +class IPv6DatagramHeaderTests(EndToEndTestCase): + """What the *reassembled* datagram's header says. + + :rfc:`8200#section-4.5` -- "The Fragment header is not present in the + reassembled packet", and "the Next Header field of the last header of the + Unfragmentable Part is obtained from the Next Header field of the first + fragment's Fragment header". Every engine used to leave the field alone, so + the reassembled datagram advertised a Fragment header on a datagram that is + by definition no longer a fragment. + + """ + + def engines(self) -> 'list[str]': + return (['default'] + + (['dpkt'] if HAS_DPKT else []) + + (['scapy'] if HAS_SCAPY else [])) + + def test_no_engine_advertises_a_fragment_header_on_the_datagram(self) -> None: + for engine in self.engines(): + with self.subTest(engine=engine): + extractor = self.extract(fin=sample_path('ipv6.pcap'), nofile=True, + store=False, ipv6=True, reassembly=True, + reasm_strict=True, engine=engine) + datagram, = extractor.reassembly.ipv6 + + self.assertTrue(datagram.completed) + self.assertEqual(len(datagram.header), IPV6_HDR_LEN) + # the Fragment header's own Next Header value, moved up + self.assertEqual(datagram.header[6], NH_UDP) + self.assertNotEqual(datagram.header[6], NH_IPV6_FRAG) + self.assertEqual(int(datagram.id.proto), NH_UDP) + + def test_every_engine_reassembles_the_same_payload(self) -> None: + """The datagram itself, not only its header. + + ``scapy``'s ``tl`` counted the Fragment header while its ``ihl`` did not, + so the reassembly machinery wrote each fragment's payload over a span 8 + octets too wide and the datagram came out 4786 octets instead of 4778 -- + eight stray zeroes per fragment boundary, and a payload no other engine + agreed with. + + """ + payloads = {} # type: dict[str, bytes] + for engine in self.engines(): + extractor = self.extract(fin=sample_path('ipv6.pcap'), nofile=True, + store=False, ipv6=True, reassembly=True, + reasm_strict=True, engine=engine) + datagram, = extractor.reassembly.ipv6 + payloads[engine] = bytes(datagram.payload) + + for engine, payload in payloads.items(): + with self.subTest(engine=engine): + self.assertEqual(len(payload), sum(length for _, length in FRAGMENTS)) + self.assertEqual(len(payload), 4778) + self.assertEqual(len(set(payloads.values())), 1) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/toolkit/test_pcap_unit.py b/tests/toolkit/test_pcap_unit.py index 757d87bb35..28038f60a7 100644 --- a/tests/toolkit/test_pcap_unit.py +++ b/tests/toolkit/test_pcap_unit.py @@ -67,7 +67,14 @@ def _make_ipv6(self, *, with_fragment: bool = True): # toolkit passes it through to ``fo`` unscaled. ``id`` is deliberately # different from the IPv6 header's flow label below, so that a ``bufid`` # keyed on the wrong one of the two is visible. + # + # ``length`` mirrors :attr:`IPv6_Frag.length + # `, which is the 8 octets + # the toolkit subtracts back off ``hdr_len`` to reach the boundary before + # the Fragment header. A fake without it cannot show that subtraction + # happening at all. fragment = types.SimpleNamespace( + length=8, info=types.SimpleNamespace(next=TransType.TCP, offset=8, mf=True, id=4321), ) extension_headers = {ExtensionHeader.IPv6_Frag: fragment} if with_fragment else {} @@ -77,8 +84,10 @@ def _make_ipv6(self, *, with_fragment: bool = True): src=ip_address('2001:db8::1'), dst=ip_address('2001:db8::2'), label=7, + # 40 octets of IPv6 header plus the 8 of the Fragment header, and + # a payload length that agrees with ``fragment.payload`` below hdr_len=48, - raw_len=10, + raw_len=6, fragment=types.SimpleNamespace(header=b'V' * 48, payload=b'v6data'), ), ) @@ -161,7 +170,14 @@ def test_pcap_ipv4_ipv6_tcp_and_traceflow_helpers(self) -> None: self.assertNotEqual(v6.bufid[2], 7) self.assertEqual(v6.fo, 8) self.assertTrue(v6.mf) - self.assertEqual(v6.header, b'V' * 48) + # ``hdr_len`` counts the 8-octet Fragment header, but the reassembled + # packet holds no Fragment header at all (:rfc:`8200#section-4.5`), so + # ``ihl`` and ``header`` stop 8 octets short of it -- and ``tl`` with them, + # since the reassembly machinery writes the payload over ``tl - ihl`` + self.assertEqual(v6.ihl, 40) + self.assertEqual(v6.header, b'V' * 40) + self.assertEqual(v6.tl, 46) + self.assertEqual(v6.tl - v6.ihl, len(v6.payload)) self.assertEqual(bytes(v6.payload), b'v6data') self.assertIsNone(toolkit.ipv6_reassembly(FakeFrame({}, types.SimpleNamespace(number=1)))) self.assertIsNone(toolkit.ipv6_reassembly( @@ -217,7 +233,12 @@ def test_pcapng_helpers_and_block_to_frame_timestamp_scaling(self) -> None: self.assertEqual(v6.bufid[2], 4321) self.assertNotEqual(v6.bufid[2], 7) self.assertEqual(v6.fo, 8) - self.assertEqual(v6.header, b'V' * 48) + # and it stops short of the Fragment header the same way, so a datagram + # does not reassemble differently per capture format + self.assertEqual(v6.ihl, 40) + self.assertEqual(v6.header, b'V' * 40) + self.assertEqual(v6.tl, 46) + self.assertEqual(v6.tl - v6.ihl, len(v6.payload)) self.assertIsNone(toolkit.ipv6_reassembly(FakeFrame({}, frame.info))) self.assertIsNone(toolkit.ipv6_reassembly( FakeFrame({'IPv6': self._make_ipv6(with_fragment=False)}, frame.info), From 637d2a466c3790ad8bc56a83108d5f4d58258f33 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Wed, 16 Sep 2026 17:31:39 -0400 Subject: [PATCH 2/8] reassembly: analyse a reassembled datagram's payload on first read (#420) `IP.submit` called `Protocol.analyze` eagerly on every datagram it built, and IP reassembly builds one for *every* frame -- not only the fragmented ones, because `toolkit/pcap.py` dismisses an IPv4 frame only when its DF flag is set, so a frame with `DF=0, MF=0, FO=0` reaches `ip.py:73`, allocates a buffer and is submitted as a trivially complete datagram. `http.pcap` holds 1117 IPv4 frames, none of them fragmented, and yielded 1117 "datagrams" -- each one a second full parse of a payload most callers never look at. `Datagram.packet` is now analysed on first read. `Deferred` holds the bound analyser, the protocol type and the payload, and `Datagram.__analyse__` runs it once and keeps the result. `Info` builds its mapping view out of `__dict__`, so a naively lazy field disappears from `to_dict()`, `keys()` and `repr()` -- or shows up there as the placeholder. Listing `packet` in `__additional__` is what avoids that: `Info` already stores a field whose name collides with a builtin name under a mangled key and maps it back on the way out, so the key stays in every view under its own name while attribute reads fall through to `__getattr__`. `__getitem__`, `__str__`, `__repr__` and `to_dict` resolve before answering; `__contains__` does not, since `Mapping.__contains__` would otherwise run a full parse just to decide the field exists. `bytes(datagram[:TDL])` is now taken once rather than twice, which is why forcing every `packet` is slightly cheaper than the old eager path rather than equal to it. Measured, `http.pcap`, best of 5, one interpreter per shape: shape before after delta baseline (no reassembly) 1046.7ms 1031.1ms - reassembly=True ip=True 1881.1ms 1111.2ms -40.9% i.e. the feature's own cost +834.4ms +77.3ms -90.7% ... with every packet read - 1793.8ms reassembly=True ip=True, gc off 1801.3ms 1114.7ms i.e. the GC bill +79.8ms ~0ms `test.pcap` (34 frames, 21 trivial datagrams): 40.7ms -> 29.7ms, -27.0%; feature cost +12.8ms -> +1.3ms, -89.8%. Behaviour-neutral, checked three ways rather than assumed: every observable of a `Datagram` -- attribute read, `len`, iteration, `to_dict`, `dict()`, `keys`, `items`, `get`, `in`, `str`, `repr`, `hasattr`, and `copy`/`deepcopy`/`pickle` -- is identical before and after, for a complete and an incomplete datagram and with `packet` read first and not at all; every reassembly result over all 14 fixtures in both the `ip` and `tcp` shapes is identical (1509 lines); and serialising all 14 fixtures to `tree` and `json`, with and without reassembly, is byte-identical to the pre-#415 tree over 60 files and 33 MB. Not changed, deliberately: whether an unfragmented frame should be emitted as a datagram at all. That is a design decision about what consumers see, and it is the owner's to make -- see the report accompanying this branch. `reassembly/tcp.py:298` has the same eager `analyze`, but it is driven by FIN/RST and submits 222 times per `http.pcap` pass rather than 1117, so it is left for a separate change. 827 passed, 17 skipped. mypy 124 errors either side, pylint message multiset identical (bar `cyclic-import`, non-deterministic run to run). --- .../pcapkit/foundation/reassembly/ip/ip.rst | 4 + .../pcapkit/foundation/reassembly/ip/ipv4.rst | 11 ++ .../pcapkit/foundation/reassembly/ip/ipv6.rst | 11 ++ pcapkit/foundation/reassembly/data/ip.py | 118 ++++++++++++++++- pcapkit/foundation/reassembly/ip.py | 13 +- tests/foundation/reassembly/test_ip.py | 123 ++++++++++++++++++ 6 files changed, 271 insertions(+), 9 deletions(-) diff --git a/docs/source/pcapkit/foundation/reassembly/ip/ip.rst b/docs/source/pcapkit/foundation/reassembly/ip/ip.rst index 2a8b2d978b..ea96166537 100644 --- a/docs/source/pcapkit/foundation/reassembly/ip/ip.rst +++ b/docs/source/pcapkit/foundation/reassembly/ip/ip.rst @@ -46,6 +46,10 @@ Data Models :members: :show-inheritance: +.. autoclass:: pcapkit.foundation.reassembly.data.ip.Deferred + :members: + :show-inheritance: + Type Variables -------------- diff --git a/docs/source/pcapkit/foundation/reassembly/ip/ipv4.rst b/docs/source/pcapkit/foundation/reassembly/ip/ipv4.rst index dd5524e746..756d4c7adc 100644 --- a/docs/source/pcapkit/foundation/reassembly/ip/ipv4.rst +++ b/docs/source/pcapkit/foundation/reassembly/ip/ipv4.rst @@ -82,6 +82,17 @@ Terminology | |--> 'packet' : (None) |--> (Info) data ... + .. note:: + + ``packet`` is analysed on the first read, not when the datagram is + submitted. A datagram is submitted for *every* frame -- an unfragmented + one included, since nothing upstream filters it out -- and the analysis + is a second full parse of the payload, so running it eagerly charged + every caller for a result most never read. Reading the attribute, or any + mapping view of it (``datagram['packet']``, ``to_dict()``, ``items()``, + ``repr()``), runs it and keeps the result; see + :class:`~pcapkit.foundation.reassembly.data.ip.Deferred`. + reasm.ipv4.buffer Data structure for internal buffering when performing reassembly algorithms (:attr:`IPv4._buffer `) diff --git a/docs/source/pcapkit/foundation/reassembly/ip/ipv6.rst b/docs/source/pcapkit/foundation/reassembly/ip/ipv6.rst index 235bc3a53c..150670b97c 100644 --- a/docs/source/pcapkit/foundation/reassembly/ip/ipv6.rst +++ b/docs/source/pcapkit/foundation/reassembly/ip/ipv6.rst @@ -115,6 +115,17 @@ Terminology still describes the first fragment rather than the reassembled datagram; use ``len(payload)`` instead. + .. note:: + + ``packet`` is analysed on the first read, not when the datagram is + submitted. A datagram is submitted for *every* frame -- an unfragmented + one included, since nothing upstream filters it out -- and the analysis + is a second full parse of the payload, so running it eagerly charged + every caller for a result most never read. Reading the attribute, or any + mapping view of it (``datagram['packet']``, ``to_dict()``, ``items()``, + ``repr()``), runs it and keeps the result; see + :class:`~pcapkit.foundation.reassembly.data.ip.Deferred`. + reasm.ipv6.buffer Data structure for internal buffering when performing reassembly algorithms (:attr:`IPv6._buffer `) diff --git a/pcapkit/foundation/reassembly/data/ip.py b/pcapkit/foundation/reassembly/data/ip.py index 0655e53845..f63d707840 100644 --- a/pcapkit/foundation/reassembly/data/ip.py +++ b/pcapkit/foundation/reassembly/data/ip.py @@ -7,12 +7,12 @@ from pcapkit.utilities.compat import Tuple __all__ = [ - 'Packet', 'DatagramID', 'Datagram', 'Buffer', 'BufferID', + 'Packet', 'DatagramID', 'Datagram', 'Buffer', 'BufferID', 'Deferred', ] if TYPE_CHECKING: from ipaddress import IPv4Address, IPv6Address - from typing import Optional, overload + from typing import Any, Callable, Optional, overload from typing_extensions import Literal, TypeAlias @@ -25,6 +25,51 @@ BufferID: 'TypeAlias' = Tuple[_AT, _AT, int, 'TransType'] +class Deferred: + """A postponed analysis of a reassembled payload. + + :attr:`Datagram.packet` is a second, full parse of the payload the datagram + just reassembled, and IP reassembly submits a datagram for *every* frame -- + not only the fragmented ones, since a frame that is not fragmented in any + sense still reaches + :meth:`IP.reassembly ` and is + submitted from there as a trivially complete datagram. Running the parse + eagerly therefore re-parsed captures that hold no fragments at all: + :file:`http.pcap` has 1117 IPv4 frames and none of them fragmented, and the + parse was 86% of the cost of IP reassembly over it. + + Holding the call here defers it to the first read of + :attr:`Datagram.packet`, so a caller that wants the parsed payload still gets + exactly the object the eager call produced, and one that does not never pays + for it. + + Args: + analyze: The analyser to call, i.e. + :meth:`Protocol.analyze ` + bound to the reassembly object's protocol. + proto: Payload protocol type. + payload: Reassembled payload to parse. + + """ + + __slots__ = ('analyze', 'proto', 'payload') + + def __init__(self, analyze: 'Callable[[TransType, bytes], Protocol]', + proto: 'TransType', payload: 'bytes') -> 'None': + self.analyze = analyze + self.proto = proto + self.payload = payload + + def __call__(self) -> 'Protocol': + """Run the postponed analysis. + + Returns: + Parsed payload. + + """ + return self.analyze(self.proto, self.payload) + + @info_final class Packet(Info, Generic[_AT]): """Data model for :term:`IPv4 ` and/or @@ -74,6 +119,14 @@ class Datagram(Info, Generic[_AT]): """Data model for :term:`IPv4 ` and/or :term:`IPv6 ` reassembled datagram.""" + #: Listing ``packet`` here is what makes :attr:`packet` lazy. :class:`Info` + #: stores a field whose name is a *builtin* name under a mangled key and maps + #: it back on the way out, so ``packet`` never lands in :attr:`__dict__` + #: itself -- which routes reading it through :meth:`__getattr__`, where a + #: :class:`Deferred` analysis can be run, while ``dict(datagram)``, + #: :meth:`to_dict` and iteration still report the field under its own name. + __additional__ = ['packet'] + #: Completed flag. completed: 'bool' #: Original packet identifier. @@ -84,17 +137,72 @@ class Datagram(Info, Generic[_AT]): header: 'bytes' #: Reassembled IP payload. payload: 'bytes | tuple[bytes, ...]' - #: Parsed IP payload. + #: Parsed IP payload. Analysed on first read, not at construction time; a + #: :class:`Deferred` may be passed in its place, and reading this attribute + #: then runs it and keeps the result. packet: 'Optional[Protocol]' if TYPE_CHECKING: @overload #pylint: disable=used-before-assignment - def __init__(self, completed: 'Literal[True]', id: 'DatagramID[_AT]', index: 'tuple[int, ...]', header: 'bytes', payload: 'bytes', packet: 'Protocol') -> 'None': ... # pylint: disable=unused-argument,super-init-not-called,multiple-statements,line-too-long,redefined-builtin + def __init__(self, completed: 'Literal[True]', id: 'DatagramID[_AT]', index: 'tuple[int, ...]', header: 'bytes', payload: 'bytes', packet: 'Protocol | Deferred') -> 'None': ... # pylint: disable=unused-argument,super-init-not-called,multiple-statements,line-too-long,redefined-builtin @overload def __init__(self, completed: 'Literal[False]', id: 'DatagramID[_AT]', index: 'tuple[int, ...]', header: 'bytes', payload: 'tuple[bytes, ...]', packet: 'None') -> 'None': ... # pylint: disable=unused-argument,super-init-not-called,multiple-statements,line-too-long,redefined-builtin - def __init__(self, completed: 'bool', id: 'DatagramID[_AT]', index: 'tuple[int, ...]', header: 'bytes', payload: 'bytes | tuple[bytes, ...]', packet: 'Optional[Protocol]') -> 'None': ... # pylint: disable=unused-argument,super-init-not-called,multiple-statements,line-too-long,redefined-builtin + def __init__(self, completed: 'bool', id: 'DatagramID[_AT]', index: 'tuple[int, ...]', header: 'bytes', payload: 'bytes | tuple[bytes, ...]', packet: 'Optional[Protocol | Deferred]') -> 'None': ... # pylint: disable=unused-argument,super-init-not-called,multiple-statements,line-too-long,redefined-builtin + + def __analyse__(self) -> 'Optional[Protocol]': + """Resolve a deferred analysis, at most once. + + Returns: + Parsed IP payload, or :data:`None` for an incomplete datagram. + + """ + key = self.__map__.get('packet', 'packet') + value = self.__dict__[key] + if isinstance(value, Deferred): + value = value() + self.__dict__[key] = value + return value + + def __getattr__(self, name: 'str') -> 'Any': + # NOTE: reached only for names absent from ``__dict__``, which ``packet`` + # always is -- see ``__additional__`` above. Everything else has to raise, + # or a typo would silently answer with a parsed payload. + if name != 'packet': + raise AttributeError(f'{type(self).__name__!r} object has no attribute {name!r}') + return self.__analyse__() + + def __getitem__(self, name: 'str') -> 'Any': + if name == 'packet': + return self.__analyse__() + return super().__getitem__(name) + + def __contains__(self, name: 'object') -> 'bool': + # NOTE: ``Mapping.__contains__`` answers by fetching the value, which + # would run the deferred analysis merely to decide that the field exists. + # ``packet`` is a declared field, so it is always there. + return name == 'packet' or super().__contains__(name) + + def __str__(self) -> 'str': + self.__analyse__() + return super().__str__() + + def __repr__(self) -> 'str': + self.__analyse__() + return super().__repr__() + + def to_dict(self) -> 'dict[str, Any]': + """Convert :class:`Datagram` into :obj:`dict`. + + Returns: + The datagram's fields, with ``packet`` analysed if it had not been + read yet -- a :obj:`dict` holding a :class:`Deferred` would leak an + implementation detail into what is meant to be plain data. + + """ + self.__analyse__() + return super().to_dict() @info_final diff --git a/pcapkit/foundation/reassembly/ip.py b/pcapkit/foundation/reassembly/ip.py index 996ee58593..1f76a20191 100644 --- a/pcapkit/foundation/reassembly/ip.py +++ b/pcapkit/foundation/reassembly/ip.py @@ -17,7 +17,7 @@ from typing import TYPE_CHECKING, Generic from pcapkit.foundation.reassembly.data.ip import (_AT, Buffer, BufferID, Datagram, DatagramID, - Packet) + Deferred, Packet) from pcapkit.foundation.reassembly.reassembly import ReassemblyBase as Reassembly if TYPE_CHECKING: @@ -196,7 +196,7 @@ def submit(self, buf: 'Buffer[_AT]', *, bufid: 'tuple[_AT, _AT, int, TransType]' ret.append(packet) # if datagram is reassembled in whole else: - payload = datagram[:TDL] + payload = bytes(datagram[:TDL]) packet = Datagram( completed=True, id=DatagramID( @@ -207,8 +207,13 @@ def submit(self, buf: 'Buffer[_AT]', *, bufid: 'tuple[_AT, _AT, int, TransType]' ), index=tuple(index), header=header, - payload=bytes(payload), - packet=self.protocol.analyze(bufid[3], bytes(payload)), + payload=payload, + # NOTE: ``analyze`` is a second full parse of the payload, and a + # datagram is submitted for every frame rather than only for the + # fragmented ones, so running it here charges every caller for a + # result most of them never read. ``Deferred`` postpones it to the + # first read of ``Datagram.packet``. + packet=Deferred(self.protocol.analyze, bufid[3], payload), ) ret.append(packet) diff --git a/tests/foundation/reassembly/test_ip.py b/tests/foundation/reassembly/test_ip.py index da86a47c1f..0f428b86fa 100644 --- a/tests/foundation/reassembly/test_ip.py +++ b/tests/foundation/reassembly/test_ip.py @@ -110,5 +110,128 @@ class TestIP(IP): ) +@unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed') +class DeferredAnalysisTests(unittest.TestCase): + """``Datagram.packet`` is analysed on first read, not at submit time. + + The analysis is a second full parse of the reassembled payload, and a + datagram is submitted for *every* frame -- ``pcapkit/toolkit/pcap.py`` + dismisses an IPv4 frame only when its **DF** flag is set, so a frame with + ``DF=0, MF=0, FO=0`` is not fragmented in any sense and still arrives here. + On ``http.pcap``, which holds no fragments at all, that was 1117 re-parses + per extraction and 86% of the cost of IP reassembly. + + What a caller sees must not change, which is why the assertions below are + about *when* the analyser runs and not only about what it returns. + + """ + + def setUp(self) -> None: + purge_modules(['pcapkit']) + + def _reassemble(self, *, calls: 'list'): + """One complete, unfragmented datagram, and the analyser's call log.""" + from pcapkit.const.reg.transtype import TransType + from pcapkit.foundation.reassembly.data.ip import Packet + from pcapkit.foundation.reassembly.ip import IP + + class Analyzer: + @classmethod + def analyze(cls, proto: object, payload: bytes) -> object: + calls.append((proto, payload)) + return {'proto': proto, 'payload': payload} + + class TestIP(IP): + __protocol_type__ = Analyzer + + src = ip_address('192.0.2.1') + dst = ip_address('198.51.100.2') + reasm = TestIP() + reasm(Packet((src, dst, 42, TransType.UDP), 1, 0, 20, False, 25, + b'ip-header', bytearray(b'hello'))) + datagram, = reasm.datagram + return datagram + + def test_submitting_a_datagram_does_not_analyse_it(self) -> None: + calls = [] # type: list + datagram = self._reassemble(calls=calls) + + # the datagram is complete and its payload is there, but nothing has been + # parsed -- which is the whole point + self.assertTrue(datagram.completed) + self.assertEqual(datagram.payload, b'hello') + self.assertEqual(calls, []) + + def test_reading_packet_analyses_once_and_keeps_the_result(self) -> None: + calls = [] # type: list + datagram = self._reassemble(calls=calls) + + first = datagram.packet + self.assertEqual(first, {'proto': datagram.id.proto, 'payload': b'hello'}) + self.assertEqual(len(calls), 1) + + # a second read must not re-parse, and must be the same object rather + # than an equal one -- a caller holding ``datagram.packet`` and reading it + # again would otherwise get a different parse tree each time + self.assertIs(datagram.packet, first) + self.assertEqual(len(calls), 1) + + def test_the_mapping_view_reports_packet_and_forces_the_analysis(self) -> None: + """``dict(datagram)`` and friends must not expose the deferral. + + ``Info`` builds its mapping view out of ``__dict__``, so a lazy field is + one that can silently vanish from ``to_dict()``, ``keys()`` and ``repr()`` + -- or, worse, show up there as the placeholder object. + + """ + for reader in ('to_dict', 'str', 'repr', 'getitem', 'get', 'items'): + with self.subTest(reader=reader): + calls = [] # type: list + datagram = self._reassemble(calls=calls) + + # every view lists the field before anything has been read + self.assertIn('packet', datagram) + self.assertIn('packet', sorted(datagram)) + self.assertIn('packet', datagram.keys()) + self.assertEqual(calls, []) + + expected = {'proto': datagram.id.proto, 'payload': b'hello'} + if reader == 'to_dict': + self.assertEqual(datagram.to_dict()['packet'], expected) + elif reader == 'str': + self.assertIn("'payload': b'hello'", str(datagram)) + elif reader == 'repr': + self.assertIn("'payload': b'hello'", repr(datagram)) + elif reader == 'getitem': + self.assertEqual(datagram['packet'], expected) + elif reader == 'get': + self.assertEqual(datagram.get('packet'), expected) + else: + self.assertEqual(dict(datagram.items())['packet'], expected) + self.assertEqual(len(calls), 1) + + def test_an_unknown_attribute_still_raises(self) -> None: + """The lazy read is reached through ``__getattr__``, which must not swallow.""" + calls = [] # type: list + datagram = self._reassemble(calls=calls) + + self.assertFalse(hasattr(datagram, 'nope')) + with self.assertRaises(AttributeError): + datagram.nope # pylint: disable=pointless-statement + self.assertEqual(calls, []) + + def test_the_deferred_holder_is_a_plain_callable(self) -> None: + """It has to be callable and comparable by identity, nothing more.""" + from pcapkit.const.reg.transtype import TransType + from pcapkit.foundation.reassembly.data.ip import Deferred + + seen = [] # type: list + deferred = Deferred(lambda proto, payload: seen.append((proto, payload)) or 'parsed', + TransType.UDP, b'hello') + self.assertEqual(seen, []) + self.assertEqual(deferred(), 'parsed') + self.assertEqual(seen, [(TransType.UDP, b'hello')]) + + if __name__ == '__main__': unittest.main() From a9b8603c3eccbf70ae405123489718598f0f2130 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Wed, 16 Sep 2026 18:52:31 -0400 Subject: [PATCH 3/8] reassembly: move ReassemblyData and Deferred into their own data module Per review on #424. `Deferred` was in `data/ip.py`, which misfiled it: it holds an analyser, a protocol and a payload, and calls the analyser -- nothing in it is IP-specific. `ReassemblyData` was in `data/__init__.py`, which is the same problem from the other side, a class living in a package initialiser. Both now sit in `pcapkit/foundation/reassembly/data/data.py`, following `pcapkit/protocols/data/data.py`, and `data/__init__.py` is re-exports only. `Deferred` remains importable from `data.ip` for anyone who already had it. Its docstring now separates the mechanism from the case that motivated it: IP reassembly submits a datagram per frame, which is why the eager parse cost 86% of IP reassembly over a capture with no fragments, but nothing about postponing the parse is IP-only. It also records that TCP reassembly builds its `packet` eagerly too and can use this unmodified -- being FIN/RST-driven, 222 submits per `http.pcap` pass against 1117, it is a far smaller cost and wants its own change. Flow tracing was considered and needs nothing: its data models carry no parsed protocol at all -- `Packet` holds the already-extracted frame it was handed, and `Buffer`/`Index` hold a dumper, indices and a label -- and there is no `analyze()` call anywhere in `foundation/traceflow/`. Its per-packet cost was the dumper rebuilding a `Frame`, a different problem fixed separately. Full suite 851 passed, 17 skipped. --- .../foundation/reassembly/data/__init__.py | 27 ++---- pcapkit/foundation/reassembly/data/data.py | 87 +++++++++++++++++++ pcapkit/foundation/reassembly/data/ip.py | 48 +--------- 3 files changed, 94 insertions(+), 68 deletions(-) create mode 100644 pcapkit/foundation/reassembly/data/data.py diff --git a/pcapkit/foundation/reassembly/data/__init__.py b/pcapkit/foundation/reassembly/data/__init__.py index fe949d384d..d1b69dbb2b 100644 --- a/pcapkit/foundation/reassembly/data/__init__.py +++ b/pcapkit/foundation/reassembly/data/__init__.py @@ -1,6 +1,9 @@ # -*- coding: utf-8 -*- """data models for reassembly""" +# shared +from pcapkit.foundation.reassembly.data.data import Deferred, ReassemblyData + # IP reassembly from pcapkit.foundation.reassembly.data.ip import Buffer as IP_Buffer from pcapkit.foundation.reassembly.data.ip import BufferID as IP_BufferID @@ -18,31 +21,11 @@ from pcapkit.foundation.reassembly.data.tcp import Packet as TCP_Packet __all__ = [ + 'ReassemblyData', 'Deferred', + 'IP_Packet', 'IP_DatagramID', 'IP_Datagram', 'IP_Buffer', 'IP_BufferID', 'TCP_Packet', 'TCP_DatagramID', 'TCP_Datagram', 'TCP_Buffer', 'TCP_Fragment', 'TCP_HoleDescriptor', 'TCP_BufferID', ] - -from typing import TYPE_CHECKING - -from pcapkit.corekit.infoclass import Info, info_final - -if TYPE_CHECKING: - from typing import Optional - - -@info_final -class ReassemblyData(Info): - """Data storage for reassembly.""" - - #: IPv4 reassembled data. - ipv4: 'tuple[IP_Datagram, ...]' - #: IPv6 reassembled data. - ipv6: 'tuple[IP_Datagram, ...]' - #: TCP reassembled data. - tcp: 'tuple[TCP_Datagram, ...]' - - if TYPE_CHECKING: - def __init__(self, ipv4: 'Optional[tuple[IP_Datagram, ...]]', ipv6: 'Optional[tuple[IP_Datagram, ...]]', tcp: 'Optional[tuple[TCP_Datagram, ...]]') -> 'None': ... # pylint: disable=unused-argument,super-init-not-called,multiple-statements,line-too-long diff --git a/pcapkit/foundation/reassembly/data/data.py b/pcapkit/foundation/reassembly/data/data.py new file mode 100644 index 0000000000..558be51361 --- /dev/null +++ b/pcapkit/foundation/reassembly/data/data.py @@ -0,0 +1,87 @@ +# -*- coding: utf-8 -*- +"""shared data models for reassembly""" + +from typing import TYPE_CHECKING + +from pcapkit.corekit.infoclass import Info, info_final + +__all__ = ['ReassemblyData', 'Deferred'] + +if TYPE_CHECKING: + from typing import Callable, Optional + + from pcapkit.const.reg.transtype import TransType + from pcapkit.foundation.reassembly.data.ip import Datagram as IP_Datagram + from pcapkit.foundation.reassembly.data.tcp import Datagram as TCP_Datagram + from pcapkit.protocols.protocol import ProtocolBase as Protocol + + +class Deferred: + """A postponed analysis of a reassembled payload. + +A reassembled datagram's ``packet`` is a second, full parse of the payload + the datagram just reassembled. Nothing about postponing it is specific to any + one reassembler, which is why this lives beside + :class:`~pcapkit.foundation.reassembly.data.ReassemblyData` rather than in + either protocol's data module. + + IP reassembly is the case that made it necessary. It submits a datagram for + *every* frame -- not only the fragmented ones, since a frame that is not + fragmented in any sense still reaches + :meth:`IP.reassembly ` and is + submitted there as a trivially complete datagram -- so the eager parse + re-parsed captures holding no fragments at all: :file:`http.pcap` has 1117 + IPv4 frames, none of them fragmented, and the parse was 86% of the cost of IP + reassembly over it. + + TCP reassembly builds its ``packet`` eagerly too + (:meth:`TCP.submit `). It is a + far smaller cost there, being FIN/RST-driven rather than per-frame -- 222 + submits per :file:`http.pcap` pass against 1117 -- so it is left for its own + change, but it can use this unmodified when someone gets to it. + + Holding the call here defers it to the first read of + :attr:`Datagram.packet`, so a caller that wants the parsed payload still gets + exactly the object the eager call produced, and one that does not never pays + for it. + + Args: + analyze: The analyser to call, i.e. + :meth:`Protocol.analyze ` + bound to the reassembly object's protocol. + proto: Payload protocol type. + payload: Reassembled payload to parse. + + """ + + __slots__ = ('analyze', 'proto', 'payload') + + def __init__(self, analyze: 'Callable[[TransType, bytes], Protocol]', + proto: 'TransType', payload: 'bytes') -> 'None': + self.analyze = analyze + self.proto = proto + self.payload = payload + + def __call__(self) -> 'Protocol': + """Run the postponed analysis. + + Returns: + Parsed payload. + + """ + return self.analyze(self.proto, self.payload) + + +@info_final +class ReassemblyData(Info): + """Data storage for reassembly.""" + + #: IPv4 reassembled data. + ipv4: 'tuple[IP_Datagram, ...]' + #: IPv6 reassembled data. + ipv6: 'tuple[IP_Datagram, ...]' + #: TCP reassembled data. + tcp: 'tuple[TCP_Datagram, ...]' + + if TYPE_CHECKING: + def __init__(self, ipv4: 'Optional[tuple[IP_Datagram, ...]]', ipv6: 'Optional[tuple[IP_Datagram, ...]]', tcp: 'Optional[tuple[TCP_Datagram, ...]]') -> 'None': ... # pylint: disable=unused-argument,super-init-not-called,multiple-statements,line-too-long diff --git a/pcapkit/foundation/reassembly/data/ip.py b/pcapkit/foundation/reassembly/data/ip.py index f63d707840..bc48a84006 100644 --- a/pcapkit/foundation/reassembly/data/ip.py +++ b/pcapkit/foundation/reassembly/data/ip.py @@ -4,10 +4,11 @@ from typing import TYPE_CHECKING, Generic, TypeVar from pcapkit.corekit.infoclass import Info, info_final +from pcapkit.foundation.reassembly.data.data import Deferred from pcapkit.utilities.compat import Tuple __all__ = [ - 'Packet', 'DatagramID', 'Datagram', 'Buffer', 'BufferID', 'Deferred', + 'Packet', 'DatagramID', 'Datagram', 'Buffer', 'BufferID', ] if TYPE_CHECKING: @@ -25,51 +26,6 @@ BufferID: 'TypeAlias' = Tuple[_AT, _AT, int, 'TransType'] -class Deferred: - """A postponed analysis of a reassembled payload. - - :attr:`Datagram.packet` is a second, full parse of the payload the datagram - just reassembled, and IP reassembly submits a datagram for *every* frame -- - not only the fragmented ones, since a frame that is not fragmented in any - sense still reaches - :meth:`IP.reassembly ` and is - submitted from there as a trivially complete datagram. Running the parse - eagerly therefore re-parsed captures that hold no fragments at all: - :file:`http.pcap` has 1117 IPv4 frames and none of them fragmented, and the - parse was 86% of the cost of IP reassembly over it. - - Holding the call here defers it to the first read of - :attr:`Datagram.packet`, so a caller that wants the parsed payload still gets - exactly the object the eager call produced, and one that does not never pays - for it. - - Args: - analyze: The analyser to call, i.e. - :meth:`Protocol.analyze ` - bound to the reassembly object's protocol. - proto: Payload protocol type. - payload: Reassembled payload to parse. - - """ - - __slots__ = ('analyze', 'proto', 'payload') - - def __init__(self, analyze: 'Callable[[TransType, bytes], Protocol]', - proto: 'TransType', payload: 'bytes') -> 'None': - self.analyze = analyze - self.proto = proto - self.payload = payload - - def __call__(self) -> 'Protocol': - """Run the postponed analysis. - - Returns: - Parsed payload. - - """ - return self.analyze(self.proto, self.payload) - - @info_final class Packet(Info, Generic[_AT]): """Data model for :term:`IPv4 ` and/or From 9eec5a7706bd4cb573e48826953df5278ec18689 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Wed, 16 Sep 2026 19:22:07 -0400 Subject: [PATCH 4/8] traceflow: move TraceFlowData into its own data module, matching reassembly Symmetry, per review on #424. `TraceFlowData` sat in `traceflow/data/__init__.py` exactly as `ReassemblyData` sat in the reassembly one; both packages now keep their shared model in `data/data.py` and their `__init__` as re-exports only, following `pcapkit/protocols/data/data.py`. No `Deferred` here, because there is nothing yet to defer: flow tracing holds no parsed protocol and no payload bytes. `Index` carries a *filename*, a tuple of frame indices and a label, and `submit()` never sees packet data at all -- frames go straight to the dumper as they arrive rather than accumulating. Wiring application-layer analysis into flow tracing is a design change rather than a refactor, and is being raised separately. Full suite unchanged. --- pcapkit/foundation/traceflow/data/__init__.py | 23 ++++-------------- pcapkit/foundation/traceflow/data/data.py | 24 +++++++++++++++++++ 2 files changed, 29 insertions(+), 18 deletions(-) create mode 100644 pcapkit/foundation/traceflow/data/data.py diff --git a/pcapkit/foundation/traceflow/data/__init__.py b/pcapkit/foundation/traceflow/data/__init__.py index 89d464686f..3a3c4a0799 100644 --- a/pcapkit/foundation/traceflow/data/__init__.py +++ b/pcapkit/foundation/traceflow/data/__init__.py @@ -1,6 +1,9 @@ # -*- coding: utf-8 -*- """data models for flow tracing""" +# shared +from pcapkit.foundation.traceflow.data.data import TraceFlowData + # TCP flow tracing from pcapkit.foundation.traceflow.data.tcp import Buffer as TCP_Buffer from pcapkit.foundation.traceflow.data.tcp import BufferID as TCP_BufferID @@ -8,23 +11,7 @@ from pcapkit.foundation.traceflow.data.tcp import Packet as TCP_Packet __all__ = [ + 'TraceFlowData', + 'TCP_Buffer', 'TCP_BufferID', 'TCP_Index', 'TCP_Packet', ] - -from typing import TYPE_CHECKING - -from pcapkit.corekit.infoclass import Info, info_final - -if TYPE_CHECKING: - from typing import Optional - - -@info_final -class TraceFlowData(Info): - """Data storage for flow tracing.""" - - #: TCP traced flows. - tcp: 'tuple[TCP_Index, ...]' - - if TYPE_CHECKING: - def __init__(self, tcp: 'Optional[tuple[TCP_Index, ...]]') -> 'None': ... # pylint: disable=unused-argument,super-init-not-called,multiple-statements,line-too-long diff --git a/pcapkit/foundation/traceflow/data/data.py b/pcapkit/foundation/traceflow/data/data.py new file mode 100644 index 0000000000..a250dca8ee --- /dev/null +++ b/pcapkit/foundation/traceflow/data/data.py @@ -0,0 +1,24 @@ +# -*- coding: utf-8 -*- +"""shared data models for flow tracing""" + +from typing import TYPE_CHECKING + +from pcapkit.corekit.infoclass import Info, info_final + +__all__ = ['TraceFlowData'] + +if TYPE_CHECKING: + from typing import Optional + + from pcapkit.foundation.traceflow.data.tcp import Index as TCP_Index + + +@info_final +class TraceFlowData(Info): + """Data storage for flow tracing.""" + + #: TCP traced flows. + tcp: 'tuple[TCP_Index, ...]' + + if TYPE_CHECKING: + def __init__(self, tcp: 'Optional[tuple[TCP_Index, ...]]') -> 'None': ... # pylint: disable=unused-argument,super-init-not-called,multiple-statements,line-too-long From 8ae95a3aa0f6716ec148aa5ae6a876001e0da37d Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Wed, 16 Sep 2026 19:40:03 -0400 Subject: [PATCH 5/8] reassembly: defer TCP's payload analysis too, sharing the mechanism with IP Per the decision on #424: flow tracing keeps streaming and gains nothing, while TCP reassembly -- which already reassembles the stream and already calls `analyze`, eagerly, in `TCP.submit` -- takes the deferral instead. The reading half of the arrangement is now a `DeferredPacket` mixin in `data/data.py` beside `Deferred`, rather than copied into a second `Datagram`. `IP_Datagram` and `TCP_Datagram` both inherit it and both declare `__additional__ = ['packet']`, which is what makes the field lazy: `Info` stores a builtin-named field under a mangled key and maps it back, so `packet` never lands in `__dict__` and reading it routes through `__getattr__`. `tcp=True, reassembly=True` over `http.pcap`, best of 5: **1441.0 ms to 1099.0 ms, -23.7%**. Smaller in relative terms than IP's -90.7%, as expected -- this path is FIN/RST-driven, 222 submits against 1117 -- but 222 HTTP parses is still 222 parses most callers never read. Output is unchanged, checked rather than assumed: 229 datagram lines across `http.pcap`, `tcp.pcap` and `http6.cap` -- completion, indices, payload lengths and parsed type -- hash identically before and after. All 222 completed datagrams still resolve to `HTTP`, memoised on first read, and `'packet' in datagram` still answers without triggering a parse. Full suite 851 passed, 17 skipped. --- pcapkit/foundation/reassembly/data/data.py | 78 +++++++++++++++++++++- pcapkit/foundation/reassembly/data/ip.py | 58 +--------------- pcapkit/foundation/reassembly/data/tcp.py | 13 +++- pcapkit/foundation/reassembly/tcp.py | 3 +- 4 files changed, 90 insertions(+), 62 deletions(-) diff --git a/pcapkit/foundation/reassembly/data/data.py b/pcapkit/foundation/reassembly/data/data.py index 558be51361..522e24cf63 100644 --- a/pcapkit/foundation/reassembly/data/data.py +++ b/pcapkit/foundation/reassembly/data/data.py @@ -5,11 +5,13 @@ from pcapkit.corekit.infoclass import Info, info_final -__all__ = ['ReassemblyData', 'Deferred'] +__all__ = ['ReassemblyData', 'Deferred', 'DeferredPacket'] if TYPE_CHECKING: from typing import Callable, Optional + from typing import Any + from pcapkit.const.reg.transtype import TransType from pcapkit.foundation.reassembly.data.ip import Datagram as IP_Datagram from pcapkit.foundation.reassembly.data.tcp import Datagram as TCP_Datagram @@ -19,7 +21,7 @@ class Deferred: """A postponed analysis of a reassembled payload. -A reassembled datagram's ``packet`` is a second, full parse of the payload + A reassembled datagram's ``packet`` is a second, full parse of the payload the datagram just reassembled. Nothing about postponing it is specific to any one reassembler, which is why this lives beside :class:`~pcapkit.foundation.reassembly.data.ReassemblyData` rather than in @@ -72,6 +74,78 @@ def __call__(self) -> 'Protocol': return self.analyze(self.proto, self.payload) +class DeferredPacket: + """Resolves a :class:`Deferred` ``packet`` field on first read. + + A reassembled datagram's ``packet`` is the parsed form of the payload it just + reassembled, and both reassemblers can hand a :class:`Deferred` in its place. + This carries the reading half of that arrangement, so the two ``Datagram`` + models share it rather than each declaring it. + + A subclass has to list ``packet`` in its ``__additional__``. That is what makes + the field lazy at all: :class:`~pcapkit.corekit.infoclass.Info` stores a field + whose name is a *builtin* name under a mangled key and maps it back on the way + out, so ``packet`` never lands in :attr:`~object.__dict__` itself -- which + routes reading it through :meth:`__getattr__`, where the deferred analysis can + run, while ``dict(datagram)``, :meth:`to_dict` and iteration still report the + field under its own name. + + """ + + def __analyse__(self) -> 'Optional[Protocol]': + """Resolve a deferred analysis, at most once. + + Returns: + Parsed IP payload, or :data:`None` for an incomplete datagram. + + """ + key = self.__map__.get('packet', 'packet') + value = self.__dict__[key] + if isinstance(value, Deferred): + value = value() + self.__dict__[key] = value + return value + + def __getattr__(self, name: 'str') -> 'Any': + # NOTE: reached only for names absent from ``__dict__``, which ``packet`` + # always is -- see ``__additional__`` above. Everything else has to raise, + # or a typo would silently answer with a parsed payload. + if name != 'packet': + raise AttributeError(f'{type(self).__name__!r} object has no attribute {name!r}') + return self.__analyse__() + + def __getitem__(self, name: 'str') -> 'Any': + if name == 'packet': + return self.__analyse__() + return super().__getitem__(name) + + def __contains__(self, name: 'object') -> 'bool': + # NOTE: ``Mapping.__contains__`` answers by fetching the value, which + # would run the deferred analysis merely to decide that the field exists. + # ``packet`` is a declared field, so it is always there. + return name == 'packet' or super().__contains__(name) + + def __str__(self) -> 'str': + self.__analyse__() + return super().__str__() + + def __repr__(self) -> 'str': + self.__analyse__() + return super().__repr__() + + def to_dict(self) -> 'dict[str, Any]': + """Convert :class:`Datagram` into :obj:`dict`. + + Returns: + The datagram's fields, with ``packet`` analysed if it had not been + read yet -- a :obj:`dict` holding a :class:`Deferred` would leak an + implementation detail into what is meant to be plain data. + + """ + self.__analyse__() + return super().to_dict() + + @info_final class ReassemblyData(Info): """Data storage for reassembly.""" diff --git a/pcapkit/foundation/reassembly/data/ip.py b/pcapkit/foundation/reassembly/data/ip.py index bc48a84006..d39395a460 100644 --- a/pcapkit/foundation/reassembly/data/ip.py +++ b/pcapkit/foundation/reassembly/data/ip.py @@ -4,7 +4,7 @@ from typing import TYPE_CHECKING, Generic, TypeVar from pcapkit.corekit.infoclass import Info, info_final -from pcapkit.foundation.reassembly.data.data import Deferred +from pcapkit.foundation.reassembly.data.data import Deferred, DeferredPacket from pcapkit.utilities.compat import Tuple __all__ = [ @@ -71,7 +71,7 @@ def __init__(self, src: '_AT', dst: '_AT', id: 'int', proto: 'TransType') -> 'No @info_final -class Datagram(Info, Generic[_AT]): +class Datagram(DeferredPacket, Info, Generic[_AT]): """Data model for :term:`IPv4 ` and/or :term:`IPv6 ` reassembled datagram.""" @@ -107,60 +107,6 @@ def __init__(self, completed: 'Literal[False]', id: 'DatagramID[_AT]', index: 't def __init__(self, completed: 'bool', id: 'DatagramID[_AT]', index: 'tuple[int, ...]', header: 'bytes', payload: 'bytes | tuple[bytes, ...]', packet: 'Optional[Protocol | Deferred]') -> 'None': ... # pylint: disable=unused-argument,super-init-not-called,multiple-statements,line-too-long,redefined-builtin - def __analyse__(self) -> 'Optional[Protocol]': - """Resolve a deferred analysis, at most once. - - Returns: - Parsed IP payload, or :data:`None` for an incomplete datagram. - - """ - key = self.__map__.get('packet', 'packet') - value = self.__dict__[key] - if isinstance(value, Deferred): - value = value() - self.__dict__[key] = value - return value - - def __getattr__(self, name: 'str') -> 'Any': - # NOTE: reached only for names absent from ``__dict__``, which ``packet`` - # always is -- see ``__additional__`` above. Everything else has to raise, - # or a typo would silently answer with a parsed payload. - if name != 'packet': - raise AttributeError(f'{type(self).__name__!r} object has no attribute {name!r}') - return self.__analyse__() - - def __getitem__(self, name: 'str') -> 'Any': - if name == 'packet': - return self.__analyse__() - return super().__getitem__(name) - - def __contains__(self, name: 'object') -> 'bool': - # NOTE: ``Mapping.__contains__`` answers by fetching the value, which - # would run the deferred analysis merely to decide that the field exists. - # ``packet`` is a declared field, so it is always there. - return name == 'packet' or super().__contains__(name) - - def __str__(self) -> 'str': - self.__analyse__() - return super().__str__() - - def __repr__(self) -> 'str': - self.__analyse__() - return super().__repr__() - - def to_dict(self) -> 'dict[str, Any]': - """Convert :class:`Datagram` into :obj:`dict`. - - Returns: - The datagram's fields, with ``packet`` analysed if it had not been - read yet -- a :obj:`dict` holding a :class:`Deferred` would leak an - implementation detail into what is meant to be plain data. - - """ - self.__analyse__() - return super().to_dict() - - @info_final class Buffer(Info, Generic[_AT]): """Data model for :term:`IPv4 ` and/or diff --git a/pcapkit/foundation/reassembly/data/tcp.py b/pcapkit/foundation/reassembly/data/tcp.py index b06950f17a..3a0f2b95e7 100644 --- a/pcapkit/foundation/reassembly/data/tcp.py +++ b/pcapkit/foundation/reassembly/data/tcp.py @@ -4,6 +4,7 @@ from typing import TYPE_CHECKING, Generic, TypeVar from pcapkit.corekit.infoclass import Info, info_final +from pcapkit.foundation.reassembly.data.data import Deferred, DeferredPacket from pcapkit.utilities.compat import Tuple __all__ = [ @@ -77,11 +78,15 @@ def __init__(self, src: 'tuple[_AT, int]', dst: 'tuple[_AT, int]', ack: 'int') - @info_final -class Datagram(Info, Generic[_AT]): +class Datagram(DeferredPacket, Info, Generic[_AT]): """Data model for :term:`TCP `.""" #: Completed flag. completed: 'bool' + #: Listing ``packet`` here is what makes it lazy -- see + #: :class:`~pcapkit.foundation.reassembly.data.data.DeferredPacket`. + __additional__ = ['packet'] + #: Original packet identifier. id: 'DatagramID[_AT]' #: Packet numbers. @@ -91,16 +96,18 @@ class Datagram(Info, Generic[_AT]): #: Reassembled payload (application layer data). payload: 'bytes | tuple[bytes, ...]' #: Parsed reassembled payload. + #: Parsed TCP payload. Analysed on first read rather than at construction; + #: a :class:`Deferred` may be passed in its place. packet: 'Optional[Protocol]' if TYPE_CHECKING: @overload # pylint: disable=used-before-assignment - def __init__(self, completed: 'Literal[True]', id: 'DatagramID[_AT]', index: 'tuple[int, ...]', header: 'bytes', payload: 'bytes', packet: 'Protocol') -> 'None': ... # pylint: disable=unused-argument,super-init-not-called,multiple-statements,line-too-long,redefined-builtin + def __init__(self, completed: 'Literal[True]', id: 'DatagramID[_AT]', index: 'tuple[int, ...]', header: 'bytes', payload: 'bytes', packet: 'Protocol | Deferred') -> 'None': ... # pylint: disable=unused-argument,super-init-not-called,multiple-statements,line-too-long,redefined-builtin @overload def __init__(self, completed: 'Literal[False]', id: 'DatagramID[_AT]', index: 'tuple[int, ...]', header: 'bytes', payload: 'tuple[bytes, ...]', packet: 'None') -> 'None': ... # pylint: disable=unused-argument,super-init-not-called,multiple-statements,line-too-long,redefined-builtin - def __init__(self, completed: 'bool', id: 'DatagramID[_AT]', index: 'tuple[int, ...]', header: 'bytes', payload: 'bytes | tuple[bytes, ...]', packet: 'Optional[Protocol]') -> 'None': ... # pylint: disable=unused-argument,super-init-not-called,multiple-statements,line-too-long,redefined-builtin + def __init__(self, completed: 'bool', id: 'DatagramID[_AT]', index: 'tuple[int, ...]', header: 'bytes', payload: 'bytes | tuple[bytes, ...]', packet: 'Optional[Protocol | Deferred]') -> 'None': ... # pylint: disable=unused-argument,super-init-not-called,multiple-statements,line-too-long,redefined-builtin @info_final diff --git a/pcapkit/foundation/reassembly/tcp.py b/pcapkit/foundation/reassembly/tcp.py index f42528906c..0fb8434404 100644 --- a/pcapkit/foundation/reassembly/tcp.py +++ b/pcapkit/foundation/reassembly/tcp.py @@ -12,6 +12,7 @@ import sys from typing import TYPE_CHECKING +from pcapkit.foundation.reassembly.data.data import Deferred from pcapkit.foundation.reassembly.data.tcp import (Buffer, BufferID, Datagram, DatagramID, Fragment, HoleDescriptor, Packet) from pcapkit.foundation.reassembly.reassembly import ReassemblyBase as Reassembly @@ -295,7 +296,7 @@ def submit(self, buf: 'Buffer', *, bufid: 'BufferID') -> 'list[Datagram]': # ty index=tuple(buffer.ind), header=buf.hdr, payload=bytes(payload), - packet=self.protocol.analyze((bufid[1], bufid[3]), bytes(payload)), + packet=Deferred(self.protocol.analyze, (bufid[1], bufid[3]), bytes(payload)), ) datagram.append(packet) From 6f438738ecca2beaf7c966715b501a231788f580 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Wed, 16 Sep 2026 19:49:46 -0400 Subject: [PATCH 6/8] reassembly: reference the TransType enum instead of hard-coded Next Header values Per review on #424. The header walk had `44` and `51` as module literals when the library already names them: they are now `Enum_TransType.IPv6_Frag` and `Enum_TransType.AH`, so a reader does not have to trust a comment to know which protocol a number means. `_NH_IPV6_FRAG` is gone entirely, since the comparison reads better against the enum member. `_NH_AH` stays as a name -- bound to `Enum_TransType.AH` -- because its comment carries the reason the walk singles that header out at all: AH is the one extension header that does not measure its length in 8-octet units (:rfc:`4302#section-2.2`). `__protocol_type__` is `None` on `IPv6`, `IPv6_Frag` and `AH`, so there is no protocol-class attribute to prefer over the const enum here; checked rather than assumed. The two remaining literals stay literals, and now say why: `_IPV6_HDR_LEN` (40) and `_IPV6_NEXT_HEADER` (6) are byte offsets into the header rather than protocol numbers, so no enumeration carries them. Full suite 851 passed, 17 skipped; IPv6 and TCP reassembly both still resolve their deferred payloads (UDP and HTTP). --- pcapkit/foundation/reassembly/ipv6.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/pcapkit/foundation/reassembly/ipv6.py b/pcapkit/foundation/reassembly/ipv6.py index f054f97a3d..cce92a2176 100644 --- a/pcapkit/foundation/reassembly/ipv6.py +++ b/pcapkit/foundation/reassembly/ipv6.py @@ -12,6 +12,7 @@ """ from typing import TYPE_CHECKING +from pcapkit.const.reg.transtype import TransType as Enum_TransType from pcapkit.foundation.reassembly.ip import IP from pcapkit.protocols.internet.ipv6 import IPv6 as IPv6_Protocol @@ -21,19 +22,18 @@ __all__ = ['IPv6'] #: Length of the fixed IPv6 header, i.e. the offset of the first extension -#: header (:rfc:`8200#section-3`). +#: header (:rfc:`8200#section-3`). A literal because it is a *byte offset* into +#: the header rather than a protocol number, so no enumeration carries it. _IPV6_HDR_LEN = 40 -#: Offset of the Next Header field within the fixed IPv6 header. +#: Offset of the Next Header field within the fixed IPv6 header. A literal for +#: the same reason as :data:`_IPV6_HDR_LEN`. _IPV6_NEXT_HEADER = 6 -#: Next Header value of the IPv6 Fragment header (:rfc:`8200#section-4.5`). -_NH_IPV6_FRAG = 44 - -#: Next Header value of the Authentication Header. It is the one extension -#: header that does not measure its length in 8-octet units -#: (:rfc:`4302#section-2.2`), so the header walk below has to special-case it. -_NH_AH = 51 +#: Next Header value of the Authentication Header. Named here only to record +#: *why* the walk below singles it out: it is the one extension header that does +#: not measure its length in 8-octet units (:rfc:`4302#section-2.2`). +_NH_AH = Enum_TransType.AH def _next_header_offset(header: 'bytes') -> 'int': @@ -138,6 +138,6 @@ def _rectify_header(self, header: 'bytes', proto: 'TransType') -> 'bytes': if len(header) < _IPV6_HDR_LEN: return header offset = _next_header_offset(header) - if header[offset] != _NH_IPV6_FRAG: + if header[offset] != Enum_TransType.IPv6_Frag: return header return header[:offset] + bytes((int(proto),)) + header[offset + 1:] From 454bce788ba76b105a0a1d2b9e4b0f2aa730777c Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Wed, 16 Sep 2026 21:29:50 -0400 Subject: [PATCH 7/8] reassembly: inline the AH enum, and move the Deferred docs to follow the class Two review findings on #424. `_NH_AH` is gone; the comparison reads `Enum_TransType.AH` directly, with the reason the walk singles that header out moved to the use site rather than living on a constant that existed only to carry it. Four-adapter parity re-checked after the change: 40/40/1488 and `header[6] == 17` on default, dpkt and scapy alike. The docs gap was mine and larger than the comment suggested. `DeferredPacket` was in `data/data.py`'s `__all__` but never re-exported from the package, so autodoc could not import it at all; and `Deferred` was documented in `reassembly/ip/ip.rst` from when it lived in `data/ip.py`, so my new entry beside `ReassemblyData` made it a duplicate registration. The stale `ip.rst` entry is removed -- the class moved, so its documentation moves with it -- and the two `:class:` references in `ipv4.rst` and `ipv6.rst` are repointed. `TraceFlowData` was already documented; what it lacked was the same treatment for the module it now lives in. I also added `.. module::` directives for both `data.data` modules and then removed them again: they double-register everything the autoclass paths already cover, which is what produced the duplicate warning. Clean build: 85 warning/error lines, exactly main's baseline, with all four of `ReassemblyData`, `Deferred`, `DeferredPacket` and `TraceFlowData` rendering and no warning naming `data.data`. Full suite 857 passed, 17 skipped. --- docs/source/pcapkit/foundation/reassembly/index.rst | 8 ++++++++ docs/source/pcapkit/foundation/reassembly/ip/ip.rst | 3 --- docs/source/pcapkit/foundation/reassembly/ip/ipv4.rst | 2 +- docs/source/pcapkit/foundation/reassembly/ip/ipv6.rst | 2 +- pcapkit/foundation/reassembly/data/__init__.py | 4 ++-- pcapkit/foundation/reassembly/ipv6.py | 11 ++++------- 6 files changed, 16 insertions(+), 14 deletions(-) diff --git a/docs/source/pcapkit/foundation/reassembly/index.rst b/docs/source/pcapkit/foundation/reassembly/index.rst index f236da9a64..e7a254194f 100644 --- a/docs/source/pcapkit/foundation/reassembly/index.rst +++ b/docs/source/pcapkit/foundation/reassembly/index.rst @@ -56,3 +56,11 @@ Auxiliary Data .. autoclass:: pcapkit.foundation.reassembly.data.ReassemblyData :members: :show-inheritance: + +.. autoclass:: pcapkit.foundation.reassembly.data.Deferred + :members: + :show-inheritance: + +.. autoclass:: pcapkit.foundation.reassembly.data.DeferredPacket + :members: + :show-inheritance: diff --git a/docs/source/pcapkit/foundation/reassembly/ip/ip.rst b/docs/source/pcapkit/foundation/reassembly/ip/ip.rst index ea96166537..b5e3f7be68 100644 --- a/docs/source/pcapkit/foundation/reassembly/ip/ip.rst +++ b/docs/source/pcapkit/foundation/reassembly/ip/ip.rst @@ -46,9 +46,6 @@ Data Models :members: :show-inheritance: -.. autoclass:: pcapkit.foundation.reassembly.data.ip.Deferred - :members: - :show-inheritance: Type Variables -------------- diff --git a/docs/source/pcapkit/foundation/reassembly/ip/ipv4.rst b/docs/source/pcapkit/foundation/reassembly/ip/ipv4.rst index 756d4c7adc..2eaf596c17 100644 --- a/docs/source/pcapkit/foundation/reassembly/ip/ipv4.rst +++ b/docs/source/pcapkit/foundation/reassembly/ip/ipv4.rst @@ -91,7 +91,7 @@ Terminology every caller for a result most never read. Reading the attribute, or any mapping view of it (``datagram['packet']``, ``to_dict()``, ``items()``, ``repr()``), runs it and keeps the result; see - :class:`~pcapkit.foundation.reassembly.data.ip.Deferred`. + :class:`~pcapkit.foundation.reassembly.data.Deferred`. reasm.ipv4.buffer Data structure for internal buffering when performing reassembly algorithms diff --git a/docs/source/pcapkit/foundation/reassembly/ip/ipv6.rst b/docs/source/pcapkit/foundation/reassembly/ip/ipv6.rst index 150670b97c..5a222d63aa 100644 --- a/docs/source/pcapkit/foundation/reassembly/ip/ipv6.rst +++ b/docs/source/pcapkit/foundation/reassembly/ip/ipv6.rst @@ -124,7 +124,7 @@ Terminology every caller for a result most never read. Reading the attribute, or any mapping view of it (``datagram['packet']``, ``to_dict()``, ``items()``, ``repr()``), runs it and keeps the result; see - :class:`~pcapkit.foundation.reassembly.data.ip.Deferred`. + :class:`~pcapkit.foundation.reassembly.data.Deferred`. reasm.ipv6.buffer Data structure for internal buffering when performing reassembly algorithms diff --git a/pcapkit/foundation/reassembly/data/__init__.py b/pcapkit/foundation/reassembly/data/__init__.py index d1b69dbb2b..6737e6df40 100644 --- a/pcapkit/foundation/reassembly/data/__init__.py +++ b/pcapkit/foundation/reassembly/data/__init__.py @@ -2,7 +2,7 @@ """data models for reassembly""" # shared -from pcapkit.foundation.reassembly.data.data import Deferred, ReassemblyData +from pcapkit.foundation.reassembly.data.data import Deferred, DeferredPacket, ReassemblyData # IP reassembly from pcapkit.foundation.reassembly.data.ip import Buffer as IP_Buffer @@ -21,7 +21,7 @@ from pcapkit.foundation.reassembly.data.tcp import Packet as TCP_Packet __all__ = [ - 'ReassemblyData', 'Deferred', + 'ReassemblyData', 'Deferred', 'DeferredPacket', 'IP_Packet', 'IP_DatagramID', 'IP_Datagram', 'IP_Buffer', 'IP_BufferID', diff --git a/pcapkit/foundation/reassembly/ipv6.py b/pcapkit/foundation/reassembly/ipv6.py index cce92a2176..58297e1aa2 100644 --- a/pcapkit/foundation/reassembly/ipv6.py +++ b/pcapkit/foundation/reassembly/ipv6.py @@ -30,12 +30,6 @@ #: the same reason as :data:`_IPV6_HDR_LEN`. _IPV6_NEXT_HEADER = 6 -#: Next Header value of the Authentication Header. Named here only to record -#: *why* the walk below singles it out: it is the one extension header that does -#: not measure its length in 8-octet units (:rfc:`4302#section-2.2`). -_NH_AH = Enum_TransType.AH - - def _next_header_offset(header: 'bytes') -> 'int': """Locate the Next Header field of a datagram's last header. @@ -63,7 +57,10 @@ def _next_header_offset(header: 'bytes') -> 'int': # header's Next Header value -- hence ``proto`` trailing one step behind. while position + 1 < len(header): offset = position - if proto == _NH_AH: + # NOTE: AH is the one extension header that does not measure its + # length in 8-octet units (:rfc:`4302#section-2.2`), which is why it + # is singled out rather than falling to the common case below. + if proto == Enum_TransType.AH: position += (header[position + 1] + 2) * 4 else: position += (header[position + 1] + 1) * 8 From f9d9474a4f14908189a238116523a9ec676cddba Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Wed, 16 Sep 2026 22:00:21 -0400 Subject: [PATCH 8/8] docs: name the real module for the shared data classes Per review on #424: the docs described `ReassemblyData`, `Deferred`, `DeferredPacket` and `TraceFlowData` by their re-export path, which said where to import them from rather than where they are. They are now documented as `...data.data.`, under a `.. module::` directive for each of the two new modules. That is also what the sibling pages already do -- `reassembly/tcp.rst` documents `...data.tcp.Packet` and `ip/ip.rst` documents `...data.ip.Datagram`, both under a `.. module::` naming the defining module -- so the re-export paths were the odd ones out, mine included. The `.. module::` directives are back and no longer duplicate anything: the earlier duplicate-registration warning came from `Deferred` being documented in two places at once (the stale `ip/ip.rst` entry, since removed, plus the new one), not from the directive. Imports are deliberately unchanged. `extraction.py` still imports from `...data`, the public re-export, so the documentation reflects where a class lives while consumers keep a path that does not move when it does. Clean build: 85 warning/error lines, exactly main's baseline, nothing naming `data.data`, all four classes rendering under the canonical path and the `ipv4`/`ipv6` cross-references resolving to it. Foundation tests 163 passed, 11 skipped. --- docs/source/pcapkit/foundation/reassembly/index.rst | 8 +++++--- docs/source/pcapkit/foundation/reassembly/ip/ipv4.rst | 2 +- docs/source/pcapkit/foundation/reassembly/ip/ipv6.rst | 2 +- docs/source/pcapkit/foundation/traceflow/index.rst | 4 +++- pcapkit/foundation/reassembly/data/data.py | 2 +- 5 files changed, 11 insertions(+), 7 deletions(-) diff --git a/docs/source/pcapkit/foundation/reassembly/index.rst b/docs/source/pcapkit/foundation/reassembly/index.rst index e7a254194f..35aaf8357d 100644 --- a/docs/source/pcapkit/foundation/reassembly/index.rst +++ b/docs/source/pcapkit/foundation/reassembly/index.rst @@ -53,14 +53,16 @@ Auxiliary Data :members: :show-inheritance: -.. autoclass:: pcapkit.foundation.reassembly.data.ReassemblyData +.. module:: pcapkit.foundation.reassembly.data.data + +.. autoclass:: pcapkit.foundation.reassembly.data.data.ReassemblyData :members: :show-inheritance: -.. autoclass:: pcapkit.foundation.reassembly.data.Deferred +.. autoclass:: pcapkit.foundation.reassembly.data.data.Deferred :members: :show-inheritance: -.. autoclass:: pcapkit.foundation.reassembly.data.DeferredPacket +.. autoclass:: pcapkit.foundation.reassembly.data.data.DeferredPacket :members: :show-inheritance: diff --git a/docs/source/pcapkit/foundation/reassembly/ip/ipv4.rst b/docs/source/pcapkit/foundation/reassembly/ip/ipv4.rst index 2eaf596c17..8b262531c5 100644 --- a/docs/source/pcapkit/foundation/reassembly/ip/ipv4.rst +++ b/docs/source/pcapkit/foundation/reassembly/ip/ipv4.rst @@ -91,7 +91,7 @@ Terminology every caller for a result most never read. Reading the attribute, or any mapping view of it (``datagram['packet']``, ``to_dict()``, ``items()``, ``repr()``), runs it and keeps the result; see - :class:`~pcapkit.foundation.reassembly.data.Deferred`. + :class:`~pcapkit.foundation.reassembly.data.data.Deferred`. reasm.ipv4.buffer Data structure for internal buffering when performing reassembly algorithms diff --git a/docs/source/pcapkit/foundation/reassembly/ip/ipv6.rst b/docs/source/pcapkit/foundation/reassembly/ip/ipv6.rst index 5a222d63aa..bca430df5a 100644 --- a/docs/source/pcapkit/foundation/reassembly/ip/ipv6.rst +++ b/docs/source/pcapkit/foundation/reassembly/ip/ipv6.rst @@ -124,7 +124,7 @@ Terminology every caller for a result most never read. Reading the attribute, or any mapping view of it (``datagram['packet']``, ``to_dict()``, ``items()``, ``repr()``), runs it and keeps the result; see - :class:`~pcapkit.foundation.reassembly.data.Deferred`. + :class:`~pcapkit.foundation.reassembly.data.data.Deferred`. reasm.ipv6.buffer Data structure for internal buffering when performing reassembly algorithms diff --git a/docs/source/pcapkit/foundation/traceflow/index.rst b/docs/source/pcapkit/foundation/traceflow/index.rst index 98ee0a9c65..b2c4c95fe5 100644 --- a/docs/source/pcapkit/foundation/traceflow/index.rst +++ b/docs/source/pcapkit/foundation/traceflow/index.rst @@ -54,6 +54,8 @@ Auxiliary Data :members: :show-inheritance: -.. autoclass:: pcapkit.foundation.traceflow.data.TraceFlowData +.. module:: pcapkit.foundation.traceflow.data.data + +.. autoclass:: pcapkit.foundation.traceflow.data.data.TraceFlowData :members: :show-inheritance: diff --git a/pcapkit/foundation/reassembly/data/data.py b/pcapkit/foundation/reassembly/data/data.py index 522e24cf63..3abda6f4ce 100644 --- a/pcapkit/foundation/reassembly/data/data.py +++ b/pcapkit/foundation/reassembly/data/data.py @@ -24,7 +24,7 @@ class Deferred: A reassembled datagram's ``packet`` is a second, full parse of the payload the datagram just reassembled. Nothing about postponing it is specific to any one reassembler, which is why this lives beside - :class:`~pcapkit.foundation.reassembly.data.ReassemblyData` rather than in + :class:`~pcapkit.foundation.reassembly.data.data.ReassemblyData` rather than in either protocol's data module. IP reassembly is the case that made it necessary. It submits a datagram for