From d47825a5159bebca493033ffafd8771f9f130c11 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Wed, 4 Nov 2026 13:44:04 -0500 Subject: [PATCH] fix(protocols): rebuild a cut header at the width its parse read it (#2761) A cut header's rebuild was read at the width make() packs, so an IPv6_Ext cut at one octet, whose length is the width its parse was given, rebuilt with a length of two. - Data: a __short_width__ key, carried by to_dict() and left out of repr and the dumps, records the length the parse read a cut header at. - keep_short_read records it beside __short_read__, from the length each layer base passes on; Protocol.from_data reads the rebuild at it. - It replaces Protocol._uncaptured and _short_of_header: read at its parse's width, the rebuild leaves its next layer the length its parse did. - The #2767 sweep drops its known gap; the IGMPv3 direct-vs-IPv4 tests set the width aside, each read at its own; utilities-or-corekit.rst records the ruling. The new tests fail without the fix; every sample capture rebuilds unchanged. --- .../conventions/utilities-or-corekit.rst | 8 ++ docs/source/pcapkit/protocols/protocol.rst | 2 - pcapkit/corekit/schema/shortread.py | 22 +++-- pcapkit/protocols/application/application.py | 2 +- pcapkit/protocols/data.py | 21 +++-- pcapkit/protocols/internet/internet.py | 2 +- pcapkit/protocols/link/link.py | 2 +- .../protocols/misc/pcap/header/protocol.py | 2 +- pcapkit/protocols/protocol.py | 81 +++++++------------ pcapkit/protocols/transport/transport.py | 2 +- .../test_cut_ext_header_values_2761_unit.py | 35 +++++++- tests/protocols/internet/test_igmp_unit.py | 5 +- .../test_igmpv3_report_count_cut_2711_unit.py | 2 + .../test_cut_header_values_2761_unit.py | 22 +---- ...test_short_read_rebuild_quiet_2703_unit.py | 4 +- 15 files changed, 116 insertions(+), 96 deletions(-) diff --git a/docs/source/contributing/conventions/utilities-or-corekit.rst b/docs/source/contributing/conventions/utilities-or-corekit.rst index 49e9425031..22360cc139 100644 --- a/docs/source/contributing/conventions/utilities-or-corekit.rst +++ b/docs/source/contributing/conventions/utilities-or-corekit.rst @@ -84,6 +84,14 @@ through them, never through an ``unpack`` override of its own or a length callab clamped to the octets present, which hides the cut from the record. Ruled in review of :pr:`2682` (:issue:`2680`), and applied to option areas on :issue:`2694`. +**A rebuild is read at the width its parse read the header at.** +:func:`~pcapkit.corekit.schema.shortread.keep_short_read` records that width beside +the record, as ``__short_width__``, and :meth:`Protocol.from_data +` reads the rebuild at it, so it holds +the values and the next layer its parse did. Ruled by the owner on :issue:`2761`, over +a clamp inside :class:`~pcapkit.protocols.internet.ipv6_ext.protocol.IPv6_Ext`, the one +protocol whose overrunning ``length`` showed the gap. + A field callback tells a value the cut left as zero from one read through the same record: :meth:`Schema.unpack ` gives it to the fields after the one the data ends inside, as ``__short_read__`` in their diff --git a/docs/source/pcapkit/protocols/protocol.rst b/docs/source/pcapkit/protocols/protocol.rst index aaf0239a34..f1350d7275 100644 --- a/docs/source/pcapkit/protocols/protocol.rst +++ b/docs/source/pcapkit/protocols/protocol.rst @@ -120,8 +120,6 @@ Internal Definitions below. .. autoattribute:: _sigterm .. autoattribute:: __reconstructing__ .. autoattribute:: __short_read__ - .. autoattribute:: _uncaptured - .. autoattribute:: _short_of_header .. autoattribute:: _past_layer_limit .. autoattribute:: __data__ diff --git a/pcapkit/corekit/schema/shortread.py b/pcapkit/corekit/schema/shortread.py index fbe5ffb750..deab764aba 100644 --- a/pcapkit/corekit/schema/shortread.py +++ b/pcapkit/corekit/schema/shortread.py @@ -35,11 +35,13 @@ __all__ = ['keep_short_read', 'replay_short_read'] -def keep_short_read(protocol: 'ProtocolBase') -> 'None': +def keep_short_read(protocol: 'ProtocolBase', length: 'Optional[int]' = None) -> 'None': """Carry a parsed header's short read into the protocol's info. Args: protocol: Protocol instance, just parsed. + length: Length of data the parse was given, :data:`None` for the octets + it holds. Called by the layer base classes, and by the PCAP global :class:`~pcapkit.protocols.misc.pcap.header.protocol.Header`, once a parse is done. @@ -49,11 +51,13 @@ def keep_short_read(protocol: 'ProtocolBase') -> 'None': ` hands it to the rebuild, whose :meth:`Protocol.pack ` cuts the header back to what was captured before it is read (:issue:`1458`, - :issue:`2703`). The key is listed + :issue:`2703`). Beside it, ``__short_width__`` records the length the parse + read the header at, which the rebuild is read at in turn, so that it holds + the values its parse read (:issue:`2761`). Both keys are listed in :attr:`Data.__included__ `, - so :meth:`Info.to_dict ` carries it - and ``from_data(info.to_dict())`` cuts its rebuild back too; it is absent for - a header that was captured whole. + so :meth:`Info.to_dict ` carries them + and ``from_data(info.to_dict())`` cuts its rebuild back too; they are 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 @@ -77,7 +81,8 @@ def keep_short_read(protocol: 'ProtocolBase') -> 'None': 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 + width = len(protocol._data) if length is None else length # pylint: disable=protected-access + protocol._info.__update__({'__short_read__': record, '__short_width__': width}) # pylint: disable=protected-access def replay_short_read(protocol: '_PB', data: 'Any') -> '_PB': @@ -102,8 +107,9 @@ def replay_short_read(protocol: '_PB', data: 'Any') -> '_PB': :meth:`Protocol.__post_init__ ` has :meth:`~pcapkit.protocols.protocol.Protocol.read` parse those octets at the - width the header declares, so the rebuild is read and verified as its - capture was (:issue:`2703`). + width ``__short_width__`` records, the one its parse read them at, so the + rebuild is read and verified as its capture was (:issue:`2703`, + :issue:`2761`). The record names the field the capture ended inside and how much of it there was. Everything in the rebuild from that point on is dropped, which diff --git a/pcapkit/protocols/application/application.py b/pcapkit/protocols/application/application.py index 4198767627..8d197ba625 100644 --- a/pcapkit/protocols/application/application.py +++ b/pcapkit/protocols/application/application.py @@ -118,7 +118,7 @@ def __post_init__(self, file: 'Optional[IO[bytes] | bytes]' = None, # NOTE: A header the data ends inside keeps that in its ``info``, so # that :meth:`from_data` rebuilds only what was captured (#1458). - keep_short_read(self) + keep_short_read(self, length) # ``read`` may have dispatched the undissected remainder through # ``_decode_next_layer``, which already set the payload and the chain diff --git a/pcapkit/protocols/data.py b/pcapkit/protocols/data.py index 8ca80b4a01..b533571909 100644 --- a/pcapkit/protocols/data.py +++ b/pcapkit/protocols/data.py @@ -18,12 +18,14 @@ class Data(Info): """Base class for data models. - The ``__next_name__``, ``__next_type__`` and ``__short_read__`` keys are - metadata, listed in :attr:`~pcapkit.corekit.infoclass.Info.__included__` - and :attr:`~pcapkit.corekit.infoclass.Info.__excluded__`: + The ``__next_name__``, ``__next_type__``, ``__short_read__`` and + ``__short_width__`` keys are metadata, listed in + :attr:`~pcapkit.corekit.infoclass.Info.__included__` and + :attr:`~pcapkit.corekit.infoclass.Info.__excluded__`: :meth:`Protocol.from_data ` - dispatches the payload on the first two and on nothing else, and cuts a - short read back on the third. :meth:`to_dict` carries them, so that + dispatches the payload on the first two and on nothing else, cuts a short + read back on the third, and reads it at the width the fourth records. + :meth:`to_dict` carries them, so that ``from_data(info.to_dict())`` rebuilds as ``from_data(info)`` does, while the dumpers and ``repr`` leave them out. A hand-written :obj:`dict` must carry them as well -- and, for IPv6, the extension header chain under @@ -35,9 +37,9 @@ class Data(Info): """ #: Metadata keys that :meth:`to_dict` carries, for a rebuild to read. - __included__ = ['__next_name__', '__next_type__', '__short_read__'] + __included__ = ['__next_name__', '__next_type__', '__short_read__', '__short_width__'] #: Metadata keys that ``repr``, ``str`` and the dumps leave out. - __excluded__ = ['__next_name__', '__next_type__', '__short_read__'] + __excluded__ = ['__next_name__', '__next_type__', '__short_read__', '__short_width__'] if TYPE_CHECKING: #: Next field name, i.e., the name of the payload field. @@ -50,6 +52,11 @@ class Data(Info): #: ``from_data(info)`` does. See #: :func:`~pcapkit.corekit.schema.shortread.keep_short_read`. __short_read__: 'tuple[str, int]' + #: Length of data the parse read the header at, where it recorded + #: ``__short_read__``; present only then. ``from_data`` reads the + #: rebuild at it, so that the rebuild holds the values its parse read + #: (:issue:`2761`). + __short_width__: 'int' class Protocol(Data): diff --git a/pcapkit/protocols/internet/internet.py b/pcapkit/protocols/internet/internet.py index 253b4e43de..b83a930e85 100644 --- a/pcapkit/protocols/internet/internet.py +++ b/pcapkit/protocols/internet/internet.py @@ -304,7 +304,7 @@ def __post_init__(self, file: 'Optional[IO[bytes] | bytes]' = None, self._exthdr_depth = depth if file is not None else self._rebuilt_exthdr_depth() super().__post_init__(file, length, extension=extension, **kwargs) # type: ignore[arg-type] - keep_short_read(self) + keep_short_read(self, length) if extension and file is not None and len(self._data) > self.length: self._data = self._data[:self.length] diff --git a/pcapkit/protocols/link/link.py b/pcapkit/protocols/link/link.py index 81d7854e0a..accc96fefe 100644 --- a/pcapkit/protocols/link/link.py +++ b/pcapkit/protocols/link/link.py @@ -211,7 +211,7 @@ def __post_init__(self, file: 'Optional[IO[bytes] | bytes]' = None, """ super().__post_init__(file, length, **kwargs) # type: ignore[arg-type] - keep_short_read(self) + keep_short_read(self, length) ########################################################################## # Utilities. diff --git a/pcapkit/protocols/misc/pcap/header/protocol.py b/pcapkit/protocols/misc/pcap/header/protocol.py index 88a2c10021..3a92195dab 100644 --- a/pcapkit/protocols/misc/pcap/header/protocol.py +++ b/pcapkit/protocols/misc/pcap/header/protocol.py @@ -300,7 +300,7 @@ def __post_init__(self, file: 'Optional[IO[bytes] | bytes]' = None, self._file.name = file.name #: pcapkit.corekit.infoclass.Info: Parsed packet data. self._info = self.unpack(length, **kwargs) - keep_short_read(self) + keep_short_read(self, length) def __len__(self) -> 'Literal[24]': """Total length of corresponding protocol.""" diff --git a/pcapkit/protocols/protocol.py b/pcapkit/protocols/protocol.py index 31b2852353..9ce9e00cb5 100644 --- a/pcapkit/protocols/protocol.py +++ b/pcapkit/protocols/protocol.py @@ -705,26 +705,14 @@ class ProtocolBase(Generic[_PT, _ST], metaclass=ProtocolMeta): #: The ``__short_read__`` record of the data model :meth:`from_data` rebuilds #: this instance from. :meth:`pack` cuts the header it makes back to the - #: octets captured, and :meth:`__post_init__` reads them at the width the - #: header declares, so the rebuild is parsed and verified as its capture was - #: (:issue:`2703`). Set for the duration of that call alone, like - #: :attr:`__reconstructing__`; the class-level :data:`None` is what every - #: other code path sees. + #: octets captured, and :meth:`__post_init__` has :meth:`unpack` parse them + #: back at the width ``__short_width__`` records beside it, the one their + #: parse read them at, so the rebuild is parsed and verified as its capture + #: was (:issue:`2703`, :issue:`2761`). Set for the duration of that call + #: alone, like :attr:`__reconstructing__`; the class-level :data:`None` is + #: what every other code path sees. __short_read__: 'Optional[tuple[str, int]]' = None - #: How many octets of its header :meth:`pack` cut back by - #: :attr:`__short_read__`, which :meth:`__post_init__` adds back to the width - #: it reads the rebuild at. Set between those two calls alone. - _uncaptured: 'Optional[int]' = None - - #: Whether the data model :meth:`from_data` rebuilds this instance from kept - #: a :class:`~pcapkit.protocols.misc.raw.protocol.Raw` next layer past a header - #: the capture cut short, which only a parse reading the header short of its - #: width does. :meth:`_empty_next_layer` keeps it for the rebuild, read at that - #: width (:issue:`2717`). Set for the duration of that call alone, like - #: :attr:`__short_read__`. - _short_of_header: 'bool' = False - #: Caller supplied parsing context; see :mod:`pcapkit.corekit.context`. #: :meth:`self.__init__ ` replaces this with a real #: :class:`~pcapkit.corekit.context.ContextRegistry`; the class-level @@ -982,12 +970,10 @@ def pack(self, **kwargs: 'Any') -> 'bytes': return data # NOTE: Cut back before ``unpack`` reads and verifies it, at the width - # packed here, which ``__post_init__`` passes on (#2703). + # ``from_data`` passes on (#2703, #2761). header.__dict__['__short_read__'] = target.__short_read__ header._cut_short_read() # pylint: disable=protected-access - captured = bytes(header) - target._uncaptured = len(data) - len(captured) # pylint: disable=protected-access - return captured + return bytes(header) def unpack(self, length: 'Optional[int]' = None, **kwargs: 'Any') -> '_PT': """Unpack (parse) packet data. @@ -1410,7 +1396,8 @@ def from_data(cls, data: '_PT | dict[str, Any]', **kwargs: 'Any') -> 'Self': ``data`` may be the :obj:`dict` that :meth:`Info.to_dict ` returns, which carries the metadata the rebuild reads: ``__next_name__`` and ``__next_type__``, - which the payload is dispatched on, ``__short_read__``, and, for + which the payload is dispatched on, ``__short_read__`` and + ``__short_width__``, and, for :class:`~pcapkit.protocols.internet.ipv6.protocol.IPv6`, the extension header chain under ``__exthdr__``. A hand-written :obj:`dict` must carry them too, or no payload (nor IPv6 extension header) is rebuilt and no short @@ -1418,14 +1405,14 @@ def from_data(cls, data: '_PT | dict[str, Any]', **kwargs: 'Any') -> 'Self': its other keys (:issue:`2003`). A header the capture cut short, as ``__short_read__`` records, is cut - back before it is read, and read at the width it declares, so the - rebuild parses and verifies the octets captured as the capture did - (:issue:`2703`), holding the values its parse read from them, not those - :meth:`make` fills in past the cut (:issue:`2761`). Inside a payload - :meth:`_make_payload` rebuilds, which gives only its octets, the header - made is read instead. It keeps the ``Raw`` next layer its data model holds, - which a parse reading the header short of that width leaves it - (:issue:`2717`). + back before it is read, and read at the width ``__short_width__`` + records, the one its parse read it at, so the rebuild parses and + verifies the octets captured as the capture did (:issue:`2703`), + holding the values its parse read from them, not those :meth:`make` + fills in past the cut (:issue:`2761`), and the ``Raw`` next layer a + parse reading the header short of its width leaves it (:issue:`2717`). + Inside a payload :meth:`_make_payload` rebuilds, which gives only its + octets, the header made is read instead. Args: data: Protocol data. @@ -1467,19 +1454,17 @@ def from_data(cls, data: '_PT | dict[str, Any]', **kwargs: 'Any') -> 'Self': # built directly. So is ``__short_read__``, which ``pack`` cuts back by (#2703). self.__reconstructing__ = True self.__short_read__ = cast('Optional[tuple[str, int]]', data.get('__short_read__')) - # NOTE: A cut header kept a Raw next layer only where its parse read it short of - # its width, which the rebuild, read at that width, is told of (#2717). - if self.__short_read__ is not None: - from pcapkit.protocols.misc.raw.protocol import Raw # isort: skip # pylint: disable=import-outside-toplevel - next_type = data.get('__next_type__') - self._short_of_header = isinstance(next_type, type) and issubclass(next_type, Raw) + # NOTE: A cut header is read at the width its parse read it at, as recorded, so + # the rebuild holds the values and the next layer its parse did (#2717, #2761). + width = data.get('__short_width__') + if self.__short_read__ is not None and width is not None: + kwargs.setdefault('length', width) try: # initialize protocol instance self.__init__(**kwargs) # type: ignore[misc] finally: del self.__reconstructing__ del self.__short_read__ - vars(self).pop('_short_of_header', None) return self @@ -1706,14 +1691,10 @@ def __post_init__(self, file: 'Optional[IO[bytes] | bytes]' = None, """ if file is None: _data = self.pack(**kwargs) - # NOTE: A header rebuilt from a short read holds only what was captured, - # read at the width it declares, as a capture is at its parent's (#2703). - uncaptured = vars(self).pop('_uncaptured', None) - if uncaptured is not None and length is None: - length = len(_data) + uncaptured - # NOTE: Parsed back from the octets captured, not read from what ``make`` filled - # in past the cut (#2761); a payload ``_make_payload`` rebuilds gives its octets. - if uncaptured is not None and not rebuilding.get(): + # NOTE: A header rebuilt from a short read holds only what was captured, parsed + # back at the width ``from_data`` passes, not read from what ``make`` filled in + # past the cut (#2703, #2761); a payload ``_make_payload`` rebuilds gives its octets. + if self.__short_read__ is not None and not rebuilding.get(): self.__header__ = None # type: ignore[assignment] else: _data = file if isinstance(file, bytes) else file.read(length) # type: ignore[arg-type] @@ -3063,16 +3044,16 @@ def _empty_next_layer(self, file_: 'bytes', length: 'int') -> 'Optional[Type[Pro IPv6 extension header overrunning the payload (:issue:`2477`). Such a next layer is kept raw, not dispatched on a code the cut may have read as zeros (:issue:`1170`). :meth:`from_data` reads the rebuild of - a cut header at the width it declares (:issue:`2703`), which leaves - none, so the rebuild keeps ``Raw`` where its data model did, as - :attr:`_short_of_header` says (:issue:`2717`). + a cut header at the width its parse read it at, which leaves it the + same length, so the rebuild keeps ``Raw`` where its data model did + (:issue:`2717`, :issue:`2761`). .. versionadded:: 1.6.0 """ if file_ and length != 0: return None - if not file_ and (length < 0 or self._short_of_header): + if not file_ and length < 0: from pcapkit.protocols.misc.raw.protocol import Raw # isort: skip # pylint: disable=import-outside-toplevel return Raw from pcapkit.protocols.misc.no_payload.protocol import NoPayload # isort: skip # pylint: disable=import-outside-toplevel diff --git a/pcapkit/protocols/transport/transport.py b/pcapkit/protocols/transport/transport.py index 27ed9e86f6..b7488180a9 100644 --- a/pcapkit/protocols/transport/transport.py +++ b/pcapkit/protocols/transport/transport.py @@ -255,7 +255,7 @@ def __post_init__(self, file: 'Optional[IO[bytes] | bytes]' = None, """ super().__post_init__(file, length, **kwargs) # type: ignore[arg-type] - keep_short_read(self) + keep_short_read(self, length) ########################################################################## # Utilities. diff --git a/tests/protocols/internet/test_cut_ext_header_values_2761_unit.py b/tests/protocols/internet/test_cut_ext_header_values_2761_unit.py index 2feb50d2d9..8974e8a890 100644 --- a/tests/protocols/internet/test_cut_ext_header_values_2761_unit.py +++ b/tests/protocols/internet/test_cut_ext_header_values_2761_unit.py @@ -6,9 +6,10 @@ was read from the header ``make()`` built, whose Reserved fields are zero-filled past the cut and whose option area a ``PadN`` pads out to the length the header declares; the parse read the octets captured. Each field the issue names is checked against those -octets, in the parse and in its rebuild from the data model and from the :obj:`dict`, -but ``IPv6_Ext``'s ``length``, which :file:`tests/protocols/test_cut_header_values_2761_unit.py` -records as still divergent. +octets, in the parse and in its rebuild from the data model and from the :obj:`dict`. +``IPv6_Ext``'s ``length`` is the width the parse was given where the header overruns +it, and the rebuild, read at the two octets ``make()`` packs, took that for its own; +``__short_width__`` now records the width, and the rebuild is read at it. The octets are built here; this module is unit-tier and reads no capture file. @@ -37,6 +38,13 @@ #: Alert is refused. OPTION_CUTS = (1, 2, 4, 5, 6) +#: A generic extension header (RFC 6564, section 4) cut after its Next Header of UDP, +#: and the widths it is read at: none, the octet captured, and the eight octets its Hdr +#: Ext Len of zero declares. The data model's ``length`` is what each width holds of +#: those eight. +GENERIC = bytes([17]) +GENERIC_WIDTHS = {None: 1, 1: 1, 8: 8} + class CutExtensionHeaderValuesTests(unittest.TestCase): """The issue's fields read the octets captured, parsed and rebuilt.""" @@ -81,6 +89,27 @@ def test_ipv6_frag_reserved_octet(self) -> None: for rebuilt in self.rebuilds(layer, raw): self.assertEqual(rebuilt.info.reserved_octet, FRAG[1:count]) + def test_ipv6_ext_length(self) -> None: + # The width is recorded beside the cut, and the rebuild read at it holds the + # length its parse read, not the two octets make() packs. + from pcapkit.protocols.internet.ipv6_ext.protocol import IPv6_Ext + + for width, length in GENERIC_WIDTHS.items(): + with self.subTest(width=width): + layer = IPv6_Ext(GENERIC) if width is None else IPv6_Ext(GENERIC, width) + self.assertEqual((layer.info.length, layer.info.get('__short_width__')), (length, length)) + self.assertEqual(layer.info.next, None if length < 8 else 17) + self.assertIn('__short_width__', layer.info.to_dict()) + self.assertNotIn('__short_width__', repr(layer.info)) + for rebuilt in self.rebuilds(layer, GENERIC): + self.assertEqual(rebuilt.info.length, length) + with self.subTest(width='whole'): + # a header captured whole records no width, nor does its rebuild + whole = IPv6_Ext(GENERIC + bytes(7)) + self.assertEqual(whole.info.length, 8) + self.assertNotIn('__short_width__', whole.info.to_dict()) + self.assertNotIn('__short_width__', IPv6_Ext.from_data(whole.info.to_dict()).info.to_dict()) + def test_hopopt_and_ipv6_opts_options(self) -> None: # No octet before the seventh is a PadN, so neither parse nor rebuild holds one. from pcapkit.const.ipv6.option import Option diff --git a/tests/protocols/internet/test_igmp_unit.py b/tests/protocols/internet/test_igmp_unit.py index 7f89882c6c..f6c49109b2 100644 --- a/tests/protocols/internet/test_igmp_unit.py +++ b/tests/protocols/internet/test_igmp_unit.py @@ -1385,7 +1385,10 @@ def test_a_cut_is_recorded_as_inside_ipv4(self) -> None: carried = IPv4(outer[:len(outer) - len(packet) + keep]).payload direct = self.parse(packet[:keep]) self.assertEqual(direct.info['__short_read__'], carried.info['__short_read__']) - self.assertEqual(direct.info.to_dict(), carried.info.to_dict()) + ours, theirs = direct.info.to_dict(), carried.info.to_dict() + # each read at its own width, the octets given and IPv4's length (#2761) + self.assertEqual((ours.pop('__short_width__'), theirs.pop('__short_width__')), (keep, len(packet))) + self.assertEqual(ours, theirs) class IGMPv3RecordCountTests(unittest.TestCase): diff --git a/tests/protocols/internet/test_igmpv3_report_count_cut_2711_unit.py b/tests/protocols/internet/test_igmpv3_report_count_cut_2711_unit.py index 19b2ee90c8..bf27fffb62 100644 --- a/tests/protocols/internet/test_igmpv3_report_count_cut_2711_unit.py +++ b/tests/protocols/internet/test_igmpv3_report_count_cut_2711_unit.py @@ -165,6 +165,8 @@ def test_a_cut_parses_the_records_captured_as_behind_ipv4(self) -> None: self.assertIsInstance(carried, self.IGMPv3) ours, theirs = direct.info.to_dict(), carried.info.to_dict() records, filled = ours.pop('records'), theirs.pop('records') + # each read at its own width, the octets given and IPv4's length (#2761) + self.assertEqual((ours.pop('__short_width__'), theirs.pop('__short_width__')), (keep, len(msg))) self.assertEqual(ours, theirs) self.assertEqual(len(records), sum(start < keep for start in starts(SHAPES[label]))) self.assertEqual(records, filled[:len(records)]) diff --git a/tests/protocols/test_cut_header_values_2761_unit.py b/tests/protocols/test_cut_header_values_2761_unit.py index 9930f98b53..c0a6aa45a5 100644 --- a/tests/protocols/test_cut_header_values_2761_unit.py +++ b/tests/protocols/test_cut_header_values_2761_unit.py @@ -7,7 +7,8 @@ :meth:`Protocol._fill_short_read ` gives a reserved field, a ``PadN`` padding an option area out, a nested schema at its full width. The parse read the octets captured. The rebuild now parses those octets -back, so a round trip reproduces the parsed values as well as the octets. +back, at the width its parse read them at, so a round trip reproduces the parsed values +as well as the octets, with no exception. Every concrete protocol class, found by walking the protocol classes rather than listed, is built from its ``make()`` defaults and cut at every octet of that header, and so are @@ -77,17 +78,6 @@ 'reads its block by the section and interfaces before it', } -#: Cuts whose rebuild still reads a value its parse did not, by class and the octets -#: captured, in hex: the keys that differ, and why. An entry whose cut stops differing -#: so fails, as does one the sweep never reaches. -KNOWN = { - ('pcapkit.protocols.internet.ipv6_ext.protocol.IPv6_Ext', '11'): ( - ('length',), - # Protocol.__post_init__ reads the rebuild at the width make() packs (#2703), - # past the octet IPv6_Ext.read was given, and an overrun is that long (#2761). - 'IPv6_Ext.read takes an overrunning header for the width it is given'), -} - def _concrete() -> 'Iterator[tuple[str, type]]': """Every concrete protocol class, by qualified name.""" @@ -173,17 +163,13 @@ def test_every_class_is_cut(self) -> None: self.assertEqual(name in NOT_CUT, not built and name not in OCTETS) def test_every_cut_rebuilds_its_values(self) -> None: - reached = set() # type: set[tuple[str, str]] + # read at the width its parse read the header at, as __short_width__ records for name, raw, layer in self.cuts(): - key = (name, raw.hex()) - fields = KNOWN.get(key, ((),))[0] - reached.update({key} & set(KNOWN)) for path, data in (('model', layer.info), ('dict', layer.info.to_dict())): with self.subTest(protocol=name, captured=raw.hex(), path=path): rebuilt = type(layer).from_data(data) self.assertEqual(rebuilt.data, raw) - self.assertEqual(_differing(layer.info, rebuilt.info), fields) - self.assertEqual(reached, set(KNOWN)) + self.assertEqual(_differing(layer.info, rebuilt.info), ()) if __name__ == '__main__': diff --git a/tests/protocols/test_short_read_rebuild_quiet_2703_unit.py b/tests/protocols/test_short_read_rebuild_quiet_2703_unit.py index 0cead70425..1e141427af 100644 --- a/tests/protocols/test_short_read_rebuild_quiet_2703_unit.py +++ b/tests/protocols/test_short_read_rebuild_quiet_2703_unit.py @@ -245,7 +245,7 @@ def test_a_rebuilt_instance_keeps_no_short_read_of_its_own(self) -> None: with self.subTest(captured=count, path=path): rebuilt, _ = self.parse(lambda: type(layer).from_data(data)) # pylint: disable=cell-var-from-loop self.assertEqual(rebuilt.data, msg[:count]) - self.assertEqual({'__short_read__', '_uncaptured'} & set(vars(rebuilt)), set()) + self.assertNotIn('__short_read__', vars(rebuilt)) self.assertIsNone(rebuilt.__short_read__) def test_a_failed_rebuild_keeps_no_short_read_either(self) -> None: @@ -274,7 +274,7 @@ def recorded(proto: 'Any', **kwargs: 'Any') -> bytes: gre.from_data(data) with self.subTest(case=label, check='instance'): self.assertEqual(len(built), 1) - self.assertEqual({'__short_read__', '_uncaptured'} & set(vars(built[0])), set()) + self.assertNotIn('__short_read__', vars(built[0])) self.assertIsNone(built[0].__short_read__)