fix(corekit): zero-fill a cut list of schema items to its length (#2686)

docs/source/pcapkit/protocols/application/httpv2.rst

@@ -149,10 +149,6 @@ Header Schemas
    :members:
    :show-inheritance:
 
-.. autoclass:: pcapkit.protocols.schema.application.httpv2.SettingPairListField
-   :members:
-   :show-inheritance:
-
 .. autoclass:: pcapkit.protocols.schema.application.httpv2.SettingsFrame
    :members:
    :show-inheritance:

docs/source/pcapkit/protocols/internet/igmpv3.rst

@@ -86,10 +86,6 @@ Header Schemas
    :members:
    :show-inheritance:
 
-.. autoclass:: pcapkit.protocols.schema.internet.igmpv3.GroupRecordListField
-   :members:
-   :show-inheritance:
-
 Data Models
 -----------
 

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
+from pcapkit.corekit.fields.field import FieldBase, _zero_fill_items
 from pcapkit.corekit.fields.misc import SchemaField
 from pcapkit.corekit.fields.numbers import NumberField
 from pcapkit.corekit.multidict import OrderedMultiDict
@@ -174,8 +174,19 @@ def unpack(self, buffer: 'bytes | IO[bytes]', packet: 'dict[str, Any]') -> 'byte
             Unpacked field value.
 
         Raises:
-            FieldValueError: If the items overrun the field, or if a schema item
-                consumes nothing from ``buffer`` -- see the note below.
+            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.
+
+        When ``buffer`` falls short of the field's length, schema items are read
+        from its octets zero-filled to that length, as
+        :meth:`FieldBase.unpack <pcapkit.corekit.fields.field.FieldBase.unpack>`
+        reads any other field, so the items cover the length declared.
+
+        .. 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`).
 
         """
         length = self._length
@@ -215,17 +226,22 @@ def unpack(self, buffer: 'bytes | IO[bytes]', packet: 'dict[str, Any]') -> 'byte
         # :mod:`pcapkit.corekit.fields.misc`'s entry in :data:`sys.modules`
         # afterwards.
         is_schema = isinstance(self._item_type, SchemaField)
+        if is_schema and length > 0:
+            start = file.tell()
+            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()))
 
         # 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 reading a stream that
-        # has already run out records nothing, so ``length -= len(data)`` makes no
-        # progress and the loop spins forever. Reachable from a TCP segment: a
-        # ``SACK`` option declaring more octets than the option area holds leaves
-        # ``sack``'s ``ListField`` reading ``SACKBlock`` off an exhausted stream.
-        # 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.
+        # 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,
+        # 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.
         #
         # ``start`` is where the field itself begins. The comparison needs stream
         # positions, but the diagnostic wants an offset into the field, and the two

pcapkit/corekit/fields/field.py

@@ -83,7 +83,8 @@
 #: *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]``.
+#: 32-bit band above reads and writes ``[1]``, and :func:`_zero_fill_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',
     default=cast('Optional[list[int]]', None),
@@ -118,6 +119,65 @@ def _zero_pad_budget() -> 'Iterator[list[int]]':
         _zero_pad_ledger.reset(token)
 
 
+def _zero_fill_items(name: 'str', length: 'int', data: 'bytes') -> '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`).
+
+    Args:
+        name: Field name.
+        length: Octets the field declares.
+        data: Octets actually present, fewer than ``length``.
+
+    Returns:
+        ``data``, zero-filled to ``length``.
+
+    Raises:
+        FieldValueError: If ``length`` is past
+            :data:`~pcapkit.utilities.config.MAX_ZERO_PAD_LENGTH`, or if the
+            fill takes this parse's padding past
+            :data:`~pcapkit.utilities.config.ZERO_PAD_BUDGET_RATIO` times the
+            octets it has actually read.
+
+    """
+    ceiling = config.MAX_ZERO_PAD_LENGTH
+    if ceiling != -1 and length > ceiling:
+        raise FieldValueError(
+            f'Field {name} declares a length of {length} octet(s), '
+            f'but only {len(data)} octet(s) are available.'
+        )
+
+    ledger = _zero_pad_ledger.get()
+    if ledger is None:
+        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.
+    padding = length - len(data)
+    shortfall, ratio = config.MAX_ZERO_PAD_SHORTFALL, config.ZERO_PAD_BUDGET_RATIO
+    if shortfall != -1:
+        if ceiling != -1 and ratio != -1:
+            allowance = ratio * (ledger[0] + len(data))
+            if ledger[1] + padding > allowance:
+                raise FieldValueError(
+                    f'Field {name} would zero-fill {padding} octet(s) of items, '
+                    f'taking this parse to {ledger[1] + padding} octet(s) of padding '
+                    f'against {ledger[0] + len(data)} octet(s) actually read, past the '
+                    f'{allowance} octet(s) allowed.'
+                )
+        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.
+    ledger[0] -= padding
+    return data + bytes(padding)
+
+
 #: Pattern[str]: Matches a leading, optionally byte-order-prefixed negative
 #: count in a :attr:`FieldBase.template`, e.g. the ``-1`` in ``'-1s'`` or in
 #: ``'>-1s'``.

pcapkit/protocols/application/httpv2.py

@@ -208,10 +208,10 @@ class HTTPv2(HTTPBase[Data_HTTP, Schema_HTTP],
        * A whole frame after the first in a segment is a further layer of this
          class, where it was kept as :class:`~pcapkit.protocols.misc.raw.Raw`
          (:issue:`2536`).
-       * A ``SETTINGS`` frame the capture cut inside its settings parses as far
-         as its octets go, where it was :class:`~pcapkit.protocols.misc.raw.Raw`,
-         and :meth:`make` takes ``frame_length``, so it rebuilds as captured
-         (:issue:`2659`).
+       * 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.Raw` (:issue:`2659`,
+         :issue:`2686`).
 
     """
 
@@ -318,11 +318,6 @@ def unpack(self, length: 'Optional[int]' = None, **kwargs: 'Any') -> 'Data_HTTP'
             holds one frame per packet and has no form for a frameless segment.
 
         """
-        # NOTE: A frame ``pack`` made short of its Length parses at that Length (#2659).
-        made_length = vars(self).pop('_made_length', None)
-        if length is None:
-            length = made_length
-
         preface_len = len(_HTTP2_PREFACE)
         if self.__header__ is None and self._data[:preface_len] == _HTTP2_PREFACE \
                 and (length is None or length >= preface_len):
@@ -427,7 +422,6 @@ def make(self,  # type: ignore[override]
              frame: 'Optional[bytes | Data_HTTP | Schema_FrameType | dict[str, Any]]' = None,
              payload: 'bytes | ProtocolBase | Schema' = b'',
              preface: 'bool' = False,  # pylint: disable=unused-argument
-             frame_length: 'Optional[int]' = None,
              **kwargs: 'Any') -> 'Schema_HTTP':
         """Make (construct) packet data.
 
@@ -447,12 +441,6 @@ def make(self,  # type: ignore[override]
             payload: Octets after the frame, e.g. the next frame.
             preface: Whether the HTTP/2 connection preface goes ahead of the
                 frame; :meth:`pack` writes it.
-            frame_length: Length of the frame payload, as transmitted on the
-                wire, the 9-octet header excluded (:rfc:`9113#section-4.1`);
-                computed from ``frame`` when not given, so a ``SETTINGS`` frame
-                the capture cut inside its settings rebuilds as captured. A
-                Length above the one ``frame`` packs to is written as given,
-                as on a frame the capture cut short.
             **kwargs: Arbitrary keyword arguments.
 
         Returns:
@@ -462,11 +450,7 @@ def make(self,  # type: ignore[override]
             ProtocolError: If ``frame`` is none of the forms this maker reads --
                 raw :obj:`bytes`, a :obj:`dict` of frame arguments, an
                 :class:`~pcapkit.protocols.data.application.httpv2.HTTPv2` data
-                model or a :class:`~pcapkit.protocols.schema.schema.Schema`; or
-                if ``frame_length`` is not an :obj:`int` from the Length
-                ``frame`` packs to up to ``0xFFFFFF``, or exceeds that Length
-                with a non-empty ``payload``, which the frame would read as its
-                own (:issue:`2617`).
+                model or a :class:`~pcapkit.protocols.schema.schema.Schema`.
 
         .. versionchanged:: 1.5.0
 
@@ -517,16 +501,6 @@ def make(self,  # type: ignore[override]
             flags_val[f'bit_{bit}'] = (int(flags) & (1 << bit)) >> bit
 
         length = self._make_http_length(frame_val, flags)
-        if frame_length is not None:
-            if isinstance(frame_length, bool) or not isinstance(frame_length, int) \
-                    or not length <= frame_length <= 0xFFFFFF:
-                raise ProtocolError(f'HTTP/2: [Type {type_val}] invalid frame_length: {frame_length!r} '
-                                    f'for a frame payload of {length} octet(s)')
-            # NOTE: Past the frame payload, the Length would read the next frame as its own.
-            if frame_length > length and payload is not None and len(payload) > 0:
-                raise ProtocolError(f'HTTP/2: [Type {type_val}] invalid frame_length: {frame_length!r} '
-                                    f'for a frame payload of {length} octet(s) with a payload after it')
-            length = frame_length
 
         return Schema_HTTP(
             length=length,
@@ -558,22 +532,11 @@ def pack(self, **kwargs: 'Any') -> 'bytes':
             type's own schema. Any other call leaves the instance's header as it
             was (:issue:`1769`).
 
-            A frame whose payload falls short of its Length, as
-            :meth:`from_data` rebuilds a ``SETTINGS`` frame the capture cut
-            inside its settings, is parsed back at that Length, as the capture
-            was, rather than refused as a frame the data cannot back
-            (:issue:`2659`).
-
         """
         data = super().pack(**kwargs)
         # NOTE: Constructing, by the test ``ProtocolBase._packing_target`` makes.
         if '_info' not in vars(self):
             self.__header__ = None  # type: ignore[assignment]
-
-            start = len(_HTTP2_PREFACE) if kwargs.get('preface', False) else 0
-            declared = start + 9 + int.from_bytes(data[start:start + 3], 'big')
-            if declared > len(data):
-                self._made_length = declared
         return data
 
     @classmethod
@@ -641,8 +604,10 @@ def _make_data(cls, data: 'Data_HTTP') -> 'dict[str, Any]':  # type: ignore[over
             Key-value pairs for protocol construction.
 
         """
-        # NOTE: The Length as on the wire, so a cut SETTINGS frame keeps it (#2659);
-        # ``length`` would be taken by ``ProtocolBase.__init__`` as octets to parse.
+        # 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.
         return {
             'type': data.type,
             'flags': data.flags.__value__ if data.flags is not None else 0,
@@ -651,7 +616,6 @@ def _make_data(cls, data: 'Data_HTTP') -> 'dict[str, Any]':  # type: ignore[over
             'frame': data,
             'payload': cls._make_payload(data),
             'preface': bool(data.get('preface')),
-            'frame_length': data.get('length'),
         }
 
     @staticmethod

pcapkit/protocols/schema/application/httpv2.py

@@ -12,7 +12,6 @@ class is renamed :class:`~pcapkit.protocols.schema.application.httpv2.HTTPv2`,
 
 import collections
 import enum
-import io
 from typing import TYPE_CHECKING, cast
 
 from pcapkit.const.http.error_code import ErrorCode as Enum_ErrorCode
@@ -36,7 +35,7 @@ class is renamed :class:`~pcapkit.protocols.schema.application.httpv2.HTTPv2`,
 ]
 
 if TYPE_CHECKING:
-    from typing import IO, Any, Optional
+    from typing import Any, Optional
 
     from pcapkit.corekit.fields.field import FieldBase as Field
     from pcapkit.protocols.protocol import ProtocolBase
@@ -344,46 +343,6 @@ class SettingPair(Schema):
         def __init__(self, id: 'Enum_Setting', value: 'int') -> 'None': ...
 
 
-class SettingPairListField(ListField):
-    """Setting list that ends with the data.
-
-    :attr:`SettingsFrame.settings` is sized by the frame's Length, which exceeds
-    the octets present once the capture cuts the frame short. A setting read
-    past them consumes nothing, which :meth:`ListField.unpack
-    <pcapkit.corekit.fields.collections.ListField.unpack>` refuses to loop on,
-    so the list is sized by the octets present instead: the setting the data
-    ends inside is kept as far as it goes, and :meth:`Schema.unpack
-    <pcapkit.protocols.schema.schema.Schema.unpack>` records the short read
-    (:issue:`2659`).
-
-    """
-
-    def unpack(self, buffer: 'bytes | IO[bytes]', packet: 'dict[str, Any]') -> 'list[SettingPair]':
-        """Unpack field value from :obj:`bytes`.
-
-        Args:
-            buffer: Field buffer.
-            packet: Packet data.
-
-        Returns:
-            Unpacked settings.
-
-        """
-        if isinstance(buffer, bytes):
-            present = len(buffer)
-        else:
-            start = buffer.tell()
-            present = buffer.seek(0, io.SEEK_END) - start
-            buffer.seek(start, io.SEEK_SET)
-
-        declared = self._length
-        self._length = min(declared, present)
-        try:
-            return cast('list[SettingPair]', super().unpack(buffer, packet))
-        finally:
-            self._length = declared
-
-
 @schema_final
 class SettingsFrame(FrameType, code=Enum_Frame.SETTINGS):
     """Header schema for HTTP/2 ``SETTINGS`` frames."""
@@ -393,9 +352,8 @@ class Flags(FrameType.Flags):
 
         ACK = BIT_0 = 0x1
 
-    #: Settings, read no further than the data goes (see
-    #: :class:`SettingPairListField`).
-    settings: 'list[SettingPair]' = SettingPairListField(
+    #: Settings.
+    settings: 'list[SettingPair]' = ListField(
         length=lambda pkt: pkt['__length__'],
         item_type=SchemaField(length=6, schema=SettingPair),
     )