fix(protocols): report a rebuilt frame's messages once and record cuts on the layer (#2711)

docs/source/contributing/conventions/protocol-family-grouping.rst

@@ -229,7 +229,10 @@ What stays loud:
   mode, installs the hooks.
 * **Any other error class.** An error the fallback absorbs is as loud as it was
   constructed, and :func:`~pcapkit.corekit.decorators.beholder` logs it at
-  :data:`logging.WARNING` as well.
+  :data:`logging.WARNING` as well. Inside a payload
+  :meth:`Protocol._make_payload <pcapkit.protocols.protocol.Protocol._make_payload>`
+  rebuilds, it is quiet, as the owner ruled on :issue:`2711`: the layer the payload
+  is rebuilt for parses the same octets again and reports it there, once.
 * **A refusal built under an inner** ``CaughtByCaller()``. A block naming no class,
   such as the one :meth:`~pcapkit.corekit.enum.EnumRegistry.register` enters, makes
   errors loud again inside the fallback's.

docs/source/contributing/pep.rst

@@ -969,7 +969,10 @@ offload heuristic on :issue:`2470`.
   ICMP and IGMP go by the original length as well (:issue:`2689`). A GRE or IGMP
   header the data ends inside is
   :attr:`~pcapkit.corekit.checksum.internet.ChecksumStatus.TRUNCATED` too
-  (:issue:`2689`).
+  (:issue:`2689`). Each of these cuts is recorded on the message's own header, so
+  the message rebuilt on its own reads
+  :attr:`~pcapkit.corekit.checksum.internet.ChecksumStatus.TRUNCATED` as well, an
+  ICMP one included (:issue:`2711`).
 * **DCCP covers what its Checksum Coverage gives** (:rfc:`4340#section-9.2`), over
   a pseudo-header that carries the whole packet's length, as TCP's does, so a
   fragment is

pcapkit/corekit/decorators.py

@@ -30,7 +30,7 @@
 import traceback
 from typing import TYPE_CHECKING, cast
 
-from pcapkit.utilities.contextvars import beholding
+from pcapkit.utilities.contextvars import beholding, rebuilding
 from pcapkit.utilities.exceptions import (CaughtByCaller, ProtocolError, StreamEOFError,
                                           StructError, stacklevel)
 from pcapkit.utilities.logging import DEVMODE, VERBOSE, get_logger
@@ -124,6 +124,13 @@ def beholder(
     exception is logged here at :data:`logging.WARNING`, and is as loud as it
     was constructed.
 
+    Inside a payload :meth:`Protocol._make_payload
+    <pcapkit.protocols.protocol.Protocol._make_payload>` rebuilds, the block
+    names every :exc:`Exception`, which is what the ``except`` below catches,
+    and nothing is logged here: the layer the payload is rebuilt for parses
+    the same octets again, outside the rebuild, and its own :func:`beholder`
+    reports the error there (:issue:`2711`).
+
     The recovery runs after the block has ended. Under the outermost
     :func:`beholder` of a parse, a
     :exc:`~pcapkit.utilities.exceptions.ProtocolError` it raises therefore
@@ -155,7 +162,8 @@ def beholder(
 
        A :exc:`~pcapkit.utilities.exceptions.ProtocolError` the decorated
        function builds is quiet, and one this decorator absorbs is not logged
-       here either (:issue:`2588`).
+       here either (:issue:`2588`). Inside a payload being rebuilt, so is every
+       other error (:issue:`2711`).
 
     :param func: decorated function
     :meta decorator:
@@ -173,13 +181,16 @@ def behold(*args: 'P.args', **kwargs: 'P.kwargs') -> 'R_beholder':
         except IndexError:
             length = None
 
+        # NOTE: A rebuilt payload's parse is repeated by the layer it is rebuilt
+        # for, which reports what this one would (#2711).
+        rebuilt = rebuilding.get()
         try:
             # call method
             token = beholding.set(True)
             try:
                 # NOTE: The whole call is quiet for a ProtocolError, which the ``except`` below
                 # records; the recovery runs after it, loud unless an outer block is active (#2588).
-                with CaughtByCaller(ProtocolError):
+                with CaughtByCaller(Exception if rebuilt else ProtocolError):
                     return func(*args, **kwargs)
             finally:
                 beholding.reset(token)
@@ -192,7 +203,7 @@ def behold(*args: 'P.args', **kwargs: 'P.kwargs') -> 'R_beholder':
 
             # NOTE: Keyed on the class, not on quietness: a quiet end-of-file StructError is still
             # logged here, and a ProtocolError built loud has logged at CRITICAL already (#2588).
-            if not isinstance(exc, ProtocolError):
+            if not isinstance(exc, ProtocolError) and not rebuilt:
                 logger.warning(str(exc), stack_info=DEVMODE, stacklevel=stacklevel())
 
                 if VERBOSE:

pcapkit/corekit/fields/collections.py

@@ -76,10 +76,23 @@ class ListField(FieldBase[List[_TL]], Generic[_TL]):
         item_type: Field type of the contained items.
         callback: Callback function to be called upon
             :meth:`self.__call__ <pcapkit.corekit.fields.field.FieldBase.__call__>`.
+        count: Number of items a field before this one counts, for a list
+            running to the end of the data; if a callable is given, it should
+            return an integer value and accept the current packet as its only
+            argument. A negative value, the default, counts none.
 
     This field is used to represent a list of fields, as in the case of lists of
     constant-length-field items in a protocol.
 
+    A list whose octets hold fewer items than ``count`` ended with the data
+    before the rest, and reports them as :attr:`shortfall`, so that
+    :meth:`Schema.unpack <pcapkit.protocols.schema.schema.Schema.unpack>`
+    records the cut; the items absent are not made up.
+
+    .. versionchanged:: 1.6.0
+
+       Takes ``count`` (:issue:`2711`).
+
     """
 
     @property
@@ -92,9 +105,20 @@ def optional(self) -> 'bool':
         """Field is optional."""
         return True
 
+    @property
+    def count(self) -> 'int':
+        """Number of items counted; negative where none are."""
+        return self._count
+
+    @property
+    def shortfall(self) -> 'int':
+        """Number of the items counted that the octets last unpacked did not hold."""
+        return self._shortfall
+
     def __init__(self, length: 'int | Callable[[dict[str, Any]], int]' = lambda _: -1,
                  item_type: 'Optional[FieldBase]' = None,
-                 callback: 'Callable[[Self, dict[str, Any]], None]' = lambda *_: None) -> 'None':
+                 callback: 'Callable[[Self, dict[str, Any]], None]' = lambda *_: None,
+                 count: 'int | Callable[[dict[str, Any]], int]' = -1) -> 'None':
         #self._name = '<list>'
         if not hasattr(self, '_name'):
             self._name = f'<{type(self).__name__[:-5].lower()}>'
@@ -108,6 +132,12 @@ def __init__(self, length: 'int | Callable[[dict[str, Any]], int]' = lambda _: -
         self._length = length
         self._template = '0s'
 
+        self._count_callback = None
+        if not isinstance(count, int):
+            self._count_callback, count = count, -1
+        self._count = count
+        self._shortfall = 0
+
     def __call__(self, packet: 'dict[str, Any]') -> 'Self':
         """Update field attributes.
 
@@ -126,6 +156,8 @@ def __call__(self, packet: 'dict[str, Any]') -> 'Self':
         if new_self._length_callback is not None:
             new_self._length = new_self._length_callback(packet)
             new_self._template = f'{new_self._length}s'
+        if new_self._count_callback is not None:
+            new_self._count = new_self._count_callback(packet)
         return new_self
 
     def pack(self, value: 'Optional[list[_TL]]', packet: 'dict[str, Any]') -> 'bytes':
@@ -306,6 +338,11 @@ def _unpack_items(self, file: 'IO[bytes]', packet: 'dict[str, Any]', is_schema:
                 data = field.unpack(buffer, packet)
 
             temp.append(data)
+
+        # NOTE: Items are read to the end of the octets, so those counted past them are
+        # not there to read: the data ended first, and the field reads short (#2711).
+        if self._count >= 0:
+            self._shortfall = max(self._count - len(temp), 0)
         return temp
 
 

pcapkit/corekit/schema/shortread.py

@@ -22,6 +22,8 @@
 import io
 from typing import TYPE_CHECKING, cast
 
+from pcapkit.utilities.contextvars import frame_length
+
 if TYPE_CHECKING:
     from typing import Any, Optional, TypeVar
 
@@ -53,10 +55,26 @@ def keep_short_read(protocol: 'ProtocolBase') -> 'None':
     and ``from_data(info.to_dict())`` cuts its rebuild back too; it is absent for
     a header that was captured whole.
 
+    Where the capture cut the frame short, as
+    :data:`~pcapkit.utilities.contextvars.frame_length` gives its lengths, a
+    layer whose message runs to the end of the frame records the cut too, on the
+    field it runs to the end in, through :meth:`Schema._note_short_read
+    <pcapkit.protocols.schema.schema.Schema._note_short_read>`, so its rebuild
+    reads as cut, as the layer did inside the frame (:issue:`2711`). A layer
+    whose own header declares where its message ends records none, whether the
+    cut falls at that end or past it; see :meth:`Schema._frame_end_field
+    <pcapkit.protocols.schema.schema.Schema._frame_end_field>`.
+
     """
     header = cast('Optional[Schema]', getattr(protocol, '__header__', None))
     if header is None:
         return
+    lengths = frame_length.get()
+    if lengths is not None and lengths[1] > lengths[0]:
+        name = header._frame_end_field()  # pylint: disable=protected-access
+        if name is not None:
+            octets = header.__buffer__[name]
+            header._note_short_read(name, len(octets) + lengths[1] - lengths[0], octets)  # pylint: disable=protected-access
     record = header.__dict__.get('__short_read__')
     if record is not None:
         protocol._info.__update__({'__short_read__': record})  # pylint: disable=protected-access

pcapkit/protocols/internet/icmpv4.py

@@ -97,7 +97,10 @@ class ICMPv4(ICMP[Data_ICMPv4, Schema_ICMPv4],
 
        The checksum is computed when :meth:`make` is given none, and verified
        on read under :data:`~pcapkit.utilities.config.CHECKSUM_VERIFY`,
-       unless the enclosing IP layer is a fragment (:issue:`2470`).
+       unless the enclosing IP layer is a fragment (:issue:`2470`). A message
+       whose header records a cut reads
+       :attr:`~pcapkit.corekit.checksum.internet.ChecksumStatus.TRUNCATED`
+       (:issue:`2711`).
 
     """
 
@@ -202,9 +205,16 @@ def read(self, length: 'Optional[int]' = None, *,
         # NOTE: The checksum covers the whole message, with no pseudo-header (RFC 792);
         # an IP fragment carries only part of one.
         if config.CHECKSUM_VERIFY:
-            from pcapkit.corekit.checksum.internet import verify_message  # isort: skip # pylint: disable=import-outside-toplevel
+            from pcapkit.corekit.checksum.internet import (  # isort: skip # pylint: disable=import-outside-toplevel
+                ChecksumStatus, verify_message)
+            # NOTE: A message whose header records a cut leaves part of what the
+            # checksum covers uncaptured, as for GRE and IGMP (#2711).
+            if '__short_read__' in schema.__dict__:
+                status = ChecksumStatus.TRUNCATED
+            else:
+                status = verify_message(self.alias, self._data, schema.checksum, length=length)
             icmp.__update__([
-                ('checksum_status', verify_message(self.alias, self._data, schema.checksum, length=length)),
+                ('checksum_status', status),
             ])
 
         # NOTE: The quoted original datagram stays raw in the message body,

pcapkit/protocols/link/gre.py

@@ -111,7 +111,8 @@ class GRE(Link[Data_GRE, Schema_GRE],
     as :attr:`~pcapkit.corekit.checksum.internet.ChecksumStatus.TRUNCATED`,
     as :data:`~pcapkit.utilities.contextvars.frame_length` tells it
     (:issue:`2653`), and so does a header the data ends inside
-    (:issue:`2689`).
+    (:issue:`2689`). The header records such a cut, so the message rebuilt on
+    its own reads as cut too (:issue:`2711`).
 
     """
 

pcapkit/protocols/protocol.py

@@ -1424,6 +1424,13 @@ def __post_init__(self, file: 'Optional[IO[bytes] | bytes]' = None,
             For construction arguments, please refer to
             :meth:`self.make <pcapkit.protocols.protocol.Protocol.make>`.
 
+        Note:
+            Inside a payload :meth:`_make_payload` rebuilds, the warnings of the
+            parse are dropped, as the layer the payload is rebuilt for parses
+            the same octets again and reports them there (:issue:`2711`). Those
+            of a parse that raises are reported as the error leaves, unless
+            :func:`~pcapkit.corekit.decorators.beholder` replaces the layer.
+
         """
         if file is None:
             _data = self.pack(**kwargs)
@@ -1439,8 +1446,20 @@ def __post_init__(self, file: 'Optional[IO[bytes] | bytes]' = None,
         self._data = _data
         #: io.BytesIO: Source packet stream.
         self._file = io.BytesIO(self._data)
-        #: pcapkit.protocols.data.data.Data: Parsed packet data.
-        self._info = self.unpack(length, **kwargs)
+        if not rebuilding.get():
+            #: pcapkit.protocols.data.data.Data: Parsed packet data.
+            self._info = self.unpack(length, **kwargs)
+            return
+
+        # NOTE: The enclosing layer parses these octets again and reports what
+        # they carry; as in ``_parse_next_layer``, a raise keeps them (#2711).
+        with hold_warnings() as held:
+            try:
+                self._info = self.unpack(length, **kwargs)
+            except Exception:
+                held.discard = beholding.get()
+                raise
+            held.discard = True
 
     def __init_subclass__(cls, /, schema: 'Optional[Type[_ST]]' = None,
                           data: 'Optional[Type[_PT]]' = None,
@@ -2561,8 +2580,10 @@ def _make_payload(cls, data: 'Data') -> 'ProtocolBase':
         parses its own octets again. So rebuilding a frame stops at
         :data:`~pcapkit.utilities.config.FRAME_LAYER_LIMIT` where parsing it did, rather than that many
         layers below each layer of it (:issue:`1610`). Nor does the payload's
-        parse warn of the limit: this protocol's own parse goes over the same
-        octets, so the rebuilt frame warns of it once, as the parsed one did.
+        parse report anything, the limit included: this protocol's own parse
+        goes over the same octets, so the rebuilt frame reports each warning,
+        and each error a next-layer fallback absorbs, once, as the parsed one
+        did (:issue:`2711`).
 
         Args:
             data: protocol data

pcapkit/protocols/schema/internet/igmp.py

@@ -43,8 +43,10 @@ class IGMP(Schema):
     #: Group address.
     group: 'IPv4Address' = IPv4AddressField()
     #: Payload, empty for a standards-conformant IGMPv1/v2 message, since the
-    #: 8-octet header is the whole message.
-    payload: 'bytes' = PayloadField()
+    #: 8-octet header is the whole message. It is the rest of the IP payload,
+    #: which the checksum covers too, so a cut inside it is recorded
+    #: (:issue:`2711`).
+    payload: 'bytes' = PayloadField(length=lambda pkt: pkt['__length__'])
 
     if TYPE_CHECKING:
         def __init__(self, type: 'Enum_Type', max_resp_time: 'int', checksum: 'bytes',  # pylint: disable=unused-argument,super-init-not-called,multiple-statements,redefined-builtin

pcapkit/protocols/schema/internet/igmpv3.py

@@ -11,7 +11,9 @@
 from pcapkit.corekit.fields.misc import ConditionalField, PayloadField, SchemaField, SwitchField
 from pcapkit.corekit.fields.numbers import EnumField, UInt8Field, UInt16Field
 from pcapkit.corekit.fields.strings import BitField, BytesField
+from pcapkit.protocols.schema.internet.ipv6 import IP_MAX_LENGTH
 from pcapkit.protocols.schema.schema import Schema, schema_final
+from pcapkit.utilities.contextvars import frame_depth
 from pcapkit.utilities.exceptions import ProtocolError
 
 __all__ = ['IGMPv3', 'Query', 'QueryExtension', 'Report', 'GroupRecord']
@@ -90,6 +92,43 @@ def query_extension_present(pkt: 'dict[str, Any]') -> 'bool':
     return pkt['__length__'] >= 4
 
 
+def report_record_count(pkt: 'dict[str, Any]') -> 'int':
+    """Count callback for :attr:`Report.records`.
+
+    Args:
+        pkt: Packet data.
+
+    Returns:
+        :attr:`Report.num_group_records`, for a message parsed on its own;
+        otherwise ``-1``, counting none: inside a frame, the layer before the
+        message or the frame's own lengths say where it ends, and on the
+        packing path ``'records'`` is already in ``pkt``, as no count is
+        without :attr:`Report.num_group_records`.
+
+    No layer before a message parsed on its own gives its length, and none
+    tells a cut from the end of the message, so the count does: records it
+    counts past the data are a cut, however the cut falls, on a field boundary
+    too, and :meth:`Schema.unpack
+    <pcapkit.protocols.schema.schema.Schema.unpack>` records it, the records
+    present alone being read (:issue:`2711`).
+
+    Raises:
+        ProtocolError: If the records counted, at the 8 fixed octets of each,
+            behind the 8 octets ahead of them, reach past the
+            :data:`~pcapkit.protocols.schema.internet.ipv6.IP_MAX_LENGTH` octets
+            an IP payload can hold, so the count is no cut but one the message
+            cannot carry (:issue:`2711`).
+
+    """
+    count = pkt.get('num_group_records')  # type: Optional[int]
+    if count is None or 'records' in pkt or frame_depth.get():
+        return -1
+    # NOTE: The 8 octets ahead of the records count against the IP payload too.
+    if 8 + 8 * count > IP_MAX_LENGTH:
+        raise ProtocolError(f'IGMPv3: {count} group records cannot fit in one IP payload')
+    return count
+
+
 @schema_final
 class QueryExtension(Schema):
     """Header schema for the IGMPv3 Query's source-list extension.
@@ -230,7 +269,8 @@ class Report(Schema):
         :class:`~pcapkit.protocols.schema.internet.hip.LocatorSetParameter`).
         :meth:`post_process` keeps only the first :attr:`num_group_records`
         records and gives the rest to :attr:`payload`, which is
-        :rfc:`3376#section-4.2.11`'s "Additional Data".
+        :rfc:`3376#section-4.2.11`'s "Additional Data". Where the data ends
+        before the records counted, see :func:`report_record_count`.
 
     """
 
@@ -247,6 +287,7 @@ class Report(Schema):
     records: 'list[GroupRecord]' = ListField(
         length=lambda pkt: pkt['__length__'],
         item_type=SchemaField(schema=GroupRecord),
+        count=report_record_count,
     )
     #: Payload; empty for a standards-conformant message with
     #: :attr:`num_group_records` matching the octets actually present.

pcapkit/protocols/schema/internet/ipv4.py

@@ -755,9 +755,11 @@ class IPv4(Schema):
     #: Payload. A Total Length of 0, as TCP segmentation offload leaves it in a
     #: capture taken on the sending host, takes the rest of the data and leaves
     #: no trailer; any other Total Length shorter than the header leaves the
-    #: payload empty and the rest to the trailer (:issue:`1547`).
+    #: payload empty and the rest to the trailer (:issue:`1547`). The rest is
+    #: sized as what is left of the length given, so a frame the capture cut
+    #: records its cut on it (:issue:`2711`).
     payload: 'bytes' = PayloadField(length=lambda pkt: (
-        -1 if pkt['length'] == 0 else max(pkt['length'] - pkt['vihl']['ihl'] * 4, 0)))
+        pkt['__length__'] if pkt['length'] == 0 else max(pkt['length'] - pkt['vihl']['ihl'] * 4, 0)))
     #: Octets captured past the Total Length, such as Ethernet minimum-frame
     #: padding. A :class:`~pcapkit.corekit.fields.misc.PayloadField` with no
     #: length takes whatever the payload leaves on unpack and writes its value