fix(corekit): give back the octets read when a zero-filled list fails (#2705)

pcapkit/corekit/fields/collections.py

@@ -4,7 +4,7 @@
 import io
 from typing import TYPE_CHECKING, Generic, TypeVar, cast
 
-from pcapkit.corekit.fields.field import FieldBase, _zero_fill_items
+from pcapkit.corekit.fields.field import FieldBase, _zero_filled_items
 from pcapkit.corekit.fields.misc import SchemaField
 from pcapkit.corekit.fields.numbers import NumberField
 from pcapkit.corekit.multidict import OrderedMultiDict
@@ -175,8 +175,8 @@ def unpack(self, buffer: 'bytes | IO[bytes]', packet: 'dict[str, Any]') -> 'byte
 
         Raises:
             FieldValueError: If the items overrun the field, if a schema item
-                consumes nothing from ``buffer`` -- see the note below -- or if
-                schema items short of the field's length cannot be zero-filled.
+                consumes nothing from ``buffer``, or if schema items short of the
+                field's length cannot be zero-filled.
 
         When ``buffer`` falls short of the field's length, schema items are read
         from its octets zero-filled to that length, as
@@ -212,7 +212,7 @@ def unpack(self, buffer: 'bytes | IO[bytes]', packet: 'dict[str, Any]') -> 'byte
         # before, from the first one -- is still an instance of the original.
         # ``isinstance`` against the new class would report :data:`False` for an
         # item that plainly is a ``SchemaField``, ``is_schema`` would go the
-        # wrong way, and this method's other branch would bill each item by its
+        # wrong way, and the item loop's other branch would bill each item by its
         # *declared* length instead of by what it actually consumed -- so a
         # malformed TCP SACK option's length check could raise
         # :exc:`~pcapkit.utilities.exceptions.FieldValueError` from here instead
@@ -231,13 +231,34 @@ def unpack(self, buffer: 'bytes | IO[bytes]', packet: 'dict[str, Any]') -> 'byte
             present = file.seek(0, io.SEEK_END) - start
             file.seek(start, io.SEEK_SET)
             if present < length:
-                file = io.BytesIO(_zero_fill_items(self.name, length, file.read()))
+                with _zero_filled_items(self.name, length, file.read()) as filled:
+                    return self._unpack_items(io.BytesIO(filled), packet, is_schema)
+        return self._unpack_items(file, packet, is_schema)
+
+    def _unpack_items(self, file: 'IO[bytes]', packet: 'dict[str, Any]', is_schema: 'bool') -> 'list[_TL]':
+        """Unpack the field's items from ``file``.
+
+        Args:
+            file: Stream the items are read from.
+            packet: Packet data.
+            is_schema: Whether the items are schemas.
+
+        Returns:
+            Unpacked items.
+
+        Raises:
+            FieldValueError: If the items overrun the field, or if a schema item
+                consumes nothing from ``file`` -- see the note below.
+
+        """
+        length = self._length
+        item_type = cast('FieldBase', self._item_type)
 
         # NOTE: The item-typed branch below sizes each item by ``field.length``,
         # which is what it read, but the schema branch sizes it by ``len(data)``,
         # which is only what the schema *recorded*. A schema that reads nothing
         # records nothing, so ``length -= len(data)`` makes no progress and the
-        # loop spins forever. The zero-fill above gives every item octets to read,
+        # loop spins forever. The zero-fill in ``unpack`` gives every item octets,
         # but an item schema may still consume none of them. Remembering where the
         # previous item ended is what bounds the iteration count, since it does
         # not depend on what the schema reports. Cf. #431, which is the same
@@ -252,7 +273,7 @@ def unpack(self, buffer: 'bytes | IO[bytes]', packet: 'dict[str, Any]') -> 'byte
 
         temp = []  # type: list[_TL]
         while length > 0:
-            field = self._item_type(packet)
+            field = item_type(packet)
 
             if is_schema:
                 data = cast('SchemaField', field).unpack(file, packet)

pcapkit/corekit/fields/field.py

@@ -83,7 +83,7 @@
 #: *not* weighed against it: a shortfall within :data:`~pcapkit.utilities.config.MAX_ZERO_PAD_SHORTFALL`
 #: neither consults this ledger nor is charged to it, which is what keeps every
 #: legitimate short read answering the same however much preceded it. Only the
-#: 32-bit band above reads and writes ``[1]``, and :func:`_zero_fill_items`, whose
+#: 32-bit band above reads and writes ``[1]``, and :func:`_zero_filled_items`, whose
 #: fill of a list of schema items has no free band (:issue:`2686`).
 _zero_pad_ledger = contextvars.ContextVar(
     'pcapkit.corekit.fields.field._zero_pad_ledger',
@@ -119,21 +119,22 @@ def _zero_pad_budget() -> 'Iterator[list[int]]':
         _zero_pad_ledger.reset(token)
 
 
-def _zero_fill_items(name: 'str', length: 'int', data: 'bytes') -> 'bytes':
+@contextlib.contextmanager
+def _zero_filled_items(name: 'str', length: 'int', data: 'bytes') -> 'Iterator[bytes]':
     """Zero-fill the octets of a list of schema items to its declared width.
 
     :meth:`FieldBase.unpack` reads a field the data ends inside as though its
     missing octets were zeros, and :meth:`ListField.unpack
     <pcapkit.corekit.fields.collections.ListField.unpack>` gives a list of
     schema items the same reading here, so its items cover the width it
-    declares (:issue:`2686`).
+    declares (:issue:`2686`). The items are read inside the block.
 
     Args:
         name: Field name.
         length: Octets the field declares.
         data: Octets actually present, fewer than ``length``.
 
-    Returns:
+    Yields:
         ``data``, zero-filled to ``length``.
 
     Raises:
@@ -172,10 +173,15 @@ def _zero_fill_items(name: 'str', length: 'int', data: 'bytes') -> 'bytes':
                 )
         ledger[1] += padding
 
-    # NOTE: The items credit the zeros they read as octets supplied, so they are
-    # taken off in advance and the fill earns no allowance of its own.
+    # NOTE: The items credit the zeros they read as octets supplied, so they are taken off in
+    # advance; items that fail credit an unknown share, so the octets read go back as they were.
+    read = ledger[0]
     ledger[0] -= padding
-    return data + bytes(padding)
+    try:
+        yield data + bytes(padding)
+    except BaseException:
+        ledger[0] = read
+        raise
 
 
 #: Pattern[str]: Matches a leading, optionally byte-order-prefixed negative

pcapkit/protocols/application/httpv2.py

@@ -604,10 +604,8 @@ def _make_data(cls, data: 'Data_HTTP') -> 'dict[str, Any]':  # type: ignore[over
             Key-value pairs for protocol construction.
 
         """
-        # NOTE: No ``length``: :meth:`make` computes the frame's Length from the
-        # frame it builds, and a ``length`` keyword would instead be taken by
-        # ``ProtocolBase.__init__`` as the number of octets to parse back --
-        # the payload length alone, nine octets short of the frame.
+        # NOTE: No ``length``: ``make`` computes the Length from the frame it builds, and
+        # ``ProtocolBase.__init__`` would take a ``length`` as the octets to parse back.
         return {
             'type': data.type,
             'flags': data.flags.__value__ if data.flags is not None else 0,

tests/corekit/test_fields_list_zero_fill_2686_unit.py

@@ -121,6 +121,37 @@ def test_the_zeros_earn_no_allowance(self) -> None:
                 self.field(ratio * 6 + 1).unpack(b'', {})
             self.assertEqual(ledger, [6, ratio * 6])
 
+    def test_a_failed_fill_gives_back_the_octets_read(self) -> None:
+        """Items that fail inside the fill leave the octets read as they were, so one
+        bad list does not refuse the later cuts of its context; the fill stays charged.
+        Only a failure restores them: a later fill that succeeds is counted as usual."""
+        from pcapkit.corekit.fields.numbers import UInt16Field
+        from pcapkit.protocols.schema.schema import Schema, schema_final
+        from pcapkit.utilities.exceptions import ProtocolError
+
+        @schema_final
+        class NonZero(Schema):
+            """A 16-bit value that refuses to be zero once parsed."""
+
+            value: int = UInt16Field()
+
+            def post_process(self, packet: 'dict') -> 'Schema':
+                if self.value == 0:
+                    raise ProtocolError('a zero item')
+                return self
+
+        field = self.ListField(length=12, item_type=self.SchemaField(length=2, schema=NonZero))
+        with self.budget() as ledger:
+            self.field(12).unpack(bytes(12), {})
+            self.assertEqual(ledger, [12, 0])
+            with self.assertRaisesRegex(ProtocolError, 'a zero item'):
+                field.unpack(b'\x00\x01', {})
+            self.assertEqual(ledger, [12, 10])
+
+            # a fill that succeeds keeps the octets its items read, as before the failure
+            self.field(12).unpack(bytes(2), {})
+            self.assertEqual(ledger, [14, 20])
+
     def test_a_fresh_context_gets_a_ledger_of_its_own(self) -> None:
         """A fill that is the first read of a context starts that context's ledger."""
         import contextvars