From ae5fffc909a09188e237bc9578c92a8214bccb8b Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Tue, 6 Oct 2026 16:44:56 -0400 Subject: [PATCH] fix(pcap): Header.pack() writes the same magic for every endian/nanosecond setting; _make_data returns magic_number - Header._make_data returned the raw `magic_number`, which make() does not take, so from_data dropped it with a warning and rebuilt every header as the host-order microsecond default (d4c3b2a1 on little-endian hosts). It now returns `byteorder` and `nanosecond`, so from_data(Header(...).info) is byte-identical for all four magics. - A no-argument pack() rebuilding from make() defaults is the contract of every protocol (Ethernet behaves the same), so it is left as is. - Drop the Header entry, the table's only one, from the recorded _make_data mismatches in test_construction_keyword_check_unit. - test_header_frame_unit's stub MagicNumber gains the byteorder and nanosecond fields the real one carries. Closes #1125 --- pcapkit/protocols/misc/pcap/header.py | 3 +- .../misc/pcap/test_header_frame_unit.py | 4 +- .../pcap/test_header_make_data_magic_unit.py | 57 +++++++++++++++++++ .../test_construction_keyword_check_unit.py | 32 +---------- 4 files changed, 63 insertions(+), 33 deletions(-) create mode 100644 tests/protocols/misc/pcap/test_header_make_data_magic_unit.py diff --git a/pcapkit/protocols/misc/pcap/header.py b/pcapkit/protocols/misc/pcap/header.py index f0a2ffe514..7cba752a40 100644 --- a/pcapkit/protocols/misc/pcap/header.py +++ b/pcapkit/protocols/misc/pcap/header.py @@ -305,7 +305,8 @@ def _make_data(cls, data: 'Data_Header') -> 'dict[str, Any]': # type: ignore[ov """ return { - 'magic_number': data.magic_number.data, + 'byteorder': data.magic_number.byteorder, + 'nanosecond': data.magic_number.nanosecond, 'version_major': data.version.major, 'version_minor': data.version.minor, 'thiszone': data.thiszone, diff --git a/tests/protocols/misc/pcap/test_header_frame_unit.py b/tests/protocols/misc/pcap/test_header_frame_unit.py index c75f78531f..e52a40bac5 100644 --- a/tests/protocols/misc/pcap/test_header_frame_unit.py +++ b/tests/protocols/misc/pcap/test_header_frame_unit.py @@ -51,7 +51,7 @@ def test_header_make_length_index_and_make_data(self) -> None: network=LinkType.ETHERNET, ) data = DummyData( - magic_number=DummyData(data=b'\xa1\xb2\x3c\x4d'), + magic_number=DummyData(data=b'\xa1\xb2\x3c\x4d', byteorder='big', nanosecond=True), version=SimpleNamespace(major=2, minor=4), thiszone=0, sigfigs=0, @@ -71,6 +71,8 @@ def test_header_make_length_index_and_make_data(self) -> None: self.assertEqual(override_schema.version_minor, 9) self.assertEqual(override_schema.network, LinkType.ETHERNET) self.assertEqual(Header._make_data(data)['network'], LinkType.ETHERNET) + self.assertEqual(Header._make_data(data)['byteorder'], 'big') + self.assertTrue(Header._make_data(data)['nanosecond']) with self.assertRaises(UnsupportedCall): Header.__index__() diff --git a/tests/protocols/misc/pcap/test_header_make_data_magic_unit.py b/tests/protocols/misc/pcap/test_header_make_data_magic_unit.py new file mode 100644 index 0000000000..0c510e7d1d --- /dev/null +++ b/tests/protocols/misc/pcap/test_header_make_data_magic_unit.py @@ -0,0 +1,57 @@ +from __future__ import annotations + +import importlib.util +import io +import unittest +import warnings + +from tests._support import reimport_once_per_class + +RUNTIME_DEPS = ('tbtrim', 'aenum', 'chardet', 'dictdumper') +HAS_RUNTIME = all(importlib.util.find_spec(name) is not None for name in RUNTIME_DEPS) + +MAGIC = { + ('little', False): b'\xd4\xc3\xb2\xa1', + ('little', True): b'\x4d\x3c\xb2\xa1', + ('big', False): b'\xa1\xb2\xc3\xd4', + ('big', True): b'\xa1\xb2\x3c\x4d', +} + + +@unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed') +class PCAPHeaderMakeDataMagicUnitTests(unittest.TestCase): + """``Header._make_data`` keeps the byte order and resolution (GH-1125).""" + + def setUp(self) -> None: + reimport_once_per_class(self) + + def test_make_data_keys_are_make_keywords(self) -> None: + from pcapkit.protocols.misc.pcap.header import Header + from pcapkit.protocols.protocol import _declared_keywords + + accepted = _declared_keywords(Header) + self.assertIsNotNone(accepted) + made = Header._make_data(Header(byteorder='big', nanosecond=True).info) + self.assertEqual(set(made) - set(accepted), set()) + + def test_from_data_round_trips_every_magic(self) -> None: + from pcapkit.const.reg.linktype import LinkType + from pcapkit.protocols.misc.pcap.header import Header + + for (byteorder, nanosecond), magic in MAGIC.items(): + with self.subTest(byteorder=byteorder, nanosecond=nanosecond): + built = Header(byteorder=byteorder, nanosecond=nanosecond, + thiszone=-3600, snaplen=1500, network=LinkType.ETHERNET) + self.assertEqual(built.data[:4], magic) + + with warnings.catch_warnings(): + warnings.simplefilter('error') + rebuilt = Header.from_data(built.info) + self.assertEqual(rebuilt.data, built.data) + + parsed = Header(io.BytesIO(built.data)) + self.assertEqual(Header.from_data(parsed.info).data, built.data) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/protocols/test_construction_keyword_check_unit.py b/tests/protocols/test_construction_keyword_check_unit.py index 4c42d9ad56..1c1477276d 100644 --- a/tests/protocols/test_construction_keyword_check_unit.py +++ b/tests/protocols/test_construction_keyword_check_unit.py @@ -601,8 +601,7 @@ def test_from_data_warns_instead_of_raising(self) -> None: ``__init__``, so nobody typed those keywords: a mismatch is a defect in the protocol's own pair of mappings, and the caller who meets it cannot fix it. Raising there would also convert a latent defect of that shape into a - broken ``from_data`` -- see - :meth:`test_the_known_make_data_mismatches_are_recorded`. + broken ``from_data``. ``L2TPv2`` is subclassed with a ``_make_data`` that returns one key no signature declares, so the case does not depend on a real mismatch that @@ -631,35 +630,6 @@ def _make_data(cls, data: 'Any') -> 'dict[str, Any]': self.assertIn("'prio'", str(messages[0].message)) self.assertIn('_make_data', str(messages[0].message)) - def test_the_known_make_data_mismatches_are_recorded(self) -> None: - """The latent defects this check made visible, named so they can be fixed. - - Each of these has ``_make_data`` returning a key that no signature of the - same protocol declares, so :meth:`ProtocolBase.from_data - ` drops that field -- - a capture's byte order. It is recorded rather than fixed here because it - is a defect in its own protocol rather than in this mechanism. - - Written as an expected-failure table for the reason the round-trip module - writes its own that way: fixing one of these turns this red and the entry - gets deleted, where a silent skip would leave the defect recorded forever. - - """ - from pcapkit.protocols.misc.pcap.header import Header - from pcapkit.protocols.protocol import _declared_keywords - - # protocol -> the ``_make_data`` key it returns that nothing declares - recorded = {Header: 'magic_number'} - - for protocol, key in recorded.items(): - with self.subTest(protocol=protocol.__name__): - accepted = _declared_keywords(protocol) - self.assertIsNotNone(accepted) - self.assertNotIn(key, accepted, ( - f'{protocol.__name__} now declares {key!r}, so its _make_data ' - f'mismatch is fixed -- delete this entry' - )) - @unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed') class AsymmetryTests(unittest.TestCase):