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

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

@@ -28,7 +28,7 @@ Octets      Bits        Name                               Description
   8          68   ``igmpv3.s_flag``                  Suppress Router-Side Processing (S)
   8          69   ``igmpv3.qrv``                     Querier's Robustness Variable (QRV)
   9          72   ``igmpv3.qqic``                    Querier's Query Interval Code
-  10         80                                      Number of Sources (N)
+  10         80   ``igmpv3.num_sources``             Number of Sources (N)
   12         96   ``igmpv3.sources``                 Source Address [1..N]
 ======= ========= ================================== ===================================
 

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_filled_items
+from pcapkit.corekit.fields.field import FieldBase, _give_back_fill, _zero_filled_items
 from pcapkit.corekit.fields.misc import SchemaField
 from pcapkit.corekit.fields.numbers import NumberField
 from pcapkit.corekit.multidict import OrderedMultiDict
@@ -213,18 +213,29 @@ 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, the schema item it
-        ends inside is read as though its missing octets were zeros, as
+        When ``buffer`` falls short of the field's length, the 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, and the list ends with that item: none is read
-        from the zeros past it, which hold no octet of the data.
+        from past the data, which holds no octet of it. A ``__data_present__``
+        key in ``packet`` says that ``buffer`` holds the data that many octets
+        in, and past them the zeros an enclosing list of schema items was
+        filled out with, as :meth:`Schema.unpack
+        <pcapkit.protocols.schema.Schema.unpack>` tells the fields of the item
+        the data ends inside; the list then ends with the data there.
+
+        A ``__zero_tail__`` key in ``packet``, true, says instead that the
+        octets past ``buffer`` are zeros of the data, as radiotap.org reads the
+        octets a producer leaves off a TLV item (:issue:`2655`), so the list
+        reads its items on through them to its length.
 
         .. versionchanged:: 1.6.0
 
-           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`).
+           A list the data ends inside ends with the data, the item it ends
+           inside read with its missing octets as zeros, where a schema item
+           past the data was refused as consuming nothing and a plain value
+           past it was read as zeros, and so does a list in an item the data
+           ends inside (:issue:`2686`, :issue:`2756`, :issue:`2760`).
 
         """
         length = self._length
@@ -264,13 +275,23 @@ 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:
+        if length > 0:
             start = file.tell()
-            present = file.seek(0, io.SEEK_END) - start
+            octets = file.seek(0, io.SEEK_END) - start
             file.seek(start, io.SEEK_SET)
-            if present < length:
-                with _zero_filled_items(self.name, length, file.read()) as filled:
-                    return self._unpack_items(filled, packet, is_schema, present)
+            # NOTE: Past ``__data_present__`` octets the buffer holds an enclosing fill (#2760).
+            held = cast('int', packet.get('__data_present__', -1))
+            present = held if 0 <= held < octets else octets
+            # NOTE: Octets past the data that are zeros of it are read as items too (#2655, #2760).
+            stop = -1 if packet.get('__zero_tail__') else present
+            if present < length and is_schema:
+                with _zero_filled_items(self.name, length, file.read(), present) as filled:
+                    return self._unpack_items(filled, packet, is_schema, stop)
+            if present < length and stop >= 0:
+                # NOTE: A plain value the data ends inside zero-fills itself, as any field does.
+                items = self._unpack_items(file, packet, is_schema, present)
+                _give_back_fill(octets - (file.tell() - start))
+                return items
         return self._unpack_items(file, packet, is_schema)
 
     def _unpack_items(self, file: 'IO[bytes]', packet: 'dict[str, Any]', is_schema: 'bool',
@@ -281,9 +302,9 @@ def _unpack_items(self, file: 'IO[bytes]', packet: 'dict[str, Any]', is_schema:
             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.
+            present: Octets of ``file`` the data holds, past which ``file``
+                ends or holds zeros a fill made up; negative where it holds the
+                data to the field's length.
 
         Returns:
             Unpacked items.
@@ -317,15 +338,22 @@ 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).
+            # or of nothing, one the capture never held, so the list ends with the data (#2756, #2760).
             if 0 <= present <= offset - start:
+                # NOTE: A length no whole number of plain values fills is refused, as it is whole.
+                if not is_schema and length % item_type(packet).length:
+                    raise FieldValueError(f'Field {self.name} has invalid length.')
                 break
             field = item_type(packet)
 
             if is_schema:
                 # NOTE: A counted list runs to the end of the data, so an item it counts
                 # running past the octets left is cut, not overrunning a length (#2749).
                 context = dict(packet, __data_end__=True) if len(temp) < self._count else packet
+                # NOTE: The item the data ends inside reads the fill past it, which the lists in it
+                # are told of, so they end with the data too (#2760).
+                if present >= 0:
+                    context = dict(context, __data_present__=present - (offset - start))
                 data = cast('SchemaField', field).unpack(file, context)
 
                 end = file.tell()
@@ -354,6 +382,7 @@ def _unpack_items(self, file: 'IO[bytes]', packet: 'dict[str, Any]', is_schema:
 
                 buffer = file.read(field.length)
                 data = field.unpack(buffer, packet)
+                offset = file.tell()
 
             temp.append(data)
 

pcapkit/corekit/fields/field.py

@@ -121,7 +121,8 @@ def _zero_pad_budget() -> 'Iterator[list[int]]':
 
 
 @contextlib.contextmanager
-def _zero_filled_items(name: 'str', length: 'int', data: 'bytes') -> 'Iterator[IO[bytes]]':
+def _zero_filled_items(name: 'str', length: 'int', data: 'bytes',
+                       present: 'int' = -1) -> '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
@@ -134,11 +135,14 @@ def _zero_filled_items(name: 'str', length: 'int', data: 'bytes') -> 'Iterator[I
     Args:
         name: Field name.
         length: Octets the field declares.
-        data: Octets actually present, fewer than ``length``.
+        data: Octets of the field's buffer, fewer than ``length``, or holding
+            past ``present`` the zeros of an enclosing fill.
+        present: Octets of ``data`` the data holds; negative for all of them.
 
     Yields:
         A stream of ``data``, zero-filled to ``length``. The zeros left unread
-        past where the stream ends up are not counted as octets read.
+        past where the stream ends up are not counted as octets read, an
+        enclosing fill's included (:issue:`2760`).
 
     Raises:
         FieldValueError: If ``length`` is past
@@ -148,11 +152,13 @@ def _zero_filled_items(name: 'str', length: 'int', data: 'bytes') -> 'Iterator[I
             octets it has actually read.
 
     """
+    if present < 0:
+        present = len(data)
     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.'
+            f'but only {present} octet(s) are available.'
         )
 
     ledger = _zero_pad_ledger.get()
@@ -177,8 +183,9 @@ def _zero_filled_items(name: 'str', length: 'int', data: 'bytes') -> 'Iterator[I
         ledger[1] += padding
 
     # NOTE: The items credit the zeros they read as octets supplied, so they are taken off in
-    # advance and those left unread given back (#2756); items that fail credit an unknown share,
-    # so the octets read go back as they were.
+    # advance and those left unread given back, an enclosing fill's too, which it took off
+    # (#2756, #2760); 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))
@@ -187,7 +194,26 @@ def _zero_filled_items(name: 'str', length: 'int', data: 'bytes') -> 'Iterator[I
     except BaseException:
         ledger[0] = read
         raise
-    ledger[0] += length - max(filled.tell(), len(data))
+    ledger[0] += length - max(filled.tell(), present)
+
+
+def _give_back_fill(count: 'int') -> 'None':
+    """Give back the ``count`` zeros of a fill that a list of plain values left unread.
+
+    Args:
+        count: Zeros of an enclosing fill in the list's buffer that no item
+            read; nothing is given back for none.
+
+    :func:`_zero_filled_items` takes the zeros of its fill off the octets read
+    in advance, as the items reading them credit them, and gives back those its
+    stream is left short of. A list in the item the data ends inside reads none
+    of its buffer past the data (:issue:`2760`), so the zeros it leaves there
+    are given back here.
+
+    """
+    ledger = _zero_pad_ledger.get()
+    if ledger is not None and count > 0:
+        ledger[0] += count
 
 
 #: Pattern[str]: Matches a leading, optionally byte-order-prefixed negative

pcapkit/protocols/internet/igmpv3/data.py

@@ -86,6 +86,10 @@ class Query(IGMPv3):
     #: Querier's Query Interval, in seconds, decoded from :attr:`qqic` by the
     #: same formula as :attr:`max_resp_time` (:rfc:`3376#section-4.1.7`).
     qqi: 'int'
+    #: Number of source addresses, as received on the wire
+    #: (:rfc:`3376#section-4.1.8`); more than :attr:`sources` holds where the
+    #: data ends inside them.
+    num_sources: 'int'
     #: Source addresses being queried (:rfc:`3376#section-4.1.9`); an empty
     #: :obj:`tuple` for a General or Group-Specific Query.
     sources: 'tuple[IPv4Address, ...]'
@@ -94,7 +98,7 @@ class Query(IGMPv3):
         def __init__(self, version: 'Literal[3]', type: 'Enum_Type', length: 'int', max_resp_code: 'int',
                      max_resp_time: 'int', checksum: 'bytes', group: 'IPv4Address',
                      resv: 'int', s_flag: 'bool', qrv: 'int', qqic: 'int', qqi: 'int',
-                     sources: 'tuple[IPv4Address, ...]') -> 'None': ...  # pylint: disable=unused-argument,super-init-not-called,multiple-statements,redefined-builtin,line-too-long
+                     num_sources: 'int', sources: 'tuple[IPv4Address, ...]') -> 'None': ...  # pylint: disable=unused-argument,super-init-not-called,multiple-statements,redefined-builtin,line-too-long
 
 
 @info_final

pcapkit/protocols/internet/igmpv3/protocol.py

@@ -25,7 +25,7 @@
   8          68   ``igmpv3.s_flag``                  Suppress Router-Side Processing (S)
   8          69   ``igmpv3.qrv``                     Querier's Robustness Variable (QRV)
   9          72   ``igmpv3.qqic``                    Querier's Query Interval Code
-  10         80                                      Number of Sources (N)
+  10         80   ``igmpv3.num_sources``             Number of Sources (N)
   12         96   ``igmpv3.sources``                 Source Address [1..N]
 ======= ========= ================================== ===================================
 
@@ -50,6 +50,7 @@
 .. versionadded:: 1.6.0
 
 """
+import ipaddress
 from typing import TYPE_CHECKING, cast
 
 from pcapkit.const.igmp.record_type import RecordType as Enum_RecordType
@@ -275,6 +276,7 @@ def read(self, length: 'Optional[int]' = None, **kwargs: 'Any') -> 'Data_IGMPv3'
                 qrv=qrv,
                 qqic=qqic,
                 qqi=qqi,
+                num_sources=ext.num_sources,
                 sources=sources,
             )  # type: Data_IGMPv3
         elif isinstance(body, Schema_Report):
@@ -318,6 +320,7 @@ def make(self,
              qqic: 'Optional[int]' = None,
              qqi: 'Optional[int]' = None,
              sources: 'Optional[Iterable[IPv4Address | str | bytes | int]]' = None,
+             num_sources: 'Optional[int]' = None,
              reserved_1: 'int' = 0,
              reserved_2: 'bytes' = b'\x00\x00',
              num_group_records: 'Optional[int]' = None,
@@ -363,6 +366,11 @@ def make(self,
                 is given; only meaningful for a Query.
             sources: Source addresses being queried
                 (:rfc:`3376#section-4.1.9`); only meaningful for a Query.
+            num_sources: Number of sources, as transmitted on the wire
+                (:rfc:`3376#section-4.1.8`); the length of ``sources`` when not
+                given; only meaningful for a Query. A count above the length of
+                ``sources`` is written as given, as on a Query the capture cut
+                inside its sources.
             reserved_1: The reserved octet following ``type``
                 (:rfc:`3376#section-4.2.1`); only meaningful for a Report.
             reserved_2: The reserved 16 bits before the group-record count
@@ -388,7 +396,8 @@ def make(self,
                 or code outside 0 to 255, or is neither a Query nor a Report;
                 if ``max_resp_time`` is not an :obj:`int` from 0 to 255; if
                 ``num_group_records`` is not an :obj:`int` from the length of
-                ``records`` to 65535; if ``checksum`` or ``reserved_2`` is not
+                ``records`` to 65535, or ``num_sources`` one from the length of
+                ``sources``; if ``checksum`` or ``reserved_2`` is not
                 two octets; or if a record's ``record_type`` is refused, see
                 :meth:`_make_group_record`.
 
@@ -425,10 +434,15 @@ def make(self,
             if qqic is None:
                 qqic = _encode_resp_code(0 if qqi is None else qqi)
             source_list = [] if sources is None else list(sources)
+            if num_sources is None:
+                num_sources = len(source_list)
+            elif self._make_uint(num_sources, bits=16, name='num_sources') < len(source_list):
+                raise ProtocolError(f'{self.alias}: invalid num_sources: {num_sources!r} '
+                                    f'for {len(source_list)} source(s)')
             ext = Schema_QueryExtension(
                 flags={'resv': resv, 's_flag': int(bool(s_flag)), 'qrv': qrv},
                 qqic=qqic,
-                num_sources=len(source_list),
+                num_sources=num_sources,
                 sources=source_list,
                 payload=b'',
             )
@@ -493,6 +507,18 @@ def _make_data(cls, data: 'Data_IGMPv3') -> 'dict[str, Any]':  # type: ignore[ov
             }
 
         query = cast('Data_Query', data)
+        # NOTE: The count as parsed, while the sources are those a cut begins (11 octets into the body,
+        # octet 12 of the message), zeros past it; the info keeps no octets captured to compare (#2760).
+        num_sources = None
+        record = query.get('__short_read__')
+        if record is not None and record[0] == 'body':
+            cut = max(record[1] - 11, 0)
+            try:
+                packed = b''.join(ipaddress.IPv4Address(source).packed for source in query.sources)
+            except (TypeError, ValueError):  # edited, to be refused by ``make``
+                packed = b''
+            if len(packed) == -(-cut // 4) * 4 and not packed[cut:].strip(b'\x00'):
+                num_sources = query.get('num_sources')
         return {
             'version': query.get('version'),
             'type': query.type,
@@ -504,6 +530,7 @@ def _make_data(cls, data: 'Data_IGMPv3') -> 'dict[str, Any]':  # type: ignore[ov
             'qrv': query.qrv,
             'qqic': query.qqic,
             'sources': query.sources,
+            'num_sources': num_sources,
             'payload': cls._make_payload(data),
         }
 

pcapkit/protocols/link/radiotap/protocol.py

@@ -2150,7 +2150,9 @@ def _read_tlv_data(self, tlv_type: 'Enum_FieldType', data: 'bytes') -> 'Data_Fie
         # what it is given; the refusal is handled here, so it is quiet.
         with CaughtByCaller(FieldValueError):
             try:
-                schema = cast('Schema', field_schema.unpack(octets, length, None))  # type: ignore[call-arg,misc]
+                # NOTE: The octets left off are zeros of the item, so its lists read on (#2655, #2760).
+                schema = cast('Schema', field_schema.unpack(octets, length,  # type: ignore[call-arg,misc]
+                                                            {'__zero_tail__': True}))
             except FieldValueError:
                 return data
         if layout is None: