From ad7cd10a35ea675e9e4a69bdae5c4a1ae5dfcc1f Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Fri, 30 Oct 2026 19:22:44 -0400 Subject: [PATCH 1/2] fix(protocols)!: parse a malformed IPv6 extension header as Raw (#2477) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RFC 8200 §4 has a node discard such a packet, so the walk stops at it. - IPv6._import_next_layer: drop the IPv6_Ext fallback and __generic_ext_codes__. - The Raw the walk stopped on is the upper layer, so its error is logged once. - IPv6_Ext keeps Shim6, its only direct dispatch. - IPv6_Ext.error and read(error=) are no longer filled; see info.error on Raw. - Tests pin Raw for malformed and truncated HOPOPT/Routing/Opts/MH/HIP/AH. - mh.py docstrings and the round-trip tests no longer describe the fallback. --- .../extension-header-subclassing.rst | 16 +- .../pcapkit/protocols/internet/ipv6.rst | 3 - .../pcapkit/protocols/internet/ipv6_ext.rst | 49 ++--- pcapkit/protocols/data/internet/ipv6.py | 3 +- pcapkit/protocols/data/internet/ipv6_ext.py | 5 +- pcapkit/protocols/internet/ipv6.py | 171 ++++++---------- pcapkit/protocols/internet/ipv6_ext.py | 75 ++++--- pcapkit/protocols/internet/mh.py | 28 ++- .../test_ipv6_ext_after_fragment_unit.py | 40 ++-- .../internet/test_ipv6_ext_hint_unit.py | 8 +- .../protocols/internet/test_ipv6_ext_unit.py | 183 ++++++++++++++---- .../test_ipv6_exthdr_cut_rebuild_unit.py | 24 ++- tests/protocols/internet/test_ipv6_unit.py | 2 +- tests/protocols/internet/test_mh_unit.py | 53 ++--- .../test_declared_length_overrun_unit.py | 17 +- .../test_short_read_roundtrip_unit.py | 11 -- .../test_trial_parse_raises_2308_unit.py | 13 +- 17 files changed, 341 insertions(+), 360 deletions(-) diff --git a/docs/source/contributing/conventions/extension-header-subclassing.rst b/docs/source/contributing/conventions/extension-header-subclassing.rst index 1228b901d7..b870570f27 100644 --- a/docs/source/contributing/conventions/extension-header-subclassing.rst +++ b/docs/source/contributing/conventions/extension-header-subclassing.rst @@ -158,17 +158,13 @@ carries the same extension-mode contract as its siblings. inside the *encrypted* trailer, so with no key material there is nothing to continue on: ESP's own data model reports ``next`` as :obj:`None`, and :meth:`IPv6._decode_next_layer ` -ends the ordinary way one iteration later. So ESP is absent from -:attr:`IPv6.__generic_ext_codes__ ` --- not because it lacks a parser, which it has, but because it cannot hand the walk a -successor. +ends the ordinary way one iteration later -- not because ESP lacks a parser, which +it has, but because it cannot hand the walk a successor. The full reasoning, including how ESP's terminal case differs from 253/254's, is -written where the code is and is deliberately not restated at length here: see the -``#:`` comment on -:attr:`IPv6.__generic_ext_codes__ ` -(:file:`pcapkit/protocols/internet/ipv6.py`) and the module docstring of -:mod:`pcapkit.protocols.internet.ipv6_ext`. +written where the code is and is deliberately not restated at length here: see +:meth:`IPv6._import_next_layer ` +and the module docstring of :mod:`pcapkit.protocols.internet.ipv6_ext`. .. caution:: @@ -176,4 +172,4 @@ written where the code is and is deliberately not restated at length here: see t extension header" (wrong, and :rfc:`8200#section-4.5` quoted out of scope) is a different claim from "ESP cannot be walked past" (right, and about :rfc:`4303`'s wire format). Collapsing them is how ESP ends up either dropped - from the extension-header contract or wrongly added to the generic-dispatch set. + from the extension-header contract or wrongly walked past. diff --git a/docs/source/pcapkit/protocols/internet/ipv6.rst b/docs/source/pcapkit/protocols/internet/ipv6.rst index 7646353f7a..f810cf6162 100644 --- a/docs/source/pcapkit/protocols/internet/ipv6.rst +++ b/docs/source/pcapkit/protocols/internet/ipv6.rst @@ -45,9 +45,6 @@ Octets Bits Name Description .. automethod:: __index__ - .. autoattribute:: __generic_ext_codes__ - :no-value: - .. automethod:: _read_ip_hextet Protocol Constants diff --git a/docs/source/pcapkit/protocols/internet/ipv6_ext.rst b/docs/source/pcapkit/protocols/internet/ipv6_ext.rst index ecc0deca48..027867219a 100644 --- a/docs/source/pcapkit/protocols/internet/ipv6_ext.rst +++ b/docs/source/pcapkit/protocols/internet/ipv6_ext.rst @@ -14,8 +14,8 @@ unavailable on a header parsed as part of an IPv6 chain. Each subclass guards its own ``protocol`` the same way; this class's :attr:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext.protocol` deliberately does not. It also implements a -**generic** extractor for IPv6 extension headers [*]_, standing in whenever a -header's own dedicated parser is unavailable or has failed. :rfc:`6564#section-4` +**generic** extractor for IPv6 extension headers [*]_, for a header with no +dedicated parser. :rfc:`6564#section-4` requires, with an RFC 2119 **MUST**, that any IPv6 extension header defined since April 2012 carry the same first two octets: @@ -123,32 +123,25 @@ carry a ``next`` at all?) rather than on a list of codes. Entry Paths ----------- -This class is reached in these ways: - -1. **An unrecognised protocol number.** - :attr:`~pcapkit.const.ipv6.extension_header.ExtensionHeader.Shim6` is a real - IANA extension header (:rfc:`5533`) with no dedicated parser, so - :meth:`Internet._lookup_next_layer ` - would default it to plain :class:`~pcapkit.protocols.misc.raw.Raw`. That - has no ``next`` field, so the walk in - :meth:`IPv6._decode_next_layer ` would - crash on it (:issue:`891`). This module registers this class for that code - instead, at the bottom of the module, so dispatch reaches a working generic - parser directly. The registration is global, shared by every - :class:`~pcapkit.protocols.internet.internet.Internet` subclass, so - :meth:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext.__post_init__` gates on ``version == 6`` to - keep it from also activating for an IPv4 payload that carries protocol - number 140; see its docstring. -2. **A recognised header whose own parser raises.** ``HOPOPT``, - ``IPv6-Route``, ``IPv6-Opts``, ``MH``, ``HIP``, ``IPv6-Frag`` and ``AH`` all - have dedicated classes. When one of *those* raises, - :func:`~pcapkit.corekit.decorators.beholder` - (:meth:`Protocol._import_next_layer `) - would substitute plain :class:`~pcapkit.protocols.misc.raw.Raw`, which loses - ``next`` and crashes the walk as in path 1. - :meth:`IPv6._import_next_layer ` catches - the failure itself, one layer in from ``beholder``, and substitutes this - class: the bad header costs only itself, not the rest of the chain. +This class is reached by direct dispatch only. +:attr:`~pcapkit.const.ipv6.extension_header.ExtensionHeader.Shim6` is a real +IANA extension header (:rfc:`5533`) with no dedicated parser, so +:meth:`Internet._lookup_next_layer ` +would default it to plain :class:`~pcapkit.protocols.misc.raw.Raw`, whose +missing ``next`` field would end the walk in +:meth:`IPv6._decode_next_layer ` +there (:issue:`891`). This module registers this class for that code instead, +at the bottom of the module, so dispatch reaches a working generic parser +directly. The registration is global, shared by every +:class:`~pcapkit.protocols.internet.internet.Internet` subclass, so +:meth:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext.__post_init__` gates on +``version == 6`` to keep it from also activating for an IPv4 payload that +carries protocol number 140; see its docstring. + +A header with a dedicated class whose parser raises does not reach this class. +:func:`~pcapkit.corekit.decorators.beholder` makes it plain +:class:`~pcapkit.protocols.misc.raw.Raw`, and the walk stops there, as +:rfc:`8200#section-4` has a node discard such a packet (:issue:`2477`). .. _ipv6-ext-overrun-guard: diff --git a/pcapkit/protocols/data/internet/ipv6.py b/pcapkit/protocols/data/internet/ipv6.py index 73bd0e61d7..370c785b20 100644 --- a/pcapkit/protocols/data/internet/ipv6.py +++ b/pcapkit/protocols/data/internet/ipv6.py @@ -70,8 +70,7 @@ class IPv6(Protocol): #: Extension header chain, in wire order, as pairs of the parser #: class and its info. The per-header keys hold the same infos, but in #: the order of the keys of this :class:`~pcapkit.corekit.infoclass.Info`, - #: which differs on PyPy, so the chain is rebuilt from this alone; the - #: parser class keeps the IPv6_Ext a header fell back to + #: which differs on PyPy, so the chain is rebuilt from this alone #: (:issue:`1473`, :issue:`2003`). __exthdr__: 'tuple[tuple[Type[ProtocolBase], Data], ...]' diff --git a/pcapkit/protocols/data/internet/ipv6_ext.py b/pcapkit/protocols/data/internet/ipv6_ext.py index 8e55365b8b..ff9a33f39f 100644 --- a/pcapkit/protocols/data/internet/ipv6_ext.py +++ b/pcapkit/protocols/data/internet/ipv6_ext.py @@ -47,7 +47,6 @@ class IPv6_Ext(Protocol): length: 'int' #: Header-specific content after the two fixed octets, as consumed. data: 'bytes' - #: Original parsing error, if this instance was reached as a - #: :func:`~pcapkit.corekit.decorators.beholder` fallback rather than by - #: direct dispatch. + #: Parsing error a caller passed to + #: :meth:`read `, if any. error: 'Optional[Exception]' diff --git a/pcapkit/protocols/internet/ipv6.py b/pcapkit/protocols/internet/ipv6.py index 449721ddd7..23e05dbd2a 100644 --- a/pcapkit/protocols/internet/ipv6.py +++ b/pcapkit/protocols/internet/ipv6.py @@ -76,10 +76,10 @@ class IPv6(IP[Data_IPv6, Schema_IPv6], .. versionchanged:: 1.5.0 - * A Shim6 header, which has no dedicated parser, and a header in - :attr:`__generic_ext_codes__` whose dedicated parser raises are parsed - by :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext`, so the - rest of the chain is still walked (:issue:`891`). + * A Shim6 header, which has no dedicated parser, and an extension + header other than ESP whose dedicated parser raises are parsed by + :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext`, so the rest + of the chain is still walked (:issue:`891`). * A rebuild from :meth:`~pcapkit.protocols.internet.internet.Internet.from_data` keeps the Payload Length a truncated capture declares @@ -90,61 +90,23 @@ class IPv6(IP[Data_IPv6, Schema_IPv6], (:issue:`1231`), and a cut extension header chain rebuilds byte for byte (:issue:`1471`, :issue:`1473`). - """ + .. versionchanged:: 1.6.0 - ########################################################################## - # Defaults. - ########################################################################## + An extension header whose dedicated parser raises is the upper layer, + parsed once as :class:`~pcapkit.protocols.misc.raw.Raw`, and the chain + walk stops there, as :rfc:`8200#section-4` has a node discard such a + packet (:issue:`2477`). - #: Extension header codes with a *dedicated* parser class in this - #: package whose layout follows :rfc:`6564#section-4`'s generic ``next`` + - #: ``Hdr Ext Len`` format (see :ref:`ipv6-ext-length-rules` for the - #: exception table). - #: When that dedicated parser raises, :meth:`_import_next_layer` - #: substitutes :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext` - #: instead of letting the generic - #: :func:`~pcapkit.corekit.decorators.beholder` fall back to plain - #: :class:`~pcapkit.protocols.misc.raw.Raw`, which has no ``next`` field - #: and so cannot continue the walk in :meth:`_decode_next_layer`. - #: - #: These groups are deliberately absent: - #: - #: * :attr:`~pcapkit.const.ipv6.extension_header.ExtensionHeader.Shim6` - #: has no dedicated parser class in this package, so there is no "own - #: parser" for it to raise from. It reaches :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext` - #: by *direct* registration (see :ref:`ipv6-ext-entry-paths`), which - #: already produces exactly this class. - #: * ``ESP`` *does* have a dedicated, registered parser - #: (:class:`~pcapkit.protocols.internet.esp.ESP`). It is excluded - #: because :rfc:`4303` places the real Next Header byte inside the - #: encrypted trailer, so its own info always *carries* a ``next`` - #: attribute, just one that is :data:`None` whenever the payload could not - #: be decrypted -- which, with no key material available to a generic - #: parse, is always. The :meth:`_decode_next_layer` walk still ends - #: there, one iteration later, because :data:`None` fails - #: :class:`~pcapkit.const.ipv6.extension_header.ExtensionHeader`'s - #: constructor at the top of the loop -- the ordinary end-of-chain path, - #: unrelated to the structural check this set exists for. - #: * ``253`` and ``254`` have no dedicated parser at all, so they resolve - #: to plain :class:`~pcapkit.protocols.misc.raw.Raw`, whose info has no - #: ``next`` *attribute*; the structural check in - #: :meth:`_decode_next_layer` catches these. A next-header byte that is - #: not in the extension-header registry at all (e.g. ``147``) fails the - #: same constructor at the top of the loop, like any unrecognised - #: upper-layer protocol code. - #: - #: :meth:`_decode_next_layer`'s walk stops cleanly at whichever of these - #: (or any other IANA code this package has not implemented) it meets, and - #: keeps this layer's own header intact. - __generic_ext_codes__ = frozenset({ - Enum_ExtensionHeader.HOPOPT, - Enum_ExtensionHeader.IPv6_Route, - Enum_ExtensionHeader.IPv6_Opts, - Enum_ExtensionHeader.Mobility_Header, - Enum_ExtensionHeader.HIP, - Enum_ExtensionHeader.IPv6_Frag, - Enum_ExtensionHeader.AH, - }) + .. versionremoved:: 1.6.0 + + ``__generic_ext_codes__``, the extension headers whose dedicated parser + raising fell back to + :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext` (:issue:`2477`). + + """ + + #: The layer a header-walk stopped on, returned unparsed as the upper layer. + _walk_upper = None # type: Optional[ProtocolBase] ########################################################################## # Properties. @@ -466,10 +428,7 @@ def _is_first_fragment(info: 'Mapping[str, Any]') -> 'bool': ` Returns: - :data:`True` at a Fragment Offset of zero. A header whose dedicated - parser raised was parsed by - :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext` instead and - has no offset, so it does not count as one. + :data:`True` at a Fragment Offset of zero. """ return info.get('offset') == 0 @@ -574,21 +533,21 @@ def _decode_next_layer(self, ipv6: 'Data_IPv6', proto: 'Optional[int]' = None, # :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext` carry # ``next`` (possibly :data:`None`, on an overrun) -- Shim6 has no # dedicated parser but is registered to ``IPv6_Ext`` directly -- - # while every other code with no dedicated parser -- ``253``, - # ``254``, or whatever IANA assigns next -- resolves to plain - # :class:`~pcapkit.protocols.misc.raw.Raw`, which does not (see - # :attr:`__generic_ext_codes__`). Reading ``info.next`` on that - # would raise ``AttributeError`` and let a further-out - # :func:`~pcapkit.corekit.decorators.beholder` degrade the + # while plain :class:`~pcapkit.protocols.misc.raw.Raw` does not. + # That is what a code with no dedicated parser -- ``253``, + # ``254``, or whatever IANA assigns next -- resolves to, and what + # :func:`~pcapkit.corekit.decorators.beholder` makes of a header + # whose dedicated parser raises, so a malformed header ends the + # walk, as :rfc:`8200#section-4` has a node discard the packet + # (#2477). Reading ``info.next`` on that would raise + # ``AttributeError`` and let a further-out ``beholder`` degrade the # *whole* packet. # - # Such a layer is not recorded as an extension header: it consumed - # nothing (``Raw.length`` is 0), so recording it would emit its - # octets twice on rebuild, once from ``__exthdr__`` and once more as - # the upper layer. Stopping here with ``proto`` unchanged instead - # hands the remaining octets to ``super()._decode_next_layer`` - # below, which decodes them once, as the upper layer. + # Not recorded as an extension header, as it consumed nothing and + # would be rebuilt twice. It is the upper layer instead, returned + # unparsed by :meth:`_import_next_layer`, so its error is logged once. if not hasattr(info, 'next'): + self._walk_upper = next_ break # NOTE: Added through a :class:`MultiDict`, so a repeated header @@ -642,8 +601,7 @@ def _decode_next_layer(self, ipv6: 'Data_IPv6', proto: 'Optional[int]' = None, # fragment carries a slice of the Fragmentable Part from its # offset on, and that slice starts with no header at all. if not self._is_first_fragment(info): - # a header parsed with no offset is not known to be later - opaque = info.get('offset') is not None + opaque = True break # record real header & payload length (headers exclude) @@ -655,8 +613,7 @@ def _decode_next_layer(self, ipv6: 'Data_IPv6', proto: 'Optional[int]' = None, 'protocol': proto, # extension header chain in wire order, with the parser of each, - # for ``from_data``; the parser keeps the IPv6_Ext a header fell - # back to (#1473) + # for ``from_data`` (#1473) '__exthdr__': tuple(_exthdr), }) @@ -667,12 +624,12 @@ def _decode_next_layer(self, ipv6: 'Data_IPv6', proto: 'Optional[int]' = None, # with no protocol to alias it as, and ``'UDP' in frame`` does not hold # of a slice of UDP payload (#1545). Reassembly takes # ``fragment`` above, which this leaves as it was. - ipv6_exthdr = ProtoChain.from_list(_protos) # type: ignore[arg-type] self._exthdr_depth = len(_protos) # the headers the upper layer follows token = _frame_depth.set(depth + len(_protos)) try: return super()._decode_next_layer(ipv6, None if opaque else proto, raw_len, packet=packet, - ipv6_exthdr=ipv6_exthdr, payload=payload) + ipv6_exthdr=ProtoChain.from_list(_protos), # type: ignore[arg-type] + payload=payload) finally: _frame_depth.reset(token) @@ -695,27 +652,20 @@ def _import_next_layer(self, proto: 'int', length: 'Optional[int]' = None, *, # Instance of next layer. Notes: - If the dedicated parser for a code in :attr:`__generic_ext_codes__` - raises, this substitutes - :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext` for it - rather than letting the exception reach the + A header whose dedicated parser raises reaches the :func:`~pcapkit.corekit.decorators.beholder` decorating this - method, which would substitute plain - :class:`~pcapkit.protocols.misc.raw.Raw` -- and :class:`~pcapkit.protocols.misc.raw.Raw` has no - ``next`` field, so it could not continue - :meth:`_decode_next_layer`'s walk. Every other exception -- - including one raised by :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext` itself, - or by :class:`~pcapkit.protocols.internet.esp.ESP`'s own dedicated parser -- still - reaches ``beholder`` unchanged, giving a plain :class:`~pcapkit.protocols.misc.raw.Raw` - for that one layer. ``253`` and ``254`` have no dedicated parser, so they reach plain - :class:`~pcapkit.protocols.misc.raw.Raw` with no exception involved; - :meth:`_decode_next_layer` stops its walk structurally on any layer whose info carries - no ``next`` attribute, :class:`~pcapkit.protocols.misc.raw.Raw` included. - :class:`~pcapkit.protocols.internet.esp.ESP`'s info always carries a ``next`` - (:data:`None` unless its payload was decrypted, since :rfc:`4303` encrypts the real - value), so neither this substitution nor that structural check engages for it, and the - walk ends after it -- see :attr:`__generic_ext_codes__`'s docstring for the full - distinction. + method, which substitutes plain + :class:`~pcapkit.protocols.misc.raw.Raw`. That carries no ``next``, + so :meth:`_decode_next_layer`'s walk stops there, as + :rfc:`8200#section-4` has a node discard such a packet + (:issue:`2477`). ``253`` and ``254`` have no dedicated parser and + reach plain :class:`~pcapkit.protocols.misc.raw.Raw` with no + exception involved, which stops the walk the same way. The layer + the walk stopped on is then returned as the upper layer, not parsed + again. + :class:`~pcapkit.protocols.internet.esp.ESP`'s info always carries + a ``next``, :data:`None` unless its payload was decrypted, since + :rfc:`4303` encrypts the real value, so the walk ends after it. The upper layer past a walk cut at :data:`EXTENSION_HEADER_LIMIT ` is the @@ -726,6 +676,10 @@ def _import_next_layer(self, proto: 'int', length: 'Optional[int]' = None, *, # (:issue:`1604`). """ + upper, self._walk_upper = self._walk_upper, None + if upper is not None: + return upper + if payload is None: file_ = self.__header__.get_payload() else: @@ -734,19 +688,6 @@ def _import_next_layer(self, proto: 'int', length: 'Optional[int]' = None, *, # length = len(file_) protocol, extra = self._next_layer_class(proto, file_, length, extension) - try: - next_ = self._parse_next_layer(protocol, file_, length, version=version, extension=extension, - alias=proto, packet=packet, layer=self._exlayer, - protocol=self._exproto, __context__=self._exctx, **extra) - except Exception as exc: - from pcapkit.protocols.internet.ipv6_ext import \ - IPv6_Ext # isort: skip # pylint: disable=import-outside-toplevel - - if not (extension and protocol is not IPv6_Ext - and proto in self.__generic_ext_codes__): - raise - - next_ = IPv6_Ext(file_, length, version=version, extension=extension, - alias=proto, error=exc, packet=packet, layer=self._exlayer, - protocol=self._exproto, __context__=self._exctx) - return next_ + return self._parse_next_layer(protocol, file_, length, version=version, extension=extension, + alias=proto, packet=packet, layer=self._exlayer, + protocol=self._exproto, __context__=self._exctx, **extra) diff --git a/pcapkit/protocols/internet/ipv6_ext.py b/pcapkit/protocols/internet/ipv6_ext.py index e4e84afeaf..4c32cc28d7 100644 --- a/pcapkit/protocols/internet/ipv6_ext.py +++ b/pcapkit/protocols/internet/ipv6_ext.py @@ -7,9 +7,8 @@ :mod:`pcapkit.protocols.internet.ipv6_ext` contains :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext` only, which is both (GitHub issue :issue:`917`) the shared **base class** of -every IPv6 extension header in this package and a **generic** extractor that -stands in for a header whose dedicated parser is unavailable or has failed. -The class docstring separates these roles. +every IPv6 extension header in this package and a **generic** extractor for +a header with no dedicated parser. The class docstring separates these roles. Length Rules ------------ @@ -89,31 +88,25 @@ Entry Paths ----------- -This class is reached in these ways: - -1. **An unrecognised protocol number.** :class:`~pcapkit.const.ipv6.extension_header.ExtensionHeader.Shim6` - is a real IANA extension header (:rfc:`5533`) with no dedicated parser, so - :meth:`Internet._lookup_next_layer ` - would default it to plain :class:`~pcapkit.protocols.misc.raw.Raw`. That - has no ``next`` field, so the walk in - :meth:`IPv6._decode_next_layer ` - would crash on it (GitHub issue :issue:`891`). This module registers this - class for that code instead (see the bottom of the module), so dispatch - reaches a working generic parser directly. The registration is global, shared by - every :class:`~pcapkit.protocols.internet.internet.Internet` subclass, so - :meth:`__post_init__` gates on ``version == 6`` to keep it from also - activating for an IPv4 payload that carries protocol number 140; see its - docstring. -2. **A recognised header whose own parser raises.** ``HOPOPT``, - ``IPv6-Route``, ``IPv6-Opts``, ``MH``, ``HIP``, ``IPv6-Frag`` and ``AH`` all - have dedicated classes. When one of *those* raises, - :func:`~pcapkit.corekit.decorators.beholder` - (:meth:`Protocol._import_next_layer `) - would substitute plain :class:`~pcapkit.protocols.misc.raw.Raw`, which loses - ``next`` and crashes the walk as in path 1. - :meth:`IPv6._import_next_layer ` - catches the failure itself, one layer in from ``beholder``, and substitutes - this class: the bad header costs only itself, not the rest of the chain. +This class is reached by direct dispatch only. +:class:`~pcapkit.const.ipv6.extension_header.ExtensionHeader.Shim6` is a real +IANA extension header (:rfc:`5533`) with no dedicated parser, so +:meth:`Internet._lookup_next_layer ` +would default it to plain :class:`~pcapkit.protocols.misc.raw.Raw`, whose +missing ``next`` field would end the walk in +:meth:`IPv6._decode_next_layer ` +there (GitHub issue :issue:`891`). This module registers this class for that +code instead (see the bottom of the module), so dispatch reaches a working +generic parser directly. The registration is global, shared by every +:class:`~pcapkit.protocols.internet.internet.Internet` subclass, so +:meth:`__post_init__` gates on ``version == 6`` to keep it from also activating +for an IPv4 payload that carries protocol number 140; see its docstring. + +A header with a dedicated class whose parser raises does not reach this class. +:func:`~pcapkit.corekit.decorators.beholder` makes it plain +:class:`~pcapkit.protocols.misc.raw.Raw`, and the walk stops there, as +:rfc:`8200#section-4` has a node discard such a packet (GitHub issue +:issue:`2477`). The Overrun Guard ----------------- @@ -199,7 +192,7 @@ class IPv6_Ext(Internet[_PT, _ST], Generic[_PT, _ST], One class plays both, on the owner's ruling for GitHub issue :issue:`917`: 1. **The concrete fallback parser** for an :rfc:`6564`-conforming header - with no dedicated class, or whose dedicated class raised. :meth:`read`, + with no dedicated class, which is Shim6. :meth:`read`, :meth:`make`, :attr:`name`, :attr:`alias` and the ``schema=``/``data=`` class keywords implement it. 2. **The base class** of the implemented extension headers @@ -235,6 +228,14 @@ class keywords implement it. .. versionadded:: 1.5.0 + .. versionchanged:: 1.6.0 + + The library no longer passes ``error`` to :meth:`read`, so + :attr:`Data_IPv6_Ext.error ` + is :data:`None` unless a caller passes one. A header whose dedicated + parser raises is :class:`~pcapkit.protocols.misc.raw.Raw`, whose + ``info.error`` holds the error (:issue:`2477`). + """ #: Every subclass overrides :meth:`make` with its own fields and forwards @@ -386,8 +387,8 @@ def id(cls) -> 'tuple[Literal["HOPOPT"], Literal["IPv6_Route"], Literal["IPv6_Fr Returns: Index ID of every direct concrete subclass, each of which narrows - its own :meth:`id` to itself. A header parsed by this class as the - generic fallback therefore matches every one of these names. + its own :meth:`id` to itself. A Shim6 header, which this class + parses, therefore matches every one of these names. Note: ``== 'IPv6_Ext'`` is :data:`False`, as ``== 'IP'`` is on @@ -404,16 +405,14 @@ def read(self, length: 'Optional[int]' = None, *, # pylint: disable=arguments-d length: Length of packet data. extension: If the protocol is used as an IPv6 extension header. **kwargs: Arbitrary keyword arguments, including these, which are - this class's own and are supplied by :meth:`IPv6._import_next_layer - ` - rather than typed by a caller: + this class's own: * ``alias`` -- the numeric extension header code this instance stands in for, e.g. ``0`` for ``HOPOPT`` or ``140`` for - ``Shim6``. - * ``error`` -- the parsing error, when this instance was - reached as a :func:`~pcapkit.corekit.decorators.beholder` - fallback rather than by direct dispatch. + ``Shim6``, supplied by :meth:`IPv6._import_next_layer + `. + * ``error`` -- a parsing error to record in the result, if + any, which the library does not pass. Returns: Parsed packet data. diff --git a/pcapkit/protocols/internet/mh.py b/pcapkit/protocols/internet/mh.py index 6417c790e2..8acae54759 100644 --- a/pcapkit/protocols/internet/mh.py +++ b/pcapkit/protocols/internet/mh.py @@ -566,10 +566,9 @@ class FastBindingAcknowledgmentStatus(EnumLookup, IntEnum): unassigned raises :exc:`~pcapkit.utilities.exceptions.EnumValueError` from :meth:`_missing_` instead of minting an ``Unassigned_N`` member, because this RFC-inline value set stays immutable (decided on - :issue:`877`). Within an IPv6 chain only the Mobility Header layer is - lost to it: :class:`~pcapkit.protocols.internet.ipv6.IPv6` substitutes - :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext` for a - Mobility Header whose parser raises, and the rest of the packet is kept. + :issue:`877`). Within an IPv6 chain the Mobility Header and everything + after it are then :class:`~pcapkit.protocols.misc.raw.Raw`, as + :rfc:`8200#section-4` has a node discard such a packet (:issue:`2477`). The class defines no ``get`` of its own: ``get``/``get_all`` come from :class:`~pcapkit.corekit.enum.EnumLookup`, like every other @@ -637,10 +636,9 @@ class IPv6AddressPrefixCode(EnumLookup, IntEnum): unassigned raises :exc:`~pcapkit.utilities.exceptions.EnumValueError` from :meth:`_missing_` instead of minting an ``Unassigned_N`` member, because this RFC-inline value set stays immutable (decided on - :issue:`877`). Within an IPv6 chain only the Mobility Header layer is - lost to it: :class:`~pcapkit.protocols.internet.ipv6.IPv6` substitutes - :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext` for a - Mobility Header whose parser raises, and the rest of the packet is kept. + :issue:`877`). Within an IPv6 chain the Mobility Header and everything + after it are then :class:`~pcapkit.protocols.misc.raw.Raw`, as + :rfc:`8200#section-4` has a node discard such a packet (:issue:`2477`). The class defines no ``get`` of its own: ``get``/``get_all`` come from :class:`~pcapkit.corekit.enum.EnumLookup`, like every other @@ -703,10 +701,9 @@ class LocalizedRoutingStatus(EnumLookup, IntEnum): unassigned raises :exc:`~pcapkit.utilities.exceptions.EnumValueError` from :meth:`_missing_` instead of minting an ``Unassigned_N`` member, because this RFC-inline value set stays immutable (decided on - :issue:`877`). Within an IPv6 chain only the Mobility Header layer is - lost to it: :class:`~pcapkit.protocols.internet.ipv6.IPv6` substitutes - :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext` for a - Mobility Header whose parser raises, and the rest of the packet is kept. + :issue:`877`). Within an IPv6 chain the Mobility Header and everything + after it are then :class:`~pcapkit.protocols.misc.raw.Raw`, as + :rfc:`8200#section-4` has a node discard such a packet (:issue:`2477`). The class defines no ``get`` of its own: ``get``/``get_all`` come from :class:`~pcapkit.corekit.enum.EnumLookup`, and since it cannot mint, an @@ -758,10 +755,9 @@ class LMAAddressCode(EnumLookup, IntEnum): unassigned raises :exc:`~pcapkit.utilities.exceptions.EnumValueError` from :meth:`_missing_` instead of minting an ``Unassigned_N`` member, because this RFC-inline value set stays immutable (decided on - :issue:`877`). Within an IPv6 chain only the Mobility Header layer is - lost to it: :class:`~pcapkit.protocols.internet.ipv6.IPv6` substitutes - :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext` for a - Mobility Header whose parser raises, and the rest of the packet is kept. + :issue:`877`). Within an IPv6 chain the Mobility Header and everything + after it are then :class:`~pcapkit.protocols.misc.raw.Raw`, as + :rfc:`8200#section-4` has a node discard such a packet (:issue:`2477`). The class defines no ``get`` of its own: ``get``/``get_all`` come from :class:`~pcapkit.corekit.enum.EnumLookup`, and since it cannot mint, an diff --git a/tests/protocols/internet/test_ipv6_ext_after_fragment_unit.py b/tests/protocols/internet/test_ipv6_ext_after_fragment_unit.py index e7c8ee9ebe..261ca276a4 100644 --- a/tests/protocols/internet/test_ipv6_ext_after_fragment_unit.py +++ b/tests/protocols/internet/test_ipv6_ext_after_fragment_unit.py @@ -5,11 +5,12 @@ ending its extension header walk at every Fragment header and handing what follows to the upper-layer dispatch. That dispatch neither passes ``extension=True`` nor ``version=6``, so Shim6 fails -:class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext`'s version gate, and a -zero-filled HIP header or a Routing header of type 3 raises with no -:class:`IPv6_Ext` to stand in. Each becomes a plain -:class:`~pcapkit.protocols.misc.raw.Raw` leaf, so the TCP behind it is never -dissected, though it is without the Fragment header. +:class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext`'s version gate and +becomes a plain :class:`~pcapkit.protocols.misc.raw.Raw` leaf, so the TCP +behind it is never dissected, though it is without the Fragment header. A +header whose dedicated parser raises, such as a zero-filled HIP header or a +Routing header of type 3, is :class:`~pcapkit.protocols.misc.raw.Raw` either +way (#2477). :rfc:`8200#section-4.5` puts every header through the upper-layer one in the first fragment, at offset zero, so the walk goes on past its Fragment header. @@ -33,8 +34,9 @@ #: A 20-octet TCP header and one octet of payload. TCP = bytes.fromhex('9c40000900000001000000005000ffff0000000078') -#: Extension headers that :class:`IPv6_Ext` ends up parsing, by name: Next -#: Header code, and the 8-octet header with TCP as its Next Header. +#: Extension headers by name: Next Header code, and the 8-octet header with +#: TCP as its Next Header. :class:`IPv6_Ext` parses Shim6, and the dedicated +#: parsers of the others raise. HEADERS = { 'Shim6': (140, bytes.fromhex('0600000112345678')), 'HIP (zero-filled)': (139, bytes.fromhex('0600000000000000')), @@ -93,31 +95,37 @@ def test_issue_frames(self) -> None: self.assertEqual(str(fragmented.protochain), 'IPv6:IPv6-Frag:IPv6-Ext:TCP:Raw') self.assert_rebuilds(fragmented) - def test_direct_forms_reach_tcp(self) -> None: + def test_direct_forms(self) -> None: from pcapkit.const.reg.transtype import TransType + from pcapkit.protocols.misc.raw import Raw for name, (code, header) in HEADERS.items(): with self.subTest(header=name): ipv6 = self._parse(datagram(code, header + TCP)) - self.assertIn('TCP', ipv6) - self.assertEqual(ipv6.info.protocol, TransType.TCP) + if name == 'Shim6': + self.assertIn('TCP', ipv6) + self.assertEqual(ipv6.info.protocol, TransType.TCP) + else: + self.assertNotIn('TCP', ipv6) + self.assertEqual(list(ipv6.extension_headers.values()), []) + self.assertIs(type(ipv6.payload), Raw) + self.assertEqual(int(ipv6.info.protocol), code) self.assert_rebuilds(ipv6) def test_first_fragment_dissects_as_without_it(self) -> None: - from pcapkit.protocols.internet.ipv6_ext import IPv6_Ext from pcapkit.protocols.internet.ipv6_frag import IPv6_Frag for name, (code, header) in HEADERS.items(): direct = self._parse(datagram(code, header + TCP)) + walked = list(direct.extension_headers.values()) for more in (False, True): with self.subTest(header=name, more=more): ipv6 = self._parse(datagram(44, fragment(code, 0, more) + header + TCP)) - self.assertIn('TCP', ipv6) + self.assertEqual('TCP' in ipv6, 'TCP' in direct) exthdr = list(ipv6.extension_headers.values()) - self.assertEqual([type(ext) for ext in exthdr], [IPv6_Frag, IPv6_Ext]) - # ``error`` holds an exception, which compares by identity - self.assertEqual(repr(exthdr[1].info), - repr(next(iter(direct.extension_headers.values())).info)) + self.assertEqual([type(ext) for ext in exthdr], + [IPv6_Frag, *(type(ext) for ext in walked)]) + self.assertEqual([ext.info for ext in exthdr[1:]], [ext.info for ext in walked]) self.assertEqual(ipv6.info.protocol, direct.info.protocol) self.assertEqual(ipv6.payload.info, direct.payload.info) self.assert_rebuilds(ipv6) diff --git a/tests/protocols/internet/test_ipv6_ext_hint_unit.py b/tests/protocols/internet/test_ipv6_ext_hint_unit.py index fb461b13be..3f09733d02 100644 --- a/tests/protocols/internet/test_ipv6_ext_hint_unit.py +++ b/tests/protocols/internet/test_ipv6_ext_hint_unit.py @@ -20,9 +20,9 @@ RUNTIME_DEPS = ('tbtrim', 'aenum', 'chardet', 'dictdumper') HAS_RUNTIME = all(importlib.util.find_spec(name) is not None for name in RUNTIME_DEPS) -#: ``alias`` values that reach the generic fallback: an absent alias, a number -#: IANA has not registered, ``Shim6`` (registered to ``IPv6_Ext`` directly) and -#: ``HOPOPT`` (the code ``IPv6`` substitutes the fallback for). +#: ``alias`` values ``IPv6_Ext`` may be parsed with: an absent alias, a +#: number IANA has not registered, ``Shim6`` (registered to ``IPv6_Ext`` +#: directly) and ``HOPOPT`` (a code with a dedicated parser). ALIASES = {'absent': {}, 'unregistered': {'alias': 200}, 'Shim6': {'alias': 140}, 'HOPOPT': {'alias': 0}} #: The IPv6 payload length the caller declares, longer than any tail below. @@ -36,7 +36,7 @@ def tail(size: int) -> bytes: @unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed') class IPv6ExtLengthHintTests(unittest.TestCase): - """The fallback's hint is the 8-octet minimum header.""" + """The generic parser's hint is the 8-octet minimum header.""" def setUp(self) -> None: reimport_once_per_class(self) diff --git a/tests/protocols/internet/test_ipv6_ext_unit.py b/tests/protocols/internet/test_ipv6_ext_unit.py index 27cdcb63df..ede4d09f56 100644 --- a/tests/protocols/internet/test_ipv6_ext_unit.py +++ b/tests/protocols/internet/test_ipv6_ext_unit.py @@ -33,9 +33,9 @@ def _ipv4_bytes(proto_byte: int, payload: bytes = b'') -> bytes: @unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed') class IPv6ExtUnitTests(unittest.TestCase): - """Tests for GitHub issue #891: a generic RFC 6564 parser for IPv6 - extension headers, so a failed or unimplemented one costs only itself - rather than the whole packet. + """Tests for GitHub issue #891: a generic RFC 6564 parser for an IPv6 + extension header with no dedicated parser, so an unimplemented one costs + only itself rather than the whole packet. """ def setUp(self) -> None: @@ -172,63 +172,139 @@ def test_make_and_make_data_round_trip_the_generic_and_ah_rules(self) -> None: self.assertEqual(values['next'], TransType.UDP) self.assertEqual(values['len'], len_octet) - # -- entry path 2: a recognised header's own parser raises --------------- + # -- a recognised header whose own parser raises ends the walk ---------- - def test_mh_own_parser_failure_falls_back_and_chain_reaches_real_udp(self) -> None: - """The actual #891 defect, with real bytes after the bad header this - time: proves the walk does not merely avoid crashing but genuinely - resumes at the real next layer. - """ + @staticmethod + def _malformed_mh() -> bytes: from pcapkit.const.mh.packet import Packet from pcapkit.const.reg.transtype import TransType - from pcapkit.protocols.internet.ipv6 import IPv6 - from pcapkit.protocols.internet.ipv6_ext import IPv6_Ext from pcapkit.protocols.internet.mh import MH, FastBindingAcknowledgmentStatus - from pcapkit.protocols.transport.udp import UDP mh_raw = bytearray(bytes(MH( next=TransType.UDP, chksum=b'\x12\x34', type=Packet.Fast_Binding_Acknowledgment, data={'status': FastBindingAcknowledgmentStatus.Insufficient_resources, 'key_mngt': True, 'seq': 0x1234, 'lifetime': 40, 'options': []}))) mh_raw[6] = 50 # unassigned FastBindingAcknowledgmentStatus byte -- MH.read raises + return bytes(mh_raw) - udp_payload = _udp_bytes(b'hello') - raw = _ipv6_bytes(int(TransType.Mobility_Header), bytes(mh_raw) + udp_payload) + def test_mh_own_parser_failure_is_raw_and_ends_the_walk(self) -> None: + """GitHub issue #2477: RFC 8200 §4 discards such a packet, so the bad + header and everything after it are ``Raw``, not walked past. + """ + from pcapkit.const.reg.transtype import TransType + from pcapkit.protocols.internet.ipv6 import IPv6 + from pcapkit.protocols.misc.raw import Raw + + body = self._malformed_mh() + _udp_bytes(b'hello') + raw = _ipv6_bytes(int(TransType.Mobility_Header), body) ipv6 = IPv6(io.BytesIO(raw), len(raw)) - exthdrs = list(ipv6.extension_headers.items(multi=True)) - self.assertEqual(len(exthdrs), 1) - genext = exthdrs[0][1] - self.assertIsInstance(genext, IPv6_Ext) - self.assertEqual(genext.next, TransType.UDP) - self.assertEqual(genext.length, len(mh_raw)) - self.assertIsInstance(genext.info.error, Exception) + self.assertEqual(list(ipv6.extension_headers.items(multi=True)), []) + self.assertIs(type(ipv6.payload), Raw) + self.assertEqual(bytes(ipv6.payload), body) + self.assertIsInstance(ipv6.payload.info.error, str) + self.assertEqual(ipv6.info.protocol, TransType.Mobility_Header) + self.assertEqual(str(ipv6.protochain), 'IPv6:Mobility_Header') + self.assertEqual(IPv6.from_data(ipv6.info).data, raw) + self.assertEqual(IPv6.from_data(ipv6.info.to_dict()).data, raw) + + def test_failed_dedicated_parse_is_raw_and_ends_the_walk(self) -> None: + """GitHub issue #2477: a truncated or malformed header with a dedicated + parser is ``Raw`` behind the headers already walked, and rebuilds. + """ + from pcapkit.const.ipv6.extension_header import ExtensionHeader as EH + from pcapkit.const.reg.transtype import TransType + from pcapkit.protocols.internet.ipv6 import IPv6 + from pcapkit.protocols.internet.ipv6_opts import IPv6_Opts + from pcapkit.protocols.misc.raw import Raw - self.assertIsInstance(ipv6.payload, UDP) - self.assertEqual(bytes(ipv6.payload.payload), b'hello') - self.assertEqual(str(ipv6.protochain), 'IPv6:IPv6-Ext:UDP:Raw') + udp = int(TransType.UDP) + router_alert = bytes([udp, 0, 0x05, 10]) + bytes(4) # 10 octets in a 6-octet area + malformed = { + EH.HOPOPT: router_alert, + EH.IPv6_Route: bytes([udp, 0, 3, 0]) + bytes(4), # Routing Type 3 + EH.IPv6_Opts: router_alert, + EH.Mobility_Header: self._malformed_mh(), + EH.HIP: bytes([udp]) + bytes(7), # Header Length 0 + } + for code, header in malformed.items(): + for form, bad in (('malformed', header), ('truncated', bytes([udp, 0, 1]))): + body = bad + _udp_bytes(b'ok') if form == 'malformed' else bad + dst = bytes([int(code), 0, 1, 4, 0, 0, 0, 0]) + raw = _ipv6_bytes(int(EH.IPv6_Opts), dst + body) + with self.subTest(code=code.name, form=form): + with warnings.catch_warnings(): + warnings.simplefilter('ignore') + ipv6 = IPv6(io.BytesIO(raw), len(raw)) + exthdrs = [type(ext) for _, ext in ipv6.extension_headers.items(multi=True)] + self.assertEqual(exthdrs, [IPv6_Opts]) + self.assertIs(type(ipv6.payload), Raw) + self.assertEqual(bytes(ipv6.payload), body) + self.assertEqual(ipv6.info.protocol, code) + self.assertEqual(str(ipv6.protochain), f'IPv6:IPv6-Opts:{code.name}') + self.assertEqual(IPv6.from_data(ipv6.info).data, raw) + self.assertEqual(IPv6.from_data(ipv6.info.to_dict()).data, raw) + + def test_failed_dedicated_parse_is_logged_once(self) -> None: + """GitHub issue #2477: the ``Raw`` the walk ends on is the upper layer, + not parsed again, so its error is logged once and no + ``SchemaWarning`` is left from the header it replaced. + """ + from pcapkit.const.ipv6.extension_header import ExtensionHeader as EH + from pcapkit.const.reg.transtype import TransType + from pcapkit.protocols.internet.ipv6 import IPv6 + from pcapkit.protocols.misc.raw import Raw + + udp = int(TransType.UDP) + for form, hopopt in (('malformed', bytes([udp, 0, 0x05, 10]) + bytes(4) + _udp_bytes(b'ok')), + ('truncated', bytes([udp, 0, 1]))): + raw = _ipv6_bytes(int(EH.HOPOPT), hopopt) + with self.subTest(form=form): + with warnings.catch_warnings(record=True) as caught, \ + self.assertLogs('pcapkit', level='WARNING') as logs: + warnings.simplefilter('always') + ipv6 = IPv6(io.BytesIO(raw), len(raw)) + self.assertIs(type(ipv6.payload), Raw) + error = ipv6.payload.info.error + self.assertEqual([(rec.levelname, error in rec.getMessage()) for rec in logs.records], + [('CRITICAL', True), ('WARNING', True)]) + self.assertEqual([w.category.__name__ for w in caught], []) + + def test_failed_ah_parse_is_raw_and_ends_the_walk(self) -> None: + """GitHub issue #2477: an AH its own parser rejects is ``Raw``, too.""" + from pcapkit.const.ipv6.extension_header import ExtensionHeader as EH + from pcapkit.const.reg.transtype import TransType + from pcapkit.protocols.internet.ipv6 import IPv6 + from pcapkit.protocols.misc.raw import Raw + + ah = bytes([int(TransType.UDP), 0]) + bytes(6) # Payload Len 0: a negative ICV length + body = ah + _udp_bytes(b'ok') + raw = _ipv6_bytes(int(EH.AH), body) + with self.assertLogs('pcapkit', level='WARNING'): + ipv6 = IPv6(io.BytesIO(raw), len(raw)) + + self.assertEqual(list(ipv6.extension_headers.items(multi=True)), []) + self.assertIs(type(ipv6.payload), Raw) + self.assertEqual(bytes(ipv6.payload), body) + self.assertIsInstance(ipv6.payload.info.error, str) + self.assertEqual(ipv6.info.protocol, EH.AH) + self.assertEqual(str(ipv6.protochain), 'IPv6:AH') + self.assertEqual(IPv6.from_data(ipv6.info).data, raw) + self.assertEqual(IPv6.from_data(ipv6.info.to_dict()).data, raw) def test_overrun_inside_a_real_chain_stops_the_walk_honestly(self) -> None: - """Same failing MH message, but its own ``Header Len`` octet is also - corrupted to a value the generic fallback cannot trust either -- - the walk must warn and stop, not invent a layer from trailing bytes. + """A Shim6 header whose ``Hdr Ext Len`` overruns the chain warns and + stops the walk, not inventing a layer from trailing bytes. """ - from pcapkit.const.mh.packet import Packet + from pcapkit.const.ipv6.extension_header import ExtensionHeader from pcapkit.const.reg.transtype import TransType from pcapkit.protocols.internet.ipv6 import IPv6 from pcapkit.protocols.internet.ipv6_ext import IPv6_Ext - from pcapkit.protocols.internet.mh import MH, FastBindingAcknowledgmentStatus from pcapkit.utilities.warnings import SchemaWarning - mh_raw = bytearray(bytes(MH( - next=TransType.UDP, chksum=b'\x12\x34', type=Packet.Fast_Binding_Acknowledgment, - data={'status': FastBindingAcknowledgmentStatus.Insufficient_resources, - 'key_mngt': True, 'seq': 0x1234, 'lifetime': 40, 'options': []}))) - mh_raw[6] = 50 # MH.read raises - mh_raw[1] = 250 # and the generic fallback's own length would overrun - + shim6 = bytes([int(TransType.UDP), 250]) + bytes(14) # (250 + 1) * 8 overruns trailing = _udp_bytes(b'should-not-be-reached') - raw = _ipv6_bytes(int(TransType.Mobility_Header), bytes(mh_raw) + trailing) + raw = _ipv6_bytes(int(ExtensionHeader.Shim6), shim6 + trailing) with warnings.catch_warnings(record=True) as caught: warnings.simplefilter('always') @@ -243,10 +319,10 @@ def test_overrun_inside_a_real_chain_stops_the_walk_honestly(self) -> None: self.assertIsNone(genext.next) # absorbed everything, including the bytes that would have been a # perfectly good UDP datagram -- the point is that it is not trusted - self.assertEqual(genext.length, len(mh_raw) + len(trailing)) + self.assertEqual(genext.length, len(shim6) + len(trailing)) self.assertEqual(str(ipv6.protochain), 'IPv6:IPv6-Ext') - # -- entry path 1: an unrecognised protocol number (Shim6) --------------- + # -- the entry path: direct dispatch for Shim6 --------------------------- def test_shim6_has_no_dedicated_parser_and_dispatches_directly(self) -> None: """``Shim6`` (140) is a real IANA extension header this package has @@ -278,7 +354,7 @@ def test_shim6_packet_parses_generically_and_chain_reaches_real_udp(self) -> Non code, genext = exthdrs[0] self.assertEqual(code, ExtensionHeader.Shim6) self.assertIsInstance(genext, IPv6_Ext) - self.assertIsNone(genext.info.error) # direct dispatch, not a beholder fallback + self.assertIsNone(genext.info.error) self.assertIsInstance(ipv6.payload, UDP) self.assertEqual(bytes(ipv6.payload.payload), b'shim6-ok') self.assertEqual(str(ipv6.protochain), 'IPv6:IPv6-Ext:UDP:Raw') @@ -383,21 +459,42 @@ def test_eq_on_the_family_name_is_false_like_ip_and_ipsec(self) -> None: self.assertFalse(fallback == 'IPv6_Ext') self.assertTrue(fallback == IPv6_Ext) - def test_fallback_for_a_malformed_member_matches_every_member_name(self) -> None: - """A HOPOPT that its own parser rejects is parsed by ``IPv6_Ext``.""" + def test_malformed_member_is_raw_and_matches_only_its_own_name(self) -> None: + """A HOPOPT that its own parser rejects is ``Raw`` (GitHub issue #2477).""" from pcapkit.const.ipv6.extension_header import ExtensionHeader from pcapkit.const.reg.transtype import TransType from pcapkit.protocols.internet.ipv6 import IPv6 from pcapkit.protocols.internet.ipv6_ext import IPv6_Ext + from pcapkit.protocols.misc.raw import Raw # Router Alert declaring 10 octets of data in a 6-octet option area hopopt = bytes([int(TransType.UDP), 0, 0x05, 10]) + bytes(4) raw = _ipv6_bytes(int(ExtensionHeader.HOPOPT), hopopt + _udp_bytes(b'ok')) ipv6 = IPv6(io.BytesIO(raw), len(raw)) + self.assertEqual(list(ipv6.extension_headers.items(multi=True)), []) + self.assertIs(type(ipv6.payload), Raw) + self.assertEqual(str(ipv6.protochain), 'IPv6:HOPOPT') + for name in ('HOPOPT', 'Raw'): + with self.subTest(selector=name): + self.assertIn(name, ipv6.protochain) + for name in (*(other for other in IPv6_Ext.id() if other != 'HOPOPT'), 'IPsec', 'UDP'): + with self.subTest(selector=name): + self.assertNotIn(name, ipv6.protochain) + + def test_shim6_matches_every_member_name(self) -> None: + """Shim6 is parsed by ``IPv6_Ext``, whose :meth:`id` names every member.""" + from pcapkit.const.ipv6.extension_header import ExtensionHeader + from pcapkit.const.reg.transtype import TransType + from pcapkit.protocols.internet.ipv6 import IPv6 + from pcapkit.protocols.internet.ipv6_ext import IPv6_Ext + + shim6 = bytes([int(TransType.UDP), 0]) + bytes(6) + raw = _ipv6_bytes(int(ExtensionHeader.Shim6), shim6 + _udp_bytes(b'ok')) + ipv6 = IPv6(io.BytesIO(raw), len(raw)) + (_, ext), = ipv6.extension_headers.items(multi=True) self.assertIs(type(ext), IPv6_Ext) - self.assertIsInstance(ext.info.error, Exception) self.assertEqual(str(ipv6.protochain), 'IPv6:IPv6-Ext:UDP:Raw') for name in (*IPv6_Ext.id(), 'IPv6_Ext', 'IPsec'): with self.subTest(selector=name): diff --git a/tests/protocols/internet/test_ipv6_exthdr_cut_rebuild_unit.py b/tests/protocols/internet/test_ipv6_exthdr_cut_rebuild_unit.py index 6dc492d5d6..0a0576bbf3 100644 --- a/tests/protocols/internet/test_ipv6_exthdr_cut_rebuild_unit.py +++ b/tests/protocols/internet/test_ipv6_exthdr_cut_rebuild_unit.py @@ -7,11 +7,9 @@ was handed, so the rebuild wrote the extension headers twice -- Destination Options, TCP cut at 57 octets rebuilt 65. -GitHub issue #1473: a header whose dedicated parser raised is parsed by -:class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext` and written under its -``ext`` key, but the ``to_dict`` rebuild chose the dedicated class from the -``next`` code, which then missed fields only it parses -- ``AttributeError: -'HOPOPT' object has no attribute 'options'``. +GitHub issue #2477: a header whose dedicated parser raises is kept as +:class:`~pcapkit.protocols.misc.raw.Raw`, and the walk ends there, so a cut one +rebuilds from that :class:`~pcapkit.protocols.misc.raw.Raw`. Every case builds its own octets and reads no capture. Classes are imported inside each test, after :func:`~tests._support.reimport_once_per_class`. @@ -27,7 +25,7 @@ #: Test tier (#1958): a sweep that rebuilds every cut of each extension header. __tier__ = 'LATEST' -HOP, TCP, DST, AH, NONXT, HIP = 0, 6, 60, 51, 59, 139 +HOP, TCP, DST, AH, NONXT, HIP, SHIM6 = 0, 6, 60, 51, 59, 139, 140 #: A TCP header and one octet of data. SEGMENT = struct.pack('!HHIIBBHHH', 1, 2, 0, 0, 0x50, 2, 0, 0, 0) + b'x' @@ -57,6 +55,7 @@ def hip(nxt: int) -> bytes: 'hop-nonxt': (HOP, bytes([NONXT, 1, 1, 12]) + bytes(12)), 'hip': (HIP, hip(NONXT)), 'dst-hip': (DST, opts(HIP) + hip(NONXT)), + 'shim6-tcp': (SHIM6, opts(TCP) + SEGMENT), } @@ -102,20 +101,19 @@ def test_raw_upper_layer_starts_past_the_extension_headers(self) -> None: self.assertEqual(ip6.payload.data, data[48:]) self.assertEqual(len(IPv6.from_data(ip6.info).data), 57) - def test_dict_rebuild_keeps_the_generic_parser(self) -> None: - from pcapkit.const.ipv6.extension_header import ExtensionHeader + def test_cut_header_is_raw_and_ends_the_walk(self) -> None: from pcapkit.protocols.internet.ipv6 import IPv6 - from pcapkit.protocols.internet.ipv6_ext import IPv6_Ext + from pcapkit.protocols.misc.raw import Raw for cut in (48, 50, 52, 55): data = packet('hop-nonxt')[:cut] with self.subTest(cut=cut): ip6 = IPv6(data, len(data)) - self.assertIs(type(ip6._exthdr[ExtensionHeader.HOPOPT]), IPv6_Ext) - chain = ip6.info.to_dict()['__exthdr__'] - self.assertEqual([parser for parser, _ in chain], [IPv6_Ext]) + self.assertEqual(list(ip6.extension_headers.items(multi=True)), []) + self.assertIs(type(ip6.payload), Raw) + self.assertEqual(ip6.payload.data, data[40:]) + self.assertEqual(ip6.info.to_dict()['__exthdr__'], ()) self.assertEqual(IPv6.from_data(ip6.info.to_dict()).data, data) - if __name__ == '__main__': unittest.main() diff --git a/tests/protocols/internet/test_ipv6_unit.py b/tests/protocols/internet/test_ipv6_unit.py index eaa96fca89..7bc0f9a884 100644 --- a/tests/protocols/internet/test_ipv6_unit.py +++ b/tests/protocols/internet/test_ipv6_unit.py @@ -162,7 +162,7 @@ def test_ipv6_decode_next_layer_handles_extension_headers_and_fragments(self) -> 'dst': ip_address('2001:db8::2'), }) fragment = object.__new__(IPv6_Frag) - fragment._info = DummyDict(next=TransType.UDP) + fragment._info = DummyDict(next=TransType.UDP, offset=0) with mock.patch.object(proto, '_import_next_layer', return_value=fragment) as import_next: with mock.patch.object(Internet, '_decode_next_layer', return_value='decoded') as decode: diff --git a/tests/protocols/internet/test_mh_unit.py b/tests/protocols/internet/test_mh_unit.py index 47ed4a2d3f..eb8e83bb3a 100644 --- a/tests/protocols/internet/test_mh_unit.py +++ b/tests/protocols/internet/test_mh_unit.py @@ -1713,8 +1713,7 @@ def test_mh_local_enums_raise_and_do_not_alias(self) -> None: properties the issue measured that are not specific to any one of them: the ``get()``-level aliasing defect, the ``_missing_`` range invariant, and the consequence of the raise once it reaches a real - capture -- which turns out to differ by wrapping protocol, see the - last two subtests. + capture, over IPv4 and over IPv6. """ import socket import struct @@ -1792,9 +1791,7 @@ def test_mh_local_enums_raise_and_do_not_alias(self) -> None: # any exception. Over IPv4, which dispatches to Mobility Header # through the base, non-chain-walking ``_import_next_layer``, # that is the whole story: the raise costs one MH message, not - # the packet or the capture. IPv4 is deliberately the wrapping - # protocol in *this* subtest -- the next one shows IPv6 does not - # get off this lightly. + # the packet or the capture. def wrap_in_ipv4(mh_payload: bytes) -> IPv4: header = struct.pack( '>BBHHHBBH4s4s', 0x45, 0, 20 + len(mh_payload), 0, 0, 64, @@ -1823,35 +1820,12 @@ def wrap_in_ipv4(mh_payload: bytes) -> IPv4: self.assertIsInstance(ip.payload, Raw) self.assertEqual(bytes(ip.payload), bytes(lra_raw)) - with self.subTest('over IPv6, the raise costs only the MH slot (GitHub issue #891)'): - # Mobility Header is *also* one of IPv6's own chained extension - # headers (``Enum_ExtensionHeader.Mobility_Header``, value 135, in - # ``pcapkit/const/ipv6/extension_header.py``), so IPv6 never - # reaches the fallback through the base ``_import_next_layer`` at - # all -- it walks its own ``@beholder``-decorated override - # (``pcapkit/protocols/internet/ipv6.py:_import_next_layer``). - # - # Without GitHub issue #891's catch, that override lets any - # exception from MH's own parser reach ``@beholder`` unchanged, - # which substitutes plain ``Raw`` for the MH slot; ``Raw``'s info - # carries no ``.next``, so ``proto = info.next`` - # (``pcapkit/protocols/internet/ipv6.py:_decode_next_layer``) - # raises ``AttributeError`` on it regardless -- and it is *that*, - # unrelated, exception that a further-out ``@beholder`` - # (Ethernet's) catches, degrading the whole IPv6 packet, header - # fields and MH message alike, to ``Raw``. - # - # ``IPv6._import_next_layer`` has its own catch (#891), one layer - # in from ``@beholder``: for an extension header whose wire - # format RFC 6564 guarantees (Mobility Header among them), the MH - # slot becomes - # :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext` - # instead of ``Raw``. It parses the two guaranteed octets - # generically -- next header ``UDP`` and a length that exactly - # matches this ``fback_raw`` payload -- so ``IPv6.read`` returns - # normally: IPv6's own header fields (source, destination, hop - # limit) survive, and the chain ends honestly at the bad - # header's replacement rather than swallowing the packet. + with self.subTest('over IPv6, the raise ends the walk at the MH slot (GitHub issue #2477)'): + # IPv6 walks Mobility Header as an extension header through its own + # ``@beholder``-decorated ``_import_next_layer``, which makes the + # slot ``Raw``. ``Raw`` carries no ``next``, so the walk stops + # there, as RFC 8200 §4 discards the packet, and IPv6's own header + # fields survive. def wrap_in_ethernet_ipv6(mh_payload: bytes) -> Ethernet: ipv6_header = struct.pack( '>IHBB', 6 << 28, len(mh_payload), int(TransType.Mobility_Header), 64, @@ -1861,16 +1835,13 @@ def wrap_in_ethernet_ipv6(mh_payload: bytes) -> Ethernet: return Ethernet(io.BytesIO(raw), len(raw)) from pcapkit.protocols.internet.ipv6 import IPv6 - from pcapkit.protocols.internet.ipv6_ext import IPv6_Ext eth = wrap_in_ethernet_ipv6(bytes(fback_raw)) self.assertIsInstance(eth.payload, IPv6) - self.assertEqual(str(eth.protochain), 'Ethernet:IPv6:IPv6-Ext') - - genext = next(iter(eth.payload.extension_headers.items(multi=True)))[1] - self.assertIsInstance(genext, IPv6_Ext) - self.assertEqual(genext.next, TransType.UDP) - self.assertEqual(genext.length, len(fback_raw)) + self.assertEqual(str(eth.protochain), 'Ethernet:IPv6:Mobility_Header') + self.assertEqual(list(eth.payload.extension_headers.items(multi=True)), []) + self.assertIs(type(eth.payload.payload), Raw) + self.assertEqual(bytes(eth.payload.payload), bytes(fback_raw)) def test_mh_message_flags_pack_each_bit_independently(self) -> None: """Regression test: a cleared flag must not be emitted as a set bit. diff --git a/tests/protocols/test_declared_length_overrun_unit.py b/tests/protocols/test_declared_length_overrun_unit.py index fd90186f27..77def4db02 100644 --- a/tests/protocols/test_declared_length_overrun_unit.py +++ b/tests/protocols/test_declared_length_overrun_unit.py @@ -9,9 +9,9 @@ octets with none present rebuilt 24 octets from 16. Such an option raises :exc:`~pcapkit.utilities.exceptions.ProtocolError`. -Inside a parent layer the octets are kept as captured -- raw extension data -under IPv6, a :class:`~pcapkit.protocols.misc.raw.Raw` payload elsewhere -- and -the parent rebuilds byte for byte. +Inside a parent layer the octets are kept as captured, as a +:class:`~pcapkit.protocols.misc.raw.Raw` payload, and the parent rebuilds byte +for byte. An option that begins at the end of the data reads nothing at all, and is still left to the end-of-option-list handling an over-long option area relies on. An @@ -151,14 +151,9 @@ def test_an_overrunning_layer_is_kept_as_captured_by_its_parent(self) -> None: warnings.simplefilter('ignore') parsed = protocol(frame, len(frame)) - if name == 'IPv6': - # an extension header is kept as raw extension data - self.assertIn('has an option that runs past the end of the data', str(parsed.info.ext.error)) - else: - # any other layer is kept as a raw payload, as captured - self.assertIsInstance(parsed.payload, Raw) - self.assertIn('has an option that runs past the end of the data', str(parsed.payload.info.error)) - self.assertEqual(parsed.payload.data, OVERRUNS[case][2]) + self.assertIsInstance(parsed.payload, Raw) + self.assertIn('has an option that runs past the end of the data', str(parsed.payload.info.error)) + self.assertEqual(parsed.payload.data, OVERRUNS[case][2]) self.assertEqual(protocol.from_data(parsed.info).data, frame) def test_an_option_area_declared_past_the_data_is_kept_as_captured(self) -> None: diff --git a/tests/protocols/test_short_read_roundtrip_unit.py b/tests/protocols/test_short_read_roundtrip_unit.py index d82f8726d3..f3c81b28c0 100644 --- a/tests/protocols/test_short_read_roundtrip_unit.py +++ b/tests/protocols/test_short_read_roundtrip_unit.py @@ -90,12 +90,6 @@ def _quiet(case: unittest.TestCase) -> None: warnings.simplefilter('ignore') -def _has_fallback(proto: object) -> bool: - from pcapkit.protocols.internet.ipv6_ext import IPv6_Ext - - return any(type(ext) is IPv6_Ext for ext in getattr(proto, 'extension_headers', {}).values()) - - def _resolve(path: str) -> type: module, _, name = path.partition(':') return getattr(importlib.import_module(f'pcapkit.protocols.{module}'), name) @@ -124,11 +118,6 @@ def test_every_family_cut_at_every_offset(self) -> None: continue self.assertEqual(proto.data, raw) self.assertEqual(type(proto).from_data(proto.info).data, raw) - if _has_fallback(proto): - # from the dict form, IPv6.from_data rebuilds an - # IPv6_Ext standing in for a dedicated parser with - # that parser's class: a separate defect, in ipv6.py - continue self.assertEqual(type(proto).from_data(proto.info.to_dict()).data, raw) def test_extension_header_cut_at_every_offset(self) -> None: diff --git a/tests/protocols/test_trial_parse_raises_2308_unit.py b/tests/protocols/test_trial_parse_raises_2308_unit.py index e397336260..3a9769e078 100644 --- a/tests/protocols/test_trial_parse_raises_2308_unit.py +++ b/tests/protocols/test_trial_parse_raises_2308_unit.py @@ -41,7 +41,7 @@ def _ipv6(next_header: 'int') -> 'bytes': + bytes(15) + b'\x01' + bytes(15) + b'\x02' + b'\x00') -#: The warnings of the kept, cut ``IPv6_Ext`` that each :func:`_ipv6` frame ends in. +#: The warnings of the kept, cut ``IPv6_Ext`` that a Shim6 :func:`_ipv6` frame ends in. IPV6_EXT_CUT = ['packet length < 0: -1', 'IPv6: extension header declares a length of 8 octet(s) with 1 octet(s) ' 'left in the chain; stopping the walk instead of skipping past it'] @@ -86,11 +86,14 @@ def test_error_filter_keeps_the_cut_as_the_error(self) -> None: self.assertEqual(parsed.payload.info.error, ERROR) # type: ignore[attr-defined] self.assertEqual(messages, []) - def test_replaced_by_ipv6_ext_reports_only_the_replacement(self) -> None: - """A cut Hop-by-Hop header raises and is parsed as ``IPv6_Ext``, which alone warns.""" + def test_cut_ipv6_extension_header_is_raw_and_warns_nothing(self) -> None: + """A cut Hop-by-Hop header raises and is kept as ``Raw`` (#2477), which warns nothing.""" + from pcapkit.protocols.misc.raw import Raw + parsed, messages = self._parse(_ipv6(0)) - self.assertEqual(str(parsed.protochain), 'Ethernet:IPv6:IPv6-Ext') # type: ignore[attr-defined] - self.assertEqual(messages, IPV6_EXT_CUT) + self.assertEqual(str(parsed.protochain), 'Ethernet:IPv6:HOPOPT') # type: ignore[attr-defined] + self.assertIs(type(parsed.payload.payload), Raw) # type: ignore[attr-defined] + self.assertEqual(messages, []) def test_kept_trial_still_warns(self) -> None: """A cut Shim6 header is held, kept, and reports its warnings as before.""" From 69d6728d75739348a9d1d6f94ae2df218495b055 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Fri, 30 Oct 2026 21:13:21 -0400 Subject: [PATCH 2/2] fix(protocols): reuse the walk's upper layer only when it is Raw - IPv6._decode_next_layer: a NoPayload the walk stops on was reused as the upper layer. An overrunning Routing header with nothing after it gave NoPayload where base gives an empty Raw. Reuse only a Raw. - test_ipv6_ext_unit: pin that case as an empty Raw. - test_mh_unit: over IPv6 the MH is replaced by Raw, not IPv6_Ext. - Registry comments: IPv6_Ext is the generic parser, not a fallback. --- pcapkit/protocols/data/__init__.py | 2 +- pcapkit/protocols/data/internet/__init__.py | 4 +-- pcapkit/protocols/internet/ipv6.py | 34 ++++++------------- pcapkit/protocols/internet/ipv6_ext.py | 16 ++++----- pcapkit/protocols/schema/internet/__init__.py | 4 +-- .../protocols/internet/test_ipv6_ext_unit.py | 21 +++++++++++- .../test_ipv6_exthdr_cut_rebuild_unit.py | 1 + tests/protocols/internet/test_mh_unit.py | 5 +-- 8 files changed, 47 insertions(+), 40 deletions(-) diff --git a/pcapkit/protocols/data/__init__.py b/pcapkit/protocols/data/__init__.py index f818d81927..e4f1a78517 100644 --- a/pcapkit/protocols/data/__init__.py +++ b/pcapkit/protocols/data/__init__.py @@ -123,7 +123,7 @@ # IPv6 Fragment Header 'IPv6_Frag', - # IPv6 Extension Header (base + generic fallback) + # IPv6 Extension Header (base + generic) 'IPv6_Ext', # IPv6 Destination Options Header diff --git a/pcapkit/protocols/data/internet/__init__.py b/pcapkit/protocols/data/internet/__init__.py index 421c83561f..65c8195776 100644 --- a/pcapkit/protocols/data/internet/__init__.py +++ b/pcapkit/protocols/data/internet/__init__.py @@ -135,7 +135,7 @@ # IPv6 Fragment Header from pcapkit.protocols.data.internet.ipv6_frag import IPv6_Frag -# IPv6 Extension Header (generic fallback) +# IPv6 Extension Header (generic) from pcapkit.protocols.data.internet.ipv6_ext import IPv6_Ext # IPv6 Destination Options @@ -427,7 +427,7 @@ # IPv6 Fragment Header 'IPv6_Frag', - # IPv6 Extension Header (generic fallback) + # IPv6 Extension Header (generic) 'IPv6_Ext', # IPv6 Destination Options Header diff --git a/pcapkit/protocols/internet/ipv6.py b/pcapkit/protocols/internet/ipv6.py index 23e05dbd2a..41d7a3ef07 100644 --- a/pcapkit/protocols/internet/ipv6.py +++ b/pcapkit/protocols/internet/ipv6.py @@ -105,7 +105,7 @@ class IPv6(IP[Data_IPv6, Schema_IPv6], """ - #: The layer a header-walk stopped on, returned unparsed as the upper layer. + #: The ``Raw`` a header-walk stopped on, returned unparsed as the upper layer. _walk_upper = None # type: Optional[ProtocolBase] ########################################################################## @@ -527,27 +527,15 @@ def _decode_next_layer(self, ipv6: 'Data_IPv6', proto: 'Optional[int]' = None, _frame_depth.reset(token) info = next_.info - # A layer with no ``next`` field cannot safely continue the walk. - # This is a *structural* check, not a fixed set of codes: every - # dedicated parser and - # :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext` carry - # ``next`` (possibly :data:`None`, on an overrun) -- Shim6 has no - # dedicated parser but is registered to ``IPv6_Ext`` directly -- - # while plain :class:`~pcapkit.protocols.misc.raw.Raw` does not. - # That is what a code with no dedicated parser -- ``253``, - # ``254``, or whatever IANA assigns next -- resolves to, and what - # :func:`~pcapkit.corekit.decorators.beholder` makes of a header - # whose dedicated parser raises, so a malformed header ends the - # walk, as :rfc:`8200#section-4` has a node discard the packet - # (#2477). Reading ``info.next`` on that would raise - # ``AttributeError`` and let a further-out ``beholder`` degrade the - # *whole* packet. - # - # Not recorded as an extension header, as it consumed nothing and - # would be rebuilt twice. It is the upper layer instead, returned - # unparsed by :meth:`_import_next_layer`, so its error is logged once. + # A layer with no ``next`` field ends the walk, as RFC 8200 section 4 + # has a node discard the packet on a malformed header (#2477). + # It is not recorded as an extension header. + # A Raw is reused as the upper layer, so its error logs once. if not hasattr(info, 'next'): - self._walk_upper = next_ + from pcapkit.protocols.misc.raw import Raw # isort: skip # pylint: disable=import-outside-toplevel + # Exact type on purpose: Raw subclasses exist, so isinstance would over-match. + if type(next_) is Raw: # pylint: disable=unidiomatic-typecheck + self._walk_upper = next_ break # NOTE: Added through a :class:`MultiDict`, so a repeated header @@ -660,8 +648,8 @@ def _import_next_layer(self, proto: 'int', length: 'Optional[int]' = None, *, # :rfc:`8200#section-4` has a node discard such a packet (:issue:`2477`). ``253`` and ``254`` have no dedicated parser and reach plain :class:`~pcapkit.protocols.misc.raw.Raw` with no - exception involved, which stops the walk the same way. The layer - the walk stopped on is then returned as the upper layer, not parsed + exception involved, which stops the walk the same way. A ``Raw`` the + walk stopped on is then returned as the upper layer, not parsed again. :class:`~pcapkit.protocols.internet.esp.ESP`'s info always carries a ``next``, :data:`None` unless its payload was decrypted, since diff --git a/pcapkit/protocols/internet/ipv6_ext.py b/pcapkit/protocols/internet/ipv6_ext.py index 4c32cc28d7..51c9005c7d 100644 --- a/pcapkit/protocols/internet/ipv6_ext.py +++ b/pcapkit/protocols/internet/ipv6_ext.py @@ -304,9 +304,8 @@ def protocol(self) -> 'Optional[Enum_ExtensionHeader] | Optional[str]': :attr:`Protocol.protocol ` meaning ("name of next layer protocol"). On the owner's ruling for GitHub issue :issue:`891` it is deliberately repointed at this - instance's *own* identity, i.e. which header's format it parsed, such as - :attr:`~pcapkit.const.ipv6.extension_header.ExtensionHeader.HOPOPT` - or :attr:`~pcapkit.const.ipv6.extension_header.ExtensionHeader.Shim6`. + instance's *own* identity, i.e. which header's format it parsed, which + for this class is only :attr:`~pcapkit.const.ipv6.extension_header.ExtensionHeader.Shim6`. The identity is resolved per instance from the numeric code handed to the constructor (``alias``), which :meth:`pcapkit.protocols.internet.ipv6.IPv6._decode_next_layer` already @@ -407,9 +406,9 @@ def read(self, length: 'Optional[int]' = None, *, # pylint: disable=arguments-d **kwargs: Arbitrary keyword arguments, including these, which are this class's own: - * ``alias`` -- the numeric extension header code this instance - stands in for, e.g. ``0`` for ``HOPOPT`` or ``140`` for - ``Shim6``, supplied by :meth:`IPv6._import_next_layer + * ``alias`` -- the numeric code of the extension header this + instance parses, ``140`` for ``Shim6`` (the only code + registered to this class), supplied by :meth:`IPv6._import_next_layer `. * ``error`` -- a parsing error to record in the result, if any, which the library does not pass. @@ -656,10 +655,9 @@ def __index__(cls) -> 'NoReturn': because :class:`~pcapkit.protocols.misc.raw.Raw` has no identity to report at all, this class *does* have one -- see :attr:`protocol` -- but it is resolved per instance, - not per class: one :class:`IPv6_Ext` stands in for - :attr:`~pcapkit.const.ipv6.extension_header.ExtensionHeader.HOPOPT`, + not per class: one :class:`IPv6_Ext` parses :attr:`~pcapkit.const.ipv6.extension_header.ExtensionHeader.Shim6` - and any other RFC 6564-conforming code alike, while + (and any other RFC 6564-format code registered to it), while :meth:`__index__` is a ``@classmethod`` with nowhere to put a value that differs per instance. diff --git a/pcapkit/protocols/schema/internet/__init__.py b/pcapkit/protocols/schema/internet/__init__.py index 4a3dc3fe12..dbc944333d 100644 --- a/pcapkit/protocols/schema/internet/__init__.py +++ b/pcapkit/protocols/schema/internet/__init__.py @@ -136,7 +136,7 @@ # IPv6 Fragment Header from pcapkit.protocols.schema.internet.ipv6_frag import IPv6_Frag -# IPv6 Extension Header (generic fallback) +# IPv6 Extension Header (generic) from pcapkit.protocols.schema.internet.ipv6_ext import IPv6_Ext # IPv6 Destination Options @@ -434,7 +434,7 @@ # IPv6 Fragment Header 'IPv6_Frag', - # IPv6 Extension Header (generic fallback) + # IPv6 Extension Header (generic) 'IPv6_Ext', # IPv6 Destination Options diff --git a/tests/protocols/internet/test_ipv6_ext_unit.py b/tests/protocols/internet/test_ipv6_ext_unit.py index ede4d09f56..88ecb16bcb 100644 --- a/tests/protocols/internet/test_ipv6_ext_unit.py +++ b/tests/protocols/internet/test_ipv6_ext_unit.py @@ -292,6 +292,25 @@ def test_failed_ah_parse_is_raw_and_ends_the_walk(self) -> None: self.assertEqual(IPv6.from_data(ipv6.info).data, raw) self.assertEqual(IPv6.from_data(ipv6.info.to_dict()).data, raw) + def test_overrun_routing_with_nothing_after_is_empty_raw(self) -> None: + """A Routing header overrunning the payload, with nothing after it, is an + empty ``Raw``, not ``NoPayload``: the walk reuses its upper layer only + when that is ``Raw``. + """ + from pcapkit.const.ipv6.extension_header import ExtensionHeader as EH + from pcapkit.protocols.internet.ipv6 import IPv6 + from pcapkit.protocols.misc.raw import Raw + + routing = bytes([int(EH.IPv6_Route), 2]) + bytes(6) # 24 octets declared, 8 present + raw = _ipv6_bytes(int(EH.IPv6_Route), routing) + with warnings.catch_warnings(): + warnings.simplefilter('ignore') + ipv6 = IPv6(io.BytesIO(raw), len(raw)) + + self.assertIs(type(ipv6.payload), Raw) + self.assertEqual(bytes(ipv6.payload), b'') + self.assertEqual(ipv6.info.protocol, EH.IPv6_Route) + def test_overrun_inside_a_real_chain_stops_the_walk_honestly(self) -> None: """A Shim6 header whose ``Hdr Ext Len`` overruns the chain warns and stops the walk, not inventing a layer from trailing bytes. @@ -780,7 +799,7 @@ def test_every_subclass_defines_the_required_members_itself(self) -> None: self.assertTrue( member in klass.__dict__, f'{klass.__qualname__} inherits {member!r} from IPv6_Ext, whose ' - f'value is the generic fallback parser\'s and is wrong for a ' + f'value is the generic parser\'s and is wrong for a ' f'header with an identity of its own; define it on ' f'{klass.__qualname__} itself', ) diff --git a/tests/protocols/internet/test_ipv6_exthdr_cut_rebuild_unit.py b/tests/protocols/internet/test_ipv6_exthdr_cut_rebuild_unit.py index 0a0576bbf3..5b20a31c93 100644 --- a/tests/protocols/internet/test_ipv6_exthdr_cut_rebuild_unit.py +++ b/tests/protocols/internet/test_ipv6_exthdr_cut_rebuild_unit.py @@ -115,5 +115,6 @@ def test_cut_header_is_raw_and_ends_the_walk(self) -> None: self.assertEqual(ip6.info.to_dict()['__exthdr__'], ()) self.assertEqual(IPv6.from_data(ip6.info.to_dict()).data, data) + if __name__ == '__main__': unittest.main() diff --git a/tests/protocols/internet/test_mh_unit.py b/tests/protocols/internet/test_mh_unit.py index eb8e83bb3a..c57dcabb8e 100644 --- a/tests/protocols/internet/test_mh_unit.py +++ b/tests/protocols/internet/test_mh_unit.py @@ -1463,8 +1463,9 @@ def test_mh_rfc5568_local_enums_cover_unregistered_value_sets(self) -> None: *name*, aliases a second unknown key onto the first at a shared ``-1`` sentinel. So an unassigned code does not grow the class. Over IPv4 ``@beholder`` replaces the MH layer and everything above it with - :class:`~pcapkit.protocols.misc.raw.Raw`; over IPv6 only the MH header - is replaced, by ``IPv6_Ext``, and the layers above it still parse -- + :class:`~pcapkit.protocols.misc.raw.Raw`; over IPv6 the MH header is + replaced by ``Raw``, which ends the walk, so the layers above stay + unparsed (:issue:`2477`) -- see ``test_mh_local_enums_raise_and_do_not_alias`` for that distinction and the other two RFC-inline enums. """