fix(corekit): end a cut list of schema items with the data, not in zero-filled items (#2756)

pcapkit/corekit/fields/collections.py

@@ -213,15 +213,18 @@ def unpack(self, buffer: 'bytes | IO[bytes]', packet: 'dict[str, Any]') -> 'byte
                 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
+        When ``buffer`` falls short of the field's length, the schema item it
+        ends inside is read as though its missing octets were zeros, as
         :meth:`FieldBase.unpack <pcapkit.corekit.fields.field.FieldBase.unpack>`
-        reads any other field, so the items cover the length declared.
+        reads any other field, and the list ends with that item: none is read
+        from the zeros past it, which hold no octet of the data.
 
         .. versionchanged:: 1.6.0
 
-           Schema items short of the field's length are zero-filled to it, where
-           an item past the data was refused as consuming nothing (:issue:`2686`).
+           A list of schema items the data ends inside ends with the data, the
+           item it ends inside read with its missing octets as zeros, where an
+           item past the data was refused as consuming nothing (:issue:`2686`,
+           :issue:`2756`).
 
         """
         length = self._length
@@ -267,16 +270,20 @@ def unpack(self, buffer: 'bytes | IO[bytes]', packet: 'dict[str, Any]') -> 'byte
             file.seek(start, io.SEEK_SET)
             if present < length:
                 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(filled, packet, is_schema, present)
         return self._unpack_items(file, packet, is_schema)
 
-    def _unpack_items(self, file: 'IO[bytes]', packet: 'dict[str, Any]', is_schema: 'bool') -> 'list[_TL]':
+    def _unpack_items(self, file: 'IO[bytes]', packet: 'dict[str, Any]', is_schema: 'bool',
+                      present: 'int' = -1) -> '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.
+            present: Octets of ``file`` the data holds, past which it holds the
+                zeros :meth:`unpack` filled it with; negative where it holds
+                no zeros.
 
         Returns:
             Unpacked items.
@@ -293,8 +300,9 @@ def _unpack_items(self, file: 'IO[bytes]', packet: 'dict[str, Any]', is_schema:
         # 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 in ``unpack`` gives every item octets,
-        # but an item schema may still consume none of them. Remembering where the
+        # loop spins forever. The zero-fill in ``unpack`` gives the item the data
+        # ends inside octets to read, and every item before it reads octets of the
+        # data, 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
         # defect in the ``OptionField`` subclass.
@@ -308,6 +316,10 @@ def _unpack_items(self, file: 'IO[bytes]', packet: 'dict[str, Any]', is_schema:
 
         temp = []  # type: list[_TL]
         while length > 0:
+            # NOTE: An item starting past the octets present would be made of the zero-fill alone,
+            # one the capture never held, so the list ends with the data (#2756).
+            if 0 <= present <= offset - start:
+                break
             field = item_type(packet)
 
             if is_schema:

pcapkit/corekit/fields/field.py

@@ -23,6 +23,7 @@
 import abc
 import contextlib
 import contextvars
+import io
 import re
 import struct
 from typing import TYPE_CHECKING, Generic, TypeVar, cast
@@ -120,22 +121,24 @@ def _zero_pad_budget() -> 'Iterator[list[int]]':
 
 
 @contextlib.contextmanager
-def _zero_filled_items(name: 'str', length: 'int', data: 'bytes') -> 'Iterator[bytes]':
+def _zero_filled_items(name: 'str', length: 'int', data: 'bytes') -> 'Iterator[IO[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`). The items are read inside the block.
+    <pcapkit.corekit.fields.collections.ListField.unpack>` gives the schema
+    item the data ends inside the same reading here (:issue:`2686`). The items
+    are read inside the block, and none from the zeros past that item, which
+    hold no octet of the data (:issue:`2756`).
 
     Args:
         name: Field name.
         length: Octets the field declares.
         data: Octets actually present, fewer than ``length``.
 
     Yields:
-        ``data``, zero-filled to ``length``.
+        A stream of ``data``, zero-filled to ``length``. The zeros left unread
+        past where the stream ends up are not counted as octets read.
 
     Raises:
         FieldValueError: If ``length`` is past
@@ -157,8 +160,8 @@ def _zero_filled_items(name: 'str', length: 'int', data: 'bytes') -> 'Iterator[b
         ledger = [0, 0]
         _zero_pad_ledger.set(ledger)
 
-    # NOTE: The fill is made-up schema objects, not zeros, so it gets no free band or one-off
-    # allowance: a 15-octet SETTINGS frame declaring 65,532 octets made 10,922 pairs in 7.5 MB.
+    # NOTE: The fill makes up the rest of the item the data ends inside, a schema object, not
+    # zeros, so it gets no free band or one-off allowance (#2686); past that item it makes none.
     padding = length - len(data)
     shortfall, ratio = config.MAX_ZERO_PAD_SHORTFALL, config.ZERO_PAD_BUDGET_RATIO
     if shortfall != -1:
@@ -174,14 +177,17 @@ def _zero_filled_items(name: 'str', length: 'int', data: 'bytes') -> 'Iterator[b
         ledger[1] += padding
 
     # 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.
+    # advance and those left unread given back (#2756); items that fail credit an unknown share,
+    # so the octets read go back as they were.
     read = ledger[0]
     ledger[0] -= padding
+    filled = io.BytesIO(data + bytes(padding))
     try:
-        yield data + bytes(padding)
+        yield filled
     except BaseException:
         ledger[0] = read
         raise
+    ledger[0] += length - max(filled.tell(), len(data))
 
 
 #: Pattern[str]: Matches a leading, optionally byte-order-prefixed negative

pcapkit/protocols/application/httpv2/protocol.py

@@ -207,9 +207,10 @@ class HTTPv2(HTTPBase[Data_HTTP, Schema_HTTP],
          class, where it was kept as :class:`~pcapkit.protocols.misc.raw.protocol.Raw`
          (:issue:`2536`).
        * A ``SETTINGS`` frame the capture cut inside its settings parses, with the
-         settings past the data read as zeros, as a cut ``DATA`` frame's data is,
-         where it was :class:`~pcapkit.protocols.misc.raw.protocol.Raw` (:issue:`2659`,
-         :issue:`2686`).
+         setting the cut lands inside read as zeros past it, as a cut ``DATA``
+         frame's data is, and no setting past the data, where it was
+         :class:`~pcapkit.protocols.misc.raw.protocol.Raw` (:issue:`2659`,
+         :issue:`2686`, :issue:`2756`).
        * Each enumeration field of :meth:`make` and of the frame makers takes
          a member or an :class:`int` code, resolved with the enumeration's
          ``get``; a name, a :obj:`bool` for a field wider than one bit, or a
@@ -1437,6 +1438,15 @@ def _make_http_settings(self, frame: 'Optional[Data_SettingsFrame]' = None, *,
            frame with no parameters, which :rfc:`9113#section-6.5` permits,
            rather than being rejected (:issue:`2274`).
 
+        .. versionchanged:: 1.6.0
+
+           A ``frame`` the capture cut inside its settings, as its
+           ``__short_read__`` records, packs the settings its parse read from the
+           octets its ``packet`` holds with zeros after them to the Length it
+           declares, since that parse reads no setting past the data, so its
+           rebuild declares that Length too; with its settings edited, the Length
+           is computed as for a whole frame (:issue:`2756`).
+
         """
         if frame is not None:
             ack = frame.flags.ACK
@@ -1478,6 +1488,16 @@ def _make_http_settings(self, frame: 'Optional[Data_SettingsFrame]' = None, *,
             raise ProtocolError(f'HTTP/2: [Type {Enum_Frame.SETTINGS}] invalid settings: '
                                 f'{settings!r}')
 
+        # NOTE: An unedited cut frame packs the octets its ``packet`` captured, zero to the pair, and
+        # keeps its Length in the zeros past them (#2756); a dict rebuilds as the base data model.
+        record = frame.get('__short_read__') if isinstance(frame, Data_HTTP) else None
+        if frame is not None and record is not None and isinstance(settings_val, list):
+            packed = b''.join(pair.pack() for pair in settings_val)
+            # A cut in the frame header leaves no setting captured.
+            read = cast('bytes', frame.get('packet', b''))[:record[1]] if record[0] == 'frame' else b''
+            if packed == read + bytes(-len(read) % 6) and len(packed) < frame.length:
+                settings_val = packed + bytes(frame.length - len(packed))
+
         return Schema_SettingsFrame(
             settings=settings_val,
         ), flags

tests/corekit/test_fields_list_zero_fill_2686_unit.py

@@ -5,12 +5,13 @@
 field the data ends inside as though its missing octets were zeros.
 :meth:`ListField.unpack <pcapkit.corekit.fields.collections.ListField.unpack>`
 refused the same reading for schema items: an item past the data consumed nothing,
-and the loop guard raised. Now the items read the octets present zero-filled to the
-field's length, so a cut list covers the length it declares and rebuilds from it.
+and the loop guard raised. The item the data ends inside reads instead the octets
+present zero-filled to the field's length, and the list ends with it: no item is read
+past it from the zeros alone, which hold no octet of the data (#2756).
 
-Every zero-filled item is a schema object rather than a zero octet, so the fill is
+The item the fill completes is a schema object rather than a zero octet, so the fill is
 charged to the running budget with no free band and no one-off allowance, and the
-zeros the items then read earn no allowance of their own.
+zeros the items read earn no allowance of their own, nor do those they leave unread.
 
 Every case builds its own octets.
 
@@ -72,7 +73,7 @@ def test_a_short_buffer_and_stream_are_zero_filled_alike(self) -> None:
         for name, buffer in (('bytes', present), ('stream', stream)):
             with self.subTest(buffer=name), self.budget(), time_limit():
                 parsed = self.field(18).unpack(buffer, {})
-                self.assertEqual(self.pairs(parsed), [(1, 4096), (3, 0), (0, 0)])
+                self.assertEqual(self.pairs(parsed), [(1, 4096), (3, 0)])
         self.assertEqual(stream.tell(), 2 + len(present))
 
     def test_a_whole_list_is_read_as_it_was(self) -> None:
@@ -86,14 +87,14 @@ def test_a_whole_list_is_read_as_it_was(self) -> None:
         self.assertEqual(stream.tell(), 12)
 
     def test_a_cut_schema_packs_back_as_captured(self) -> None:
-        """The short read is recorded on the list, which packs at its full length and is
-        cut back to the octets captured, from the schema and from its dict."""
+        """The short read is recorded on the list, which holds the pairs the octets
+        captured begin and is cut back to those octets, from the schema and from its dict."""
         whole = bytes.fromhex('0003' '000100001000' '000300000064' '000400010000')
         for keep in range(2, len(whole)):
             cut = whole[:keep]
             with self.subTest(keep=keep):
                 schema = self.Pairs.unpack(cut, len(whole))
-                self.assertEqual(len(schema.pairs), 3)
+                self.assertEqual(len(schema.pairs), -(-(keep - 2) // 6))
                 self.assertEqual(schema.__dict__['__short_read__'], ('pairs', keep - 2))
                 self.assertEqual(schema.pack(), cut)
                 self.assertEqual(self.Pairs.from_dict(schema.to_dict()).pack(), cut)
@@ -124,7 +125,8 @@ def test_the_zeros_earn_no_allowance(self) -> None:
     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."""
+        Only a failure restores them: a later fill that succeeds is counted as usual,
+        the zeros it leaves unread past its items given back (#2756)."""
         from pcapkit.corekit.fields.numbers import UInt16Field
         from pcapkit.protocols.schema import Schema, schema_final
         from pcapkit.utilities.exceptions import ProtocolError
@@ -145,12 +147,12 @@ def post_process(self, packet: 'dict') -> 'Schema':
             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])
+                field.unpack(b'\x00', {})
+            self.assertEqual(ledger, [12, 11])
 
             # 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])
+            self.assertEqual(ledger, [14, 21])
 
     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."""
@@ -160,7 +162,7 @@ def fill() -> 'tuple[int, list[int]]':
             parsed = self.field(12).unpack(bytes(6), {})
             return len(parsed), self.ledger.get()
 
-        self.assertEqual(contextvars.Context().run(fill), (2, [6, 6]))
+        self.assertEqual(contextvars.Context().run(fill), (1, [6, 6]))
 
     def test_a_length_past_the_ceiling_is_refused(self) -> None:
         ceiling = self.config.MAX_ZERO_PAD_LENGTH
@@ -171,7 +173,7 @@ def test_a_length_past_the_ceiling_is_refused(self) -> None:
     def test_a_lifted_budget_fills_any_shortfall_within_the_ceiling(self) -> None:
         for name in ('ZERO_PAD_BUDGET_RATIO', 'MAX_ZERO_PAD_SHORTFALL'):
             with self.subTest(name), mock.patch.object(self.config, name, -1), self.budget(), time_limit():
-                self.assertEqual(len(self.field(6000).unpack(b'', {})), 1000)
+                self.assertEqual(len(self.field(6000).unpack(bytes(1), {})), 1)
 
 
 if __name__ == '__main__':