From 4c8ce4fb38df8745b9d437ccfbf0ab6364cdab6e Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Wed, 21 Oct 2026 11:07:17 -0400 Subject: [PATCH 1/2] fix(corekit): refuse a non-bytes value for the c format on every engine (#2015) GraalPy's struct packs a bytearray for the c format, where CPython's raises struct.error, so Field.pack now refuses anything but bytes for c itself with StructError (a struct.error subclass). The test pins that error and adds a lenient struct stand-in that reproduces GraalPy's behaviour on CPython. --- pcapkit/corekit/fields/field.py | 12 +++- .../test_fields_bytearray_pack_2004_unit.py | 55 +++++++++++++++++-- 2 files changed, 60 insertions(+), 7 deletions(-) diff --git a/pcapkit/corekit/fields/field.py b/pcapkit/corekit/fields/field.py index 12eb9e7212..890d8f9f1c 100644 --- a/pcapkit/corekit/fields/field.py +++ b/pcapkit/corekit/fields/field.py @@ -29,7 +29,7 @@ from pcapkit.corekit.sentinels import NO_VALUE from pcapkit.utilities import config -from pcapkit.utilities.exceptions import FieldValueError, NoDefaultValue, ProtocolError +from pcapkit.utilities.exceptions import FieldValueError, NoDefaultValue, ProtocolError, StructError __all__ = ['Field'] @@ -365,6 +365,10 @@ def pack(self, value: 'Optional[_T]', packet: 'dict[str, Any]') -> 'bytes': Returns: Packed field value. + Raises: + StructError: If :attr:`template` is a ``c`` format and the value is + not :obj:`bytes`. + """ if value is None: if self._default is NO_VALUE: @@ -380,6 +384,12 @@ def pack(self, value: 'Optional[_T]', packet: 'dict[str, Any]') -> 'bytes': # does a bytearray for the ``c`` format, which CPython rejects too. if isinstance(pre_processed, bytearray) and self.template[-1:] in ('s', 'p'): pre_processed = bytes(pre_processed) + # NOTE: CPython's ``c`` format takes bytes alone, GraalPy's takes a + # bytearray as well (#2015), so the refusal is made here rather than left + # to the engine's struct. + if self.template[-1:] == 'c' and not isinstance(pre_processed, bytes): + raise StructError(f'Field {self.name}: char format requires a bytes object ' + f'of length 1, not {type(pre_processed).__name__}') return struct.pack(self.template, pre_processed) def post_process(self, value: 'Any', packet: 'dict[str, Any]') -> '_T': # pylint: disable=unused-argument diff --git a/tests/corekit/test_fields_bytearray_pack_2004_unit.py b/tests/corekit/test_fields_bytearray_pack_2004_unit.py index d43ff544c3..61f461a377 100644 --- a/tests/corekit/test_fields_bytearray_pack_2004_unit.py +++ b/tests/corekit/test_fields_bytearray_pack_2004_unit.py @@ -12,6 +12,10 @@ :func:`struct.pack` then pins that the output is unchanged and that a value refused before -- a :obj:`memoryview`, a :obj:`bool`, a :obj:`str` -- still is. +GitHub issue #2015. GraalPy's ``c`` format takes a :obj:`bytearray`, where +CPython's refuses it, so the field refuses anything but :obj:`bytes` for ``c`` +itself, with :exc:`~pcapkit.utilities.exceptions.StructError`. + """ import struct @@ -54,6 +58,24 @@ def pack(self, fmt: str, *values: object) -> bytes: return struct.pack(fmt, *values) +class _LenientStruct: + """A :mod:`struct` whose :func:`pack` takes any buffer for ``c``, as GraalPy's does. + + GraalPy's ``c`` format packs a :obj:`bytearray` (#2015), where CPython's + raises :exc:`struct.error`. Everything else is the real :mod:`struct`. + + """ + + def __getattr__(self, name: str) -> object: + return getattr(struct, name) + + def pack(self, fmt: str, *values: object) -> bytes: + if fmt[-1:] == 'c': + values = tuple(bytes(value) if isinstance(value, (bytearray, memoryview)) else value + for value in values) + return struct.pack(fmt, *values) + + class TestBytearrayPack(unittest.TestCase): """Pin how :meth:`Field.pack` hands a :obj:`bytearray` to :func:`struct.pack`.""" @@ -99,16 +121,37 @@ def test_refused_values_stay_refused(self) -> None: with self.assertRaises((struct.error, TypeError)): field.pack(value, {}) # type: ignore[attr-defined] - def test_bytearray_for_the_c_format_stays_refused(self) -> None: - # CPython's ``c`` format refuses a bytearray as well, so converting one - # there would accept what no interpreter accepted before. + def _char(self) -> 'object': from pcapkit.corekit.fields.strings import BytesField char = BytesField(length=1) char._template = 'c' - self.assertEqual(char.pack(b'a', {}), b'a') - with self.assertRaises((struct.error, TypeError)): - char.pack(bytearray(b'a'), {}) + return char + + def test_bytearray_for_the_c_format_stays_refused(self) -> None: + # CPython's ``c`` format refuses a bytearray as well, so converting one + # there would accept what no interpreter accepted before. + from pcapkit.utilities.exceptions import StructError + + char = self._char() + self.assertEqual(char.pack(b'a', {}), b'a') # type: ignore[attr-defined] + for value in (bytearray(b'a'), _ByteArray(b'a'), memoryview(b'a')): + with self.subTest(value=type(value).__name__): + with self.assertRaises(StructError): + char.pack(value, {}) # type: ignore[attr-defined] + + def test_c_format_refuses_a_buffer_under_a_struct_that_takes_it(self) -> None: + # GraalPy's ``c`` format takes a bytearray (#2015), so Field.pack refuses + # one itself rather than leaving it to the engine's struct. + from pcapkit.utilities.exceptions import StructError + + char = self._char() + with mock.patch('pcapkit.corekit.fields.field.struct', _LenientStruct()): + self.assertEqual(char.pack(b'a', {}), b'a') # type: ignore[attr-defined] + for value in (bytearray(b'a'), _ByteArray(b'a'), memoryview(b'a')): + with self.subTest(value=type(value).__name__): + with self.assertRaises(StructError): + char.pack(value, {}) # type: ignore[attr-defined] if __name__ == '__main__': From f9fe66a76df2117dad87755e54c93f1358f68bbd Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Wed, 21 Oct 2026 11:35:54 -0400 Subject: [PATCH 2/2] fix(corekit): convert a bytearray for the c format instead of refusing it (#2015) Field.pack now hands a bytearray for c over as bytes, as it already does for s and p, so it packs identically under CPython's struct (which refuses one) and GraalPy's (which takes one). The StructError refusal is dropped. A memoryview is still passed through unconverted. Tests pin the c packing under the real struct and a lenient stand-in. --- pcapkit/corekit/fields/field.py | 19 ++------ .../test_fields_bytearray_pack_2004_unit.py | 48 +++++++++---------- 2 files changed, 27 insertions(+), 40 deletions(-) diff --git a/pcapkit/corekit/fields/field.py b/pcapkit/corekit/fields/field.py index 890d8f9f1c..841555b9a1 100644 --- a/pcapkit/corekit/fields/field.py +++ b/pcapkit/corekit/fields/field.py @@ -29,7 +29,7 @@ from pcapkit.corekit.sentinels import NO_VALUE from pcapkit.utilities import config -from pcapkit.utilities.exceptions import FieldValueError, NoDefaultValue, ProtocolError, StructError +from pcapkit.utilities.exceptions import FieldValueError, NoDefaultValue, ProtocolError __all__ = ['Field'] @@ -365,10 +365,6 @@ def pack(self, value: 'Optional[_T]', packet: 'dict[str, Any]') -> 'bytes': Returns: Packed field value. - Raises: - StructError: If :attr:`template` is a ``c`` format and the value is - not :obj:`bytes`. - """ if value is None: if self._default is NO_VALUE: @@ -380,16 +376,11 @@ def pack(self, value: 'Optional[_T]', packet: 'dict[str, Any]') -> 'bytes': # NOTE: CPython's ``s`` and ``p`` formats take a bytearray, PyPy's take # bytes alone (``TypeError: expected bytes, got bytearray object``), so a # bytearray is handed over as bytes, packing identically on both (#2004). - # Nothing CPython refuses is let through: a memoryview stays refused, as - # does a bytearray for the ``c`` format, which CPython rejects too. - if isinstance(pre_processed, bytearray) and self.template[-1:] in ('s', 'p'): + # The ``c`` format is converted too: CPython's refuses a bytearray and + # GraalPy's takes one, so converting it packs alike everywhere (#2015). + # A memoryview is left to the engine, which CPython refuses for all three. + if isinstance(pre_processed, bytearray) and self.template[-1:] in ('s', 'p', 'c'): pre_processed = bytes(pre_processed) - # NOTE: CPython's ``c`` format takes bytes alone, GraalPy's takes a - # bytearray as well (#2015), so the refusal is made here rather than left - # to the engine's struct. - if self.template[-1:] == 'c' and not isinstance(pre_processed, bytes): - raise StructError(f'Field {self.name}: char format requires a bytes object ' - f'of length 1, not {type(pre_processed).__name__}') return struct.pack(self.template, pre_processed) def post_process(self, value: 'Any', packet: 'dict[str, Any]') -> '_T': # pylint: disable=unused-argument diff --git a/tests/corekit/test_fields_bytearray_pack_2004_unit.py b/tests/corekit/test_fields_bytearray_pack_2004_unit.py index 61f461a377..8849409e2d 100644 --- a/tests/corekit/test_fields_bytearray_pack_2004_unit.py +++ b/tests/corekit/test_fields_bytearray_pack_2004_unit.py @@ -13,8 +13,8 @@ refused before -- a :obj:`memoryview`, a :obj:`bool`, a :obj:`str` -- still is. GitHub issue #2015. GraalPy's ``c`` format takes a :obj:`bytearray`, where -CPython's refuses it, so the field refuses anything but :obj:`bytes` for ``c`` -itself, with :exc:`~pcapkit.utilities.exceptions.StructError`. +CPython's refuses it, so the field hands a :obj:`bytearray` for ``c`` over as +:obj:`bytes` too, and it packs identically on every interpreter. """ @@ -128,30 +128,26 @@ def _char(self) -> 'object': char._template = 'c' return char - def test_bytearray_for_the_c_format_stays_refused(self) -> None: - # CPython's ``c`` format refuses a bytearray as well, so converting one - # there would accept what no interpreter accepted before. - from pcapkit.utilities.exceptions import StructError - - char = self._char() - self.assertEqual(char.pack(b'a', {}), b'a') # type: ignore[attr-defined] - for value in (bytearray(b'a'), _ByteArray(b'a'), memoryview(b'a')): - with self.subTest(value=type(value).__name__): - with self.assertRaises(StructError): - char.pack(value, {}) # type: ignore[attr-defined] - - def test_c_format_refuses_a_buffer_under_a_struct_that_takes_it(self) -> None: - # GraalPy's ``c`` format takes a bytearray (#2015), so Field.pack refuses - # one itself rather than leaving it to the engine's struct. - from pcapkit.utilities.exceptions import StructError - - char = self._char() - with mock.patch('pcapkit.corekit.fields.field.struct', _LenientStruct()): - self.assertEqual(char.pack(b'a', {}), b'a') # type: ignore[attr-defined] - for value in (bytearray(b'a'), _ByteArray(b'a'), memoryview(b'a')): - with self.subTest(value=type(value).__name__): - with self.assertRaises(StructError): - char.pack(value, {}) # type: ignore[attr-defined] + def test_bytearray_for_the_c_format_packs_as_bytes(self) -> None: + # CPython's ``c`` format refuses a bytearray and GraalPy's takes one, so + # Field.pack converts it, and both pack it as the equal bytes (#2015). + for label, stand_in in (('real struct', struct), ('lenient struct', _LenientStruct())): + for value in (bytearray(b'a'), _ByteArray(b'a')): + with self.subTest(struct=label, value=type(value).__name__): + char = self._char() + with mock.patch('pcapkit.corekit.fields.field.struct', stand_in): + self.assertEqual(char.pack(value, {}), char.pack(b'a', {})) # type: ignore[attr-defined] + self.assertEqual(char.pack(value, {}), b'a') # type: ignore[attr-defined] + + def test_memoryview_for_the_c_format_is_left_to_struct(self) -> None: + # Only a bytearray is converted; CPython refuses a memoryview for ``c``, + # as for ``s`` and ``p``, so Field.pack hands one over untouched and the + # engine's struct decides, rather than pcapkit accepting it (#2015). + value = memoryview(b'a') + stand_in = mock.Mock(wraps=_LenientStruct()) + with mock.patch('pcapkit.corekit.fields.field.struct', stand_in): + self._char().pack(value, {}) # type: ignore[attr-defined] + self.assertIs(stand_in.pack.call_args.args[1], value) if __name__ == '__main__':