diff --git a/pcapkit/corekit/fields/collections.py b/pcapkit/corekit/fields/collections.py index 0e9de0e3db..29182bbe31 100644 --- a/pcapkit/corekit/fields/collections.py +++ b/pcapkit/corekit/fields/collections.py @@ -135,9 +135,15 @@ def pack(self, value: 'Optional[list[_TL]]', packet: 'dict[str, Any]') -> 'bytes Returns: Packed field value. + Without an ``item_type``, :meth:`unpack` returns the field's octets as + a single :obj:`bytes`, so such a value is written as-is rather than + iterated item by item. + """ if value is None: return b'' + if self._item_type is None and isinstance(value, bytes): + return value from pcapkit.protocols.schema.schema import \ Schema # pylint: disable=import-outside-top-level diff --git a/tests/corekit/test_field_roundtrip_unit.py b/tests/corekit/test_field_roundtrip_unit.py index 7b6d153f85..d1b6c5aa4a 100644 --- a/tests/corekit/test_field_roundtrip_unit.py +++ b/tests/corekit/test_field_roundtrip_unit.py @@ -563,13 +563,6 @@ class FieldRoundTripTests(KnownFailureTable, unittest.TestCase): '(pcapkit/protocols/schema/link/vlan.py:47-51), which no protocol parses with', 'WIRE', '', ('NumberField/bit-length-12/wire/ffff', 'NumberField/bit-length-12/wire/f000', 'EnumField/bit-length-3/wire/ff')), - Gap(1489, 'ListField without an item_type unpacks to bytes ' - '(pcapkit/corekit/fields/collections.py:179), but ListField.pack iterates its ' - 'value item by item and refuses each int of those bytes ' - '(pcapkit/corekit/fields/collections.py:146, :154). Not reachable through ' - 'Schema.pack, which writes a bytes value itself ' - '(pcapkit/protocols/schema/schema.py:846)', - 'PACK', 'has invalid value', ('ListField/bytes/*',)), ) def setUp(self) -> None: diff --git a/tests/corekit/test_listfield_untyped_pack_1489_unit.py b/tests/corekit/test_listfield_untyped_pack_1489_unit.py new file mode 100644 index 0000000000..ebc1a96926 --- /dev/null +++ b/tests/corekit/test_listfield_untyped_pack_1489_unit.py @@ -0,0 +1,66 @@ +# -*- coding: utf-8 -*- +"""An untyped ``ListField`` packs back the ``bytes`` its ``unpack`` returns. + +GitHub issue #1489. Without an ``item_type``, :meth:`ListField.unpack +` returns the field's +octets as one :obj:`bytes`, but :meth:`ListField.pack +` iterated that value item +by item and rejected each :obj:`int` with ``FieldValueError``. ``pack`` now +writes such a value as-is; a typed ``ListField`` is unchanged. +""" + +import unittest + +from pcapkit.corekit.fields.collections import ListField +from pcapkit.corekit.fields.numbers import UInt16Field +from pcapkit.utilities.exceptions import FieldValueError + + +def named(field: 'ListField') -> 'ListField': + """Name ``field`` by hand, as a schema would.""" + field.name = 'items' + return field + + +class UntypedListFieldPackTests(unittest.TestCase): + """``pack(unpack(raw))`` is byte-exact for an untyped ``ListField``.""" + + def test_fixed_length_round_trip(self) -> None: + field = named(ListField(length=4))({}) + value = field.unpack(b'abcd', {}) + self.assertEqual(value, b'abcd') + self.assertEqual(field.pack(value, {}), b'abcd') + + def test_callable_length_round_trip(self) -> None: + for raw in (b'', b'\x00', b'\xff\x00\x7f', bytes(range(256))): + with self.subTest(raw=raw): + packet = {'n': len(raw)} + field = named(ListField(length=lambda pkt: pkt['n']))(packet) + self.assertEqual(field.pack(field.unpack(raw, packet), packet), raw) + + def test_list_of_bytes_items_still_joined(self) -> None: + field = named(ListField(length=4))({}) + self.assertEqual(field.pack([b'ab', b'cd'], {}), b'abcd') + + def test_list_of_ints_still_rejected(self) -> None: + field = named(ListField(length=2))({}) + with self.assertRaisesRegex(FieldValueError, 'has invalid value'): + field.pack([1, 2], {}) + + +class TypedListFieldPackTests(unittest.TestCase): + """A typed ``ListField`` still packs item by item.""" + + def test_typed_round_trip(self) -> None: + field = named(ListField(length=4, item_type=UInt16Field()))({}) + value = field.unpack(b'\x00\x01\x00\x02', {}) + self.assertEqual(value, [1, 2]) + self.assertEqual(field.pack(value, {}), b'\x00\x01\x00\x02') + + def test_typed_bytes_value_still_iterated(self) -> None: + field = named(ListField(length=4, item_type=UInt16Field()))({}) + self.assertEqual(field.pack(b'\x01\x02', {}), b'\x00\x01\x00\x02') + + +if __name__ == '__main__': + unittest.main()