fix(protocols): record a cut option area or SCTP padding where the data ends (#2680) (#2682)

Schema.unpack gave back a cut OptionField area's remainder by its declared
width, keeping and rewinding octets never read, and charged the cut to the
field before it parsed; #2679's SCTP.unpack override worked round that for
SCTP alone and could not read a stream that does not seek. padding_length
capped SCTP padding at the octets left, so a chunk cut inside it packed at
full width: 34 octets in, 36 out.

- Schema.unpack gives back no more of the remainder than it read, and charges
  the cut to the option field only when none is given back.
- ChunkListField reports its remainder in declared terms; the override is gone.
- padding_length returns the padding the Length calls for. OptionField.unpack
  asks _accepts_short_read before refusing a short option (#1325), and
  refuses by default, since a general padding rule accepts the PadN overruns
  #1325 refuses; PaddedOptionField accepts SCTP's, which the Length leaves out.
- Tests: SCTP items cut at every padding offset, alone, nested and bundled, on
  bytes, streams and envelopes; a TCP option area cut at every offset; SCTP on
  a stream that cannot seek; an SCTP item overrunning its chunk, still refused;
  an option reporting more than it read. #2679's tests pass unchanged.

Every unittest leg passes; captures parse identically to main.

Closes #2680
Closes #2685

Author: JarryShaw

docs/source/pcapkit/protocols/transport/sctp.rst

@@ -323,6 +323,10 @@ Header Schemas
    :members:
    :show-inheritance:
 
+.. autoclass:: pcapkit.protocols.schema.transport.sctp.PaddedOptionField
+   :members:
+   :show-inheritance:
+
 .. autoclass:: pcapkit.protocols.schema.transport.sctp.ChunkListField
    :members:
    :show-inheritance:

pcapkit/corekit/fields/collections.py

@@ -449,7 +449,8 @@ def unpack(self, buffer: 'bytes | IO[bytes]', packet: 'dict[str, Any]') -> 'list
             FieldValueError: If an option consumes nothing from ``buffer``, since
                 the loop below has then no way to get past it.
             ProtocolError: If an option that begins with octets present reads
-                past the end of the data.
+                past the end of the data, unless :meth:`_accepts_short_read`
+                accepts it.
 
         .. versionchanged:: 1.5.0
 
@@ -553,7 +554,7 @@ def unpack(self, buffer: 'bytes | IO[bytes]', packet: 'dict[str, Any]') -> 'list
 
             # unpack option using option schema
             data = schema.unpack(file, length, packet)  # type: ignore[call-arg,misc,var-annotated]
-            if tracker.short and consumed > 0:
+            if tracker.short and consumed > 0 and not self._accepts_short_read(data):
                 raise ProtocolError(
                     f'Field {self.name} has an option that runs past the end of '
                     f'the data: {code!r} at offset {offset - start} of '
@@ -598,3 +599,21 @@ def unpack(self, buffer: 'bytes | IO[bytes]', packet: 'dict[str, Any]') -> 'list
 
         self._option_padding = length
         return temp
+
+    def _accepts_short_read(self, option: '_TS') -> 'bool':  # pylint: disable=unused-argument
+        """Whether an option that read past the end of the data may stand.
+
+        Args:
+            option: Option just unpacked, which read short.
+
+        Returns:
+            :data:`False`, so that :meth:`unpack` refuses the option
+            (:issue:`1325`): padding cut short may be content the option's own
+            length declares, as a ``PadN`` option's is. A list whose items'
+            lengths leave out their trailing padding may override this, as
+            :class:`~pcapkit.protocols.schema.transport.sctp.PaddedOptionField`
+            does; :meth:`Schema.pack <pcapkit.protocols.schema.schema.Schema.pack>`
+            then writes back only the octets read.
+
+        """
+        return False

pcapkit/protocols/schema/schema.py

@@ -732,9 +732,10 @@ def unpack(cls, data: 'bytes | IO[bytes]',
             declares the size of the whole area it may read, but stops at the
             end-of-option-list marker and reports the unconsumed remainder
             through ``__option_padding__``. Since that remainder has not been
-            parsed, we rewind ``data`` by it, so that the fields which size
-            themselves from ``__option_padding__`` read the remainder itself
-            rather than the same number of octets from beyond it.
+            parsed, we rewind ``data`` by as much of it as was read, so that the
+            fields which size themselves from ``__option_padding__`` read the
+            remainder itself rather than the same number of octets from beyond
+            it, and record a cut inside it (:issue:`2685`).
 
         """
         # force cast arg type since decorator changed their signatures
@@ -798,7 +799,7 @@ def unpack(cls, data: 'bytes | IO[bytes]',
             # NOTE: bounded as the payload read above is (#1975).
             byte = data.read(min(length, sys.maxsize))
             self.__buffer__[field.name] = byte
-            if not isinstance(field, ForwardMatchField):
+            if not isinstance(field, (ForwardMatchField, OptionField)):
                 self._note_short_read(field.name, length, byte)
 
             value = field.unpack(byte, packet.copy())
@@ -842,15 +843,22 @@ def unpack(cls, data: 'bytes | IO[bytes]',
                 data.seek(-len(byte), io.SEEK_CUR)
                 self.__buffer__[field.name] = b''
             elif isinstance(field, OptionField) and field.option_padding > 0:
-                # the option list ended before the declared field length was
-                # exhausted; give the unconsumed remainder back to ``data``
-                # so that the following fields can read it
-                data.seek(-field.option_padding, io.SEEK_CUR)
+                # NOTE: The option list ended before the declared area did, and
+                # ``option_padding`` is the rest of that area; give back as much
+                # of it as was read, so that the following fields read it and
+                # record a cut inside it themselves (#2685).
                 consumed = length - field.option_padding
+                kept = min(consumed, len(byte))
+                if kept < len(byte):
+                    data.seek(kept - len(byte), io.SEEK_CUR)
 
-                self.__buffer__[field.name] = byte[:consumed]
+                self.__buffer__[field.name] = byte[:kept]
                 packet['__length__'] -= consumed
+                if kept == len(byte):
+                    self._note_short_read(field.name, length, byte)
             else:
+                if isinstance(field, OptionField):
+                    self._note_short_read(field.name, length, byte)
                 packet['__length__'] -= length
 
             if packet['__length__'] < 0:

pcapkit/protocols/schema/transport/sctp.py

@@ -3,7 +3,7 @@
 """header schema for stream control transmission protocol"""
 
 import io
-from typing import TYPE_CHECKING, cast
+from typing import TYPE_CHECKING
 
 from pcapkit.const.reg.apptype import AppType as Enum_AppType
 from pcapkit.const.reg.apptype import TransportProtocol as Enum_TransportProtocol
@@ -50,9 +50,7 @@
 
 if TYPE_CHECKING:
     from ipaddress import IPv4Address, IPv6Address
-    from typing import IO, Any, Callable, Optional
-
-    from typing_extensions import Self
+    from typing import IO, Any, Callable
 
     from pcapkit.protocols.protocol import ProtocolBase
 
@@ -96,13 +94,15 @@ def padding_length(pkt: 'dict[str, Any]') -> 'int':
         pkt: Packet data.
 
     Returns:
-        Number of padding bytes, clamped to the number of bytes left in the
-        enclosing structure, so that a final chunk captured without (all of)
-        its padding still parses, although :rfc:`9260#section-3.2` has the
-        sender pad every chunk. When packing, it is clamped as well
-        to the ``__padding__`` width :func:`pre_pack_padding` hands over, so a
-        final chunk captured without (all of) its padding rebuilds without it
-        (:issue:`1474`).
+        Number of padding bytes the ``length`` calls for. A final chunk captured
+        without (all of) its padding, although :rfc:`9260#section-3.2` has the
+        sender pad every chunk, reads short, which :meth:`Schema.unpack
+        <pcapkit.protocols.schema.schema.Schema.unpack>` records so that
+        :meth:`Schema.pack <pcapkit.protocols.schema.schema.Schema.pack>` writes
+        back only the octets read, and :class:`PaddedOptionField` lets it parse
+        (:issue:`2680`). When packing, it is clamped to the ``__padding__``
+        width :func:`pre_pack_padding` hands over, so such a chunk rebuilt from
+        its data model is short of its padding as well (:issue:`1474`).
 
     """
     length = pkt.get('length') or 0
@@ -111,11 +111,7 @@ def padding_length(pkt: 'dict[str, Any]') -> 'int':
     width = pkt.get('__padding__')
     if isinstance(width, int) and width >= 0:
         padding = min(padding, width)
-
-    remaining = pkt.get('__length__')
-    if not isinstance(remaining, int) or remaining < 0:
-        return padding
-    return min(padding, remaining)
+    return padding
 
 
 def pre_pack_padding(schema: 'Schema', packet: 'dict[str, Any]') -> 'None':
@@ -195,7 +191,8 @@ def nested_length(base: 'int') -> 'Callable[[dict[str, Any]], int]':
       ``__option_padding__``. The last chunk of a packet may omit that padding
       altogether, so the chunks wrap this callback in :func:`bounded`: the span
       then ends with the data, and the final parameter or cause reads only the
-      padding that is there (:issue:`1479`).
+      padding that is there (:issue:`1479`), recording the cut
+      (:issue:`2680`).
     * On construction, a separate padding field could not compute its own size:
       :meth:`Schema.pack <pcapkit.protocols.schema.schema.Schema.pack>` passes
       one shared ``packet`` mapping down into the nested schemas, and each
@@ -213,6 +210,39 @@ def callback(pkt: 'dict[str, Any]') -> 'int':
     return callback
 
 
+class PaddedOptionField(OptionField):
+    """List of chunks, parameters or error causes, the last of which may be cut
+    inside its trailing padding.
+
+    :meth:`OptionField.unpack <pcapkit.corekit.fields.collections.OptionField.unpack>`
+    refuses an item that reads past the end of the data (:issue:`1325`). The
+    last chunk of a packet may lack (some of) its padding, though, and so may
+    the last parameter or cause of that chunk, which share it
+    (:func:`nested_length`). An item's ``length`` leaves its padding out and
+    the receiver ignores it (:rfc:`9260#section-3.2`), and
+    :class:`ChunkListField` hands on only chunks whose ``length`` octets are
+    all present, so an item short of its padding alone is cut rather than
+    overrun. It records that as a short read of its ``padding``, and is kept
+    (:issue:`2680`).
+
+    """
+
+    def _accepts_short_read(self, option: 'Schema') -> 'bool':
+        """Whether ``option`` read short inside its trailing padding alone.
+
+        Args:
+            option: Item just unpacked, which read short.
+
+        Returns:
+            Whether the short read :meth:`Schema.unpack
+            <pcapkit.protocols.schema.schema.Schema.unpack>` recorded is of the
+            item's ``padding``.
+
+        """
+        record = option.__dict__.get('__short_read__')
+        return record is not None and record[0] == 'padding'
+
+
 class ErrorCause(EnumSchema[Enum_CauseCode]):
     """Header schema for SCTP error causes."""
 
@@ -644,7 +674,7 @@ class INITChunk(Chunk, code=Enum_Chunk.Initiation):
     init_tsn: 'int' = UInt32Field()
     #: Optional and variable-length parameters, including the chunk's own
     #: trailing padding; see :func:`nested_length`.
-    parameters: 'list[Parameter]' = OptionField(
+    parameters: 'list[Parameter]' = PaddedOptionField(
         length=bounded(nested_length(20)),
         base_schema=Parameter,
         type_name='type',
@@ -674,7 +704,7 @@ class INITACKChunk(Chunk, code=Enum_Chunk.Initiation_Acknowledgement):
     init_tsn: 'int' = UInt32Field()
     #: Optional and variable-length parameters, including the chunk's own
     #: trailing padding; see :func:`nested_length`.
-    parameters: 'list[Parameter]' = OptionField(
+    parameters: 'list[Parameter]' = PaddedOptionField(
         length=bounded(nested_length(20)),
         base_schema=Parameter,
         type_name='type',
@@ -729,7 +759,7 @@ class HeartbeatChunk(Chunk, code=Enum_Chunk.Heartbeat_Request):
 
     #: Heartbeat information parameters, including the chunk's own trailing
     #: padding; see :func:`nested_length`.
-    parameters: 'list[Parameter]' = OptionField(
+    parameters: 'list[Parameter]' = PaddedOptionField(
         length=bounded(nested_length(4)),
         base_schema=Parameter,
         type_name='type',
@@ -747,7 +777,7 @@ class HeartbeatACKChunk(Chunk, code=Enum_Chunk.Heartbeat_Acknowledgement):
 
     #: Heartbeat information parameters, including the chunk's own trailing
     #: padding; see :func:`nested_length`.
-    parameters: 'list[Parameter]' = OptionField(
+    parameters: 'list[Parameter]' = PaddedOptionField(
         length=bounded(nested_length(4)),
         base_schema=Parameter,
         type_name='type',
@@ -770,7 +800,7 @@ class AbortChunk(Chunk, code=Enum_Chunk.Abort):
     })
     #: Zero or more error causes, including the chunk's own trailing padding;
     #: see :func:`nested_length`.
-    error: 'list[ErrorCause]' = OptionField(
+    error: 'list[ErrorCause]' = PaddedOptionField(
         length=bounded(nested_length(4)),
         base_schema=ErrorCause,
         type_name='code',
@@ -813,7 +843,7 @@ class ErrorChunk(Chunk, code=Enum_Chunk.Operation_Error):
 
     #: One or more error causes, including the chunk's own trailing padding;
     #: see :func:`nested_length`.
-    error: 'list[ErrorCause]' = OptionField(
+    error: 'list[ErrorCause]' = PaddedOptionField(
         length=bounded(nested_length(4)),
         base_schema=ErrorCause,
         type_name='code',
@@ -875,7 +905,7 @@ def whole_chunks_length(data: 'bytes') -> 'int':
     Returns:
         Number of octets up to the end of the last chunk whose declared
         ``length`` octets are all present, with as much of its padding as there
-        is, see :func:`padding_length`. A ``length`` below the four-octet chunk
+        is, see :class:`PaddedOptionField`. A ``length`` below the four-octet chunk
         header (:rfc:`9260#section-3.2`) delimits nothing, so the run stops
         there too; four zero octets would otherwise read as a DATA chunk.
 
@@ -890,17 +920,20 @@ def whole_chunks_length(data: 'bytes') -> 'int':
     return end
 
 
-class ChunkListField(OptionField):
+class ChunkListField(PaddedOptionField):
     """Chunk list that stops at the last whole chunk.
 
     SCTP has no packet length field, so octets after the last whole chunk --
     fewer than a chunk header, or a chunk whose ``length`` runs past the data --
     cannot be told apart from a chunk cut short. They are not parsed but
     reported as option padding, which :meth:`Schema.unpack
     <pcapkit.protocols.schema.schema.Schema.unpack>` hands back to the stream for
-    :attr:`SCTP.trailer` to keep as captured (:issue:`1468`). The area reaches no
-    further than the octets present, as :meth:`SCTP.unpack` sizes it
-    (:issue:`2673`).
+    :attr:`SCTP.trailer` to keep as captured (:issue:`1468`). The padding is the
+    rest of the area declared, as for any
+    :class:`~pcapkit.corekit.fields.collections.OptionField`, so an area the data
+    ends inside hands back only the octets present (:issue:`2685`). The last
+    whole chunk may be cut inside its padding, and is kept as
+    :class:`PaddedOptionField` keeps one (:issue:`2680`).
 
     """
 
@@ -928,7 +961,7 @@ def unpack(self, buffer: 'bytes | IO[bytes]', packet: 'dict[str, Any]') -> 'list
             chunks = super().unpack(data[:whole], packet)
         finally:
             self._length = area
-        self._option_padding += len(data) - whole
+        self._option_padding += max(area, 0) - whole
         return chunks
 
 
@@ -967,49 +1000,6 @@ class SCTP(Schema):
     #: and :attr:`IPv4.trailer <pcapkit.protocols.schema.internet.ipv4.IPv4.trailer>`.
     trailer: 'bytes' = PayloadField(default=b'')
 
-    @classmethod
-    def unpack(cls, data: 'bytes | IO[bytes]',  # type: ignore[override]
-               length: 'Optional[int]' = None, packet: 'Optional[dict[str, Any]]' = None) -> 'Self':
-        """Unpack :obj:`bytes` into :class:`SCTP`, no further than the octets present.
-
-        Args:
-            data: Packed data.
-            length: Length of data, as the enclosing layer declares it.
-            packet: Unpacked data.
-
-        Returns:
-            Unpacked data as :class:`SCTP`.
-
-        SCTP has no packet length field (:rfc:`9260#section-3`), so a ``length``
-        past the octets present, as an IPv4 Total Length the capture cut short of
-        declares, is cut to them. Sized past them, :attr:`chunks` would record the
-        data as ending inside it while :class:`ChunkListField` hands the octets
-        after the last whole chunk to :attr:`trailer`, so :meth:`Schema.pack
-        <pcapkit.protocols.schema.schema.Schema.pack>` would drop them and
-        ``bytes(schema)`` count them twice (:issue:`2673`).
-
-        When no octet follows the last whole chunk, which the capture may have cut
-        inside its padding, the data does end inside :attr:`chunks`, so the cut is
-        recorded there as :meth:`Schema.unpack
-        <pcapkit.protocols.schema.schema.Schema.unpack>` records a short read, and
-        packing writes back only the octets read.
-
-        """
-        cut = False
-        if length is not None:
-            if isinstance(data, bytes):
-                present = len(data)
-            else:
-                start = data.tell()
-                present = data.seek(0, io.SEEK_END) - start
-                data.seek(start, io.SEEK_SET)
-            cut, length = length > present, min(length, present)
-
-        schema = cast('Self', super().unpack(data, length, packet))  # type: ignore[call-arg,misc]
-        if cut and not schema.__buffer__['trailer'] and '__short_read__' not in schema.__dict__:
-            schema.__dict__['__short_read__'] = ('chunks', len(schema.__buffer__['chunks']))
-        return schema
-
     if TYPE_CHECKING:
         def __init__(self, srcport: 'Enum_AppType | int', dstport: 'Enum_AppType | int',
                      vtag: 'int', chksum: 'bytes',