diff --git a/pcapkit/protocols/internet/__init__.py b/pcapkit/protocols/internet/__init__.py index f7e1ed14f..d2ded0fad 100644 --- a/pcapkit/protocols/internet/__init__.py +++ b/pcapkit/protocols/internet/__init__.py @@ -33,7 +33,7 @@ # Ethertype IEEE 802 Numbers from pcapkit.const.reg.ethertype import EtherType as ETHERTYPE -# Deprecated / Base Classes +# Base Classes from pcapkit.protocols.internet.ip import IP from pcapkit.protocols.internet.ipsec import IPsec diff --git a/pcapkit/protocols/internet/esp.py b/pcapkit/protocols/internet/esp.py index 86fff04f0..be4764f9d 100644 --- a/pcapkit/protocols/internet/esp.py +++ b/pcapkit/protocols/internet/esp.py @@ -22,8 +22,7 @@ ? ? ``esp.icv`` Integrity Check Value (ICV, variable) ======= ========= ===================== ============================================== -Unlike every other protocol in :mod:`pcapkit`, ESP is **not** self -describing. :rfc:`4303` places the ``Pad Length`` and ``Next Header`` +ESP is **not** self describing. :rfc:`4303` places the ``Pad Length`` and ``Next Header`` fields *inside* the ciphertext, and leaves the length of the ``Integrity Check Value`` to be determined by the Security Association (SA), which is negotiated out of band. Therefore: @@ -56,7 +55,7 @@ ) extraction = pcapkit.extract('esp.pcap', context=ESPContext(sa)) -Registered algorithms, and supported ones +Registered Algorithms, and Supported Ones ----------------------------------------- ESP has no algorithm registry of its own -- an SA's algorithms are negotiated @@ -139,7 +138,7 @@ :attr:`Cipher.ENCR_AES_CBC ` are the same member. -Known limitations +Known Limitations ----------------- * **Extended Sequence Numbers (ESN,** :rfc:`4303` **ยง2.2.1) are not @@ -458,10 +457,9 @@ def get(cls, value: 'Integrity | str | int') -> 'IntegritySuite': class ESPStatus(EnumLookup, enum.IntEnum): """Outcome of ESP payload processing. - Re-parented onto :class:`~pcapkit.corekit.enum.EnumLookup` per GitHub - issue :issue:`930`, finishing :issue:`877`'s phase 2 -- pure re-parenting, since this - class defines neither ``get`` nor ``_missing_`` of its own to reconcile - with the base. + ``get`` / ``get_all`` come from :class:`~pcapkit.corekit.enum.EnumLookup`; + the class defines no ``_missing_``, so an unknown value raises + :exc:`ValueError`. """ @@ -525,7 +523,7 @@ class SecurityAssociation: Raises: ProtocolError: If the algorithms or key lengths are inconsistent, or - ``destination`` is a :obj:`bool` (c.f. :issue:`491`) -- :obj:`bool` is an + ``destination`` is a :obj:`bool` -- :obj:`bool` is an :class:`int` subclass, and :func:`ipaddress.ip_address` treats any :class:`int` below ``2**32`` as IPv4, so without this check ``destination=True`` would silently become @@ -563,7 +561,7 @@ def __init__(self, spi: 'Optional[int]' = None, *, # subclass, and ``ipaddress.ip_address()`` treats any ``int`` below # ``2**32`` as IPv4 -- so without this check, ``destination=True`` # would silently become ``IPv4Address('0.0.0.1')``, with no - # exception and no warning (c.f. #491). + # exception and no warning. raise ProtocolError( f'invalid destination: must not be a bool, not {destination!r} -- ' f'pass int({destination!r}) if the numeric value is what is wanted') @@ -975,18 +973,16 @@ class ESP(IPsec[Data_ESP, Schema_ESP], IPv6_Ext[Data_ESP, Schema_ESP], schema=Schema_ESP, data=Data_ESP): """This class implements Encapsulating Security Payload. - Double-inherited (GitHub issue :issue:`917`), mirroring - :class:`~pcapkit.protocols.internet.ah.AH`: IANA's *IPv6 Extension - Header Types* registry lists ``ESP`` at 50 (:rfc:`4303#section-3.1.1` - has it appear after the hop-by-hop, routing and fragmentation - extension headers in the IPv6 header chain), and this package's own + Double-inherited, mirroring :class:`~pcapkit.protocols.internet.ah.AH`: + IANA's *IPv6 Extension Header Types* registry lists ``ESP`` at 50 + (:rfc:`4303#section-3.1.1` has it follow the hop-by-hop, routing and + fragmentation extension headers in the IPv6 header chain), and this + package's own :class:`~pcapkit.const.ipv6.extension_header.ExtensionHeader` registry - agrees (``ESP = 50``), so it must honour the same extension-mode - contract as its siblings. The same section separately states that, - in the context of IPv4, ESP is placed after the IP header and - before the next-layer protocol -- the primary-source evidence that - it also travels directly as an IPv4 payload, which is what - qualifies it for a base besides + agrees, so it must honour the same extension-mode contract as its + siblings. The same section places ESP after the IP header and before the + next-layer protocol in IPv4, so it also travels directly as an IPv4 + payload, which is what qualifies it for a base besides :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext`. :attr:`payload` and :attr:`protochain` come from :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext`; :attr:`protocol` @@ -995,7 +991,7 @@ class ESP(IPsec[Data_ESP, Schema_ESP], IPv6_Ext[Data_ESP, Schema_ESP], Note: :rfc:`8200#section-4.5` says outright that ESP "is not considered an extension header". The library follows IANA's registry rather than - that sentence, on the owner's ruling for GitHub issue :issue:`895`. + that sentence (decided on :issue:`895`). """ @@ -1015,13 +1011,13 @@ def alias(self) -> 'Literal["ESP"]': Spelled out rather than left to :attr:`ProtocolBase.alias `'s class-name default, because - :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext` now sits + :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext` sits between this class and that default in the MRO and carries a concrete ``'IPv6-Ext'`` of its own. Inheriting it would rename this header in every :class:`~pcapkit.corekit.protochain.ProtoChain` string and in :meth:`IPv6._decode_next_layer `'s packet - dict key. The value is exactly what the default produced before. + dict key. The value equals the class-name default. """ return 'ESP' diff --git a/pcapkit/protocols/internet/ip.py b/pcapkit/protocols/internet/ip.py index ae85dd8e8..eb680b956 100644 --- a/pcapkit/protocols/internet/ip.py +++ b/pcapkit/protocols/internet/ip.py @@ -8,9 +8,8 @@ :class:`~pcapkit.protocols.internet.ip.IP` only, which is a base class for Internet Protocol (IP) protocol family [*]_, eg. -:class:`~pcapkit.protocols.internet.ipv4.IPv4`, -:class:`~pcapkit.protocols.internet.ipv6.IPv6`, and -:class:`~pcapkit.protocols.internet.ipsec.IPsec`. +:class:`~pcapkit.protocols.internet.ipv4.IPv4` and +:class:`~pcapkit.protocols.internet.ipv6.IPv6`. .. [*] https://en.wikipedia.org/wiki/Internet_Protocol @@ -27,12 +26,14 @@ class IP(Internet[_PT, _ST], Generic[_PT, _ST]): # pylint: disable=abstract-method - """This class implements all protocols in IP family. + """Base class for the IP protocol family. - Internet Protocol version 4 (:class:`~pcapkit.protocols.internet.ipv4.IPv4`) [:rfc:`791`] - Internet Protocol version 6 (:class:`~pcapkit.protocols.internet.ipv6.IPv6`) [:rfc:`2460`] - - Authentication Header (:class:`~pcapkit.protocols.internet.ah.AH`) [:rfc:`4302`] - - Encapsulating Security Payload (:class:`~pcapkit.protocols.internet.esp.ESP`) [:rfc:`4303`] + + The IPsec protocols (:class:`~pcapkit.protocols.internet.ah.AH` and + :class:`~pcapkit.protocols.internet.esp.ESP`) derive from + :class:`~pcapkit.protocols.internet.ipsec.IPsec` instead. """ diff --git a/pcapkit/protocols/internet/ipv6.py b/pcapkit/protocols/internet/ipv6.py index 28b8d0dae..3ed2c45c1 100644 --- a/pcapkit/protocols/internet/ipv6.py +++ b/pcapkit/protocols/internet/ipv6.py @@ -60,59 +60,47 @@ class IPv6(IP[Data_IPv6, Schema_IPv6], # Defaults. ########################################################################## - #: Extension header codes that have a *dedicated* parser class in this - #: package whose own layout follows :rfc:`6564#section-4`'s generic - #: ``next`` + ``Hdr Ext Len`` format (see the module docstring of - #: :mod:`pcapkit.protocols.internet.ipv6_ext` for the exception - #: table in full). When that dedicated parser raises, - #: :meth:`_import_next_layer` substitutes - #: :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext` + #: 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 the module docstring of + #: :mod:`pcapkit.protocols.internet.ipv6_ext` 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.utilities.decorators.beholder` fall back to plain #: :class:`~pcapkit.protocols.misc.raw.Raw`, which has no ``next`` field - #: and used to crash the whole packet at :meth:`_decode_next_layer`'s - #: ``proto = info.next`` (GitHub issue :issue:`891`). + #: and so cannot continue the walk in :meth:`_decode_next_layer`. #: - #: :attr:`~pcapkit.const.ipv6.extension_header.ExtensionHeader.Shim6` - #: is deliberately absent, even though its wire format also conforms: - #: this package has never had a dedicated parser for it to begin with - #: (``pcapkit/protocols/internet/NotImplemented/shim6.py`` is a 0-byte - #: placeholder, excluded from the wheel by ``MANIFEST.in``), so there is - #: no "own parser" here for it to raise from -- ``Shim6`` reaches - #: :class:`IPv6_Ext` by *direct* registration instead (see the - #: bottom of :mod:`pcapkit.protocols.internet.ipv6_ext`), which - #: already produces exactly this class without needing this set to name - #: it. ``ESP``, ``253`` and ``254`` are absent, but not for the same - #: reason as each other, and not because a generic fallback would help - #: them: + #: Three groups are deliberately absent: #: + #: * :attr:`~pcapkit.const.ipv6.extension_header.ExtensionHeader.Shim6` + #: has no dedicated parser (``pcapkit/protocols/internet/NotImplemented/shim6.py`` + #: is a 0-byte placeholder), so there is no "own parser" for it to raise + #: from. It reaches :class:`IPv6_Ext` by *direct* registration (see the + #: bottom of :mod:`pcapkit.protocols.internet.ipv6_ext`), which already + #: produces exactly this class. #: * ``ESP`` *does* have a dedicated, registered parser - #: (:class:`~pcapkit.protocols.internet.esp.ESP`) -- it is excluded + #: (: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 (unlike ``253``/``254`` below), 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 below still ends there, one iteration - #: later, because :data:`None` fails + #: 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 *existing* end-of-chain - #: path, unrelated to the structural check this set exists for. + #: 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* -- this is what the structural check catches. - #: (``BIT-EMU``/147 used to sit here too, until GitHub issue :issue:`925` found - #: it was never in IANA's authoritative extension-header registry to - #: begin with; :class:`~pcapkit.const.ipv6.extension_header - #: .ExtensionHeader` no longer carries it, so a next-header byte of 147 - #: now fails that same constructor at the *top* of the loop instead -- - #: the ordinary end-of-chain path any unrecognised upper-layer protocol - #: code already takes, one step earlier than it used to.) + #: ``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 instead of losing the whole - #: packet as it used to. + #: (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, @@ -416,30 +404,26 @@ def _decode_next_layer(self, ipv6: 'Data_IPv6', proto: 'Optional[int]' = None, # is what gets handed to ``super()._decode_next_layer`` below payload = payload[next_.length:] - # GitHub issue #891: a layer with no ``next`` field cannot - # safely continue the walk. This is a *structural* check -- - # does the parsed info even carry a ``next`` attribute? -- not - # a fixed set of codes, and deliberately so: HOPOPT, IPv6-Route, - # IPv6-Opts, MH, HIP, IPv6-Frag and AH all have dedicated - # parsers whose data carries ``next``, and Shim6 and any - # recognised header whose own parser raised are both handled by - # :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext`, - # which also carries ``next`` (possibly :data:`None`, on an - # overrun -- see its module docstring). Every IANA extension - # header code this package has not implemented a dedicated - # parser for -- today that is ``253`` and ``254``, and tomorrow - # it is whatever IANA assigns next -- has no - # generic fallback either (see :attr:`__generic_ext_codes__`'s - # docstring for why), so :meth:`_import_next_layer` returns a - # plain :class:`~pcapkit.protocols.misc.raw.Raw`, whose info - # carries no ``next`` at all. Reading ``info.next`` on that - # unconditionally is what used to raise ``AttributeError`` - # here and let a further-out :func:`~pcapkit.utilities.decorators.beholder` - # catch it and degrade the *whole* packet -- the actual #891 - # defect, for every code nobody has implemented. Stopping here - # instead keeps this layer's own fields (still recorded above, - # in ``self._exthdr`` and in the packet dict) and reports no - # further next header, exactly like the overrun case. + # A layer with no ``next`` field cannot safely continue the walk. + # This is a *structural* check -- does the parsed info even carry a + # ``next`` attribute? -- not a fixed set of codes, and deliberately + # so: HOPOPT, IPv6-Route, IPv6-Opts, MH, HIP, IPv6-Frag and AH all + # have dedicated parsers whose data carries ``next``, and Shim6 and + # any recognised header whose own parser raised are both handled by + # :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext`, which also + # carries ``next`` (possibly :data:`None`, on an overrun -- see its + # module docstring). Every IANA extension header code without a + # dedicated parser -- ``253`` and ``254`` today, and whatever IANA + # assigns next -- has no generic fallback either (see + # :attr:`__generic_ext_codes__`'s docstring for why), so + # :meth:`_import_next_layer` returns a plain + # :class:`~pcapkit.protocols.misc.raw.Raw`, whose info carries no + # ``next`` at all. Reading ``info.next`` on that unconditionally + # would raise ``AttributeError`` and let a further-out + # :func:`~pcapkit.utilities.decorators.beholder` degrade the *whole* + # packet. Stopping here instead keeps this layer's own fields (still + # recorded above, in ``self._exthdr`` and in the packet dict) and + # reports no further next header, exactly like the overrun case. # # This has to run -- and, on a hit, has to set ``proto`` -- # *before* the fragment-header special case below: IPv6-Frag @@ -495,31 +479,24 @@ def _import_next_layer(self, proto: 'int', length: 'Optional[int]' = None, *, # 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 + :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext` for it + rather than letting the exception reach the :func:`~pcapkit.utilities.decorators.beholder` decorating this - method, which would otherwise substitute plain + method, which would substitute plain :class:`~pcapkit.protocols.misc.raw.Raw` -- and ``Raw`` has no - ``next`` field, which is what used to crash the whole packet at - :meth:`_decode_next_layer`'s ``proto = info.next`` (GitHub issue - :issue:`891`). Every other exception -- including one raised by - ``IPv6_Ext`` itself, or by ``ESP``'s own dedicated - parser -- still reaches ``beholder`` unchanged, so *this - method's own* behaviour for anything outside that closed set is - exactly what it was before this method learned the - substitution: a plain ``Raw`` for that one layer. What changed - for ``253`` and ``254`` -- which have no dedicated parser at - all, so they were *already* reaching plain ``Raw`` with no - exception involved -- is one level up: - :meth:`_decode_next_layer` now stops its walk structurally on + ``next`` field, so it could not continue + :meth:`_decode_next_layer`'s walk. Every other exception -- + including one raised by ``IPv6_Ext`` itself, or by ``ESP``'s own + dedicated parser -- still reaches ``beholder`` unchanged, giving a + plain ``Raw`` for that one layer. ``253`` and ``254`` have no + dedicated parser, so they reach plain ``Raw`` with no exception + involved; :meth:`_decode_next_layer` stops its walk structurally on any layer whose info carries no ``next`` attribute, ``Raw`` - included, instead of reading ``info.next`` unconditionally and - crashing the whole packet. ``ESP`` is unaffected either way: its - info always carries a ``next`` (:data:`None`, since :rfc:`4303` - encrypts the real value), so neither this substitution nor that - structural check ever engages for it, and the walk ends after it - exactly as it always has -- see :attr:`__generic_ext_codes__`'s - docstring for the full distinction. + included. ``ESP``'s info always carries a ``next`` (:data:`None`, + 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. """ if TYPE_CHECKING: diff --git a/pcapkit/protocols/internet/mh.py b/pcapkit/protocols/internet/mh.py index 657254565..4964a2335 100644 --- a/pcapkit/protocols/internet/mh.py +++ b/pcapkit/protocols/internet/mh.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- # pylint: disable=fixme -"""mobility header +"""Mobility Header :mod:`pcapkit.protocols.internet.mh` contains :class:`~pcapkit.protocols.internet.mh.MH` only, @@ -574,52 +574,6 @@ class FastBindingAcknowledgmentStatus(EnumLookup, IntEnum): above that it was rejected. Note: - Re-parented onto :class:`~pcapkit.corekit.enum.EnumLookup` per GitHub - issue :issue:`930`, finishing :issue:`877`'s phase 2. :meth:`_missing_` below is - untouched, since :class:`EnumLookup` does not touch that hook. - - This class carried its own hand-rolled ``get()`` override through - :issue:`930`, and briefly again through GitHub issue :issue:`935`'s first attempt, - which widened the override to accept ``default`` rather than delete - it outright. An earlier lean on issue :issue:`935` had preferred that - widening; a later ruling in review of the attempt went the other - way: delete both ``get`` overrides in this module rather than widen - them, so :class:`FastBindingAcknowledgmentStatus` and - :class:`IPv6AddressPrefixCode` inherit - :meth:`~pcapkit.corekit.enum.EnumLookup.get` outright. That also - removes the ``# type: ignore[override]`` suppressions the overrides - needed, and makes all seven re-parented classes behave alike on - ``get``, which had not been literally true. Measured before acting - on it: the override's own docstring called it a "Backport support - for original codes", but this class mints no alias -- - ``__members__`` and ``list(cls)`` agree at 6 -- so what the override - actually did was resolve an :class:`int` by direct construction and - a name by subscript, exactly the dual resolution - :meth:`~pcapkit.corekit.enum.EnumLookup.get` already provides for - every other :class:`int`-valued registry in this tree. There was - nothing left to backport. ``get``/``get_all`` now come from the base - alone, the same as the five other re-parents :issue:`930` finished alongside - this one -- including :class:`LocalizedRoutingStatus` and - :class:`LMAAddressCode` below, whose own hand-rolled ``get()`` - GitHub issue :issue:`880` had already deleted outright, for the same reason: - zero callers depended on anything the base does not already do. - - A behaviour change comes with the deletion, deliberately: the - override branched on ``isinstance(key, int)`` and routed every - other type -- ``None``, a :class:`float`, ... -- through the - *name* path, so ``get(None)`` and ``get(1.5)`` used to answer with - a quiet :exc:`~pcapkit.utilities.exceptions.EnumKeyError` here - while the base -- branching on ``isinstance(key, str)`` instead -- - answers every other :class:`~pcapkit.corekit.enum.EnumLookup` - subclass, and now this one too, with a loud - :exc:`~pcapkit.utilities.exceptions.EnumValueError`. Nothing in - this tree calls ``get`` with such a key: the 20 call sites this - class and :class:`IPv6AddressPrefixCode` had between them, all in - tests, all passed an :class:`int` or a :class:`str`, so the - divergence was live but unreached -- and its removal is what makes - "all seven behave alike" literally true, rather than true only for - the keys a caller happens to pass today. - :rfc:`5568#section-6.2.3` defines these values inline and IANA keeps no registry of them, so the enumeration lives here rather than in :mod:`pcapkit.const.mh`. It is also **not** interchangeable with the @@ -630,23 +584,23 @@ class and :class:`IPv6AddressPrefixCode` had between them, all in supported* there. The enumeration is **closed**: an in-range value :rfc:`5568` leaves - unassigned is not minted a placeholder member. Per the owner's ruling - on GitHub issue :issue:`877`, this RFC-inline value set stays immutable, so - :meth:`_missing_` raises :exc:`~pcapkit.utilities.exceptions.EnumValueError` - instead of extending the class. That is not a capture-level failure - -- sibling frames are unaffected -- but the cost is bigger than one - message. Mobility Header is itself one of IPv6's own chained - extension headers, and IPv6's own extension-header walk then does - ``proto = info.next`` (:mod:`pcapkit.protocols.internet.ipv6`, line - 338) on whatever :meth:`_import_next_layer` handed back; a ``Raw`` - fallback's info carries no ``.next``. That :exc:`AttributeError`, - not this exception, is what a further-out ``@beholder`` actually - catches -- degrading the **whole IPv6 packet**, header fields - included, to :class:`~pcapkit.protocols.misc.raw.Raw`, rather than - just this one MH message. The walk defect predates this change and - already fires on a malformed extension header; a well-formed packet - naming merely an unassigned byte is simply a more likely way to - reach it. See GitHub issue :issue:`880`. + 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. + + The class defines no ``get`` of its own: ``get``/``get_all`` come from + :class:`~pcapkit.corekit.enum.EnumLookup`, like every other + :class:`int`-valued registry here, so a key that is neither an + :class:`int` nor a :class:`str` raises + :exc:`~pcapkit.utilities.exceptions.EnumValueError`. A ``get`` override + was rejected (decided on :issue:`935`; the override was deleted, not + widened to accept ``default``): this class mints no alias + (``__members__`` and ``list(cls)`` both hold 6 members), so an override + could only repeat the base's dual :class:`int`/name resolution. """ @@ -678,9 +632,9 @@ def _missing_(cls, value: 'int') -> 'NoReturn': Raises: EnumValueError: Always. :rfc:`5568#section-6.2.3` names this value - set inline with no IANA registry behind it, and the owner's - ruling on GitHub issue :issue:`877` is that it stays immutable rather - than minting an ``Unassigned_N`` placeholder member. + set inline with no IANA registry behind it, and it stays + immutable (decided on :issue:`877`) rather than minting an + ``Unassigned_N`` placeholder member. """ raise EnumValueError('%r is not a valid %s' % (value, cls.__name__)) @@ -694,54 +648,6 @@ class IPv6AddressPrefixCode(EnumLookup, IntEnum): :rfc:`5568#section-6.4.2`. Note: - Re-parented onto :class:`~pcapkit.corekit.enum.EnumLookup` per GitHub - issue :issue:`930`, finishing :issue:`877`'s phase 2. :meth:`_missing_` below is - untouched, since :class:`EnumLookup` does not touch that hook. - - This class carried its own hand-rolled ``get()`` override through - :issue:`930`, and briefly again through GitHub issue :issue:`935`'s first attempt, - which widened the override to accept ``default`` rather than delete - it outright. An earlier lean on issue :issue:`935` had preferred that - widening; a later ruling in review of the attempt went the other - way: delete both ``get`` overrides in this module rather than widen - them, so :class:`FastBindingAcknowledgmentStatus` and - :class:`IPv6AddressPrefixCode` inherit - :meth:`~pcapkit.corekit.enum.EnumLookup.get` outright. That also - removes the ``# type: ignore[override]`` suppressions the overrides - needed, and makes all seven re-parented classes behave alike on - ``get``, which had not been literally true. Measured before acting - on it: the override's own docstring called it a "Backport support - for original codes", but this class mints no alias -- - ``__members__`` and ``list(cls)`` agree at 4 -- so what the override - actually did was resolve an :class:`int` by direct construction and - a name by subscript, exactly the dual resolution - :meth:`~pcapkit.corekit.enum.EnumLookup.get` already provides for - every other :class:`int`-valued registry in this tree. There was - nothing left to backport. ``get``/``get_all`` now come from the base - alone, the same as the five other re-parents :issue:`930` finished alongside - this one -- including - :class:`~pcapkit.protocols.internet.mh.LocalizedRoutingStatus` and - :class:`~pcapkit.protocols.internet.mh.LMAAddressCode` below, whose - own hand-rolled ``get()`` GitHub issue :issue:`880` had already deleted - outright, for the same reason: zero callers depended on anything the - base does not already do. - - A behaviour change comes with the deletion, deliberately: the - override branched on ``isinstance(key, int)`` and routed every - other type -- ``None``, a :class:`float`, ... -- through the - *name* path, so ``get(None)`` and ``get(1.5)`` used to answer with - a quiet :exc:`~pcapkit.utilities.exceptions.EnumKeyError` here - while the base -- branching on ``isinstance(key, str)`` instead -- - answers every other :class:`~pcapkit.corekit.enum.EnumLookup` - subclass, and now this one too, with a loud - :exc:`~pcapkit.utilities.exceptions.EnumValueError`. Nothing in - this tree calls ``get`` with such a key: the 20 call sites this - class and :class:`FastBindingAcknowledgmentStatus` had between - them, all in tests, all passed an :class:`int` or a :class:`str`, - so the divergence was live but unreached -- and its removal is - what makes "all seven behave alike" literally true, rather than - true only for the keys a caller happens to pass today. - :rfc:`5568#section-6.4.2` defines these values inline and IANA keeps no registry of them, so the enumeration lives here rather than in :mod:`pcapkit.const.mh`. The identical code space of the neighbor @@ -749,23 +655,23 @@ class and :class:`FastBindingAcknowledgmentStatus` had between likewise unregistered. The enumeration is **closed**: an in-range value :rfc:`5568` leaves - unassigned is not minted a placeholder member. Per the owner's ruling - on GitHub issue :issue:`877`, this RFC-inline value set stays immutable, so - :meth:`_missing_` raises :exc:`~pcapkit.utilities.exceptions.EnumValueError` - instead of extending the class. That is not a capture-level failure - -- sibling frames are unaffected -- but the cost is bigger than one - message. Mobility Header is itself one of IPv6's own chained - extension headers, and IPv6's own extension-header walk then does - ``proto = info.next`` (:mod:`pcapkit.protocols.internet.ipv6`, line - 338) on whatever :meth:`_import_next_layer` handed back; a ``Raw`` - fallback's info carries no ``.next``. That :exc:`AttributeError`, - not this exception, is what a further-out ``@beholder`` actually - catches -- degrading the **whole IPv6 packet**, header fields - included, to :class:`~pcapkit.protocols.misc.raw.Raw`, rather than - just this one MH message. The walk defect predates this change and - already fires on a malformed extension header; a well-formed packet - naming merely an unassigned byte is simply a more likely way to - reach it. See GitHub issue :issue:`880`. + 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. + + The class defines no ``get`` of its own: ``get``/``get_all`` come from + :class:`~pcapkit.corekit.enum.EnumLookup`, like every other + :class:`int`-valued registry here, so a key that is neither an + :class:`int` nor a :class:`str` raises + :exc:`~pcapkit.utilities.exceptions.EnumValueError`. A ``get`` override + was rejected (decided on :issue:`935`; the override was deleted, not + widened to accept ``default``): this class mints no alias + (``__members__`` and ``list(cls)`` both hold 4 members), so an override + could only repeat the base's dual :class:`int`/name resolution. """ @@ -791,9 +697,9 @@ def _missing_(cls, value: 'int') -> 'NoReturn': Raises: EnumValueError: Always. :rfc:`5568#section-6.4.2` names this value - set inline with no IANA registry behind it, and the owner's - ruling on GitHub issue :issue:`877` is that it stays immutable rather - than minting an ``Unassigned_N`` placeholder member. + set inline with no IANA registry behind it, and it stays + immutable (decided on :issue:`877`) rather than minting an + ``Unassigned_N`` placeholder member. """ raise EnumValueError('%r is not a valid %s' % (value, cls.__name__)) @@ -807,13 +713,6 @@ class LocalizedRoutingStatus(EnumLookup, IntEnum): was processed successfully, values of ``128`` and above that it was rejected. Note: - Re-parented onto :class:`~pcapkit.corekit.enum.EnumLookup` per GitHub - issue :issue:`930`, finishing :issue:`877`'s phase 2 -- pure re-parenting as far as - ``get``/``get_all`` are concerned, since this class defines no - ``get`` of its own to reconcile with the base; its own - :meth:`_missing_` below is untouched, since :class:`EnumLookup` does - not touch that hook. - :rfc:`6705#section-10.2` defines these values inline and IANA keeps no registry of them -- neither a dedicated one nor entries in the general *Status Codes* registry -- so the enumeration lives here rather than in @@ -822,37 +721,20 @@ class LocalizedRoutingStatus(EnumLookup, IntEnum): ``128`` and ``129`` mean something else entirely. The enumeration is **closed**: an in-range value :rfc:`6705` leaves - unassigned is not minted a placeholder member. Per the owner's ruling - on GitHub issue :issue:`877`, this RFC-inline value set stays immutable, so - :meth:`_missing_` raises :exc:`~pcapkit.utilities.exceptions.EnumValueError` - instead of extending the class. That is not a capture-level failure - -- sibling frames are unaffected -- but the cost is bigger than one - message. Mobility Header is itself one of IPv6's own chained - extension headers, and IPv6's own extension-header walk then does - ``proto = info.next`` (:mod:`pcapkit.protocols.internet.ipv6`, line - 338) on whatever :meth:`_import_next_layer` handed back; a ``Raw`` - fallback's info carries no ``.next``. That :exc:`AttributeError`, - not this exception, is what a further-out ``@beholder`` actually - catches -- degrading the **whole IPv6 packet**, header fields - included, to :class:`~pcapkit.protocols.misc.raw.Raw`, rather than - just this one MH message. The walk defect predates this change and - already fires on a malformed extension header; a well-formed packet - naming merely an unassigned byte is simply a more likely way to - reach it. See GitHub issue :issue:`880`. - - There is no hand-rolled ``get()`` backport here -- nor, since GitHub - issue :issue:`935`, on :class:`FastBindingAcknowledgmentStatus` or - :class:`IPv6AddressPrefixCode` either: it had zero callers repo-wide - -- tests included -- so GitHub issue :issue:`880` deleted it outright rather - than rebuilding it on the immutable contract, the same conclusion - :issue:`935` reached separately for the other two, on a ruling given in - review of that work: delete those two overrides rather than widen - them to match the base, which an earlier lean on the issue had - preferred. GitHub issue :issue:`930`'s re-parenting above gives this class - ``get``/``get_all`` again, but as the base's own bare lookup rather - than a bespoke override -- it still cannot mint, so an unassigned - value raises through ``get`` exactly as it does through the bare - constructor. + 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. + + The class defines no ``get`` of its own: ``get``/``get_all`` come from + :class:`~pcapkit.corekit.enum.EnumLookup`, and since it cannot mint, an + unassigned value raises through ``get`` exactly as it does through the + bare constructor. A ``get`` override was rejected (decided on + :issue:`880`): it had no callers, and rebuilding it on the immutable + contract would add nothing to the base. """ @@ -874,9 +756,9 @@ def _missing_(cls, value: 'int') -> 'NoReturn': Raises: EnumValueError: Always. :rfc:`6705#section-10.2` names this value - set inline with no IANA registry behind it, and the owner's - ruling on GitHub issue :issue:`877` is that it stays immutable rather - than minting an ``Unassigned_N`` placeholder member. + set inline with no IANA registry behind it, and it stays + immutable (decided on :issue:`877`) rather than minting an + ``Unassigned_N`` placeholder member. """ raise EnumValueError('%r is not a valid %s' % (value, cls.__name__)) @@ -889,49 +771,25 @@ class LMAAddressCode(EnumLookup, IntEnum): address family the option carries, c.f., :rfc:`5949#section-6.2.2`. Note: - Re-parented onto :class:`~pcapkit.corekit.enum.EnumLookup` per GitHub - issue :issue:`930`, finishing :issue:`877`'s phase 2 -- pure re-parenting as far as - ``get``/``get_all`` are concerned, since this class defines no - ``get`` of its own to reconcile with the base; its own - :meth:`_missing_` below is untouched, since :class:`EnumLookup` does - not touch that hook. - :rfc:`5949#section-6.2.2` defines these values inline and IANA keeps no registry of them, so the enumeration lives here rather than in :mod:`pcapkit.const.mh`. The enumeration is **closed**: an in-range value :rfc:`5949` leaves - unassigned is not minted a placeholder member. Per the owner's ruling - on GitHub issue :issue:`877`, this RFC-inline value set stays immutable, so - :meth:`_missing_` raises :exc:`~pcapkit.utilities.exceptions.EnumValueError` - instead of extending the class. That is not a capture-level failure - -- sibling frames are unaffected -- but the cost is bigger than one - message. Mobility Header is itself one of IPv6's own chained - extension headers, and IPv6's own extension-header walk then does - ``proto = info.next`` (:mod:`pcapkit.protocols.internet.ipv6`, line - 338) on whatever :meth:`_import_next_layer` handed back; a ``Raw`` - fallback's info carries no ``.next``. That :exc:`AttributeError`, - not this exception, is what a further-out ``@beholder`` actually - catches -- degrading the **whole IPv6 packet**, header fields - included, to :class:`~pcapkit.protocols.misc.raw.Raw`, rather than - just this one MH message. The walk defect predates this change and - already fires on a malformed extension header; a well-formed packet - naming merely an unassigned byte is simply a more likely way to - reach it. See GitHub issue :issue:`880`. - - There is no hand-rolled ``get()`` backport here -- nor, since GitHub - issue :issue:`935`, on :class:`FastBindingAcknowledgmentStatus` or - :class:`IPv6AddressPrefixCode` either: it had zero callers repo-wide - -- tests included -- so GitHub issue :issue:`880` deleted it outright rather - than rebuilding it on the immutable contract, the same conclusion - :issue:`935` reached separately for the other two, on a ruling given in - review of that work: delete those two overrides rather than widen - them to match the base, which an earlier lean on the issue had - preferred. GitHub issue :issue:`930`'s re-parenting above gives this class - ``get``/``get_all`` again, but as the base's own bare lookup rather - than a bespoke override -- it still cannot mint, so an unassigned - value raises through ``get`` exactly as it does through the bare - constructor. + 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. + + The class defines no ``get`` of its own: ``get``/``get_all`` come from + :class:`~pcapkit.corekit.enum.EnumLookup`, and since it cannot mint, an + unassigned value raises through ``get`` exactly as it does through the + bare constructor. A ``get`` override was rejected (decided on + :issue:`880`): it had no callers, and rebuilding it on the immutable + contract would add nothing to the base. """ @@ -953,9 +811,9 @@ def _missing_(cls, value: 'int') -> 'NoReturn': Raises: EnumValueError: Always. :rfc:`5949#section-6.2.2` names this value - set inline with no IANA registry behind it, and the owner's - ruling on GitHub issue :issue:`877` is that it stays immutable rather - than minting an ``Unassigned_N`` placeholder member. + set inline with no IANA registry behind it, and it stays + immutable (decided on :issue:`877`) rather than minting an + ``Unassigned_N`` placeholder member. """ raise EnumValueError('%r is not a valid %s' % (value, cls.__name__)) @@ -965,7 +823,7 @@ class MH(IPv6_Ext[Data_MH, Schema_MH], schema=Schema_MH, data=Data_MH): """This class implements Mobility Header. - This class currently supports parsing of the following MH message types, + This class supports parsing of the following MH message types, which are registered in the :attr:`self.__message__ ` attribute: @@ -1048,7 +906,7 @@ class MH(IPv6_Ext[Data_MH, Schema_MH], - :meth:`~pcapkit.protocols.internet.mh.MH._read_msg_sr` - :meth:`~pcapkit.protocols.internet.mh.MH._make_msg_sr` - This class currently supports parsing the following MH options, which are + This class supports parsing the following MH options, which are registered in the :attr:`self.__option__ ` attribute: @@ -1272,7 +1130,7 @@ class MH(IPv6_Ext[Data_MH, Schema_MH], - :meth:`~pcapkit.protocols.internet.mh.MH._read_opt_dlif_lladdr` - :meth:`~pcapkit.protocols.internet.mh.MH._make_opt_dlif_lladdr` - This class currently supports parsing of the following MH CGA extensions, + This class supports parsing of the following MH CGA extensions, which are registered in the :attr:`self.__extension__ ` attribute: @@ -1338,7 +1196,7 @@ class MH(IPv6_Ext[Data_MH, Schema_MH], #: DefaultDict[Enum_Option, str | tuple[OptionParser, OptionConstructor]]: #: Option type to method mapping. Method names are expected to be referred - #: to the class by ``_read_option_${name}`` and/or ``_make_opt_${name}``, + #: to the class by ``_read_opt_${name}`` and/or ``_make_opt_${name}``, #: and if such name not found, the value should then be a method that can #: parse the option by itself. __option__ = collections.defaultdict( @@ -1420,7 +1278,7 @@ class MH(IPv6_Ext[Data_MH, Schema_MH], #: DefaultDict[Enum_CGAExtension, str | tuple[ExtensionParser, ExtensionConstructor]]: #: CGA extension type to method mapping. Method names are expected to be referred - #: to the class by ``_read_extension_${name}`` and/or ``_make_ext_${name}``, + #: to the class by ``_read_ext_${name}`` and/or ``_make_ext_${name}``, #: and if such name not found, the value should then be a method that can #: parse the CGA extension by itself. __extension__ = collections.defaultdict( @@ -1449,14 +1307,14 @@ def alias(self) -> 'Literal["MH"]': Spelled out rather than left to :attr:`ProtocolBase.alias `'s class-name default, because - :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext` now sits + :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext` sits between this class and that default in the MRO and carries a concrete - ``'IPv6-Ext'`` of its own (GitHub issue :issue:`917`). Inheriting it would + ``'IPv6-Ext'`` of its own. Inheriting it would rename this header in every :class:`~pcapkit.corekit.protochain.ProtoChain` string and in :meth:`IPv6._decode_next_layer `'s packet - dict key. The value is exactly what the default produced before. + dict key. The value equals the class-name default. """ return 'MH' @@ -1561,15 +1419,12 @@ def _mh_message_length(header_len: 'int') -> 'int': Mobility Header, in units of 8 octets, excluding the first 8 octets"* -- i.e. the total header is ``8 + 8 * header_len`` octets, or equivalently ``(header_len + 1) * 8``. Every ``_read_msg_*`` - below reports this value back as the parsed message's own - ``.length``, which :meth:`~pcapkit.protocols.internet.mh.MH.read` - then subtracts from the outer packet length to find the next - layer's length -- precisely the role ``Hdr Ext Len`` played in - :issue:`487`, and the same read-side duplication :meth:`make`'s write-side - expression (``(len(data_val) + 6) // 8 - 1``, this formula's - inverse) had already been unified out of. Do NOT drop the ``+ 1``: - the units either side of it differ (octets vs. 8-octet units), and - dropping the offset silently reinterprets the field. + below reports this value as the parsed message's own ``.length``, + which :meth:`~pcapkit.protocols.internet.mh.MH.read` subtracts from the + outer packet length to find the next layer's length. It is the inverse + of :meth:`make`'s ``(len(data_val) + 6) // 8 - 1``. Do NOT drop the + ``+ 1``: the units either side of it differ (octets vs. 8-octet + units), and dropping the offset silently reinterprets the field. Args: header_len: raw ``Header Len`` field value, as read off the wire. @@ -1640,9 +1495,9 @@ def make(self, # NOTE: The header has to be a multiple of 8 octets, so the message data # needs padding until ``len(data) + 6`` is aligned. Rounding ``length`` up - # without emitting that padding -- which is what ``math.ceil`` used to do - # here -- declares a header longer than the bytes that follow it, and the - # re-parse then reads whatever happens to be past the end of the buffer. + # without emitting that padding (as ``math.ceil`` would) declares a header + # longer than the bytes that follow it, and the re-parse then reads + # whatever happens to be past the end of the buffer. data_val = self._pad_mh_message(data_val) return Schema_MH( @@ -2909,14 +2764,11 @@ def _mh_option_length(schema_length: 'int') -> 'int': of the option, in octets, excluding the Option Type and Option Length fields"* -- so the whole option, which is what every ``_read_opt_*`` below reports back as the parsed option's own - ``.length``, is two octets more. This is the exact ``+2``/``-2`` - mismatch independently fixed in six places (see the ``Note:`` - on :meth:`_read_opt_pad` below, which explains why a ``Pad1`` - option -- the one option with no ``Option Length`` field at all -- - is this helper's sole exception); collecting the read-side half of - it into one helper is so a future fix to this arithmetic only has - to happen once. Do NOT drop the ``+ 2``: that is precisely this - mismatch. + ``.length``, is two octets more. The read-side call sites share this + helper so the arithmetic lives in one place; a ``Pad1`` option -- the + one option with no ``Option Length`` field at all -- is its sole + exception (see the ``Note:`` on :meth:`_read_opt_pad` below). Do NOT + drop the ``+ 2``. The ``+ 2`` is specific to an :rfc:`6275#section-6.2` mobility option, whose Option Type and Option Length are one octet each. It @@ -2924,7 +2776,7 @@ def _mh_option_length(schema_length: 'int') -> 'int': a CGA extension's Extension Type and Extension Data Length are two octets each [:rfc:`4581#section-2`], so those readers use :meth:`_mh_extension_length` instead. Reusing this helper for them - reported every parsed CGA extension two octets short (:issue:`512`). + would report every parsed CGA extension two octets short. Note that only the *stored-length* read-side call sites are collected here -- most ``_make_opt_*`` methods recompute the wire @@ -4866,8 +4718,7 @@ def _read_fid_suboptions( ` consults, so packing a perfectly good option fails with :exc:`~pcapkit.utilities.exceptions.FieldValueError`. Python 3.11 and - newer give each class its own cache and the checks behave, which is why - this was invisible on a modern interpreter. + newer give each class its own cache, so the checks behave there. The code is on the wire and the registry keys on it, so it is both the cheaper discriminator and the only one that cannot be poisoned. The @@ -6403,12 +6254,11 @@ def _mh_extension_length(schema_length: 'int') -> 'int': non-experimental assigned type, whose ``Ext Len`` is the *"[l]ength of the Extension in octets, not including the first 4 octets"*. - Passing these lengths through :meth:`_mh_option_length` reported every - parsed CGA extension two octets short (:issue:`512`): an 8-octet extension with - an ``Extension Data Length`` of ``4`` came back as ``6``. Note - :meth:`_make_cga_extensions` has always measured ``len(schema.pack())`` - instead, so the write side was already right and only the read side - disagreed with the wire. + Passing these lengths through :meth:`_mh_option_length` would report + every parsed CGA extension two octets short: an 8-octet extension with + an ``Extension Data Length`` of ``4`` would come back as ``6``. The write + side is unaffected, since :meth:`_make_cga_extensions` measures + ``len(schema.pack())`` instead. Args: schema_length: raw ``Extension Data Length`` field value, as read off the wire. @@ -7633,7 +7483,7 @@ def _pad_mh_message(self, data: 'Schema_Packet | bytes') -> 'Schema_Packet | byt Appending is necessary rather than optional: ``length`` is ``(len(data) + 6) // 8 - 1``, which floors, so leaving an opaque body - short emitted 10, 12 or 14 octets while declaring 8, and a parser reads + short would emit 10, 12 or 14 octets while declaring 8, and a parser reads 8 and misinterprets the remainder. Since the caller asked for a packet to be built and the shortfall is recoverable, completing it beats refusing -- the warning is there because the emitted body is then not @@ -7767,8 +7617,8 @@ def _make_opt_pad(self, type: 'Enum_Option', option: 'Optional[Data_PadOption]' *whole* option, whereas :attr:`Schema_PadOption.length ` is the ``Option Length`` field -- two octets fewer, and absent altogether - for a ``Pad1``. Copying one into the other unconverted is why - re-making a parsed ``PadN`` used to come back two octets too long. + for a ``Pad1``. Copying one into the other unconverted would make a + re-made parsed ``PadN`` come back two octets too long. """ if option is not None: @@ -7979,7 +7829,7 @@ def _make_opt_mn_id(self, type: 'Enum_Option', option: 'Optional[Data_MNIDOption (RFC 4283's ``user@realm`` form) rather than a numeric identifier, so there is no non-arbitrary int-to-text mapping the way there is int-to-address or int-to-octets, and an - :obj:`int` is rejected there (c.f. :issue:`467`). + :obj:`int` is rejected there. **kwargs: Arbitrary keyword arguments. Returns: @@ -7992,7 +7842,7 @@ def _make_opt_mn_id(self, type: 'Enum_Option', option: 'Optional[Data_MNIDOption ``::1`` for ``IPv6_Address`` and to a one-octet identifier for the other six, neither of which a caller passing a flag can plausibly have meant; pass ``int(...)`` to get the numeric - value (c.f. :issue:`469`). If ``identifier`` is a negative :obj:`int` + value. If ``identifier`` is a negative :obj:`int` (no subtype has a wire form for one), an :obj:`int` of any value with the ``NAI`` subtype, an :obj:`int` of ``2**128`` or above with the ``IPv6_Address`` subtype (whose wire form is a @@ -8001,9 +7851,9 @@ def _make_opt_mn_id(self, type: 'Enum_Option', option: 'Optional[Data_MNIDOption type its subtype's field cannot hold at all: anything but :obj:`str` for ``NAI``, anything but :obj:`bytes`/ :obj:`bytearray`/:obj:`int` for the other six -- an :obj:`int` - is converted rather than rejected there, per :issue:`467` -- or anything + is converted rather than rejected there -- or anything :class:`ipaddress.IPv6Address` itself does not accept for - ``IPv6_Address`` (c.f. :issue:`469`). + ``IPv6_Address``. """ if option is not None: @@ -8022,15 +7872,15 @@ def _make_opt_mn_id(self, type: 'Enum_Option', option: 'Optional[Data_MNIDOption # exactly the class of leak this handler exists to stop, and which a # guard living inside the ``elif isinstance(identifier, int)`` branch # could not catch, since the ``IPv6_Address`` dispatch never reaches - # it (c.f. #467). + # it. try: # ``Enum_MNIDSubtype(subtype_val)`` round-trips a plain int back # into a named member for the message below -- but its own # ``_missing_`` only auto-extends 9-15 and 16-255, so 0, # negatives and anything above 255 make the constructor itself # raise a bare ``ValueError``, which would defeat the point of - # this guard (c.f. #467). Caught here and the raw value - # used instead rather than let it propagate. + # this guard. Caught here and the raw value used instead + # rather than let it propagate. subtype_repr = repr(Enum_MNIDSubtype(subtype_val)) except ValueError: subtype_repr = repr(subtype_val) @@ -8046,8 +7896,7 @@ def _make_opt_mn_id(self, type: 'Enum_Option', option: 'Optional[Data_MNIDOption # ``BytesField`` subtypes. An MN-ID of ``True`` is a caller mistake # in every case rather than a value anyone means, so it is refused # before either path can give it a plausible-looking wire form. A - # caller who genuinely wants the integer should pass ``int(flag)`` - # (c.f. #469 review). + # caller who genuinely wants the integer should pass ``int(flag)``. raise ProtocolError( f'{self.alias}: [OptNo {type}] MN-ID identifier must not be a ' f'bool, not {identifier!r} -- pass int({identifier!r}) if the ' @@ -8059,7 +7908,7 @@ def _make_opt_mn_id(self, type: 'Enum_Option', option: 'Optional[Data_MNIDOption # 16-octet address (:class:`~pcapkit.corekit.fields.ipaddress.IPv6AddressField` # ignores any declared length), so ``identifier`` is normalised to that wire # form here as well, keeping the packed bytes and the declared length derived - # from one value instead of two independent computations (c.f. #448). + # from one value instead of two independent computations. if subtype_val == Enum_MNIDSubtype.IPv6_Address: if isinstance(identifier, int) and identifier >= 1 << 128: # NOTE: the upper-bound mirror of the negative-int guard above, and @@ -8070,10 +7919,10 @@ def _make_opt_mn_id(self, type: 'Enum_Option', option: 'Optional[Data_MNIDOption # unguarded, :class:`ipaddress.IPv6Address` raises # ``AddressValueError``, itself a bare :exc:`ValueError`, so this # handler would otherwise ship with its lower bound guarded and its - # upper bound leaking (c.f. #467). Checked explicitly rather - # than by wrapping the construction below, because that would also + # upper bound leaking. Checked explicitly rather than by + # wrapping the construction below, because that would also # swallow the wrong-*type* ``AddressValueError`` -- a ``str`` or - # ``None`` reaching here -- which is #469's subject, not this one's. + # ``None`` reaching here -- which the branch below handles. raise ProtocolError( f'{self.alias}: [OptNo {type}] MN-ID subtype IPv6_Address ' f'identifier must be an int below 2**128, not {identifier!r}') @@ -8087,7 +7936,7 @@ def _make_opt_mn_id(self, type: 'Enum_Option', option: 'Optional[Data_MNIDOption # bytes, int or str). The ``try`` wraps only this call, not # the whole branch, so it cannot swallow the ProtocolError # raised above for an out-of-range int, which is also a - # ValueError subclass (c.f. #467, #469). + # ValueError subclass. try: identifier = ipaddress.IPv6Address(identifier) except ValueError as error: @@ -8103,11 +7952,10 @@ def _make_opt_mn_id(self, type: 'Enum_Option', option: 'Optional[Data_MNIDOption # there is no non-arbitrary way to do that. str(identifier) packs # and round-trips fine, but an NAI is a network access identifier # ('user@realm', RFC 4283), and a bare decimal-digit string is not - # one: it is mechanically valid and semantically nonsense, exactly - # the "silently accepting a value that cannot pack" #467 removed, - # just relocated to "silently accepting a value that packs into - # the wrong thing". Rejected instead, with the explicit spelling - # a caller who really wants a decimal-digit NAI can use. + # one: it is mechanically valid and semantically nonsense, i.e. + # a value that is silently accepted and packs into the wrong + # thing. Rejected instead, with the explicit spelling a caller + # who really wants a decimal-digit NAI can use. raise ProtocolError( f'{self.alias}: [OptNo {type}] MN-ID subtype NAI identifier ' f'must be str, not int -- pass str({identifier!r}) if a ' @@ -8117,13 +7965,10 @@ def _make_opt_mn_id(self, type: 'Enum_Option', option: 'Optional[Data_MNIDOption # numeric identifier, so unlike NAI there IS a non-arbitrary wire # form: its own minimal big-endian encoding. That is self-consistent # with the declared length by construction and round-trips exactly. - # ``id_len = math.ceil(identifier.bit_length() / 8)`` was the right - # width all along -- the pre-#467 defect was never the sizing, it - # was that ``identifier`` itself stayed an ``int`` afterwards and - # was handed to ``BytesField`` unconverted, which ``struct.pack()`` - # cannot do anything with. #467 initially rejected outright instead - # of noticing that; converting is what this revision does (c.f. - # #467). ``bit_length()`` is 0 for 0 itself, which would + # ``math.ceil(identifier.bit_length() / 8)`` is the right width; + # the ``int`` itself must be converted, because handing it to + # ``BytesField`` unconverted leaves ``struct.pack()`` nothing it can + # pack. ``bit_length()`` is 0 for 0 itself, which would # otherwise declare a zero-octet identifier -- collapsing "the # identifier's value is 0" into "there is no identifier" -- so the # width is floored at one octet, matching what any reasonable @@ -8138,11 +7983,11 @@ def _make_opt_mn_id(self, type: 'Enum_Option', option: 'Optional[Data_MNIDOption # ipaddress.IPv6Address (no ``__len__`` either, so this branch's # own ``len()`` call below would be the one to raise). Guarded # here, before ``len()``, rather than relying on whichever of - # those two happens to fire first (c.f. #469). Deliberately not + # those two happens to fire first. Deliberately not # decoding a ``bytes`` identifier here: an NAI that happens to be # ASCII-encodable is still the caller handing over the wrong # representation, the same "accepts a value that means the wrong - # thing" #467 removed for int, just relocated to bytes. + # thing" as for an int. if not isinstance(identifier, str): raise ProtocolError( f'{self.alias}: [OptNo {type}] MN-ID subtype NAI ' @@ -8157,8 +8002,7 @@ def _make_opt_mn_id(self, type: 'Enum_Option', option: 'Optional[Data_MNIDOption # format demands a bytes object), TypeError for float/None/an # ipaddress.IPv6Address (no ``__len__``, so this branch's own # ``len()`` call below would raise instead). Guarded here, - # before ``len()``, for the same reason as the NAI branch above - # (c.f. #469). + # before ``len()``, for the same reason as the NAI branch above. # # bytearray is accepted alongside bytes -- unlike every other # wrong type here, it already round-trips correctly through @@ -9063,9 +8907,9 @@ def _make_opt_bid(self, type: 'Enum_Option', option: 'Optional[Data_BindingIdent # the option length below is derived from the family, so it cannot # wait for the schema -- and a bare conversion therefore turns a # ``bool`` into a perfectly ordinary ``IPv4Address`` that the - # schema's own guard can no longer tell from a real address. Before - # this, ``address=True`` packed as ``23080001000000000001``, i.e. a - # care-of address of ``0.0.0.1`` (c.f. #508). + # schema's own guard can no longer tell from a real address: it + # would pack ``address=True`` as ``23080001000000000001``, i.e. a + # care-of address of ``0.0.0.1``. addr = parse_ip_address( address, f'{self.alias}: [OptNo {type}] invalid care-of address') length = 8 if addr.version == 4 else 20 @@ -9308,8 +9152,8 @@ def _make_opt_lmaa(self, type: 'Enum_Option', option: 'Optional[Data_LMAAddressO # two cannot be emitted disagreeing. Normalising *here*, ahead of the # schema, is also why the conversion goes through ``parse_ip_address``: # ``ipaddress.ip_address(True)`` is ``0.0.0.1``, and the schema's own - # guard cannot see that it was ever a ``bool``. Before this, - # ``address=True`` packed as ``2906010000000001`` (c.f. #508). + # guard cannot see that it was ever a ``bool``, and it would pack + # ``address=True`` as ``2906010000000001``. addr = parse_ip_address( address, f'{self.alias}: [OptNo {type}] invalid address') @@ -9510,8 +9354,7 @@ def _make_fid_suboption(self, code: 'Enum_FlowIDSuboption', # NOTE: Through ``parse_ip_address`` because the sub-option length # below is derived from the family here, ahead of the schema, so a # bare ``ipaddress.ip_address`` would launder a ``bool`` past the - # schema's guard. Before this, ``address=True`` packed as - # ``0506000000000001`` (c.f. #508). + # schema's guard, packing ``address=True`` as ``0506000000000001``. addr = parse_ip_address( address, f'{self.alias}: [OptNo {code}] invalid target care-of address') return Schema_TargetCareofAddressSuboption( @@ -10021,10 +9864,9 @@ def _make_opt_dmnp(self, type: 'Enum_Option', option: 'Optional[Data_DelegatedMN # NOTE: Through ``parse_ip_address`` because the ``V`` flag and the width # are both derived from the family here, ahead of the schema, so a bare # ``ipaddress.ip_address`` would launder a ``bool`` past the schema's - # guard. Before this, ``prefix=True`` with an IPv4-valid - # ``prefix_length`` packed as ``3706801800000001``; the default - # ``prefix_length=64`` masked it behind the range check below, which is - # why #508's own sweep read this site as already guarded (c.f. #508). + # guard: ``prefix=True`` with an IPv4-valid ``prefix_length`` would pack + # as ``3706801800000001``. The default ``prefix_length=64`` masks it + # behind the range check below, so this site can look already guarded. addr = parse_ip_address( prefix, f'{self.alias}: [OptNo {type}] invalid mobile network prefix') ipv4 = addr.version == 4 @@ -10139,17 +9981,14 @@ def _make_qos_attribute(self, code: 'Enum_QoSAttribute', Constructed attribute schema. Note: - The data model parameter is named ``option`` rather than ``data``, and - that is not cosmetic. The vendor-specific attribute of + The data model parameter is named ``option`` rather than ``data`` + deliberately. The vendor-specific attribute of :rfc:`7222#section-4.2.11` has a field of its own called ``data``, so - with the parameter named ``data`` a caller's ``data=`` bound to the - model parameter instead of reaching ``**kwargs`` -- and the - ``kwargs.get('data')`` fallback then always saw nothing. Building the - attribute the natural way, mirroring the data model's own field names, - silently dropped the vendor payload and wrote the length as though it - were empty. ``vendor`` and ``subtype`` survived because those names do - not collide, which made the loss look like a partial success rather - than a bug. + a parameter of that name would capture a caller's ``data=`` and leave + the ``kwargs.get('data')`` fallback empty, silently dropping the vendor + payload and writing the length as though it were empty. ``vendor`` and + ``subtype`` do not collide, so the loss would look like a partial + success rather than a bug. Dispatch is on ``code`` rather than on ``isinstance``, for the reason given in :meth:`_read_fid_suboptions`. @@ -10298,9 +10137,9 @@ def _make_opt_lma_up(self, type: 'Enum_Option', option: 'Optional[Data_LMAUserPl # NOTE: Through ``parse_ip_address`` because the option length below is # derived from the family here, ahead of the schema, so a bare # ``ipaddress.ip_address`` would launder a ``bool`` past the schema's - # guard. Before this, ``address=True`` packed as ``3b06000000000001``. + # guard, packing ``address=True`` as ``3b06000000000001``. # ``None`` is handled above and stays an absent address, which is a - # legitimate value here and not what is being rejected (c.f. #508). + # legitimate value here and not what is being rejected. addr = parse_ip_address( address, f'{self.alias}: [OptNo {type}] invalid LMA user-plane address') @@ -10890,8 +10729,8 @@ def _make_ext_multiprefix(self, type: 'Enum_CGAExtension', option: 'Optional[Dat # prefixes as a :obj:`tuple`, which # :class:`~pcapkit.corekit.fields.collections.ListField` refuses to # pack -- it raises ``ProtocolUnbound: unsupported type ``. The cast this replaced was a no-op at runtime, so - # re-making a parsed Multi-Prefix extension could not work at all. + # 'tuple'>``; a cast would be a no-op at runtime, so re-making a + # parsed Multi-Prefix extension could not work at all. prefixes = list(option.prefixes) else: prefixes = prefixes or [] @@ -10902,9 +10741,8 @@ def _make_ext_multiprefix(self, type: 'Enum_CGAExtension', option: 'Optional[Dat # **8**-octet prefix apiece, since # :attr:`~pcapkit.protocols.schema.internet.mh.MultiPrefixExtension.prefixes` # is a list of :class:`~pcapkit.corekit.fields.numbers.UInt64Field`. - # This used to read ``1 + len(prefixes) * 16``, which declared 33 - # octets where 20 were emitted for two prefixes, so a re-parse ran off - # the end of the extension. + # Counting 16 octets per prefix would declare more octets than are + # emitted, so a re-parse would run off the end of the extension. length=4 + len(prefixes) * 8, flags={ 'P': int(flag), diff --git a/tests/protocols/internet/test_mh_unit.py b/tests/protocols/internet/test_mh_unit.py index dc9339742..a208ea0a9 100644 --- a/tests/protocols/internet/test_mh_unit.py +++ b/tests/protocols/internet/test_mh_unit.py @@ -1439,10 +1439,11 @@ def test_mh_rfc5568_local_enums_cover_unregistered_value_sets(self) -> None: ``_missing_``'s own ``0 <= value <= 255`` guard and, for an unknown *name*, aliased a second unknown key onto the first at a shared ``-1`` sentinel. Both enums now raise instead of minting, so an unassigned - code degrades the MH parse to :class:`~pcapkit.protocols.misc.raw.Raw` - via ``@beholder`` rather than silently growing the class -- one - message's worth of damage over IPv4, the whole packet's over IPv6 -- - see ``test_mh_local_enums_raise_and_do_not_alias`` for that + code no longer grows 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 -- see ``test_mh_local_enums_raise_and_do_not_alias`` for that distinction and the other two RFC-inline enums. """ from pcapkit.const.mh.option import Option