From 8032e7e4116e00d371a167ad7830dabbe09f4953 Mon Sep 17 00:00:00 2001 From: Qiu-LingYan Date: Tue, 27 Oct 2026 10:33:28 +0800 Subject: [PATCH 1/4] fix(pcapng): resolve byteorder safely in _make_pcapng_options for raw bytes options _make_pcapng_options read self._byte for raw bytes option items, but only _make_block_shb and the systemd journal entry maker ever set that attribute, so making any other block (IDB, DSB, ...) with a raw bytes option raised AttributeError. Fall back to the bound section context's byteorder, or the system byteorder when no context is available. Fixes #2353 --- pcapkit/protocols/misc/pcapng.py | 6 ++- .../test_pcapng_raw_bytes_option_2353_unit.py | 43 +++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 tests/protocols/test_pcapng_raw_bytes_option_2353_unit.py diff --git a/pcapkit/protocols/misc/pcapng.py b/pcapkit/protocols/misc/pcapng.py index 6f7e5f7b6b..ba8bfc969b 100644 --- a/pcapkit/protocols/misc/pcapng.py +++ b/pcapkit/protocols/misc/pcapng.py @@ -5008,13 +5008,17 @@ def _make_pcapng_options(self, options: 'Option | list[Schema_Option | tuple[Enu if isinstance(options, bytes): return [options], len(options) + byteorder = getattr(self, '_byte', None) + if byteorder is None: + byteorder = self._ctx.section.byteorder if self._ctx is not None else sys.byteorder + has_endofopt = False total_length = 0 if isinstance(options, list): options_list = [] # type: list[Schema_Option | bytes] for schema in options: if isinstance(schema, bytes): - code = Enum_OptionType.get(int.from_bytes(schema[0:2], self._byte, signed=False), + code = Enum_OptionType.get(int.from_bytes(schema[0:2], byteorder, signed=False), namespace=namespace) if code == Enum_OptionType.opt_endofopt: # moved to the end, written once has_endofopt = True diff --git a/tests/protocols/test_pcapng_raw_bytes_option_2353_unit.py b/tests/protocols/test_pcapng_raw_bytes_option_2353_unit.py new file mode 100644 index 0000000000..86301f955b --- /dev/null +++ b/tests/protocols/test_pcapng_raw_bytes_option_2353_unit.py @@ -0,0 +1,43 @@ +# -*- coding: UTF-8 -*- +"""Regression tests for issue #2353. + +A raw ``bytes`` item inside an IDB's ``options=[...]`` list used to raise +``AttributeError: 'PCAPNG' object has no attribute '_byte'``, as +``_make_pcapng_options`` read ``self._byte`` although only the SHB (and the +systemd journal entry maker) ever set that attribute. The options maker now +falls back to the section context's byteorder, or the system byteorder when +no context is bound. +""" + +import unittest + +__all__ = ['RawBytesOptionTest'] + + +class RawBytesOptionTest(unittest.TestCase): + """Test making blocks with a raw ``bytes`` option item.""" + + def test_idb_raw_bytes_option_without_context(self) -> 'None': + """A raw ``bytes`` option in an IDB must not require ``self._byte``.""" + from pcapkit.protocols.misc.pcapng import PCAPNG + + interface = PCAPNG(num=1, sct=1, ctx=None, type=0x00000001, + block={'linktype': 1, 'snaplen': 0x40000, + 'options': [b'\x01\x00\x05\x00hello\x00\x00\x00']}) + self.assertEqual(len(interface.data) % 4, 0) + + def test_idb_raw_bytes_option_after_shb(self) -> 'None': + """Ordering SHB → IDB must keep working (byteorder already set).""" + from pcapkit.foundation.engines.pcapng import Context + from pcapkit.protocols.misc.pcapng import PCAPNG + + section = PCAPNG(num=0, sct=1, ctx=None, type=0x0A0D0D0A, block={'byteorder': 'big'}) + context = Context(section.info) + interface = PCAPNG(num=1, sct=1, ctx=context, type=0x00000001, + block={'linktype': 1, 'snaplen': 0x40000, + 'options': [b'\x01\x00\x05\x00hello\x00\x00\x00']}) + self.assertEqual(len(interface.data) % 4, 0) + + +if __name__ == '__main__': + unittest.main() From 1eaaf77c4d59f60378f451834a0851bace949055 Mon Sep 17 00:00:00 2001 From: Qiu-LingYan Date: Tue, 27 Oct 2026 13:17:51 +0800 Subject: [PATCH 2/4] fix(pcapng): same safe byteorder resolution in _make_nrb_records; tighten tests _make_nrb_records had the identical self._byte read for raw bytes records in an NRB's records=[...]; resolve it the same way as _make_pcapng_options. Tests now compare the raw-bytes build against the equivalent typed-option build, parse the result back through a real section context and read the option payload, and check a from_data rebuild is byte-exact, in both byte orders. The raw option is encoded in the section's own byte order in the big-endian case. --- pcapkit/protocols/misc/pcapng.py | 6 +- .../test_pcapng_raw_bytes_option_2353_unit.py | 101 ++++++++++++++---- 2 files changed, 83 insertions(+), 24 deletions(-) diff --git a/pcapkit/protocols/misc/pcapng.py b/pcapkit/protocols/misc/pcapng.py index ba8bfc969b..896949b7f9 100644 --- a/pcapkit/protocols/misc/pcapng.py +++ b/pcapkit/protocols/misc/pcapng.py @@ -6477,13 +6477,17 @@ def _make_nrb_records(self, records: 'Record | list[Schema_NameResolutionRecord if isinstance(records, bytes): return [records], len(records) + byteorder = getattr(self, '_byte', None) + if byteorder is None: + byteorder = self._ctx.section.byteorder if self._ctx is not None else sys.byteorder + has_record_end = False total_length = 0 if isinstance(records, list): records_list = [] # type: list[Schema_NameResolutionRecord | bytes] for schema in records: if isinstance(schema, bytes): - code = Enum_RecordType.get(int.from_bytes(schema[0:2], self._byte, signed=False)) + code = Enum_RecordType.get(int.from_bytes(schema[0:2], byteorder, signed=False)) if code == Enum_RecordType.nrb_record_end: # moved to the end, written once has_record_end = True continue diff --git a/tests/protocols/test_pcapng_raw_bytes_option_2353_unit.py b/tests/protocols/test_pcapng_raw_bytes_option_2353_unit.py index 86301f955b..b5e775fea9 100644 --- a/tests/protocols/test_pcapng_raw_bytes_option_2353_unit.py +++ b/tests/protocols/test_pcapng_raw_bytes_option_2353_unit.py @@ -1,42 +1,97 @@ # -*- coding: UTF-8 -*- -"""Regression tests for issue #2353. - -A raw ``bytes`` item inside an IDB's ``options=[...]`` list used to raise -``AttributeError: 'PCAPNG' object has no attribute '_byte'``, as -``_make_pcapng_options`` read ``self._byte`` although only the SHB (and the -systemd journal entry maker) ever set that attribute. The options maker now -falls back to the section context's byteorder, or the system byteorder when -no context is bound. +"""Tests for raw ``bytes`` option/record items in PCAP-NG block makers (issue #2353). + +``_make_pcapng_options`` and ``_make_nrb_records`` read ``self._byte`` for raw +``bytes`` items, although only the SHB (and the systemd journal entry maker) +ever set that attribute. The byte order therefore comes from the bound section +context when one exists, and from ``sys.byteorder`` otherwise. + +The assertions compare the built octets against the equivalent typed-option +build, parse the result back with a real section context and read the option's +actual payload, and check a ``from_data`` rebuild is byte-exact. """ import unittest -__all__ = ['RawBytesOptionTest'] +__all__ = ['RawBytesOptionTest', 'RawBytesRecordTest'] class RawBytesOptionTest(unittest.TestCase): """Test making blocks with a raw ``bytes`` option item.""" - def test_idb_raw_bytes_option_without_context(self) -> 'None': - """A raw ``bytes`` option in an IDB must not require ``self._byte``.""" + def test_raw_option_matches_typed_build_without_context(self) -> 'None': + """A raw bytes option builds octets identical to the typed build (no context).""" + from pcapkit.const.pcapng.option_type import OptionType + from pcapkit.protocols.misc.pcapng import PCAPNG + + block = {'linktype': 1, 'snaplen': 0x40000} + typed = PCAPNG(num=1, sct=1, ctx=None, type=0x00000001, + block=dict(block, options=[(OptionType.opt_comment, {'comment': 'hello'})])) + raw = PCAPNG(num=1, sct=1, ctx=None, type=0x00000001, + block=dict(block, options=[b'\x01\x00\x05\x00hello\x00\x00\x00'])) + self.assertEqual(raw.data, typed.data) + + def test_raw_option_roundtrips_in_little_endian_section(self) -> 'None': + """Little-endian section: raw bytes option parses back with its payload.""" + from pcapkit.const.pcapng.option_type import OptionType + from pcapkit.foundation.engines.pcapng import Context from pcapkit.protocols.misc.pcapng import PCAPNG - interface = PCAPNG(num=1, sct=1, ctx=None, type=0x00000001, - block={'linktype': 1, 'snaplen': 0x40000, - 'options': [b'\x01\x00\x05\x00hello\x00\x00\x00']}) - self.assertEqual(len(interface.data) % 4, 0) + shb_bytes = PCAPNG(num=0, sct=1, ctx=None, type=0x0A0D0D0A, + block={'byteorder': 'little'}).data + shb = PCAPNG(shb_bytes, len(shb_bytes), num=0, sct=1, ctx=None) + ctx = Context(shb.info) + + block = {'linktype': 1, 'snaplen': 0x40000} + typed = PCAPNG(num=1, sct=1, ctx=ctx, type=0x00000001, + block=dict(block, options=[(OptionType.opt_comment, {'comment': 'hello'})])) + raw = PCAPNG(num=1, sct=1, ctx=ctx, type=0x00000001, + block=dict(block, options=[b'\x01\x00\x05\x00hello\x00\x00\x00'])) + self.assertEqual(raw.data, typed.data) + + parsed = PCAPNG(raw.data, len(raw.data), num=1, sct=1, ctx=ctx) + comment = parsed.info.options[OptionType.opt_comment].comment + self.assertEqual(comment, 'hello') + self.assertEqual(PCAPNG.from_data(parsed.info, num=1, sct=1, ctx=ctx).data, raw.data) - def test_idb_raw_bytes_option_after_shb(self) -> 'None': - """Ordering SHB → IDB must keep working (byteorder already set).""" + def test_raw_option_roundtrips_in_big_endian_section(self) -> 'None': + """Big-endian section: raw bytes option is encoded in the section byte order.""" + from pcapkit.const.pcapng.option_type import OptionType from pcapkit.foundation.engines.pcapng import Context from pcapkit.protocols.misc.pcapng import PCAPNG - section = PCAPNG(num=0, sct=1, ctx=None, type=0x0A0D0D0A, block={'byteorder': 'big'}) - context = Context(section.info) - interface = PCAPNG(num=1, sct=1, ctx=context, type=0x00000001, - block={'linktype': 1, 'snaplen': 0x40000, - 'options': [b'\x01\x00\x05\x00hello\x00\x00\x00']}) - self.assertEqual(len(interface.data) % 4, 0) + shb_bytes = PCAPNG(num=0, sct=1, ctx=None, type=0x0A0D0D0A, + block={'byteorder': 'big'}).data + shb = PCAPNG(shb_bytes, len(shb_bytes), num=0, sct=1, ctx=None) + ctx = Context(shb.info) + + block = {'linktype': 1, 'snaplen': 0x40000} + typed = PCAPNG(num=1, sct=1, ctx=ctx, type=0x00000001, + block=dict(block, options=[(OptionType.opt_comment, {'comment': 'hello'})])) + raw = PCAPNG(num=1, sct=1, ctx=ctx, type=0x00000001, + block=dict(block, options=[b'\x00\x01\x00\x05hello\x00\x00\x00'])) + self.assertEqual(raw.data, typed.data) + + parsed = PCAPNG(raw.data, len(raw.data), num=1, sct=1, ctx=ctx) + comment = parsed.info.options[OptionType.opt_comment].comment + self.assertEqual(comment, 'hello') + self.assertEqual(PCAPNG.from_data(parsed.info, num=1, sct=1, ctx=ctx).data, raw.data) + + +class RawBytesRecordTest(unittest.TestCase): + """Test making an NRB with a raw ``bytes`` record item.""" + + def test_raw_record_matches_typed_build_without_context(self) -> 'None': + """A raw bytes record builds octets identical to the typed build (no context).""" + from pcapkit.const.pcapng.record_type import RecordType + from pcapkit.protocols.misc.pcapng import PCAPNG + + typed = PCAPNG(num=1, sct=1, ctx=None, type=0x00000004, + block={'records': [(RecordType.nrb_record_ipv4, + {'ip': '8.8.8.8', 'names': ['1.1.1.1']})]}) + raw = PCAPNG(num=1, sct=1, ctx=None, type=0x00000004, + block={'records': [b'\x01\x00\x0c\x00\x08\x08\x08\x081.1.1.1\x00']}) + self.assertEqual(raw.data, typed.data) if __name__ == '__main__': From 6706c7b1e366bb2da1165af1a1c6a12a046cd7f3 Mon Sep 17 00:00:00 2001 From: Qiu-LingYan Date: Tue, 27 Oct 2026 15:49:33 +0800 Subject: [PATCH 3/4] test(pcapng): observe resolved byte order via the duplicate-option check A raw if_name option followed by a typed if_name option is rejected only when both decode to the same code, so the rejection pins the resolved byte order in a little-endian section, a big-endian section and without a context. The no-context tests derive their raw bytes from sys.byteorder so they hold on big-endian hosts, and the module docstring states the current behaviour. For the records maker the decoded type is only compared against the all-zero end-record sentinel, which is byte-order invariant, so its byte order is not externally observable; the NRB test pins the produced octets against the typed build. --- .../test_pcapng_raw_bytes_option_2353_unit.py | 78 ++++++++++++++++--- 1 file changed, 66 insertions(+), 12 deletions(-) diff --git a/tests/protocols/test_pcapng_raw_bytes_option_2353_unit.py b/tests/protocols/test_pcapng_raw_bytes_option_2353_unit.py index b5e775fea9..ee3077e127 100644 --- a/tests/protocols/test_pcapng_raw_bytes_option_2353_unit.py +++ b/tests/protocols/test_pcapng_raw_bytes_option_2353_unit.py @@ -1,21 +1,37 @@ # -*- coding: UTF-8 -*- """Tests for raw ``bytes`` option/record items in PCAP-NG block makers (issue #2353). -``_make_pcapng_options`` and ``_make_nrb_records`` read ``self._byte`` for raw -``bytes`` items, although only the SHB (and the systemd journal entry maker) -ever set that attribute. The byte order therefore comes from the bound section -context when one exists, and from ``sys.byteorder`` otherwise. - -The assertions compare the built octets against the equivalent typed-option -build, parse the result back with a real section context and read the option's -actual payload, and check a ``from_data`` rebuild is byte-exact. +``_make_pcapng_options`` and ``_make_nrb_records`` resolve the byte order for +raw ``bytes`` items from the instance's ``_byte`` when set, from the bound +section context otherwise, and from ``sys.byteorder`` when no context is bound. + +The option tests observe the resolved byte order through the duplicate-option +check: a raw ``if_name`` option followed by a typed ``if_name`` option is +rejected with ``ProtocolError`` only when both decode to the same code, i.e. +when the raw header is encoded in the resolved byte order. The remaining +option assertions compare the built octets against the equivalent typed-option +build, parse the result back through a real section context and read the +option's actual payload, and check a ``from_data`` rebuild is byte-exact. + +The records maker's decoded type is only compared against the all-zero +end-record sentinel, which is byte-order invariant, so its resolved byte order +is not observable from the outside; the NRB test pins the produced octets +against the typed build instead. """ +import struct +import sys import unittest __all__ = ['RawBytesOptionTest', 'RawBytesRecordTest'] +def _raw_option(code: 'int', data: 'bytes', byteorder: 'str') -> 'bytes': + """A raw ``bytes`` option item encoded in the given byte order.""" + fmt = 'HH' + return struct.pack(fmt, code, len(data)) + data + b'\x00' * (-len(data) % 4) + + class RawBytesOptionTest(unittest.TestCase): """Test making blocks with a raw ``bytes`` option item.""" @@ -28,7 +44,7 @@ def test_raw_option_matches_typed_build_without_context(self) -> 'None': typed = PCAPNG(num=1, sct=1, ctx=None, type=0x00000001, block=dict(block, options=[(OptionType.opt_comment, {'comment': 'hello'})])) raw = PCAPNG(num=1, sct=1, ctx=None, type=0x00000001, - block=dict(block, options=[b'\x01\x00\x05\x00hello\x00\x00\x00'])) + block=dict(block, options=[_raw_option(1, b'hello', sys.byteorder)])) self.assertEqual(raw.data, typed.data) def test_raw_option_roundtrips_in_little_endian_section(self) -> 'None': @@ -46,7 +62,7 @@ def test_raw_option_roundtrips_in_little_endian_section(self) -> 'None': typed = PCAPNG(num=1, sct=1, ctx=ctx, type=0x00000001, block=dict(block, options=[(OptionType.opt_comment, {'comment': 'hello'})])) raw = PCAPNG(num=1, sct=1, ctx=ctx, type=0x00000001, - block=dict(block, options=[b'\x01\x00\x05\x00hello\x00\x00\x00'])) + block=dict(block, options=[_raw_option(1, b'hello', 'little')])) self.assertEqual(raw.data, typed.data) parsed = PCAPNG(raw.data, len(raw.data), num=1, sct=1, ctx=ctx) @@ -69,7 +85,7 @@ def test_raw_option_roundtrips_in_big_endian_section(self) -> 'None': typed = PCAPNG(num=1, sct=1, ctx=ctx, type=0x00000001, block=dict(block, options=[(OptionType.opt_comment, {'comment': 'hello'})])) raw = PCAPNG(num=1, sct=1, ctx=ctx, type=0x00000001, - block=dict(block, options=[b'\x00\x01\x00\x05hello\x00\x00\x00'])) + block=dict(block, options=[_raw_option(1, b'hello', 'big')])) self.assertEqual(raw.data, typed.data) parsed = PCAPNG(raw.data, len(raw.data), num=1, sct=1, ctx=ctx) @@ -77,6 +93,41 @@ def test_raw_option_roundtrips_in_big_endian_section(self) -> 'None': self.assertEqual(comment, 'hello') self.assertEqual(PCAPNG.from_data(parsed.info, num=1, sct=1, ctx=ctx).data, raw.data) + def test_duplicate_check_observes_resolved_byte_order(self) -> 'None': + """A raw ``if_name`` option followed by a typed one is rejected (dup check) + only when the raw header decodes to the same code, i.e. when it is + encoded in the resolved byte order.""" + from pcapkit.const.pcapng.option_type import OptionType + from pcapkit.foundation.engines.pcapng import Context + from pcapkit.protocols.misc.pcapng import PCAPNG + from pcapkit.utilities.exceptions import ProtocolError + + block = {'linktype': 1, 'snaplen': 0x40000} + + with self.subTest(section='little'): + shb_bytes = PCAPNG(num=0, sct=1, ctx=None, type=0x0A0D0D0A, + block={'byteorder': 'little'}).data + ctx = Context(PCAPNG(shb_bytes, len(shb_bytes), num=0, sct=1, ctx=None).info) + with self.assertRaises(ProtocolError): + PCAPNG(num=1, sct=1, ctx=ctx, type=0x00000001, + block=dict(block, options=[_raw_option(2, b'eth0', 'little'), + (OptionType.if_name, {'name': 'eth0'})])) + + with self.subTest(section='big'): + shb_bytes = PCAPNG(num=0, sct=1, ctx=None, type=0x0A0D0D0A, + block={'byteorder': 'big'}).data + ctx = Context(PCAPNG(shb_bytes, len(shb_bytes), num=0, sct=1, ctx=None).info) + with self.assertRaises(ProtocolError): + PCAPNG(num=1, sct=1, ctx=ctx, type=0x00000001, + block=dict(block, options=[_raw_option(2, b'eth0', 'big'), + (OptionType.if_name, {'name': 'eth0'})])) + + with self.subTest(section='none'): + with self.assertRaises(ProtocolError): + PCAPNG(num=1, sct=1, ctx=None, type=0x00000001, + block=dict(block, options=[_raw_option(2, b'eth0', sys.byteorder), + (OptionType.if_name, {'name': 'eth0'})])) + class RawBytesRecordTest(unittest.TestCase): """Test making an NRB with a raw ``bytes`` record item.""" @@ -89,8 +140,11 @@ def test_raw_record_matches_typed_build_without_context(self) -> 'None': typed = PCAPNG(num=1, sct=1, ctx=None, type=0x00000004, block={'records': [(RecordType.nrb_record_ipv4, {'ip': '8.8.8.8', 'names': ['1.1.1.1']})]}) + # the typed IPv4 record builder writes the padded length into the + # header, so the raw item is padded before encoding raw = PCAPNG(num=1, sct=1, ctx=None, type=0x00000004, - block={'records': [b'\x01\x00\x0c\x00\x08\x08\x08\x081.1.1.1\x00']}) + block={'records': [_raw_option(1, b'\x08\x08\x08\x081.1.1.1\x00', + sys.byteorder)]}) self.assertEqual(raw.data, typed.data) From f42e0fb7db87a9e827bae214fb827cd273715398 Mon Sep 17 00:00:00 2001 From: Qiu-LingYan Date: Tue, 27 Oct 2026 16:49:25 +0800 Subject: [PATCH 4/4] test(pcapng): pin the _byte and host byte-order tiers; move tests to tests/protocols/misc --- .../test_pcapng_raw_bytes_option_2353_unit.py | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) rename tests/protocols/{ => misc}/test_pcapng_raw_bytes_option_2353_unit.py (85%) diff --git a/tests/protocols/test_pcapng_raw_bytes_option_2353_unit.py b/tests/protocols/misc/test_pcapng_raw_bytes_option_2353_unit.py similarity index 85% rename from tests/protocols/test_pcapng_raw_bytes_option_2353_unit.py rename to tests/protocols/misc/test_pcapng_raw_bytes_option_2353_unit.py index ee3077e127..811013ff69 100644 --- a/tests/protocols/test_pcapng_raw_bytes_option_2353_unit.py +++ b/tests/protocols/misc/test_pcapng_raw_bytes_option_2353_unit.py @@ -129,6 +129,34 @@ def test_duplicate_check_observes_resolved_byte_order(self) -> 'None': (OptionType.if_name, {'name': 'eth0'})])) + def test_byte_order_tiers_are_pinned(self) -> 'None': + """``_byte`` set by an earlier SHB build takes precedence over the + host byte order, and the host byte order is used on a fresh instance + with no context.""" + from unittest import mock + + from pcapkit.const.pcapng.option_type import OptionType + from pcapkit.protocols.misc.pcapng import PCAPNG + from pcapkit.utilities.exceptions import ProtocolError + + block = {'linktype': 1, 'snaplen': 0x40000} + + with self.subTest(tier='_byte'): + instance = PCAPNG(num=0, sct=1, ctx=None, type=0x0A0D0D0A, + block={'byteorder': 'big'}) + with self.assertRaises(ProtocolError): + instance.make(0x00000001, kind='Interface Description Block', + block=dict(block, options=[_raw_option(2, b'eth0', 'big'), + (OptionType.if_name, {'name': 'eth0'})])) + + with self.subTest(tier='host'): + with mock.patch.object(sys, 'byteorder', 'big'): + with self.assertRaises(ProtocolError): + PCAPNG(num=1, sct=1, ctx=None, type=0x00000001, + block=dict(block, options=[_raw_option(2, b'eth0', 'big'), + (OptionType.if_name, {'name': 'eth0'})])) + + class RawBytesRecordTest(unittest.TestCase): """Test making an NRB with a raw ``bytes`` record item."""