fix(corekit): keep a BitField's unnamed bits as captured (#1487)

pcapkit/corekit/fields/strings.py

@@ -265,8 +265,17 @@ class BitField(_TextField[Dict[str, Any]]):
         callback: Callback function to be called upon
             :meth:`self.__call__ <pcapkit.corekit.fields.field.FieldBase.__call__>`.
 
+    Bits that no subfield names are kept as captured: where any of them is
+    set, the unpacked value carries them under an ``__unnamed__`` key, as an
+    integer over the whole field with every named bit cleared, and packing
+    writes them back (:issue:`1487`). The key is absent when they are all
+    zero, so a value built without it packs them as zeros.
+
     """
 
+    #: Key of the unpacked value that carries the unnamed bits.
+    UNNAMED = '__unnamed__'
+
     def __init__(self, length: 'int',
                  default: 'dict[str, Any] | NoValueType' = NO_VALUE,
                  namespace: 'Optional[dict[str, NamespaceEntry]]' = None,
@@ -288,6 +297,11 @@ def __init__(self, length: 'int',
                     f'{start}-{start + size - 1} of a {width}-bit field'
                 )
 
+        #: Mask of the bits no subfield names, most significant bit first.
+        self._unnamed = (1 << width) - 1
+        for start, size in self._namespace.values():
+            self._unnamed &= ~(((1 << size) - 1) << (width - start - size))
+
     def pre_process(self, value: 'dict[str, Any]', packet: 'dict[str, Any]') -> 'bytes':  # pylint: disable=unused-argument
         """Process field value before construction (packing).
 
@@ -300,15 +314,24 @@ def pre_process(self, value: 'dict[str, Any]', packet: 'dict[str, Any]') -> 'byt
 
         Raises:
             FieldValueError: If a subfield value does not fit the bits it was
-                declared to occupy.
+                declared to occupy, or the ``__unnamed__`` value is not an
+                integer.
 
         """
         # NOTE: The buffer holds one ASCII digit per bit, so it must be seeded with
         # ``b'0'`` rather than with NUL bytes -- a zero bit written by the loop below
         # is the *character* ``b'0'``, which is itself non-zero, so a truthiness test
         # over the buffer cannot tell a cleared bit from a set one and would report
         # every named bit as set.
-        buffer = bytearray(b'0' * (self.length * 8))
+        #
+        # The unnamed bits seed it, so that they come back as captured; masking
+        # them keeps a stray named bit in the record from overriding a subfield.
+        unnamed = value.get(self.UNNAMED, 0)
+        if isinstance(unnamed, bool) or not isinstance(unnamed, int):
+            raise FieldValueError(
+                f'{type(self).__name__}: {self.UNNAMED} must be an int, not {unnamed!r}'
+            )
+        buffer = bytearray(f'{unnamed & self._unnamed:0{self.length * 8}b}'.encode())
         for name, (start, size) in self._namespace.items():
             bits = f'{value[name]:0{size}b}'
             if len(bits) > size:
@@ -335,6 +358,9 @@ def post_process(self, value: 'bytes', packet: 'dict[str, Any]') -> 'dict[str, A
         for name, (start, len) in self._namespace.items():
             end = start + len
             buffer[name] = int(binary[start:end], 2)
+        unnamed = int.from_bytes(value, 'big') & self._unnamed
+        if unnamed:
+            buffer[self.UNNAMED] = unnamed
         return buffer
 
 

tests/corekit/test_bitfield_unnamed_bits_1487_unit.py

@@ -0,0 +1,89 @@
+# -*- coding: utf-8 -*-
+"""A BitField packs back the bits its namespace does not name.
+
+GitHub issue #1487: :class:`~pcapkit.corekit.fields.strings.BitField` read only
+its named subfields and packed from all zeros, so octets with a bit outside
+every subfield set came back with it cleared. The unpacked value now carries
+those bits under ``__unnamed__`` when any is set, and a value without the key
+packs them as zeros, as before.
+
+:mod:`pcapkit` is imported inside each test, after
+:func:`~tests._support.reimport_once_per_class`, not at module load.
+
+"""
+
+import unittest
+
+from tests._support import reimport_once_per_class
+
+#: ``a`` is bit 0 and ``c`` bits 8-11; bits 1-7 and 12-15 are unnamed.
+NAMESPACE = {'a': (0, 1), 'c': (8, 4)}
+
+
+class BitFieldUnnamedBitsTests(unittest.TestCase):
+    """GitHub issue #1487."""
+
+    def setUp(self) -> None:
+        reimport_once_per_class(self)
+
+        from pcapkit.corekit.fields.strings import BitField
+
+        self.field = BitField(length=2, namespace=NAMESPACE)({})
+
+    def _round_trip(self, raw: bytes) -> bytes:
+        return self.field.pack(self.field.unpack(raw, {}), {})
+
+    def test_unnamed_bits_survive_unpack_then_pack(self) -> None:
+        for raw in (b'\xff\xff', b'\x01\x00', b'\x7f\x0f', b'\x00\x01', b'\x80\xf0', b'\x00\x00'):
+            with self.subTest(raw=raw.hex()):
+                self.assertEqual(self._round_trip(raw), raw)
+
+    def test_unpacked_value_names_the_unnamed_bits(self) -> None:
+        self.assertEqual(self.field.unpack(b'\xff\xff', {}),
+                         {'a': 1, 'c': 15, '__unnamed__': 0x7f0f})
+
+    def test_key_is_absent_when_the_unnamed_bits_are_zero(self) -> None:
+        self.assertEqual(self.field.unpack(b'\x80\xf0', {}), {'a': 1, 'c': 15})
+
+    def test_a_value_without_the_key_packs_unnamed_bits_as_zero(self) -> None:
+        self.assertEqual(self.field.pack({'a': 1, 'c': 15}, {}), b'\x80\xf0')
+        self.assertEqual(self.field.pack({'a': 0, 'c': 0}, {}), b'\x00\x00')
+
+    def test_named_subfields_win_over_the_record(self) -> None:
+        """A named bit set in ``__unnamed__`` does not override its subfield."""
+        self.assertEqual(self.field.pack({'a': 0, 'c': 0, '__unnamed__': 0xffff}, {}), b'\x7f\x0f')
+
+    def test_edited_subfield_keeps_the_captured_unnamed_bits(self) -> None:
+        value = self.field.unpack(b'\xff\xff', {})
+        value['c'] = 0
+        self.assertEqual(self.field.pack(value, {}), b'\xff\x0f')
+
+    def test_a_non_int_record_is_rejected(self) -> None:
+        from pcapkit.utilities.exceptions import FieldValueError
+
+        for bad in ('1', 1.0, True, None):
+            with self.subTest(bad=bad), self.assertRaises(FieldValueError):
+                self.field.pack({'a': 0, 'c': 0, '__unnamed__': bad}, {})
+
+    def test_schema_to_dict_from_dict_keeps_the_unnamed_bits(self) -> None:
+        from pcapkit.corekit.fields.strings import BitField
+        from pcapkit.protocols.schema.schema import Schema, schema_final
+
+        @schema_final
+        class Flags(Schema):
+            flags: dict = BitField(length=2, namespace=NAMESPACE)
+
+        raw = b'\x7f\x0f'
+        parsed = Flags.unpack(raw, len(raw), None)
+        self.assertEqual(parsed.pack(), raw)
+        self.assertEqual(Flags.from_dict(parsed.to_dict()).pack(), raw)
+
+    def test_full_namespace_never_gains_the_key(self) -> None:
+        from pcapkit.corekit.fields.strings import BitField
+
+        field = BitField(length=1, namespace={'hi': (0, 4), 'lo': (4, 4)})({})
+        self.assertEqual(field.unpack(b'\xff', {}), {'hi': 15, 'lo': 15})
+
+
+if __name__ == '__main__':
+    unittest.main()

tests/corekit/test_field_roundtrip_unit.py

@@ -550,13 +550,6 @@ class FieldRoundTripTests(KnownFailureTable, unittest.TestCase):
     STATUSES = ('OK', 'PACK', 'UNPACK', 'VALUE', 'REPACK', 'WIRE', 'DICT', 'REJECTED', 'TIMEOUT')
 
     KNOWN_FAILURES = (
-        Gap(1487, 'BitField.post_process reads only the bits its namespace names, and '
-               'pre_process seeds the rest as zeros, so octets with a bit outside every '
-               'subfield set pack back with it cleared '
-               '(pcapkit/corekit/fields/strings.py:311, :334). Not reachable from a shipped '
-               'schema: all 102 BitFields cover every bit '
-               '(test_every_schema_bitfield_covers_every_bit)',
-            'WIRE', '', ('BitField/partial/wire/ffff', 'BitField/partial/wire/0100')),
         Gap(1488, 'NumberField with bit_length narrower than its octets masks the unpacked value '
                'to bit_length bits, so set high bits are dropped and pack writes them as '
                'zeros (pcapkit/corekit/fields/numbers.py:318, :322). Shipped use: vlan.TCI '