diff --git a/docs/source/pcapkit/protocols/application/httpv2.rst b/docs/source/pcapkit/protocols/application/httpv2.rst index a5a5adf04f..7cd2178585 100644 --- a/docs/source/pcapkit/protocols/application/httpv2.rst +++ b/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: diff --git a/docs/source/pcapkit/protocols/internet/igmpv3.rst b/docs/source/pcapkit/protocols/internet/igmpv3.rst index 480b0aab42..450272889e 100644 --- a/docs/source/pcapkit/protocols/internet/igmpv3.rst +++ b/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 ----------- diff --git a/pcapkit/corekit/fields/collections.py b/pcapkit/corekit/fields/collections.py index 980f7b3166..d6eb8b411b 100644 --- a/pcapkit/corekit/fields/collections.py +++ b/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 ` + 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 diff --git a/pcapkit/corekit/fields/field.py b/pcapkit/corekit/fields/field.py index 48cb6859c0..164528be47 100644 --- a/pcapkit/corekit/fields/field.py +++ b/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 + ` 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'``. diff --git a/pcapkit/protocols/application/httpv2.py b/pcapkit/protocols/application/httpv2.py index 34f24d9950..4afffc3abd 100644 --- a/pcapkit/protocols/application/httpv2.py +++ b/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 diff --git a/pcapkit/protocols/schema/application/httpv2.py b/pcapkit/protocols/schema/application/httpv2.py index 5af50e1825..dd3770e307 100644 --- a/pcapkit/protocols/schema/application/httpv2.py +++ b/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 - ` 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 - ` 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), ) diff --git a/pcapkit/protocols/schema/internet/igmpv3.py b/pcapkit/protocols/schema/internet/igmpv3.py index a938225262..9840dd0596 100644 --- a/pcapkit/protocols/schema/internet/igmpv3.py +++ b/pcapkit/protocols/schema/internet/igmpv3.py @@ -2,8 +2,7 @@ # mypy: disable-error-code=assignment """header schema for internet group management protocol version 3""" -import io -from typing import TYPE_CHECKING, cast +from typing import TYPE_CHECKING from pcapkit.const.igmp.record_type import RecordType as Enum_RecordType from pcapkit.const.igmp.type import Type as Enum_Type @@ -13,13 +12,13 @@ from pcapkit.corekit.fields.numbers import EnumField, UInt8Field, UInt16Field from pcapkit.corekit.fields.strings import BitField, BytesField from pcapkit.protocols.schema.schema import Schema, schema_final -from pcapkit.utilities.exceptions import ProtocolError, StreamEOFError +from pcapkit.utilities.exceptions import ProtocolError __all__ = ['IGMPv3', 'Query', 'QueryExtension', 'Report', 'GroupRecord'] if TYPE_CHECKING: from ipaddress import IPv4Address - from typing import IO, Any, Optional + from typing import Any, Optional from typing_extensions import TypedDict @@ -221,46 +220,6 @@ def __init__(self, record_type: 'Enum_RecordType | int', aux_data_len: 'int', aux_data: 'bytes') -> 'None': ... # pylint: disable=unused-argument,super-init-not-called,multiple-statements -class GroupRecordListField(ListField): - """Group record list that ends with the data. - - :attr:`Report.records` is sized by the IGMP length, which exceeds the - octets present when the capture cuts the Report short. A record read past - them consumes nothing, which :meth:`ListField.unpack - ` refuses to loop on, - so the list is sized by the octets present instead: the record the data - ends inside is kept as far as it goes, and :meth:`Schema.unpack - ` records the short read - (:issue:`2650`). - - """ - - def unpack(self, buffer: 'bytes | IO[bytes]', packet: 'dict[str, Any]') -> 'list[GroupRecord]': - """Unpack field value from :obj:`bytes`. - - Args: - buffer: Field buffer. - packet: Packet data. - - Returns: - Unpacked group records. - - """ - 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[GroupRecord]', super().unpack(buffer, packet)) - finally: - self._length = declared - - @schema_final class Report(Schema): """Header schema for an IGMPv3 Membership Report's body (:rfc:`3376#section-4.2`). @@ -283,10 +242,9 @@ class Report(Schema): reserved_2: 'bytes' = BytesField(length=2, default=b'\x00\x00') #: Number of group records, as received on the wire. num_group_records: 'int' = UInt16Field() - #: Group records, read no further than the data goes (see - #: :class:`GroupRecordListField`); see :meth:`post_process` for how this is - #: kept to exactly :attr:`num_group_records` entries. - records: 'list[GroupRecord]' = GroupRecordListField( + #: Group records; see :meth:`post_process` for how this is kept to + #: exactly :attr:`num_group_records` entries. + records: 'list[GroupRecord]' = ListField( length=lambda pkt: pkt['__length__'], item_type=SchemaField(schema=GroupRecord), ) @@ -303,17 +261,15 @@ def post_process(self, packet: 'dict[str, Any]') -> 'Schema': # pylint: disable Returns: Revised schema. + A capture that ends past the records kept ends inside :attr:`payload`, so + the short read :meth:`unpack` recorded on :attr:`records` is moved there + (:issue:`2687`). + """ if len(self.records) > self.num_group_records: raw = self.__buffer__['records'] - file = io.BytesIO(raw) - valid = [] # type: list[GroupRecord] - for _ in range(self.num_group_records): - try: - valid.append(GroupRecord.unpack(file)) # type: ignore[call-arg,misc] - except StreamEOFError: # pragma: no cover - break - consumed = file.tell() + valid = self.records[:self.num_group_records] + consumed = len(self.__fields__['records'].pack(valid, {})) # NOTE: ``get_payload`` reads ``__buffer__``, and ``prepare()`` clears # ``__updated__`` after this runs, so the buffer must be set here too. @@ -321,6 +277,10 @@ def post_process(self, packet: 'dict[str, Any]') -> 'Schema': # pylint: disable self.payload = raw[consumed:] self.__buffer__['records'] = raw[:consumed] self.__buffer__['payload'] = raw[consumed:] + + record = self.__dict__.get('__short_read__') + if record is not None and record[0] == 'records' and record[1] > consumed: + self.__dict__['__short_read__'] = ('payload', record[1] - consumed) return self if TYPE_CHECKING: diff --git a/pcapkit/utilities/config.py b/pcapkit/utilities/config.py index 67029d218b..d2eaa65dc2 100644 --- a/pcapkit/utilities/config.py +++ b/pcapkit/utilities/config.py @@ -399,12 +399,21 @@ def read_flag(name: 'str', default: 'bool' = False) -> 'bool': #: leaves any real file an allowance orders of magnitude past the one event it #: can want, while bounding the sum for a file whose blocks all lie. #: +#: It bounds the zero-fill of a cut list of schema items as well, from its first +#: octet and with no one-off allowance, since each item filled is a schema object +#: rather than a zero octet; see :meth:`ListField.unpack +#: `. +#: #: Set with :envvar:`PCAPKIT_ZERO_PAD_BUDGET_RATIO`. ``-1`` lifts the running #: budget; :data:`MAX_ZERO_PAD_LENGTH` still bounds each field. #: #: .. versionadded:: 1.5.0 #: #: Moved from ``pcapkit.corekit.fields.field._ZERO_PAD_BUDGET_RATIO`` (:issue:`1726`). +#: +#: .. versionchanged:: 1.6.0 +#: +#: Bounds the zero-fill of a cut list of schema items too (:issue:`2686`). ZERO_PAD_BUDGET_RATIO = read_limit('ZERO_PAD_BUDGET_RATIO', 0x10, risk=( 'a capture whose blocks all lie about their lengths can amplify each octet read ' 'into that many zero octets in memory')) diff --git a/tests/corekit/test_fields_list_zero_fill_2686_unit.py b/tests/corekit/test_fields_list_zero_fill_2686_unit.py new file mode 100644 index 0000000000..94b7ecb793 --- /dev/null +++ b/tests/corekit/test_fields_list_zero_fill_2686_unit.py @@ -0,0 +1,147 @@ +# -*- coding: utf-8 -*- +"""A list of schema items short of its length is zero-filled to it (#2686). + +:meth:`FieldBase.unpack ` reads a +field the data ends inside as though its missing octets were zeros. +:meth:`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. + +Every zero-filled item 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. + +Every case builds its own octets. + +""" + +import io +import unittest +from unittest import mock + +from tests._support import reimport_once_per_class, time_limit + + +class ListZeroFillTests(unittest.TestCase): + """:meth:`ListField.unpack` on schema items short of the field's length.""" + + def setUp(self) -> None: + reimport_once_per_class(self) + + from pcapkit.corekit.fields import field as field_module + from pcapkit.corekit.fields.collections import ListField + from pcapkit.corekit.fields.misc import SchemaField + from pcapkit.corekit.fields.numbers import UInt16Field, UInt32Field + from pcapkit.protocols.schema.schema import Schema, schema_final + from pcapkit.utilities import config + from pcapkit.utilities.exceptions import FieldValueError + + @schema_final + class Pair(Schema): + """A 16-bit identifier and a 32-bit value, as an HTTP/2 setting is.""" + + id: int = UInt16Field() + value: int = UInt32Field() + + @schema_final + class Pairs(Schema): + """A count of pairs, then as many octets of them as ``__length__`` leaves.""" + + count: int = UInt16Field() + pairs: list = ListField(length=lambda pkt: pkt['__length__'], + item_type=SchemaField(length=6, schema=Pair)) + + self.budget = field_module._zero_pad_budget # pylint: disable=protected-access + self.ledger = field_module._zero_pad_ledger # pylint: disable=protected-access + self.ListField, self.SchemaField, self.Pair, self.Pairs = ListField, SchemaField, Pair, Pairs + self.config, self.FieldValueError = config, FieldValueError + + def field(self, length: int): + """A list of :class:`Pair` items declaring ``length`` octets.""" + return self.ListField(length=length, item_type=self.SchemaField(length=6, schema=self.Pair)) + + @staticmethod + def pairs(parsed: 'list') -> 'list[tuple[int, int]]': + return [(item.id, item.value) for item in parsed] + + def test_a_short_buffer_and_stream_are_zero_filled_alike(self) -> None: + present = bytes.fromhex('000100001000' '000300') + stream = io.BytesIO(b'\xde\xad' + present) + stream.seek(2) + 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(stream.tell(), 2 + len(present)) + + def test_a_whole_list_is_read_as_it_was(self) -> None: + """The control: a list its buffer backs fills nothing and charges nothing.""" + whole = bytes.fromhex('000100001000' '000300000064') + stream = io.BytesIO(whole + b'\xff' * 6) + with self.budget() as ledger: + self.assertEqual(self.pairs(self.field(12).unpack(whole, {})), [(1, 4096), (3, 100)]) + self.assertEqual(self.pairs(self.field(12).unpack(stream, {})), [(1, 4096), (3, 100)]) + self.assertEqual(ledger, [24, 0]) + 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.""" + 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(schema.__dict__['__short_read__'], ('pairs', keep - 2)) + self.assertEqual(schema.pack(), cut) + self.assertEqual(self.Pairs.from_dict(schema.to_dict()).pack(), cut) + + def test_the_fill_is_refused_past_the_ratio_of_octets_read(self) -> None: + """No free band: padding up to the ratio times the octets read is filled, from a + fresh ledger, and one octet past it is refused.""" + ratio = self.config.ZERO_PAD_BUDGET_RATIO + present = bytes(6) + with self.budget() as ledger: + self.field(6 + ratio * 6).unpack(present, {}) + self.assertEqual(ledger, [6, ratio * 6]) + with self.budget(), self.assertRaisesRegex( + self.FieldValueError, rf'would zero-fill {ratio * 6 + 1} octet\(s\) of items, .* past the ' + rf'{ratio * 6} octet\(s\) allowed'): + self.field(6 + ratio * 6 + 1).unpack(present, {}) + + def test_the_zeros_earn_no_allowance(self) -> None: + """A fill the items read back as zeros credits only the octets present, so a + second fill as large as the first, against no new octets, is refused.""" + ratio = self.config.ZERO_PAD_BUDGET_RATIO + with self.budget() as ledger: + self.field(6 + ratio * 6).unpack(bytes(6), {}) + with self.assertRaises(self.FieldValueError): + self.field(ratio * 6 + 1).unpack(b'', {}) + self.assertEqual(ledger, [6, ratio * 6]) + + 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.""" + import contextvars + + 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])) + + def test_a_length_past_the_ceiling_is_refused(self) -> None: + ceiling = self.config.MAX_ZERO_PAD_LENGTH + with self.budget(), self.assertRaisesRegex( + self.FieldValueError, rf'declares a length of {ceiling + 1} octet\(s\), but only 6'): + self.field(ceiling + 1).unpack(bytes(6), {}) + + 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) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/corekit/test_fields_misc_schema_length_1148_unit.py b/tests/corekit/test_fields_misc_schema_length_1148_unit.py index 7f919b0407..83181185f6 100644 --- a/tests/corekit/test_fields_misc_schema_length_1148_unit.py +++ b/tests/corekit/test_fields_misc_schema_length_1148_unit.py @@ -26,7 +26,6 @@ from pcapkit.protocols.schema.internet.hip import LocatorSetParameter from pcapkit.protocols.schema.internet.mh import CGAParametersOption from pcapkit.protocols.schema.schema import Schema -from pcapkit.utilities.exceptions import FieldValueError from pcapkit.utilities.warnings import SchemaWarning #: One plain IPv6 locator: traffic, type 0, ``Locator Length`` 4, flags, lifetime. @@ -95,19 +94,21 @@ def test_length_less_items_do_not_warn(self) -> 'None': self.assertEqual(warned, []) self.assertEqual([(item.a, item.b) for item in result.items], [(0x0001, 0x0203), (0x0405, 0x0607)]) - def test_truncated_variable_item_still_warns_and_raises(self) -> 'None': - # A remainder of zero must not become ``prepare``'s quiet - # ``StreamEOFError``, which an extractor reads as end of capture. - for name, schema, data in ( - ('hip', LocatorSetParameter, locator_set(2, truncate=10)), - ('mh', CGAParametersOption, cga_option(CGA_PARAMETER[:-3])), + def test_truncated_variable_item_still_warns_and_rebuilds(self) -> 'None': + # A truncated item must not become ``prepare``'s quiet ``StreamEOFError``, + # which an extractor reads as end of capture: the list reads it + # zero-filled to its length and records the cut (#2686). + for name, schema, data, field in ( + ('hip', LocatorSetParameter, locator_set(2, truncate=10), 'locators'), + ('mh', CGAParametersOption, cga_option(CGA_PARAMETER[:-3]), 'parameters'), ): with self.subTest(name=name): with warnings.catch_warnings(record=True) as caught: warnings.simplefilter('always') - with self.assertRaises(FieldValueError): - schema.unpack(data) + result = schema.unpack(data) self.assertTrue(any(issubclass(w.category, SchemaWarning) for w in caught)) + self.assertEqual(result.__dict__['__short_read__'][0], field) + self.assertEqual(result.pack(), data) def test_fixed_length_item_running_short_still_warns(self) -> 'None': _, warned = self.unpack(ShortFixedList, b'\x06' + bytes(range(6))) diff --git a/tests/protocols/application/test_httpv2_settings_cut_2659_unit.py b/tests/protocols/application/test_httpv2_settings_cut_2659_unit.py index b729333f8e..25c3c2f424 100644 --- a/tests/protocols/application/test_httpv2_settings_cut_2659_unit.py +++ b/tests/protocols/application/test_httpv2_settings_cut_2659_unit.py @@ -1,11 +1,13 @@ # -*- coding: utf-8 -*- -"""An HTTP/2 ``SETTINGS`` frame cut inside its settings parses as far as it goes (#2659). +"""An HTTP/2 ``SETTINGS`` frame cut inside its settings parses and rebuilds (#2659, #2686). A capture cut short hands the frame fewer octets than its Length, which the IPv4 -total length still backs. ``SettingsFrame.settings`` was sized by that Length, so past +total length still backs. ``SettingsFrame.settings`` is sized by that Length, so past the octets present a ``SettingPair`` consumed nothing and ``ListField.unpack`` raised -``FieldValueError``, leaving the segment ``Raw``. ``HTTPv2.make`` also recomputed the -Length from the settings held, so a parsed cut frame could not rebuild as captured. +``FieldValueError``, leaving the segment ``Raw``. The list now reads its settings +zero-filled to the Length, as a cut ``DATA`` frame reads its data, so ``make`` keeps +computing the Length from the frame it builds, and an edited whole frame rebuilds with +the Length of its edited payload. Every case builds its own octets. @@ -39,8 +41,8 @@ def _datagram(app: bytes) -> bytes: def _expected(present: bytes) -> 'list[tuple[int, int]]': - """The pairs ``present`` holds, the last one zero-filled as a short read is.""" - padded = present + bytes(-len(present) % 6) + """The pairs ``present`` holds, zero-filled to the frame's Length as a short read is.""" + padded = present + bytes(len(PAIRS) - len(present)) return [(int.from_bytes(padded[i:i + 2], 'big'), int.from_bytes(padded[i + 2:i + 6], 'big')) for i in range(0, len(padded), 6)] @@ -93,13 +95,20 @@ def test_cut_through_httpv2_at_every_offset(self) -> None: self.assert_cut(HTTPv2(SETTINGS[:9 + keep], len(SETTINGS)), keep) self.assertEqual(caught, []) - def test_a_setting_cut_inside_its_value_keeps_its_identifier(self) -> None: + def test_the_settings_past_the_data_read_as_zeros(self) -> None: + """The setting the data ends inside keeps its identifier, and every setting + past the data reads as identifier and value zero, as a cut ``DATA`` frame + reads its data as zeros (#2686).""" from pcapkit.const.http.setting import Setting from pcapkit.protocols.application.httpv2 import HTTPv2 http = HTTPv2(SETTINGS[:9 + 9], len(SETTINGS)) self.assertEqual(list(http.info.settings.items(multi=True)), - [(Setting.HEADER_TABLE_SIZE, 4096), (Setting.MAX_CONCURRENT_STREAMS, 0)]) + [(Setting.HEADER_TABLE_SIZE, 4096), (Setting.MAX_CONCURRENT_STREAMS, 0), + (Setting.get(0), 0)]) + + data = HTTPv2(_frame(0x0, 0x1, 1, b'hello')[:9 + 2], 9 + 5) + self.assertEqual(data.info.data, b'he\x00\x00\x00') def test_whole_frame_is_the_control(self) -> None: from pcapkit.const.http.setting import Setting @@ -140,9 +149,28 @@ def test_the_next_frame_is_not_swallowed(self) -> None: self.assertIs(type(rest), Raw) self.assertIsNone(rest.info.error) + def test_a_length_no_capture_backs_goes_raw(self) -> None: + """A Length the octets read cannot back is refused, as zero-filling it would + make a setting per six octets of it, and the segment stays ``Raw`` (#2686).""" + from pcapkit.corekit.fields.field import _zero_pad_budget + from pcapkit.protocols.application.httpv2 import HTTPv2 + from pcapkit.protocols.internet.ipv4 import IPv4 + from pcapkit.protocols.misc.raw import Raw + + # a multiple of 6, and the most an IPv4 datagram holds past the three headers + length = 65484 + packet = _datagram(length.to_bytes(3, 'big') + SETTINGS[3:9 + 6] + bytes(length - 6)) + raw = packet[:len(packet) - (length - 6)] + with _zero_pad_budget(), warnings.catch_warnings(): + warnings.simplefilter('ignore') + ipv4 = IPv4(raw, len(raw)) + self.assertEqual(str(ipv4.protochain), 'IPv4:TCP:Raw') + self.assertIs(type(ipv4.payload.payload), Raw) + self.assertNotIsInstance(ipv4.payload.payload, HTTPv2) -class SettingPairListFieldTests(unittest.TestCase): - """The settings list is sized by the octets present, no further than its Length.""" + +class SettingsListTests(unittest.TestCase): + """The settings list reads a buffer and a stream alike, zero-filled to its length.""" def setUp(self) -> None: reimport_once_per_class(self) @@ -159,8 +187,9 @@ def test_reads_bytes_and_a_live_stream_alike(self) -> None: warnings.simplefilter('always') parsed = field.unpack(buffer, {}) self.assertEqual(caught, []) - self.assertEqual([(int(pair.id), pair.value) for pair in parsed], [(1, 4096), (3, 0)]) + self.assertEqual([(int(pair.id), pair.value) for pair in parsed], [(1, 4096), (3, 0), (0, 0)]) self.assertEqual(field.length, len(PAIRS)) + self.assertEqual(stream.tell(), 2 + 9) def test_a_live_stream_is_read_no_further_than_the_length(self) -> None: from pcapkit.protocols.schema.application.httpv2 import SettingsFrame @@ -172,83 +201,59 @@ def test_a_live_stream_is_read_no_further_than_the_length(self) -> None: self.assertEqual(stream.tell(), 12) -class SettingsFrameLengthTests(unittest.TestCase): - """``HTTPv2.make`` writes the ``frame_length`` it is given only as an :obj:`int` - from the Length the frame packs to up to ``0xFFFFFF``, and above that Length - only with no payload after the frame; any other is refused (#2617, #2659).""" - - #: One setting, so the frame packs to a Length of 6. - ONE_PAIR = {'settings': [(0x1, 4096)]} +#: ``(case, frame octets, edit to its info, the frame octets the edit gives)``, a +#: frame of every type; each fixed-size frame is edited in a value, keeping its Length. +EDITS = ( + ('DATA shrunk', _frame(0x0, 0x1, 1, b'hello'), {'data': b'hi'}, _frame(0x0, 0x1, 1, b'hi')), + ('DATA grown', _frame(0x0, 0x1, 1, b'hello'), {'data': b'hello, world'}, + _frame(0x0, 0x1, 1, b'hello, world')), + ('DATA padded, shrunk', _frame(0x0, 0x8, 1, b'\x02hello\x00\x00'), {'data': b'hi'}, + _frame(0x0, 0x8, 1, b'\x02hi\x00\x00')), + ('HEADERS shrunk', _frame(0x1, 0x4, 1, b'\x82\x86'), {'fragment': b'\x82'}, _frame(0x1, 0x4, 1, b'\x82')), + ('HEADERS padded with priority, grown', _frame(0x1, 0x2c, 1, b'\x01\x80\x00\x00\x03\x10\x82\x86\x00'), + {'fragment': b'\x82\x86\x84\x87'}, _frame(0x1, 0x2c, 1, b'\x01\x80\x00\x00\x03\x10\x82\x86\x84\x87\x00')), + # the weight is one more than its octet (RFC 9113, section 5.3.2) + ('PRIORITY', _frame(0x2, 0x0, 1, b'\x80\x00\x00\x03\x10'), {'weight': 32}, + _frame(0x2, 0x0, 1, b'\x80\x00\x00\x03\x1f')), + ('RST_STREAM', _frame(0x3, 0x0, 1, b'\x00\x00\x00\x08'), {'error': 1}, _frame(0x3, 0x0, 1, b'\x00\x00\x00\x01')), + ('SETTINGS shrunk', SETTINGS, {'settings': [(0x1, 7)]}, _frame(0x4, 0x0, 0, bytes.fromhex('000100000007'))), + ('SETTINGS grown', SETTINGS, {'settings': [(0x1, 4096), (0x3, 100), (0x4, 65536), (0x5, 16384)]}, + _frame(0x4, 0x0, 0, PAIRS + bytes.fromhex('000500004000'))), + ('PUSH_PROMISE grown', _frame(0x5, 0x4, 1, b'\x00\x00\x00\x02\x82'), {'fragment': b'\x82\x86'}, + _frame(0x5, 0x4, 1, b'\x00\x00\x00\x02\x82\x86')), + ('PING', _frame(0x6, 0x0, 0, b'12345678'), {'data': b'87654321'}, _frame(0x6, 0x0, 0, b'87654321')), + ('GOAWAY shrunk', _frame(0x7, 0x0, 0, b'\x00\x00\x00\x01\x00\x00\x00\x00debug'), {'debug_data': b''}, + _frame(0x7, 0x0, 0, b'\x00\x00\x00\x01\x00\x00\x00\x00')), + ('WINDOW_UPDATE', WINDOW_UPDATE, {'increment': 1}, _frame(0x8, 0x0, 0, b'\x00\x00\x00\x01')), + ('CONTINUATION grown', _frame(0x9, 0x4, 1, b'\x82\x86\x84'), {'fragment': b'\x82\x86\x84\x87'}, + _frame(0x9, 0x4, 1, b'\x82\x86\x84\x87')), + ('unassigned shrunk', _frame(0x20, 0x0, 1, b'xyz'), {'data': b'x'}, _frame(0x20, 0x0, 1, b'x')), + ('DATA shrunk, before a frame', _frame(0x0, 0x1, 1, b'hello') + WINDOW_UPDATE, {'data': b'hi'}, + _frame(0x0, 0x1, 1, b'hi') + WINDOW_UPDATE), +) + + +class EditedFrameTests(unittest.TestCase): + """A whole frame edited in its info rebuilds with the Length of the frame it + makes, through ``from_data(info.to_dict())``, as before the ``SETTINGS`` cut + carried the parsed Length into ``make`` (#2686).""" def setUp(self) -> None: reimport_once_per_class(self) - def test_a_length_the_frame_or_the_field_cannot_carry_is_refused(self) -> None: - import re - - from pcapkit.const.http.frame import Frame - from pcapkit.protocols.application.httpv2 import HTTPv2 - from pcapkit.utilities.exceptions import ProtocolError - - cases = ( - ('past the 24-bit field', Frame.SETTINGS, self.ONE_PAIR, 2 ** 24, b'', 6), - ('a bool', Frame.SETTINGS, self.ONE_PAIR, False, b'', 6), - # each in range as an int, so only the bool clause refuses it - ('True for one octet', Frame.DATA, {'data': b'a'}, True, b'', 1), - ('False for no octets', Frame.DATA, {'data': b''}, False, b'', 0), - ('a float', Frame.SETTINGS, self.ONE_PAIR, 2.0, b'', 6), - ('a str', Frame.SETTINGS, self.ONE_PAIR, '9', b'', 6), - ('negative', Frame.SETTINGS, self.ONE_PAIR, -1, b'', 6), - ('short of the settings', Frame.SETTINGS, self.ONE_PAIR, 0, b'', 6), - ('short of the data', Frame.DATA, {'data': b'abc'}, 2, b'', 3), - ('past the settings, before a frame', Frame.SETTINGS, self.ONE_PAIR, 18, WINDOW_UPDATE, 6), - ) - for label, type_, frame, length, payload, built in cases: - with self.subTest(label), self.assertRaisesRegex( - ProtocolError, rf'^HTTP/2: \[Type \d\] invalid frame_length: {re.escape(repr(length))} ' - rf'for a frame payload of {built} octet'): - HTTPv2(type=type_, sid=0 if type_ is Frame.SETTINGS else 1, frame=frame, - frame_length=length, payload=payload) - - def test_a_length_from_the_frame_to_the_field_maximum_is_written(self) -> None: - from pcapkit.const.http.frame import Frame - from pcapkit.protocols.application.httpv2 import HTTPv2 - - # A SETTINGS Length is a multiple of 6 (RFC 9113, section 6.5), which - # ``_read_http_settings`` holds it to, so the edge one past a frame is on DATA. - cases = ( - ('the settings, before a frame', Frame.SETTINGS, self.ONE_PAIR, 6, WINDOW_UPDATE, PAIRS[:6]), - ('the data, before a frame', Frame.DATA, {'data': b'abc'}, 3, WINDOW_UPDATE, b'abc'), - ('one past the data', Frame.DATA, {'data': b'abc'}, 4, b'', b'abc'), - ('one past the data, payload None', Frame.DATA, {'data': b'abc'}, 4, None, b'abc'), - ) - for label, type_, frame, length, payload, body in cases: - with self.subTest(label): - http = HTTPv2(type=type_, sid=0 if type_ is Frame.SETTINGS else 1, frame=frame, - frame_length=length, payload=payload) - self.assertEqual(http.data[:3], length.to_bytes(3, 'big')) - self.assertEqual(http.data[9:], body + (payload or b'')) - self.assertEqual(http.info.length, length) - if payload: - self.assertEqual(http.payload.info.increment, 65535) - - # Parsed back, a Length this far past the octets meets ``MAX_ZERO_PAD_LENGTH``, - # so the field maximum is checked on what ``make`` builds. - header = object.__new__(HTTPv2).make(type=Frame.SETTINGS, frame=self.ONE_PAIR, frame_length=0xFFFFFF) - self.assertEqual(header.length, 0xFFFFFF) - self.assertEqual(bytes(header)[:3], b'\xff\xff\xff') - - def test_a_length_past_the_settings_is_kept(self) -> None: - from pcapkit.const.http.frame import Frame - from pcapkit.const.http.setting import Setting + def test_every_frame_type_rebuilds_its_edit(self) -> None: from pcapkit.protocols.application.httpv2 import HTTPv2 - http = HTTPv2(type=Frame.SETTINGS, frame={'settings': [(Setting.HEADER_TABLE_SIZE, 4096)]}, - frame_length=18) - self.assertEqual(http.data, SETTINGS[:15]) - self.assertEqual(http.info.length, 18) - self.assertEqual(http.info['__short_read__'], ('frame', 6)) - self.assertEqual(list(http.info.settings.items(multi=True)), [(Setting.HEADER_TABLE_SIZE, 4096)]) + for case, raw, edit, expected in EDITS: + with self.subTest(case): + data = HTTPv2(raw, len(raw)).info.to_dict() + for key, value in edit.items(): + data[key] = value + built = HTTPv2.from_data(data) + self.assertEqual(built.data, expected) + self.assertEqual(built.info.length, int.from_bytes(expected[:3], 'big')) + self.assertNotIn('__short_read__', built.info) + self.assertEqual(HTTPv2(expected, len(expected)).data, expected) def test_the_length_is_computed_when_not_given(self) -> None: from pcapkit.const.http.frame import Frame @@ -260,14 +265,6 @@ def test_the_length_is_computed_when_not_given(self) -> None: self.assertEqual(http.info.length, 6) self.assertNotIn('__short_read__', http.info) - def test_an_explicit_parse_length_leaves_no_state_behind(self) -> None: - from pcapkit.const.http.frame import Frame - from pcapkit.protocols.application.httpv2 import HTTPv2 - - http = HTTPv2(type=Frame.SETTINGS, frame=self.ONE_PAIR, frame_length=18, length=27) - self.assertEqual(http.info.length, 18) - self.assertNotIn('_made_length', vars(http)) - def test_a_dict_with_no_length_computes_it(self) -> None: from pcapkit.const.http.frame import Frame from pcapkit.const.http.setting import Setting diff --git a/tests/protocols/internet/test_igmp_unit.py b/tests/protocols/internet/test_igmp_unit.py index ec9020b1e0..5f7ac96aff 100644 --- a/tests/protocols/internet/test_igmp_unit.py +++ b/tests/protocols/internet/test_igmp_unit.py @@ -1146,10 +1146,11 @@ def test_make_builds_report_from_dict_records(self) -> None: class IGMPv3TruncatedReportTests(unittest.TestCase): - """A Report the capture cuts inside its record list parses as far as its - octets go and records the cut, as a cut Query does, rather than falling - back to ``Raw`` (#2650). Each datagram's IPv4 header declares the whole - Report, so the IGMP layer is handed fewer octets than its length.""" + """A Report the capture cuts inside its record list parses and records the + cut, as a cut Query does, rather than falling back to ``Raw`` (#2650); the + records past the data read as zeros (#2686). Each datagram's IPv4 header + declares the whole Report, so the IGMP layer is handed fewer octets than its + length.""" def setUp(self) -> None: reimport_once_per_class(self) @@ -1224,8 +1225,9 @@ def test_report_cut_inside_a_record_header(self) -> None: self.assert_rebuilds(cut, parsed) def test_report_cut_inside_a_source_list(self) -> None: - """Cut inside the first record's second source: one record of the two - the count declares, which the rebuild still writes as two.""" + """Cut inside the first record's second source: the source reads as far + as it was captured, and the second record the count declares reads as + zeros, as a zero-filled record of no sources (#2686).""" from pcapkit.protocols.internet.igmpv3 import IGMPv3 from pcapkit.protocols.misc.no_payload import NoPayload @@ -1237,11 +1239,11 @@ def test_report_cut_inside_a_source_list(self) -> None: self.assertEqual(igmp.info['__short_read__'], ('body', 21)) self.assertEqual(igmp.info.length, 22) self.assertEqual(igmp.info.num_group_records, 2) - self.assertEqual(len(igmp.info.records), 1) - record = igmp.info.records[0] - self.assertEqual(str(record.multicast_address), '239.1.1.1') - self.assertEqual(record.num_sources, 2) - self.assertEqual(str(record.sources[0]), '10.0.0.1') + self.assertEqual( + [(int(record.record_type), str(record.multicast_address), tuple(map(str, record.sources))) + for record in igmp.info.records], + [(1, '239.1.1.1', ('10.0.0.1', '10.0.0.0')), (0, '0.0.0.0', ())], + ) self.assertIsInstance(igmp.payload, NoPayload) self.assert_rebuilds(cut, parsed) @@ -1263,8 +1265,8 @@ def test_cut_additional_data_goes_to_the_next_layer(self) -> None: self.assert_rebuilds(cut, parsed) def test_record_list_reads_bytes_and_a_live_stream_alike(self) -> None: - """The record list is sized by the octets present, from a buffer or a - stream part-way through, and keeps its declared length afterwards.""" + """The record list reads the octets present zero-filled to its length, + from a buffer or a stream part-way through, and keeps that length (#2686).""" from pcapkit.protocols.schema.internet.igmpv3 import Report records = IGMPV3_TWO_RECORD_REPORT[8:28] @@ -1280,6 +1282,7 @@ def test_record_list_reads_bytes_and_a_live_stream_alike(self) -> None: self.assertEqual([str(record.multicast_address) for record in parsed], ['239.1.1.1', '0.0.0.0']) self.assertEqual(parsed[1].num_sources, 3) + self.assertEqual([str(source) for source in parsed[1].sources], ['0.0.0.0'] * 3) self.assertEqual(field.length, 36) diff --git a/tests/protocols/internet/test_igmpv3_report_payload_cut_2687_unit.py b/tests/protocols/internet/test_igmpv3_report_payload_cut_2687_unit.py new file mode 100644 index 0000000000..356a29c84d --- /dev/null +++ b/tests/protocols/internet/test_igmpv3_report_payload_cut_2687_unit.py @@ -0,0 +1,98 @@ +# -*- coding: utf-8 -*- +"""An IGMPv3 Report cut inside its Additional Data packs back as captured (#2687). + +``Report.post_process`` gives the octets past the counted records to ``payload`` +(:rfc:`3376#section-4.2.11`), but the short read ``Schema.unpack`` recorded named +``records``, so packing the schema cut ``records`` and blanked the ``payload`` +holding the captured octets. The protocol path rebuilds a cut layer from its info +instead, which hid it, so every case here is on the schema. + +Every case builds its own octets. + +""" + +import unittest +import warnings + +from tests._support import reimport_once_per_class + +#: A ``MODE_IS_INCLUDE`` record for 239.1.1.1 with the one source 10.0.0.1. +RECORD = bytes.fromhex('01000001' 'ef010101' '0a000001') + + +def _report(count: int, body: bytes) -> bytes: + """An IGMPv3 Report counting ``count`` records, then ``body``; its checksum is zero.""" + return bytes([0x22, 0]) + bytes(4) + count.to_bytes(2, 'big') + body + + +#: ``(case, message, where its counted records end)`` +SHAPES = ( + ('one record and extra octets', _report(1, RECORD + bytes.fromhex('cafebabe')), 8 + len(RECORD)), + ('no records and extra octets', _report(0, bytes.fromhex('ca000000' 'deadbeef' '01020304')), 8), + ('additional data holding a whole record', _report(1, RECORD + RECORD), 8 + len(RECORD)), +) + + +class ReportPayloadCutTests(unittest.TestCase): + """At every cut inside the Additional Data, with the message's own length + declared and one past it (#2687).""" + + def setUp(self) -> None: + reimport_once_per_class(self) + + def cases(self): + """Yield ``(case, declared, cut, records_end)`` for every cut inside the Additional Data.""" + for case, message, records_end in SHAPES: + for declared in (len(message), len(message) + 4): + for keep in range(records_end + 1, len(message) + 1): + yield case, declared, message[:keep], records_end + + def test_unpack_then_pack_gives_the_capture(self) -> None: + from pcapkit.protocols.schema.internet.igmpv3 import IGMPv3 + + for case, declared, cut, records_end in self.cases(): + with self.subTest(case, declared=declared, keep=len(cut)), warnings.catch_warnings(): + # a cut field warns that its length runs past the data, which is the point + warnings.simplefilter('ignore') + schema = IGMPv3.unpack(cut, declared) + self.assertEqual(schema.body.payload, cut[records_end:]) + self.assertEqual(schema.pack(), cut) + self.assertEqual(schema.pack(), cut) + + def test_from_dict_of_to_dict_gives_the_capture(self) -> None: + from pcapkit.protocols.schema.internet.igmpv3 import IGMPv3 + + for case, declared, cut, _ in self.cases(): + with self.subTest(case, declared=declared, keep=len(cut)), warnings.catch_warnings(): + warnings.simplefilter('ignore') + schema = IGMPv3.from_dict(IGMPv3.unpack(cut, declared).to_dict()) + self.assertEqual(schema.pack(), cut) + + def test_the_cut_is_recorded_on_the_payload(self) -> None: + """The short read names ``payload`` and the octets of it captured; a + message captured whole records none.""" + from pcapkit.protocols.schema.internet.igmpv3 import IGMPv3 + + for case, declared, cut, records_end in self.cases(): + with self.subTest(case, declared=declared, keep=len(cut)), warnings.catch_warnings(): + warnings.simplefilter('ignore') + report = IGMPv3.unpack(cut, declared).body + record = None if len(cut) == declared else ('payload', len(cut) - records_end) + self.assertEqual(report.__dict__.get('__short_read__'), record) + + def test_a_cut_inside_the_records_stays_on_the_records(self) -> None: + """The control: the data ends inside a record, so the cut stays on ``records``.""" + from pcapkit.protocols.schema.internet.igmpv3 import IGMPv3 + + message = SHAPES[0][1] + for keep in range(9, 8 + len(RECORD)): + with self.subTest(keep=keep), warnings.catch_warnings(): + warnings.simplefilter('ignore') + schema = IGMPv3.unpack(message[:keep], len(message)) + self.assertEqual(schema.body.__dict__.get('__short_read__'), ('records', keep - 8)) + self.assertEqual(schema.body.payload, b'') + self.assertEqual(schema.pack(), message[:keep]) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/protocols/schema/test_schema_unit.py b/tests/protocols/schema/test_schema_unit.py index 4db8e1ce41..4331649a9e 100644 --- a/tests/protocols/schema/test_schema_unit.py +++ b/tests/protocols/schema/test_schema_unit.py @@ -525,32 +525,35 @@ def test_schema_list_field_unpack_rejects_a_schema_item_consuming_nothing(self) :meth:`ListField.unpack ` sizes a schema item - by ``len(data)`` in the same way, so a budget larger than the octets behind - it leaves the item schema reading an exhausted stream, recording nothing, - and subtracting nothing. Reachable from a TCP segment whose ``SACK`` option - declares more octets than the option area holds, which - :file:`tests/protocols/transport/test_tcp_udp_unit.py` pins; this covers - the field on its own, and pins the offset as a count into the field rather - than into the stream -- the same reading as its ``OptionField`` subclass. - - Two items parse off the two octets below and the third finds the stream - exhausted, so the diagnostic's ``after 2 item(s)`` is a count of what was + by ``len(data)`` in the same way, so an item schema that reads none of the + octets left records nothing and subtracts nothing. A list short of its + length is zero-filled to it (#2686), so the item here reads nothing by + its own shape: a zero marker ends it. This pins the offset as a count into + the field rather than into the stream -- the same reading as its + ``OptionField`` subclass. + + Two items parse off the two octets below and the third reads none of the + rest, so the diagnostic's ``after 2 item(s)`` is a count of what was parsed rather than an ordinal naming the second item. """ from pcapkit.corekit.fields.collections import ListField - from pcapkit.corekit.fields.misc import SchemaField + from pcapkit.corekit.fields.misc import ForwardMatchField, SchemaField from pcapkit.corekit.fields.numbers import UInt8Field + from pcapkit.corekit.fields.strings import BytesField from pcapkit.protocols.schema.schema import Schema, schema_final from pcapkit.utilities.exceptions import FieldValueError @schema_final class Marker(Schema): - type: int = UInt8Field(default=0) + #: The marker, peeked; a zero one reads nothing below. + peek: int = ForwardMatchField(UInt8Field(default=0)) + #: The marker itself, unless it is zero. + type: bytes = BytesField(length=lambda pkt: 1 if pkt['peek'] else 0, default=b'') @schema_final class MarkerListSchema(Schema): - #: Eight octets of budget, however few are really there. + #: Eight octets of budget. markers: list[Marker] = ListField( length=8, item_type=SchemaField(length=2, schema=Marker), @@ -558,7 +561,7 @@ class MarkerListSchema(Schema): field = MarkerListSchema.__fields__['markers'] - stream = io.BytesIO(b'\xde\xad' + b'\x01\x02') + stream = io.BytesIO(b'\xde\xad' + b'\x01\x02' + bytes(6)) stream.seek(2) with self.assertRaisesRegex(FieldValueError, r'after 2 item\(s\), at offset 2 of 8\b'): diff --git a/tests/protocols/transport/test_tcp_udp_unit.py b/tests/protocols/transport/test_tcp_udp_unit.py index 85550f4cb6..4f04447ab0 100644 --- a/tests/protocols/transport/test_tcp_udp_unit.py +++ b/tests/protocols/transport/test_tcp_udp_unit.py @@ -1349,11 +1349,11 @@ def test_a_sack_option_overrunning_the_option_area_raises(self) -> None: from the wire, with a :class:`~pcapkit.corekit.fields.misc.SchemaField` item type -- and that loop subtracts each item's *parsed* size from its budget. A ``Length`` - larger than the option area leaves it reading ``SACKBlock`` off an - exhausted stream, where every field reads ``b''``, the item parses to - nothing, and the budget never falls. This is the same defect as #431, in - ``OptionField``'s base class rather than in ``OptionField`` itself, and it - is reachable from a single TCP segment. + larger than the option area once left it reading ``SACKBlock`` off an + exhausted stream, where the item parsed to nothing and the budget never + fell (the same defect as #431, in ``OptionField``'s base class). The list + now reads the blocks zero-filled to the ``Length`` (#2686), and + ``OptionField`` refuses the option for running past the data (#1325). The segment below is 24 octets: a data offset of 6, so a four-octet option area, holding a ``SACK`` option that declares 22. The well-formed segment @@ -1365,7 +1365,7 @@ def test_a_sack_option_overrunning_the_option_area_raises(self) -> None: from pcapkit.const.tcp.option import Option from pcapkit.protocols.transport.tcp import TCP - from pcapkit.utilities.exceptions import FieldValueError + from pcapkit.utilities.exceptions import ProtocolError from tests._support import time_limit def segment(data_offset: 'int', options: 'bytes') -> 'bytes': @@ -1388,7 +1388,7 @@ def segment(data_offset: 'int', options: 'bytes') -> 'bytes': self.assertEqual(bytes(proto.__header__), good) bad = segment(6, bytes([Option.SACK, 22]) + b'\x36\xcc') - with self.assertRaisesRegex(FieldValueError, 'consumed no data'): + with self.assertRaisesRegex(ProtocolError, r'runs past the end of the data: '): with time_limit(): TCP(bad, len(bad))