diff --git a/pcapkit/corekit/fields/field.py b/pcapkit/corekit/fields/field.py index 12eb9e7212..841555b9a1 100644 --- a/pcapkit/corekit/fields/field.py +++ b/pcapkit/corekit/fields/field.py @@ -376,9 +376,10 @@ 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) return struct.pack(self.template, pre_processed) diff --git a/tests/corekit/test_fields_bytearray_pack_2004_unit.py b/tests/corekit/test_fields_bytearray_pack_2004_unit.py index d43ff544c3..8849409e2d 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 hands a :obj:`bytearray` for ``c`` over as +:obj:`bytes` too, and it packs identically on every interpreter. + """ 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,33 @@ 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_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__':