From 0be8298acd94cac91fe3c4569b6aba8fc4eaba59 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Mon, 26 Oct 2026 16:43:57 -0400 Subject: [PATCH] fix(protocol): raise StructError from _make_pack for any size (#2291) Problem: for a size with no struct format code, _make_pack called int.to_bytes outside any handler, so a value out of range, a non-integer or a bad size leaked OverflowError, AttributeError or ValueError instead of the documented StructError; _make_index(..., pack=True) leaked them too. Fix: raise StructError from the errors of int.to_bytes, as the struct.pack path does from struct.error, and document StructError on _make_index. Test: test_make_pack_raises_struct_error_for_every_size. --- pcapkit/protocols/protocol.py | 9 +++++++- tests/protocols/test_protocol_base_unit.py | 24 ++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/pcapkit/protocols/protocol.py b/pcapkit/protocols/protocol.py index e25506d6b7..e70a80b3e9 100644 --- a/pcapkit/protocols/protocol.py +++ b/pcapkit/protocols/protocol.py @@ -1886,7 +1886,12 @@ def _make_pack(cls, integer: 'int', *, size: 'int' = 1, if kind is None: end = 'little' if lilendian else 'big' # type: Literal['little', 'big'] - buf = integer.to_bytes(size, end, signed=signed) + # ``int.to_bytes`` raises these where ``struct.pack`` raises + # ``struct.error`` for the other sizes (#2291). + try: + buf = integer.to_bytes(size, end, signed=signed) + except (AttributeError, OverflowError, TypeError, ValueError) as error: + raise StructError(f'{cls.__name__}: pack failed') from error else: try: fmt = f'{endian}{kind}' @@ -1964,6 +1969,8 @@ def _make_index(cls, name: 'str | int | StdlibEnum | AenumEnum', default: 'Optio Raises: ProtocolNotImplemented: If ``name`` is **NOT** in ``namespace`` and ``default`` is :data:`None`. + StructError: If ``pack`` is :data:`True` and the index could not be + packed into ``size`` octets. Note: If ``name`` is a :obj:`str` and ``namespace`` is :data:`None`, diff --git a/tests/protocols/test_protocol_base_unit.py b/tests/protocols/test_protocol_base_unit.py index 257b9bfb07..eec65f783a 100644 --- a/tests/protocols/test_protocol_base_unit.py +++ b/tests/protocols/test_protocol_base_unit.py @@ -460,6 +460,30 @@ def test_lookup_next_layer_reads_the_fallback_without_recording_it(self) -> None self.assertIsInstance(proto._import_next_layer(99, 3), Raw) self.assertEqual(set(DummyProtocol.__proto__), {7, 8}) + def test_make_pack_raises_struct_error_for_every_size(self) -> None: + # #2291: a size with no struct format code leaked the errors of + # int.to_bytes instead of the StructError the other sizes raise. + DummyProtocol, _, _ = self._make_protocol_class() + from pcapkit.utilities.exceptions import StructError + + cases = ( + (2 ** 24, {'size': 3}), # too big + (-1, {'size': 3}), # negative, unsigned + (2 ** 23, {'size': 3, 'signed': True}), # too big, signed + (1.5, {'size': 3}), # not an integer + (None, {'size': 5}), # not an integer + (1, {'size': 0}), # no room at all + (1, {'size': -1}), # negative size + (1, {'size': 3.0}), # size is not an integer + ) + for integer, kwargs in cases: + with self.subTest(integer=integer, **kwargs): + with self.assertRaises(StructError): + DummyProtocol._make_pack(integer, **kwargs) + with self.assertRaises(StructError): + DummyProtocol._make_index(2 ** 24, pack=True, size=3) + self.assertEqual(DummyProtocol._make_pack(-2, size=3, signed=True), b'\xff\xff\xfe') + def test_make_payload_branches(self) -> None: DummyProtocol, DummyData, _ = self._make_protocol_class() from pcapkit.protocols.misc.null import NoPayload