From c332e1f31748b4dda1d8ecdbfb7907f42454369c Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Tue, 22 Sep 2026 18:46:28 -0400 Subject: [PATCH] fix(pcapng): keep the captured octets every packet block declares (#646) * Every PCAP-NG packet block reported `packet == b''` while `captured_len` declared hundreds of octets. `PCAPNG.unpack` extracted the payload correctly and `ProtocolBase.__init__` then overwrote it with `self.packet.payload`, which the inherited `packet` had split at `PCAPNG.length` -- the wire's Block Total Length, not a header length -- consuming the whole block as header. * `PCAPNG.packet` is overridden to take the payload from the block schema's `__payload__` field and the header from the octets ahead of it, so the value injected into `_info` is the captured data. A PCAP-NG block carries a trailer after its payload, so no value of `length` could have made the inherited split work; the fix belongs here and not at the injection site. * Affects all three block types that carry captured octets: the Enhanced Packet Block, the Simple Packet Block and the obsolete Packet Block. * `PCAPNG.unpack` now reads that property instead of extracting the payload a second time, leaving one source of truth. * That property is a plain `@property`, not a `cached_property` like the inherited one: the inherited one caches because it reads the stream, this one only walks already-filled schema buffers, and caching it would make a second `unpack` on one instance return the first call's octets. * `ProtocolBase.packet` documents the contract it relies on. Docstring only; no behaviour change outside PCAP-NG. * Dumping through `PCAPIO` wrote record headers promising octets it never wrote -- a 104-octet PCAP for four blocks -- which pcapkit refused on re-read and scapy silently mis-parsed. It now round-trips. tests/protocols/test_pcapng_regression.py grows 4 tests to 11 and 3 subtests to 24, covering both byte orders, options present after the payload, and a snapped block; pcapkit/protocols/misc/pcapng.py holds 99.91% with its single miss unmoved, and pcapkit/protocols/protocol.py is unchanged in every coverage column, as a docstring-only change should be. Fixes #646 --- pcapkit/protocols/misc/pcapng.py | 128 +++++++- pcapkit/protocols/protocol.py | 22 +- tests/protocols/misc/test_pcapng_unit.py | 46 ++- tests/protocols/test_pcapng_regression.py | 369 +++++++++++++++++++++- 4 files changed, 549 insertions(+), 16 deletions(-) diff --git a/pcapkit/protocols/misc/pcapng.py b/pcapkit/protocols/misc/pcapng.py index bfbe1b95db..e69bfedd67 100644 --- a/pcapkit/protocols/misc/pcapng.py +++ b/pcapkit/protocols/misc/pcapng.py @@ -105,6 +105,7 @@ from pcapkit.protocols.data.misc.pcapng import WireGuardKeyLog as Data_WireGuardKeyLog from pcapkit.protocols.data.misc.pcapng import ZigBeeAPSKey as Data_ZigBeeAPSKey from pcapkit.protocols.data.misc.pcapng import ZigBeeNWKKey as Data_ZigBeeNWKKey +from pcapkit.protocols.data.protocol import Packet as Data_Packet from pcapkit.protocols.protocol import ProtocolBase as Protocol from pcapkit.protocols.schema.misc.pcapng import PCAPNG as Schema_PCAPNG from pcapkit.protocols.schema.misc.pcapng import BlockType as Schema_BlockType @@ -677,9 +678,99 @@ def name(self) -> 'str': @property def length(self) -> 'int': - """Header length of corresponding protocol.""" + """Block total length of corresponding protocol. + + Note: + This is the wire's *Block Total Length* -- the whole block, trailing + length field included -- and not a header length. The two are the + same thing for a protocol whose payload runs to the end of its + buffer, which is why + :attr:`ProtocolBase.length ` + does not distinguish them, but a PCAP-NG block carries a trailer. + :attr:`self.packet ` is overridden accordingly; see there and + #646. + + """ return self._info.length + # NOTE: A plain property, where the inherited one is a + # :func:`~pcapkit.utilities.compat.cached_property`. That is deliberate and it + # is not an oversight of the base class's caching: the inherited one caches + # because it *reads the stream*, and a second read would consume octets that + # are no longer there, whereas this one only walks buffers the schema layer has + # already filled and so costs a handful of dict lookups. + # + # Caching it would reintroduce, by a different route, the staleness this change + # exists to remove. :meth:`self.unpack ` now reports the payload + # through this property, so a cache would make a second ``unpack`` on the same + # instance return the *first* call's octets -- ``get_payload`` never even + # reached -- where the code before #646 recomputed from the schema every time. + # Nothing in the tree calls ``unpack`` twice on one instance today + # (``__post_init__`` is its only caller), so this is an invariant being kept + # rather than a bug being fixed; it was held before and there is no reason for + # it to stop holding. A data descriptor also wins over ``__dict__``, so a stale + # entry left by the inherited ``cached_property`` cannot shadow this either. + @property + def packet(self) -> 'Data_Packet': + """Header and payload octets of the current block. + + A PCAP-NG block is not a header followed by a payload. The captured + octets sit in the *middle* of a packet block, ahead of the option list + and the trailing Block Total Length, so no single split point expresses + the shape and the inherited + :attr:`ProtocolBase.packet ` + -- which reads :attr:`self.length ` octets of header and takes + everything after as payload -- cannot produce it. Since + :attr:`self.length ` is the Block Total Length, that split + consumed the entire block as header and left the payload empty: every + packet block reported ``packet == b''`` while ``captured_len`` declared + hundreds of octets, and dumping such a block through + :class:`~pcapkit.dumpkit.pcap.PCAPIO` wrote a record header promising + octets it then did not write. See #646. + + The payload is therefore the block schema's + :attr:`~pcapkit.protocols.schema.schema.Schema.__payload__` field, which + is where the schema layer put the captured octets, and the header is the + block octets ahead of it. The three block types that carry captured + octets -- :attr:`PACKET_TYPES `, i.e. the Enhanced + Packet Block, the Simple Packet Block and the obsolete Packet Block -- + each place the payload at a different offset, so the offset is summed + from the octets the schema actually unpacked rather than hard-coded per + block type. A block declaring no payload field is all header and no + payload, as before. + + """ + block = self.__header__.block + payload_name = self._get_payload_name(block) + if payload_name is None: + return Data_Packet(header=self._data, payload=b'') + + # The octets ahead of the payload field, in wire order: the outer + # schema's fields up to the one holding the block -- its 4-octet Block + # Type -- and then the block's own fields up to the payload. The block is + # found by identity rather than by name so that renaming the field + # cannot silently fold the whole block body into the header; if it is not + # found at all, the header loses the Block Type and nothing else, leaving + # the payload -- the part #646 is about -- exact either way. + names = list(self.__header__.__fields__) + stop = next((idx for idx, name in enumerate(names) + if getattr(self.__header__, name, None) is block), 0) + + block_names = list(block.__fields__) + # Safe: ``_get_payload_name`` returned a name only because it is a field. + block_stop = block_names.index(payload_name) + + header = bytearray() + for name in names[:stop]: + header += self.__header__.__buffer__.get(name, b'') + for name in block_names[:block_stop]: + header += block.__buffer__.get(name, b'') + + return Data_Packet( + header=bytes(header), + payload=block.get_payload(payload_name), + ) + @property def context(self) -> 'Context': """Context of current PCAP-NG block.""" @@ -900,13 +991,14 @@ def unpack(self, length: 'Optional[int]' = None, **kwargs: 'Any') -> 'Data_PCAPN self.__header__ = cast('Schema_PCAPNG', self.__schema__.unpack(self._file, length, packet)) # type: ignore[call-arg,misc] data = self.read(length, **kwargs) - block_schema = self.__header__.block - payload_name = getattr(block_schema, '__payload__', None) - if payload_name in getattr(block_schema, '__fields__', {}): - packet = block_schema.get_payload() - else: - packet = b'' - data.__update__(packet=packet) + + # NOTE: One source of truth for the captured octets. This used to extract + # the payload here as well, and ``ProtocolBase.__init__`` then overwrote + # the result with ``self.packet.payload`` -- which was empty, because the + # inherited ``packet`` split the block at its Block Total Length. Reading + # it through the property instead means the value injected there is the + # value computed here, rather than a second attempt at it. See #646. + data.__update__(packet=self.packet.payload) return data def read(self, length: 'Optional[int]' = None, *, _read: 'bool' = True, @@ -1128,6 +1220,26 @@ def _get_payload(self) -> 'bytes': """ return self.__header__.block.get_payload() + @staticmethod + def _get_payload_name(block: 'Schema_BlockType | bytes') -> 'Optional[str]': + """Get the name of the field carrying a block's captured packet octets. + + Args: + block: Parsed block schema. + + Returns: + The name of the block schema's payload field, or :obj:`None` when the + block declares none -- which is every block type outside + :attr:`PACKET_TYPES `, and also a block whose + body was handed over as raw :obj:`bytes` at construction time rather + than as a schema. + + """ + name = getattr(block, '__payload__', None) + if name is None or name not in getattr(block, '__fields__', {}): + return None + return name + @staticmethod def _get_local_timezone() -> 'timezone': """Get local timezone. diff --git a/pcapkit/protocols/protocol.py b/pcapkit/protocols/protocol.py index 39ec547b7a..bab56764e6 100644 --- a/pcapkit/protocols/protocol.py +++ b/pcapkit/protocols/protocol.py @@ -470,7 +470,27 @@ def protochain(self) -> 'ProtoChain': # packet data @cached_property def packet(self) -> 'Data_Packet': - """Data_Packet data of the protocol.""" + """Data_Packet data of the protocol. + + Note: + The split relies on :attr:`self.length ` being the length of + the octets *preceding* the payload, and on the payload running from + there to the end of the buffer. Both hold for a protocol laid out as + a header followed by its payload, which is nearly all of them. + + A protocol that is not laid out that way has to override this: one + whose :attr:`~length` counts something else, or one carrying a + *trailer* after the payload, gets a header that eats the payload and + a payload of ``b''``. That is what + :class:`~pcapkit.protocols.misc.pcapng.PCAPNG` did to every packet + block -- its :attr:`~pcapkit.protocols.misc.pcapng.PCAPNG.length` is + the wire's Block Total Length and the captured octets sit ahead of + the option list and the trailing length field -- and + :meth:`ProtocolBase.__init__` injects this payload into every parsed + ``_info``, so the empty value reached the dumpers and corrupted the + files they wrote. See #646. + + """ try: return self._read_packet(header=self.length) except UnsupportedCall: diff --git a/tests/protocols/misc/test_pcapng_unit.py b/tests/protocols/misc/test_pcapng_unit.py index 849381a6fa..a8536ef307 100644 --- a/tests/protocols/misc/test_pcapng_unit.py +++ b/tests/protocols/misc/test_pcapng_unit.py @@ -1878,15 +1878,25 @@ def test_pcapng_remaining_constructor_branches_and_custom_dispatch(self) -> None __packet__={'byteorder': 'little'}) self.assertEqual(len(packed), 20) + # NOTE: ``unpack`` injects the captured octets by reading ``self.packet`` + # rather than by extracting them a second time of its own (#646), so these + # stubs have to present the schema surface that property reads: the outer + # schema's ``__fields__``/``__buffer__`` up to the field holding the block, + # and the block's own up to its payload field. unpacker = object.__new__(PCAPNG) unpacker.__header__ = None payload_block = types.SimpleNamespace( __payload__='packet_data', __fields__={'packet_data': object()}, + __buffer__={'packet_data': b'payload'}, get_payload=mock.Mock(return_value=b'payload'), ) unpacker.__schema__ = types.SimpleNamespace( - unpack=mock.Mock(return_value=types.SimpleNamespace(block=payload_block))) + unpack=mock.Mock(return_value=types.SimpleNamespace( + block=payload_block, + __fields__={'type': object(), 'block': object()}, + __buffer__={'type': b'\x03\x00\x00\x00'}, + ))) unpacker._file = io.BytesIO(b'0123456789ab') unpacker._ctx = types.SimpleNamespace(section=types.SimpleNamespace(byteorder='little')) unpacker._byte = 'big' @@ -1894,35 +1904,59 @@ def test_pcapng_remaining_constructor_branches_and_custom_dispatch(self) -> None data = unpacker.unpack(12, __packet__={}) self.assertEqual(data['packet'], b'payload') self.assertEqual(unpacker._byte, 'little') - payload_block.get_payload.assert_called_once_with() - + payload_block.get_payload.assert_called_once_with('packet_data') + # Only the outer fields ahead of the block reach the header: the 4-octet + # block type, and none of the block's own, the payload being its first. + self.assertEqual(unpacker.packet.header, b'\x03\x00\x00\x00') + + # The same instance, unpacked a second time with a different block in place: + # an already-set ``__header__`` must skip re-unpacking the schema, and the + # payload must be recomputed from the block that is there now rather than + # served from the first call. That second half is why ``PCAPNG.packet`` is a + # plain property and not a ``cached_property`` -- caching it would return + # ``b'payload'`` here and never call ``get_payload`` at all. See #646. cached_block = types.SimpleNamespace( __payload__='packet_data', __fields__={'packet_data': object()}, + __buffer__={'packet_data': b'cached'}, get_payload=mock.Mock(return_value=b'cached'), ) - unpacker.__header__ = types.SimpleNamespace(block=cached_block) + unpacker.__header__ = types.SimpleNamespace( + block=cached_block, + __fields__={'type': object(), 'block': object()}, + __buffer__={'type': b'\x03\x00\x00\x00'}, + ) unpacker.read = mock.Mock(return_value=DummyData(length=12)) self.assertEqual(unpacker.unpack(12)['packet'], b'cached') unpacker.__schema__.unpack.assert_called_once() - cached_block.get_payload.assert_called_once_with() + cached_block.get_payload.assert_called_once_with('packet_data') no_ctx_unpacker = object.__new__(PCAPNG) no_ctx_unpacker.__header__ = None no_payload_block = types.SimpleNamespace( __payload__='payload', __fields__={}, + __buffer__={}, get_payload=mock.Mock(return_value=b'unused'), ) no_ctx_unpacker.__schema__ = types.SimpleNamespace( - unpack=mock.Mock(return_value=types.SimpleNamespace(block=no_payload_block))) + unpack=mock.Mock(return_value=types.SimpleNamespace( + block=no_payload_block, + __fields__={'type': object(), 'block': object()}, + __buffer__={'type': b'\x01\x00\x00\x00'}, + ))) no_ctx_unpacker._file = io.BytesIO(b'0123456789ab') + no_ctx_unpacker._data = b'0123456789ab' no_ctx_unpacker._ctx = None no_ctx_unpacker._byte = 'big' no_ctx_unpacker.read = mock.Mock(return_value=DummyData(length=12)) self.assertEqual(no_ctx_unpacker.unpack(12)['packet'], b'') self.assertEqual(no_ctx_unpacker._byte, 'big') no_payload_block.get_payload.assert_not_called() + # A block declaring no payload field is all header: the raw block octets + # verbatim, and no payload. + self.assertEqual(no_ctx_unpacker.packet.header, b'0123456789ab') + self.assertEqual(no_ctx_unpacker.packet.payload, b'') self.assertEqual(pcapng._make_block_shb(major_version=2, section_length=-1).minor, 0) self.assertEqual(pcapng._make_block_shb(major_version=1, minor_version=5, diff --git a/tests/protocols/test_pcapng_regression.py b/tests/protocols/test_pcapng_regression.py index 6b2ca4b288..17a855c7f7 100644 --- a/tests/protocols/test_pcapng_regression.py +++ b/tests/protocols/test_pcapng_regression.py @@ -1,13 +1,35 @@ from __future__ import annotations import importlib.util +import pathlib +import struct +import tempfile import unittest +import warnings -from tests._support import purge_modules, sample_path +from tests._support import close_extractor, purge_modules, sample_path RUNTIME_DEPS = ('tbtrim', 'aenum', 'chardet', 'dictdumper') HAS_RUNTIME = all(importlib.util.find_spec(name) is not None for name in RUNTIME_DEPS) +#: Block types that carry captured packet octets, i.e. the three whose schema +#: declares a ``__payload__``. Spelled as the wire values so that the test does +#: not have to import the library to know what it is looking for. +BLOCK_SHB = 0x0A0D0D0A +BLOCK_IDB = 0x00000001 +BLOCK_PACKET = 0x00000002 # obsolete Packet Block +BLOCK_SPB = 0x00000003 +BLOCK_EPB = 0x00000006 + +#: First sixteen and last sixteen octets of the first Enhanced Packet Block's +#: captured data in :file:`examples/captures/dhcp.pcapng`: a broadcast Ethernet +#: destination, the client's source address, ethertype 0x0800, and the head of the +#: IPv4 header; then the tail of the BOOTP payload. Spelled out so that a reader +#: can see the octets #646 dropped, and so that an off-by-a-field payload offset +#: fails rather than merely reporting the right length. +DHCP_EPB0_HEAD = bytes.fromhex('ffffffffffff000b8201fc4208004500') +DHCP_EPB0_TAIL = bytes.fromhex('000037040103062aff00000000000000') + @unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed') class PcapngRegressionTests(unittest.TestCase): @@ -46,5 +68,350 @@ def test_additional_pcapng_samples_extract_successfully(self) -> None: self.assertGreater(extractor.length, 0) +def _pad32(data: bytes) -> bytes: + """Pad ``data`` out to a 32-bit boundary, the way every PCAP-NG block body is.""" + return data + b'\x00' * ((4 - len(data) % 4) % 4) + + +def _block(block_type: int, body: bytes, endian: str = '<') -> bytes: + """Wrap ``body`` in a PCAP-NG block frame of the given byte order.""" + total = 12 + len(body) + return struct.pack(endian + 'II', block_type, total) + body + struct.pack(endian + 'I', total) + + +def _option(code: int, value: bytes, endian: str = '<') -> bytes: + """One PCAP-NG option: a 2-octet code, a 2-octet length, and padded value.""" + return struct.pack(endian + 'HH', code, len(value)) + _pad32(value) + + +def _ethernet(tail: bytes) -> bytes: + """A well-formed Ethernet frame carrying ``tail`` under an unassigned ethertype. + + 0x88B5 is reserved for local experimental use, so the link layer decodes and + the octets after it stay raw -- the point here is the octets, not what they + would have meant. + + """ + return (b'\x02\x00\x00\x00\x00\x01' + + b'\x02\x00\x00\x00\x00\x02' + + struct.pack('>H', 0x88B5) + + tail) + + +#: One payload per payload-carrying block type, each a different length modulo 4 +#: (0, 3 and 3 octets of block padding respectively) so that a payload offset +#: computed from the wrong field, or a payload taken with its padding attached, +#: cannot pass by coincidence. +SYNTH_PAYLOADS = { + BLOCK_EPB: _ethernet(b'EPB-payload-\x11\x22\x33\x44\x55\x66'), + BLOCK_SPB: _ethernet(b'SPB!\xa0\xa1\xa2\xa3\xa4'), + BLOCK_PACKET: _ethernet(b'obsolete-Packet-Block-\xde\xad\xbe'), +} + + +def _synthetic_pcapng(endian: str = '<') -> bytes: + """A PCAP-NG file holding one block of every payload-carrying type. + + No committed fixture has a Simple Packet Block or an (obsolete) Packet Block -- + :file:`examples/captures/dhcp.pcapng` is all Enhanced Packet Blocks -- so the + other two are built here rather than generated, which also keeps the expected + octets in the same file as the assertion. + + Both packet blocks that can carry options are given some, which no fixture + exercises: ``dhcp.pcapng``'s four blocks all have an empty option area. Options + sit *after* the captured data, so a payload offset walked from the wrong end -- + or one derived by subtracting a trailer of assumed size -- reads them as + payload, and a block with none cannot tell the difference. + + """ + timestamp = 0x0005D0F012345678 + + # The byte-order magic is written in the section's own order, which is how a + # reader detects that order in the first place. + shb = _block(BLOCK_SHB, struct.pack(endian + 'IHHq', 0x1A2B3C4D, 1, 0, -1), endian) + # ETHERNET, snaplen 65535. + idb = _block(BLOCK_IDB, struct.pack(endian + 'HHI', 1, 0, 0xFFFF), endian) + + # epb_flags (code 2, four octets) then opt_endofopt (code 0, empty). + options = _option(2, struct.pack(endian + 'I', 0), endian) + _option(0, b'', endian) + + epb_payload = SYNTH_PAYLOADS[BLOCK_EPB] + epb = _block(BLOCK_EPB, + struct.pack(endian + 'IIIII', 0, timestamp >> 32, timestamp & 0xFFFFFFFF, + len(epb_payload), len(epb_payload)) + + _pad32(epb_payload) + options, endian) + + # A Simple Packet Block carries no option area at all, by specification. + spb_payload = SYNTH_PAYLOADS[BLOCK_SPB] + spb = _block(BLOCK_SPB, + struct.pack(endian + 'I', len(spb_payload)) + _pad32(spb_payload), endian) + + pkb_payload = SYNTH_PAYLOADS[BLOCK_PACKET] + pkb = _block(BLOCK_PACKET, + struct.pack(endian + 'HHIIII', 0, 0, timestamp >> 32, timestamp & 0xFFFFFFFF, + len(pkb_payload), len(pkb_payload)) + + _pad32(pkb_payload) + options, endian) + + return shb + idb + epb + spb + pkb + + +def _epb_payloads_from_octets(raw: bytes) -> 'list[bytes]': + """Hand-parse the captured octets of every Enhanced Packet Block in ``raw``. + + Deliberately written with :mod:`struct` alone, so that the expected values owe + nothing to the code under test: an Enhanced Packet Block puts its captured data + 28 octets in, after the block type, the total length, the interface ID, the two + timestamp halves and the two lengths. + + """ + payloads = [] + offset = 0 + while offset + 12 <= len(raw): + block_type, total = struct.unpack_from(' len(raw): + break + if block_type == BLOCK_EPB: + captured_len = struct.unpack_from(' 'list[tuple[int, int, bytes]]': + """Hand-parse a PCAP file into ``(incl_len, orig_len, octets)`` per record. + + Walks by ``incl_len``, which is how every PCAP reader finds the next record, so + a record header promising octets it did not deliver desynchronises here exactly + as it does in a real reader instead of being quietly tolerated. + + """ + records = [] + offset = 24 # global header + while offset + 16 <= len(raw): + _, _, incl_len, orig_len = struct.unpack_from(' None: + purge_modules(['pcapkit']) + + def _extract(self, path: str) -> 'object': + from pcapkit.interface import extract + + with warnings.catch_warnings(): + # The obsolete Packet Block warns twice by design, and a capture ending + # at its last block warns ``EOF reached``; neither is under test here. + warnings.simplefilter('ignore') + extractor = extract(fin=path, store=True, nofile=True) + self.addCleanup(close_extractor, extractor) + return extractor + + def test_dhcp_pcapng_blocks_carry_their_captured_octets(self) -> None: + """Each Enhanced Packet Block of the committed fixture keeps its own octets.""" + path = sample_path('dhcp.pcapng') + expected = _epb_payloads_from_octets(pathlib.Path(path).read_bytes()) + self.assertEqual(len(expected), 4) + + extractor = self._extract(path) + frames = list(extractor.frame) # type: ignore[attr-defined] + self.assertEqual(len(frames), len(expected)) + + for index, (frame, octets) in enumerate(zip(frames, expected)): + with self.subTest(frame=index): + packet = bytes(frame.info.packet) + self.assertEqual(packet, octets) + self.assertEqual(len(packet), frame.info.captured_len) + + def test_dhcp_pcapng_first_block_octets_are_the_expected_ethernet_frame(self) -> None: + """The first block's octets are the DHCP discover, head and tail spelled out. + + A length assertion alone passes under several wrong fixes -- a payload read + from the wrong offset, or one that picked up the block's 32-bit padding -- + so the boundaries are pinned to literals. + + """ + extractor = self._extract(sample_path('dhcp.pcapng')) + packet = bytes(next(iter(extractor.frame)).info.packet) # type: ignore[attr-defined] + + self.assertEqual(len(packet), 314) + self.assertEqual(packet[:16], DHCP_EPB0_HEAD) + self.assertEqual(packet[-16:], DHCP_EPB0_TAIL) + + def test_every_payload_carrying_block_type_carries_its_octets(self) -> None: + """EPB, SPB and the obsolete Packet Block each keep their own payload. + + All three declare ``__payload__ = 'packet_data'`` and all three put it at a + different offset, so a fix that only reached the Enhanced Packet Block -- + the only type any committed fixture has -- would pass the test above and + still corrupt the other two. + + Run over both byte orders. The offset of the captured data is the same in + either, but the fields it is summed from are not, so a byte order the walk + mishandled would show up as a payload read from the wrong place. + + """ + for endian, name in (('<', 'little'), ('>', 'big')): + with tempfile.TemporaryDirectory() as temp: + path = pathlib.Path(temp) / 'three_block_types.pcapng' + path.write_bytes(_synthetic_pcapng(endian)) + + extractor = self._extract(str(path)) + frames = list(extractor.frame) # type: ignore[attr-defined] + + self.assertEqual(len(frames), 3) + seen = set() + for frame in frames: + block_type = int(frame.info.type) + seen.add(block_type) + with self.subTest(byteorder=name, block_type=hex(block_type)): + self.assertEqual(bytes(frame.info.packet), SYNTH_PAYLOADS[block_type]) + self.assertEqual(bytes(frame.packet.payload), SYNTH_PAYLOADS[block_type]) + self.assertEqual(len(frame.info.packet), frame.info.captured_len) + self.assertEqual(seen, set(SYNTH_PAYLOADS)) + + def test_a_snapped_block_carries_the_octets_that_are_present(self) -> None: + """``captured_len`` below ``original_len`` yields the captured octets only. + + Two shapes of snapping, because the two block types express it differently: + an Enhanced Packet Block declares both lengths, while a Simple Packet Block + declares only the on-wire one and has its captured length bounded by the + interface's ``snaplen``. Neither may come back padded out to the on-wire + length, and neither may come back empty. + + """ + timestamp = 0x0005D0F012345678 + wire = _ethernet(b'0123456789abcdefghij') # 34 octets on the wire + snaplen = 20 + captured = wire[:snaplen] + + shb = _block(BLOCK_SHB, struct.pack('> 32, timestamp & 0xFFFFFFFF, + len(captured), len(wire)) + _pad32(captured)) + spb = _block(BLOCK_SPB, struct.pack(' None: + """``packet.header`` is the octets ahead of the payload, not the whole block. + + The header used to be the entire block -- ``_read_packet(header=self.length)`` + with ``length`` being the Block Total Length -- which is the same bug seen + from the other side. + + """ + with tempfile.TemporaryDirectory() as temp: + path = pathlib.Path(temp) / 'three_block_types.pcapng' + path.write_bytes(_synthetic_pcapng()) + + extractor = self._extract(str(path)) + frames = list(extractor.frame) # type: ignore[attr-defined] + + # The offset of the captured data in each block type, counted from the block + # type field: 28 for an EPB and for the obsolete Packet Block, 12 for an SPB. + offsets = {BLOCK_EPB: 28, BLOCK_SPB: 12, BLOCK_PACKET: 28} + for frame in frames: + block_type = int(frame.info.type) + with self.subTest(block_type=hex(block_type)): + header = bytes(frame.packet.header) + payload = bytes(frame.packet.payload) + self.assertEqual(len(header), offsets[block_type]) + self.assertEqual(header + payload, + bytes(frame._data)[:len(header) + len(payload)]) + + def test_blocks_without_a_payload_field_stay_empty(self) -> None: + """A block type that carries no captured octets keeps reporting none. + + Guards the other direction: the Section Header and Interface Description + blocks declare no ``__payload__``, and must not start reporting their own + body as a payload. + + """ + from pcapkit.protocols.misc.pcapng import PCAPNG + + # ``extractor.frame`` holds only the packet blocks, so these two are read + # directly to reach their ``packet`` field at all. + raw = _synthetic_pcapng() + shb_len = struct.unpack_from(' None: + """A PCAP-NG block dumped through :class:`PCAPIO` writes the octets it promises. + + This is where the defect stopped being an API wart: the record headers came + out 16 octets apart, each declaring hundreds of octets and delivering none, + which desynchronises every reader that walks by ``incl_len``. + + """ + from pcapkit.const.reg.linktype import LinkType + from pcapkit.dumpkit.pcap import PCAPIO + from pcapkit.toolkit.pcapng import block2frame + + source = sample_path('dhcp.pcapng') + expected = _epb_payloads_from_octets(pathlib.Path(source).read_bytes()) + extractor = self._extract(source) + + with tempfile.TemporaryDirectory() as temp: + out = pathlib.Path(temp) / 'dumped.pcap' + dumper = PCAPIO(str(out), protocol=LinkType.ETHERNET, byteorder='little') + for frame in extractor.frame: # type: ignore[attr-defined] + dumper(block2frame(frame.info)) + + dumped = out.read_bytes() + + records = _pcap_records(dumped) + self.assertEqual(len(records), len(expected)) + for index, ((incl_len, orig_len, octets), payload) in enumerate(zip(records, expected)): + with self.subTest(record=index): + self.assertEqual(incl_len, len(payload)) + self.assertEqual(octets, payload) + self.assertGreaterEqual(orig_len, incl_len) + + # 24 octets of global header, then 16 of record header and the payload per + # record, with nothing left over. Before #646 this file was 104 octets for + # these four blocks -- the global header and four record headers, and not one + # octet of payload -- which is both the wrong size and, worse, a size a + # reader cannot tell is wrong until it has already lost frame sync. + self.assertEqual(len(dumped), 24 + sum(16 + len(payload) for payload in expected)) + self.assertNotEqual(len(dumped), 104) + + if __name__ == '__main__': unittest.main()