diff --git a/pcapkit/protocols/internet/ah.py b/pcapkit/protocols/internet/ah.py index b5d9c88d0..06466953a 100644 --- a/pcapkit/protocols/internet/ah.py +++ b/pcapkit/protocols/internet/ah.py @@ -16,9 +16,9 @@ 0 0 ``ah.next`` Next Header 1 8 ``ah.length`` Payload Length 2 16 Reserved (must be zero) - 4 32 ``sah.spi`` Security Parameters Index (SPI) - 8 64 ``sah.seq`` Sequence Number Field - 12 96 ``sah.icv`` Integrity Check Value (ICV) + 4 32 ``ah.spi`` Security Parameters Index (SPI) + 8 64 ``ah.seq`` Sequence Number Field + 12 96 ``ah.icv`` Integrity Check Value (ICV) ======= ========= ======================= =================================== .. [*] https://en.wikipedia.org/wiki/IPsec @@ -52,19 +52,17 @@ class AH(IPsec[Data_AH, Schema_AH], IPv6_Ext[Data_AH, Schema_AH], """This class implements Authentication Header. Double-inherited (GitHub issue :issue:`917`): ``AH`` is both a member of the - IPsec family and an IPv6 extension header -- IANA's *IPv6 Extension - Header Types* registry lists it at 51 (:rfc:`4302#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 - :class:`~pcapkit.const.ipv6.extension_header.ExtensionHeader` registry - agrees. The same section separately states that, in the context of - IPv4, AH 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 :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext`. - :class:`~pcapkit.protocols.internet.ipsec.IPsec` is first in the - bases so that its :meth:`~pcapkit.protocols.internet.ipsec.IPsec.id` - keeps precedence. + IPsec family and an IPv6 extension header. IANA's *IPv6 Extension Header + Types* registry lists it at 51 (:rfc:`4302#section-3.1.1` has it appear + after the hop-by-hop, routing and fragmentation extension headers in the + IPv6 header chain), and + :class:`~pcapkit.const.ipv6.extension_header.ExtensionHeader` agrees. The + same section places it after the IP header and before the next-layer + protocol under 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`. + :class:`~pcapkit.protocols.internet.ipsec.IPsec` comes first so that its + :meth:`~pcapkit.protocols.internet.ipsec.IPsec.id` takes precedence. """ @@ -84,14 +82,13 @@ def alias(self) -> 'Literal["AH"]': Spelled out rather than left to :attr:`ProtocolBase.alias `'s class-name default, because - :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext` now 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 - rename this header in every - :class:`~pcapkit.corekit.protochain.ProtoChain` string and in - :meth:`IPv6._decode_next_layer + :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext` sits between + this class and that default in the MRO and carries its own + ``'IPv6-Ext'`` (GitHub issue :issue:`917`). 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. """ return 'AH' diff --git a/pcapkit/protocols/internet/hip.py b/pcapkit/protocols/internet/hip.py index eb02a2adc..47298ceec 100644 --- a/pcapkit/protocols/internet/hip.py +++ b/pcapkit/protocols/internet/hip.py @@ -259,23 +259,23 @@ class HIP(IPv6_Ext[Data_HIP, Schema_HIP], Internet[Data_HIP, Schema_HIP], schema=Schema_HIP, data=Data_HIP): """This class implements Host Identity Protocol. - Double-inherited, per the maintainer's convention given in review of the - work for :issue:`917`: a header that is *only* usable as an extension header - inherits :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext` alone, - while one that is also usable as a standalone protocol names - :class:`~pcapkit.protocols.internet.internet.Internet` as well. HIP is - both, on two independent grounds: + Double-inherited, per the convention set on :issue:`917`: a header usable + *only* as an extension header inherits + :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext` alone, while one also + usable as a standalone protocol names + :class:`~pcapkit.protocols.internet.internet.Internet` as well. HIP is both, + on two independent grounds: * :rfc:`7401#section-5.1` states that "the HIP header is logically an IPv6 extension header", and IANA lists protocol 139 in its *IPv6 - Extension Header Types* registry -- 11 entries, HIP among them. - * :rfc:`7401#appendix-C.2`, "IPv4 HIP Packet (I1 Packet)", works a - checksum for an **IPv4** header carrying ``Next Header: 139`` with + Extension Header Types* registry (11 entries). + * :rfc:`7401#appendix-C.2`, "IPv4 HIP Packet (I1 Packet)", works a checksum + for an **IPv4** header carrying ``Next Header: 139`` with ``Payload Protocol: 59``. HIP therefore travels directly as an IPv4 - payload, exactly as :class:`~pcapkit.protocols.internet.ah.AH` and - :class:`~pcapkit.protocols.internet.esp.ESP` do. + payload, as do :class:`~pcapkit.protocols.internet.ah.AH` and + :class:`~pcapkit.protocols.internet.esp.ESP`. - The second ground is what separates HIP from + The second ground separates HIP from :class:`~pcapkit.protocols.internet.mh.MH` and Shim6, which are protocols in their own right but cannot appear under IPv4: :rfc:`6275#section-6.1.1` defines the Mobility Header checksum over a @@ -284,11 +284,11 @@ class HIP(IPv6_Ext[Data_HIP, Schema_HIP], Internet[Data_HIP, Schema_HIP], than as protocol 135. ``Internet`` is already reached transitively through ``IPv6_Ext``; naming - it is what records the classification, so a future reader can tell a - deliberate standalone protocol from a header that merely inherits one. + it records the classification, so a reader can tell a deliberate standalone + protocol from a header that merely inherits one. - This class currently supports parsing of the following HIP parameters, - which are registered in the :attr:`self.__parameter__ ` + This class parses the following HIP parameters, which are registered in the + :attr:`self.__parameter__ ` attribute: .. list-table:: @@ -1018,11 +1018,6 @@ def _read_locator(locator: 'Schema_Locator') -> 'Data_LocatorData | IPv6Address' locator_set = Data_LocatorSetParameter( type=schema.type, critical=bool(schema.type & 0b1), - # NOTE: This was the one reported record length in this module left - # on the pre-#651 expression, held back to match the one padding - # site left on it. #679 moves both, together with the ``Length`` - # unit they both read -- see ``LocatorSetParameter.padding`` in - # :mod:`pcapkit.protocols.schema.internet.hip`. length=parameter_total_len(schema.len), locator_set=tuple(_locs), ) @@ -1079,7 +1074,7 @@ def _read_param_puzzle(self, schema: 'Schema_PuzzleParameter', *, version: 'int' # Keep the field's on-wire width, which ``_rand`` alone cannot carry: # ``int.bit_length()`` sees the value, not the octets it was padded # into. ``schema.len`` is ``4 + RHASH_len / 8``, so the width in bits - # is ``(schema.len - 4) * 8``. See #653. + # is ``(schema.len - 4) * 8``. rhash_len=(schema.len - 4) * 8, ) return puzzle @@ -1127,16 +1122,16 @@ def _read_param_solution(self, schema: 'Schema_SolutionParameter', *, version: ' # :rfc:`7401#section-5.2.5` names this octet ``Reserved``, "zero when sent, # ignored when received", and only ``PUZZLE`` (:rfc:`7401#section-5.2.4`) # has a ``Lifetime`` at this offset. Record it verbatim rather than reading - # it as a ``2^(value - 32)`` duration: interpreting it wrote ``0x20`` into a - # field the RFC requires to be zero, and made the conformant ``0x00`` - # impossible to re-serialise. See #654. + # it as a ``2^(value - 32)`` duration: that would write ``0x20`` into a + # field the RFC requires to be zero, and make the conformant ``0x00`` + # impossible to re-serialise. _resv = schema.reserved _opak = schema.opaque _rand = schema.random # ``schema.len`` is ``4 + RHASH_len / 4`` per :rfc:`7401#section-5.2.5`, which - # is the same quantity as ``4 + 2 * (RHASH_len / 8)`` -- two equal-width fields - # of ``RHASH_len / 8`` octets -- only because ``RHASH_len`` is a whole number of - # octets. Do not reuse the ``/ 4`` shorthand on a width that is not; see #608. + # is the same quantity as ``4 + 2 * (RHASH_len / 8)`` (two equal-width fields + # of ``RHASH_len / 8`` octets) only because ``RHASH_len`` is a whole number of + # octets. Do not reuse the ``/ 4`` shorthand on a width that is not. _solt = schema.solution solution = Data_SolutionParameter( @@ -1151,8 +1146,8 @@ def _read_param_solution(self, schema: 'Schema_SolutionParameter', *, version: ' # Keep the two fields' shared on-wire width, which the values alone # cannot carry. Each is ``RHASH_len / 8`` octets and ``schema.len`` is # ``4 + RHASH_len / 4``, so the width in bits is - # ``((schema.len - 4) // 2) * 8`` -- the same halving the schema's own - # field lengths do. See #653. + # ``((schema.len - 4) // 2) * 8``, the same halving the schema's own + # field lengths do. rhash_len=((schema.len - 4) // 2) * 8, ) return solution @@ -3128,12 +3123,11 @@ def _make_locator(locator: 'Optional[Data_Locator]' = None, *, # to octets *here*, ahead of the schema, so a bare # ``ipaddress.IPv6Address`` would launder a ``bool`` into ``::1`` # and hand the schema's ``SwitchField`` plain bytes that its guard - # cannot question. Before this, ``ip=True`` packed a locator of - # ``::1`` with no error at all. The ``version=6`` argument is the - # *IP* version rather than the HIP one the message names, and it is - # what keeps the ``int`` widening this signature documents -- - # ``0x102`` is ``::102``, not the ``0.0.1.2`` that - # ``ipaddress.ip_address`` would give (c.f. #508). + # cannot question. The ``version=6`` argument is the *IP* version + # rather than the HIP one the message names, and it keeps the + # ``int`` widening this signature documents: ``0x102`` is + # ``::102``, not the ``0.0.1.2`` that ``ipaddress.ip_address`` + # would give. ip_val = parse_ip_address( ip, f'HIPv{version}: [ParamNo {code}] invalid locator', version=6) @@ -3172,16 +3166,15 @@ def _make_locator(locator: 'Optional[Data_Locator]' = None, *, type=code, # NOTE: ``Locator.len`` is ``Locator Length``, which # :rfc:`8046#section-4` gives "in 4-octet units" and which counts - # only the ``Locator`` field -- so a locator record is the eight + # only the ``Locator`` field, so a locator record is the eight # fixed octets (traffic type, locator type, locator length, # reserved-and-flags, lifetime) plus ``Locator Length`` * 4. This # parameter's ``len`` is :rfc:`7401` Section 5.2.1's ``Length``, # "length of the Contents, in bytes", so it is the sum of those - # record sizes. It was ``sum(locator['len'])`` until #679: ``4n`` + # record sizes. Summing ``locator['len']`` instead gives ``4n`` # where the contents are ``24n`` octets for plain IPv6 locators, - # which both mis-declared the record on the wire and starved the - # reader's ``ListField`` of the octets it needed -- see - # ``LocatorSetParameter.locators`` in + # mis-declaring the record and starving the reader's ``ListField`` + # -- see ``LocatorSetParameter.locators`` in # :mod:`pcapkit.protocols.schema.internet.hip`. len=sum(8 + locator['len'] * 4 for locator in locators), locators=locators, @@ -3211,28 +3204,27 @@ def _make_puzzle_lifetime(code: 'Enum_Parameter', version: 'int', to a value the one-octet field cannot hold. """ - # Keyed on :class:`~datetime.timedelta`, not on :class:`int`: the old - # ``lifetime if isinstance(lifetime, int) else lifetime.total_seconds()`` - # sent a plain ``float`` down the timedelta branch and escaped an - # :exc:`AttributeError` instead. + # Keyed on :class:`~datetime.timedelta`, not on :class:`int`: testing + # for ``int`` would send a plain ``float`` down the timedelta branch and + # escape an :exc:`AttributeError`. seconds = lifetime.total_seconds() if isinstance(lifetime, timedelta) else lifetime # ``math.log2`` raises a bare :exc:`ValueError` at zero and below. That is - # not a :class:`~pcapkit.utilities.exceptions.BaseError`, so it escapes the - # library's own error handling with a message naming neither HIP nor the - # field. It is reachable from conformant input rather than only from a - # crafted one: a ``Lifetime`` octet of ``0x00`` means ``2^-32`` seconds, - # below :class:`~datetime.timedelta`'s microsecond resolution, so parsing - # one yields ``timedelta(0)`` and re-serialising it lands here. See #654. + # not a :class:`~pcapkit.utilities.exceptions.BaseError`, so it would + # escape the library's error handling with a message naming neither HIP + # nor the field. Conformant input reaches it: a ``Lifetime`` octet of + # ``0x00`` means ``2^-32`` seconds, below + # :class:`~datetime.timedelta`'s microsecond resolution, so parsing one + # yields ``timedelta(0)`` and re-serialising it lands here. if seconds <= 0: raise ProtocolError(f'HIPv{version}: [ParamNo {code}] invalid lifetime: ' f'{seconds} is not a positive number of seconds') octet = math.floor(math.log2(seconds) + 32) - # ``UInt8Field`` wraps rather than raising -- measured, ``300`` packs as - # ``0x2c`` -- so an out-of-range lifetime would otherwise be written as some - # other perfectly valid-looking duration. + # ``UInt8Field`` wraps rather than raising (``300`` packs as ``0x2c``), so + # an out-of-range lifetime would otherwise be written as some other + # valid-looking duration. if not 0 <= octet <= 0xFF: raise ProtocolError(f'HIPv{version}: [ParamNo {code}] invalid lifetime: ' f'{seconds} seconds encodes to {octet}, outside the ' @@ -3262,20 +3254,19 @@ def _make_puzzle_field_width(code: 'Enum_Parameter', version: 'int', Only case 4 can lose a leading zero octet, and it is the only case where the width is genuinely unknowable -- a from-scratch HIPv2 build with nothing - declaring it. Reaching for it unconditionally is what re-serialised a - ``Length = 20`` ``SOLUTION`` as ``Length = 6`` (:issue:`653`) and what built, under - HIPv1, parameters this library's own reader then rejected (:issue:`655`). + declaring it. Using it unconditionally would re-serialise a + ``Length = 20`` ``SOLUTION`` as ``Length = 6`` and build, under HIPv1, + parameters this library's own reader rejects. Two things this deliberately does *not* reject, both of which look like oversights and are not: * ``rhash_len == 0``, i.e. a zero-width payload field. No real hash has a zero-length output, so no conformant packet carries one -- but it is what - case 4 yields for the default ``random=0``, it is what a ``Length = 4`` - parameter parses back to, and it is what this builder produced before this - change. Rejecting it would turn a degenerate-but-self-consistent case into - a new failure for callers that pass no value at all, which is beyond the - three defects this addresses. + case 4 yields for the default ``random=0``, and it is what a + ``Length = 4`` parameter parses back to. Rejecting it would turn a + degenerate-but-self-consistent case into a failure for callers that pass + no value at all. * a ``version`` that is neither 1 nor 2, which falls through to case 4 and is treated as HIPv2. That mirrors :meth:`_read_param_puzzle` and :meth:`_read_param_solution`, whose guards are likewise written as @@ -3356,7 +3347,7 @@ def _make_param_puzzle(self, code: 'Enum_Parameter', param: 'Optional[Data_Puzzl return Schema_PuzzleParameter( type=code, # One field of ``RHASH_len / 8`` octets after the 4-octet - # ``#K``/``Lifetime``/``Opaque`` prefix -- :rfc:`7401#section-5.2.4` + # ``#K``/``Lifetime``/``Opaque`` prefix; :rfc:`7401#section-5.2.4` # spells the same quantity ``4 + RHASH_len / 8``. len=4 + self._make_puzzle_field_width(code, version, rhash_len, random), index=index, @@ -3409,8 +3400,8 @@ def _make_param_solution(self, code: 'Enum_Parameter', param: 'Optional[Data_Sol # Both of these are `None`-sentinelled rather than overwritten # outright, unlike the data fields above. `Data_SolutionParameter` is # immutable, so a caller with a parsed parameter in hand has no other - # way to sanitise a peer's non-conformant `Reserved` -- or to re-frame - # the parameter for an association with a different `RHASH_len`. + # way to sanitise a peer's non-conformant `Reserved` or to re-frame + # the parameter for a different `RHASH_len`. if reserved is None: reserved = param.reserved if rhash_len is None: @@ -3422,13 +3413,13 @@ def _make_param_solution(self, code: 'Enum_Parameter', param: 'Optional[Data_Sol return Schema_SolutionParameter( type=code, # Two equal-width fields, ``Random #I`` and ``Puzzle solution #J``, of - # ``RHASH_len / 8`` octets each -- so the contents length is necessarily + # ``RHASH_len / 8`` octets each, so the contents length is necessarily # even after the 4-octet ``#K``/``Reserved``/``Opaque`` prefix. # :rfc:`7401#section-5.2.5` spells the same quantity - # ``4 + RHASH_len / 4``, which is an identity only because a real + # ``4 + RHASH_len / 4``, an identity only because a real # ``RHASH_len`` is a whole number of octets; ``ceil(bits / 4)`` on an # arbitrary :meth:`int.bit_length` is not that quantity and yields an odd - # width that :meth:`_read_param_solution` rejects. See #608. + # width that :meth:`_read_param_solution` rejects. len=4 + 2 * self._make_puzzle_field_width(code, version, rhash_len, random, solution), index=index, @@ -3766,18 +3757,16 @@ def _make_param_encrypted(self, code: 'Enum_Parameter', param: 'Optional[Data_En iv=iv, data=data, ) - # NOTE: ``cipher`` is not a schema field -- ``ENCRYPTED``'s own wire - # format carries no cipher ID of its own, only the preceding - # ``HIP_CIPHER`` parameter does -- so passing it as a constructor - # keyword (as this used to) drew an ``UnknownFieldWarning`` and was - # dropped, leaving ``pre_unpack`` to fall back to its own sibling - # lookup, which a standalone ``make`` call gives no ``options`` to - # search and which then always treated the parameter as cipher-less - # and silently packed the ``ENCRYPTED`` parameter without its IV. - # Setting the already-resolved ``cipher_id`` as a plain attribute - # instead reaches ``pack()``'s packet context via - # ``packet.update(self.__dict__)``, where ``pre_unpack`` now honours - # it ahead of that lookup. See #556. + # NOTE: ``cipher`` is not a schema field: ``ENCRYPTED``'s own wire + # format carries no cipher ID, only the preceding ``HIP_CIPHER`` + # parameter does. Passed as a constructor keyword it draws an + # ``UnknownFieldWarning`` and is dropped, leaving ``pre_unpack`` to fall + # back to its sibling lookup, which a standalone ``make`` call gives no + # ``options`` to search; the parameter would then be treated as + # cipher-less and silently packed without its IV. Setting the + # already-resolved ``cipher_id`` as a plain attribute instead reaches + # ``pack()``'s packet context via ``packet.update(self.__dict__)``, + # where ``pre_unpack`` honours it ahead of that lookup. schema.cipher = cipher_id return schema diff --git a/pcapkit/protocols/internet/hopopt.py b/pcapkit/protocols/internet/hopopt.py index 6e88b0892..34cbe140d 100644 --- a/pcapkit/protocols/internet/hopopt.py +++ b/pcapkit/protocols/internet/hopopt.py @@ -124,8 +124,9 @@ class HOPOPT(IPv6_Ext[Data_HOPOPT, Schema_HOPOPT], schema=Schema_HOPOPT, data=Data_HOPOPT): """This class implements IPv6 Hop-by-Hop Options. - This class currently supports parsing of the following IPv6 Hop-by-Hop - options, which are registered in the :attr:`self.__option__ ` + This class parses the following IPv6 Hop-by-Hop options, which are + registered in the + :attr:`self.__option__ ` attribute: .. list-table:: @@ -229,14 +230,13 @@ def alias(self) -> 'Literal["HOPOPT"]': Spelled out rather than left to :attr:`ProtocolBase.alias `'s class-name default, because - :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext` now 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 - rename this header in every - :class:`~pcapkit.corekit.protochain.ProtoChain` string and in - :meth:`IPv6._decode_next_layer + :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext` sits between + this class and that default in the MRO and carries its own + ``'IPv6-Ext'`` (GitHub issue :issue:`917`). 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. """ return 'HOPOPT' @@ -353,17 +353,15 @@ def make(self, next_value = self._make_index(next, next_default, namespace=next_namespace, reversed=next_reversed, pack=False) - # NOTE: No options at all is not the same thing as no options area: the - # header is at least 8 octets, 2 of which are the fixed part, so the - # remaining 6 have to be padding options rather than nothing. Passing the - # empty list through the same path is what produces them -- returning - # ``[], 0`` here instead declared an 8-octet header and emitted 2. + # NOTE: No options is not the same as no options area. The header is at + # least 8 octets, 2 of them the fixed part, so the remaining 6 must be + # padding options. Passing the empty list through the same path produces + # them; returning ``[], 0`` would declare an 8-octet header and emit 2. options_value, total_length = self._make_hopopt_options( options if options is not None else []) # NOTE: ``_make_hopopt_options`` has aligned the header, so this division - # is exact; rounding up here used to hide the 6-octet shortfall that the - # per-option alignment left behind. + # is exact and rounding up is unnecessary. length = (total_length - 6) // 8 return Schema_HOPOPT( @@ -478,30 +476,21 @@ def _hopopt_option_length(schema_len: 'int') -> 'int': excludes the Option Type and Opt Data Len fields themselves, 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 ``Data_PadOption.length`` vs. ``Schema_PadOption.length`` - below, at the surviving explanation of that fix); 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. - - Section 4.2 is the citation because it is what defines the TLV option - format, and it is where the sentence quoted above actually appears. - This cited :rfc:`8200#section-4.3` until :issue:`530`, which is a subtler - error than the one :issue:`517` fixed in the IPv6-Opts sibling: Section 4.3 - is not the wrong *header* -- it is the Hop-by-Hop Options header, - which is exactly what this class implements -- but it is the wrong - place for this arithmetic. It defines no ``Opt Data Len`` at all, - deferring the option encoding to Section 4.2 (*"one or more - TLV-encoded options, as described in Section 4.2"*), and its own only - length field is ``Hdr Ext Len``, *"the length of the Hop-by-Hop - Options header in 8-octet units, not including the first 8 octets"* -- - the whole header in 8-octet units, which is a different quantity from - one option's ``Opt Data Len`` in octets. So a reader who followed the - old citation found the right header and no such sentence. Section - 4.3 stays the right reference for the header itself and is still - cited as such below, at the ``Note:`` on the whole extension header - having to be a multiple of 8 octets. + the ``+2``/``-2`` mismatch between the two (see ``Data_PadOption.length`` + vs. ``Schema_PadOption.length`` below). The read-side half lives in this + one helper so the arithmetic has a single home. Do NOT drop the + ``+ 2``. + + Section 4.2 is the citation because it defines the TLV option format and + contains the sentence quoted above. The Hop-by-Hop Options header this + class implements is :rfc:`8200#section-4.3`, but it defers option + encoding to Section 4.2 (*"one or more TLV-encoded options, as + described in Section 4.2"*) and its only length field is + ``Hdr Ext Len``, *"the length of the Hop-by-Hop Options header in + 8-octet units, not including the first 8 octets"*: the whole header, a + different quantity from one option's ``Opt Data Len`` in octets. + Section 4.3 remains the reference for the header itself, as in the + ``Note:`` on the whole extension header being a multiple of 8 octets. Note that only the *stored-length* read-side call sites are collected here -- most ``_make_opt_*`` methods recompute the wire @@ -814,10 +803,10 @@ def _read_opt_smf_dpd(self, schema: 'Schema_SMFDPDOption', *, options: 'Option') if TYPE_CHECKING: schema = cast('Schema_SMFIdentificationBasedDPDOption', schema) - # NOTE: #775 tier 1 made an unresolvable *string* key raise KeyError - # instead of minting a member; only a hand-constructed schema hits - # this guard. An unassigned *wire* value still mints via _missing_ - # (tier 2, deferred), so it never reaches here. + # NOTE: An unresolvable *string* key raises KeyError rather than + # minting a member, so only a hand-constructed schema hits this + # guard. An unassigned *wire* value still mints via _missing_ and + # never reaches here. tid_key = None try: tid_key = schema.info['type'] @@ -1048,10 +1037,10 @@ def _read_opt_mpl(self, schema: 'Schema_MPLOption', *, options: 'Option') -> 'Da ProtocolError: If the option is malformed. """ - # NOTE: #775 tier 1 made an unresolvable *string* key raise KeyError - # instead of minting a member; only a hand-constructed schema hits - # this guard. An unassigned *wire* value still mints via _missing_ - # (tier 2, deferred), so it never reaches here. + # NOTE: An unresolvable *string* key raises KeyError rather than + # minting a member, so only a hand-constructed schema hits this guard. + # An unassigned *wire* value still mints via _missing_ and never + # reaches here. seed_key = None try: seed_key = schema.flags['type'] @@ -1295,13 +1284,13 @@ def _make_pad_options(self, offset: 'int') -> 'tuple[list[Schema_PadOption], int Note: It is the *whole* extension header, fixed part included, that has to - be a multiple of 8 octets [:rfc:`8200#section-4.3`] -- which is why + be a multiple of 8 octets [:rfc:`8200#section-4.3`], which is why the alignment is computed from ``offset`` rather than from the - length of the option that has just been emitted. Aligning each - option to 8 octets on its own leaves the options area a multiple of - 8 octets long, whereas :meth:`read` sizes it as - ``hdr_ext_len * 8 + 6`` -- six short of a multiple of 8 -- so the - constructed header declared 6 octets more than it actually carried. + length of the option just emitted. Aligning each option to 8 octets + on its own leaves the options area a multiple of 8, whereas + :meth:`read` sizes it as ``hdr_ext_len * 8 + 6``, six short of a + multiple of 8; the header would declare 6 octets more than it + carried. A ``PadN`` option spends two octets on its own type and ``Opt Data Len`` fields before any padding data, so occupying @@ -1431,7 +1420,7 @@ def _make_opt_none(self, code: 'Enum_Option', opt: 'Optional[Data_UnassignedOpti **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if opt is not None: @@ -1456,16 +1445,16 @@ def _make_opt_pad(self, code: 'Enum_Option', opt: 'Optional[Data_PadOption]' = N **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. Note: :attr:`Data_PadOption.length ` counts the *whole* option, whereas :attr:`Schema_PadOption.len ` is the - ``Opt Data Len`` field -- two octets fewer, and absent altogether for - a ``Pad1``. ``opt`` used to be ignored here, so re-making a parsed - padding option silently collapsed it to a single ``Pad1``. + ``Opt Data Len`` field: two octets fewer, and absent altogether for a + ``Pad1``. ``opt`` is honoured so that re-making a parsed padding + option keeps its size instead of collapsing to a single ``Pad1``. """ if opt is not None: @@ -1497,7 +1486,7 @@ def _make_opt_tun(self, code: 'Enum_Option', opt: 'Optional[Data_TunnelEncapsula **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if opt is not None: @@ -1527,7 +1516,7 @@ def _make_opt_ra(self, code: 'Enum_Option', opt: 'Optional[Data_RouterAlertOptio **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if opt is not None: @@ -1560,7 +1549,7 @@ def _make_opt_calipso(self, code: 'Enum_Option', opt: 'Optional[Data_CALIPSOOpti **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if opt is not None: @@ -1604,7 +1593,7 @@ def _make_opt_smf_dpd(self, code: 'Enum_Option', opt: 'Optional[Data_SMFIdentifi **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if opt is not None: @@ -1709,7 +1698,7 @@ def _make_opt_pdm(self, code: 'Enum_Option', opt: 'Optional[Data_PDMOption]' = N **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if opt is not None: @@ -1765,7 +1754,7 @@ def _make_opt_qs(self, code: 'Enum_Option', opt: 'Optional[Data_QuickStartOption **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if opt is not None: @@ -1827,7 +1816,7 @@ def _make_opt_rpl(self, code: 'Enum_Option', opt: 'Optional[Data_RPLOption]' = N **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if opt is not None: @@ -1867,7 +1856,7 @@ def _make_opt_mpl(self, code: 'Enum_Option', opt: 'Optional[Data_MPLOption]' = N **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if opt is not None: @@ -1917,7 +1906,7 @@ def _make_opt_ilnp(self, code: 'Enum_Option', opt: 'Optional[Data_ILNPOption]' = **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if opt is not None: @@ -1925,16 +1914,15 @@ def _make_opt_ilnp(self, code: 'Enum_Option', opt: 'Optional[Data_ILNPOption]' = # NOTE: ``nonce`` is packed by a NumberField whose width is this very # ``len`` (c.f. pcapkit.protocols.schema.internet.hopopt.ILNPOption), so - # the declared octet count has to be the ceiling of the bit length over - # eight -- ``bit_length() // 8`` floors instead, and wrapping a float-free - # floor division in ``math.ceil`` is a no-op, so every nonce whose bit - # length is not a multiple of eight used to be sized short and silently - # truncated on the wire (a nonce below 256 was declared as *zero* octets - # and vanished outright). ``bit_length()`` is 0 for 0 itself, which would - # likewise declare a zero-octet nonce -- collapsing "the nonce is 0" into - # "there is no nonce", when RFC 6744 gives the option a Nonce Value field - # -- so the width is floored at one octet, matching - # pcapkit.protocols.internet.mh.MH._make_opt_mn_id (c.f. #601). + # the declared octet count must be the ceiling of the bit length over + # eight. ``bit_length() // 8`` floors, and ``math.ceil`` around an + # integer division is a no-op, so any nonce whose bit length is not a + # multiple of eight would be sized short and truncated on the wire (a + # nonce below 256 as *zero* octets). ``bit_length()`` is 0 for 0, which + # would collapse "the nonce is 0" into "there is no nonce" although + # RFC 6744 gives the option a Nonce Value field, so the width is + # floored at one octet, as in + # pcapkit.protocols.internet.mh.MH._make_opt_mn_id. return Schema_ILNPOption( type=code, len=max(1, math.ceil(nonce.bit_length() / 8)), @@ -1953,7 +1941,7 @@ def _make_opt_lio(self, code: 'Enum_Option', opt: 'Optional[Data_LineIdentificat **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if opt is not None: @@ -1978,7 +1966,7 @@ def _make_opt_jumbo(self, code: 'Enum_Option', opt: 'Optional[Data_JumboPayloadO **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if opt is not None: @@ -2002,7 +1990,7 @@ def _make_opt_home(self, code: 'Enum_Option', opt: 'Optional[Data_HomeAddressOpt **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if opt is not None: @@ -2032,7 +2020,7 @@ def _make_opt_ip_dff(self, code: 'Enum_Option', opt: 'Optional[Data_IPDFFOption] **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if opt is not None: diff --git a/pcapkit/protocols/internet/internet.py b/pcapkit/protocols/internet/internet.py index a6ca9bdc0..e311a22aa 100644 --- a/pcapkit/protocols/internet/internet.py +++ b/pcapkit/protocols/internet/internet.py @@ -33,8 +33,8 @@ class Internet(ProtocolBase[_PT, _ST], Generic[_PT, _ST]): # pylint: disable=abstract-method """Abstract base class for internet layer protocol family. - This class currently supports parsing of the following protocols, which are - registered in the :attr:`self.__proto__ ` + This class parses the following protocols, which are registered in the + :attr:`self.__proto__ ` attribute: .. list-table:: @@ -136,8 +136,8 @@ def register(cls, code: 'Enum_TransType', protocol: 'ModuleDescriptor[ProtocolBa r"""Register a new protocol class. Notes: - The full qualified class name of the new protocol class - should be as ``{protocol.module}.{protocol.name}``. + The fully qualified class name should be + ``{protocol.module}.{protocol.name}``. Arguments: code: protocol code as in :class:`~pcapkit.const.reg.transtype.TransType` @@ -149,11 +149,10 @@ def register(cls, code: 'Enum_TransType', protocol: 'ModuleDescriptor[ProtocolBa class, or not a :class:`~pcapkit.protocols.protocol.Protocol` subclass. Warns: - pcapkit.utilities.warnings.RegistryWarning: If this transport-layer - protocol number is already registered, naming the displaced - entry and its replacement so a caller can tell *what* was lost. - Fires only when the incumbent differs from the replacement -- - see :meth:`ProtocolBase.register + pcapkit.utilities.warnings.RegistryWarning: If this protocol number + is already registered, naming the displaced entry and its + replacement so a caller can tell *what* was lost. Fires only + when the incumbent differs from the replacement; see :meth:`ProtocolBase.register ` for the guard this shares with ``register_protocol``. @@ -208,10 +207,9 @@ def _decode_next_layer(self, dict_: '_PT', proto: 'Optional[int]' = None, # pyl Current protocol with next layer extracted. Notes: - We added a new key ``__next_type__`` to ``dict_`` to store the - next layer protocol type, and a new key ``__next_name__`` to - store the next layer protocol name. These two keys will **NOT** - be included when :meth:`Info.to_dict ` is called. + ``dict_`` gains the key ``__next_type__`` (next layer protocol + type) and ``__next_name__`` (next layer protocol name). Neither + is included when :meth:`Info.to_dict ` is called. """ next_ = cast('ProtocolBase', # type: ignore[redundant-cast] diff --git a/pcapkit/protocols/internet/ipv4.py b/pcapkit/protocols/internet/ipv4.py index ae0100c60..fd22768b1 100644 --- a/pcapkit/protocols/internet/ipv4.py +++ b/pcapkit/protocols/internet/ipv4.py @@ -15,16 +15,19 @@ ======= ========= ====================== ============================================= 0 0 ``ip.version`` Version (``4``) 0 4 ``ip.hdr_len`` Internal Header Length (IHL) - 1 8 ``ip.dsfield.dscp`` Differentiated Services Code Point (DSCP) - 1 14 ``ip.dsfield.ecn`` Explicit Congestion Notification (ECN) + 1 8 ``ip.tos.pre`` ToS Precedence + 1 11 ``ip.tos.del`` ToS Delay + 1 12 ``ip.tos.thr`` ToS Throughput + 1 13 ``ip.tos.rel`` ToS Reliability + 1 14 ``ip.tos.ecn`` Explicit Congestion Notification (ECN) 2 16 ``ip.len`` Total Length 4 32 ``ip.id`` Identification 6 48 Reserved Bit (must be ``\\x00``) 6 49 ``ip.flags.df`` Don't Fragment (DF) 6 50 ``ip.flags.mf`` More Fragments (MF) - 6 51 ``ip.frag_offset`` Fragment Offset + 6 51 ``ip.offset`` Fragment Offset 8 64 ``ip.ttl`` Time To Live (TTL) - 9 72 ``ip.proto`` Protocol (Transport Layer) + 9 72 ``ip.protocol`` Protocol (Transport Layer) 10 80 ``ip.checksum`` Header Checksum 12 96 ``ip.src`` Source IP Address 16 128 ``ip.dst`` Destination IP Address @@ -128,8 +131,8 @@ class IPv4(IP[Data_IPv4, Schema_IPv4], schema=Schema_IPv4, data=Data_IPv4): """This class implements Internet Protocol version 4. - This class currently supports parsing of the following IPv4 options, - which are registered in the :attr:`self.__option__ ` + This class parses the following IPv4 options, which are registered in the + :attr:`self.__option__ ` attribute: .. list-table:: @@ -655,7 +658,7 @@ def _read_opt_unassigned(self, schema: 'Schema_UnassignedOption', *, options: 'O def _read_opt_eool(self, schema: 'Schema_EOOLOption', *, options: 'Option') -> 'Data_EOOLOption': # pylint: disable=unused-argument """Read IPv4 End of Option List (``EOOL``) option. - Structure of IPv4 End of Option List (``EOOL``) option [:rfc:`719`]: + Structure of IPv4 End of Option List (``EOOL``) option [:rfc:`791`]: .. code-block:: text @@ -682,7 +685,7 @@ def _read_opt_eool(self, schema: 'Schema_EOOLOption', *, options: 'Option') -> ' def _read_opt_nop(self, schema: 'Schema_NOPOption', *, options: 'Option') -> 'Data_NOPOption': # pylint: disable=unused-argument """Read IPv4 No Operation (``NOP``) option. - Structure of IPv4 No Operation (``NOP``) option [:rfc:`719`]: + Structure of IPv4 No Operation (``NOP``) option [:rfc:`791`]: .. code-block:: text @@ -1255,16 +1258,12 @@ def _make_ipv4_options(self, options: 'list[Schema_Option | tuple[Enum_OptionNum # force alignment to 32-bit boundary if data_len % 4: pad_len = 4 - (data_len % 4) - # NOTE: The terminator goes in as an EOOL option *schema*, the - # way the padding above goes in as a NOP schema. What used to be - # appended was ``Enum_OptionNumber.EOOL`` itself -- the wire code - # rather than an option -- and the enclosing ``options`` field - # takes only schemas and :obj:`bytes`, so packing the header - # failed with ``FieldValueError: Field options has invalid - # value``. Any option whose length is not already a multiple of - # four reaches this branch, so that made the packet unpackable - # whether it was built by hand or rebuilt from a parsed one. See - # #506. + # NOTE: The terminator is an EOOL option *schema*, like the NOP + # padding. The bare ``Enum_OptionNumber.EOOL`` wire code is not + # an option, and the ``options`` field takes only schemas and + # :obj:`bytes`, so packing would fail with ``FieldValueError: + # Field options has invalid value`` for any option whose + # length is not a multiple of four. pad_opt = self._make_opt_nop(Enum_OptionNumber.NOP) # type: ignore[arg-type] end_opt = self._make_opt_eool(Enum_OptionNumber.EOOL) # type: ignore[arg-type] total_length += pad_len @@ -1297,10 +1296,9 @@ def _make_ipv4_options(self, options: 'list[Schema_Option | tuple[Enum_OptionNum # force alignment to 32-bit boundary if data_len % 4: pad_len = 4 - (data_len % 4) - # NOTE: An EOOL option schema rather than the bare wire code, for the - # reason spelled out in the list branch above. This is the branch the - # ``from_data`` path takes, since a parsed packet hands its options - # back as a container. See #506. + # NOTE: An EOOL option schema rather than the bare wire code, as in + # the list branch above. This is the ``from_data`` branch, since a + # parsed packet hands its options back as a container. pad_opt = self._make_opt_nop(Enum_OptionNumber.NOP) # type: ignore[arg-type] end_opt = self._make_opt_eool(Enum_OptionNumber.EOOL) # type: ignore[arg-type] total_length += pad_len @@ -1322,7 +1320,7 @@ def _make_opt_unassigned(self, kind: 'Enum_OptionNumber', option: 'Optional[Data **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if option is not None: @@ -1344,7 +1342,7 @@ def _make_opt_eool(self, kind: 'Enum_OptionNumber', option: 'Optional[Data_EOOLO **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ return Schema_EOOLOption( @@ -1362,7 +1360,7 @@ def _make_opt_nop(self, kind: 'Enum_OptionNumber', option: 'Optional[Data_NOPOpt **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ return Schema_NOPOption( @@ -1390,7 +1388,7 @@ def _make_opt_sec(self, kind: 'Enum_OptionNumber', option: 'Optional[Data_SECOpt **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. Raises: ProtocolError: If ``authorities`` names a bit position that is not a @@ -1433,10 +1431,10 @@ def _make_opt_sec(self, kind: 'Enum_OptionNumber', option: 'Optional[Data_SECOpt f'termination indicator, not an authority') # ``max_auth`` is the highest bit *index*, so the octet count comes - # from the bit *count* one past it. Sizing from the index itself put - # a single ``GENSER`` (index 0) in a zero-octet bitmap and then - # indexed into it, raising a bare ``IndexError``; and it under-sized - # by an octet at every exact multiple of eight. See #537. + # from the bit *count* one past it. Sizing from the index would put + # a single ``GENSER`` (index 0) in a zero-octet bitmap and raise + # ``IndexError``, and under-size by an octet at every exact + # multiple of eight. max_auth = max(authorities) int_len = math.ceil((max_auth + 1) / 8) @@ -1445,10 +1443,9 @@ def _make_opt_sec(self, kind: 'Enum_OptionNumber', option: 'Optional[Data_SECOpt data_list[auth] = b'1' # Bit 0 of the *last* octet terminates the field. The intermediate - # octets keep the ``0`` they were initialised with, which is what - # says "another octet follows" -- so this single assignment is the - # whole of the indicator, and without it every option this method - # wrote was one its own reader warned about. + # octets keep their initial ``0``, meaning "another octet follows", + # so this one assignment is the whole indicator; without it the + # reader warns on every option written here. data_list[-1] = b'1' data = int(b''.join(data_list), base=2).to_bytes(int_len, 'big', signed=False) @@ -1476,7 +1473,7 @@ def _make_opt_lsr(self, kind: 'Enum_OptionNumber', option: 'Optional[Data_LSROpt **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if option is not None: @@ -1510,7 +1507,7 @@ def _make_opt_ts(self, kind: 'Enum_OptionNumber', option: 'Optional[Data_TSOptio **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if option is not None: @@ -1582,16 +1579,13 @@ def _make_opt_ts(self, kind: 'Enum_OptionNumber', option: 'Optional[Data_TSOptio raise ProtocolError(f'{self.alias}: [OptNo {kind}] invalid timestamp value: {timestamp}') pointer = 5 + len(ts_list) * 4 - # NOTE: ``ts_data``, the name of the field on - # :class:`~pcapkit.protocols.schema.internet.ipv4.TSOption`, and not the - # ``data`` this used to pass. ``data`` is the attribute that schema's - # ``post_process`` *derives* from ``ts_data``, so naming it here dropped - # every timestamp: :meth:`Schema.__update__ - # ` warns - # ``UnknownFieldWarning`` for a name it does not know and carries on, which - # left ``ts_data`` bound to its class-level ``ListField`` -- and made the - # IPv4 Timestamp option unbuildable through ``make``, since - # ``post_process`` then iterated the field object itself. See #552. + # NOTE: ``ts_data`` is the field on + # :class:`~pcapkit.protocols.schema.internet.ipv4.TSOption`; ``data`` is + # the attribute its ``post_process`` *derives* from ``ts_data``. Passing + # ``data`` would drop every timestamp, since :meth:`Schema.__update__ + # ` only warns + # ``UnknownFieldWarning`` for an unknown name, leaving ``ts_data`` bound + # to its class-level ``ListField`` for ``post_process`` to iterate. return Schema_TSOption( type=kind, length=length, @@ -1617,7 +1611,7 @@ def _make_opt_e_sec(self, kind: 'Enum_OptionNumber', option: 'Optional[Data_ESEC **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if option is not None: @@ -1648,7 +1642,7 @@ def _make_opt_rr(self, kind: 'Enum_OptionNumber', option: 'Optional[Data_RROptio **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if option is not None: @@ -1679,7 +1673,7 @@ def _make_opt_sid(self, kind: 'Enum_OptionNumber', option: 'Optional[Data_SIDOpt **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if option is not None: @@ -1705,7 +1699,7 @@ def _make_opt_ssr(self, kind: 'Enum_OptionNumber', option: 'Optional[Data_SSROpt **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if option is not None: @@ -1736,7 +1730,7 @@ def _make_opt_mtup(self, kind: 'Enum_OptionNumber', option: 'Optional[Data_MTUPO **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if option is not None: @@ -1760,7 +1754,7 @@ def _make_opt_mtur(self, kind: 'Enum_OptionNumber', option: 'Optional[Data_MTURO **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if option is not None: @@ -1790,7 +1784,7 @@ def _make_opt_tr(self, kind: 'Enum_OptionNumber', option: 'Optional[Data_TROptio **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if option is not None: @@ -1826,7 +1820,7 @@ def _make_opt_rtralt(self, kind: 'Enum_OptionNumber', option: 'Optional[Data_RTR **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if option is not None: @@ -1865,7 +1859,7 @@ def _make_opt_qs(self, kind: 'Enum_OptionNumber', option: 'Optional[Data_QuickSt **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if option is not None: diff --git a/pcapkit/protocols/internet/ipv6_ext.py b/pcapkit/protocols/internet/ipv6_ext.py index 894e8d6b2..71ff0f09c 100644 --- a/pcapkit/protocols/internet/ipv6_ext.py +++ b/pcapkit/protocols/internet/ipv6_ext.py @@ -6,12 +6,10 @@ :mod:`pcapkit.protocols.internet.ipv6_ext` contains :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext` -only, which serves two roles at once (GitHub issue :issue:`917`): it is the -shared **base class** of every IPv6 extension header in this package, -and it implements a **generic** extractor for IPv6 extension headers, -standing in for one whenever the header's own dedicated parser is -unavailable or has failed. See the class docstring for the division -between the two. +only, which serves two roles (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 divides the two. Why this is safe in general ---------------------------- @@ -31,12 +29,12 @@ so the first two octets of a *conforming* header are parseable without knowing anything else about it. :rfc:`6564#section-5` is explicit that -this is **not retroactive** -- *"[i]t applies only to newly defined -extension headers"* -- which is why the pre-existing headers need the -closed exception table below rather than being assumed to conform. -(:rfc:`8200#section-4.8` restates the same layout, but without a MUST, -and mislabels the generic length field as *"Length of the Destination -Options header"*; cite :rfc:`6564#section-4`, not that section.) +this is **not retroactive** -- it applies only to newly defined extension +headers -- which is why the pre-existing headers need the closed exception +table below rather than being assumed to conform. (:rfc:`8200#section-4.8` +restates the layout without a MUST and mislabels the generic length field as +the length of the Destination Options header; cite :rfc:`6564#section-4` +instead.) The exception table, verified against IANA's ``protocol-numbers-1.csv`` *IPv6 Extension Header* column and each cited RFC: @@ -62,41 +60,31 @@ ======================================================= =========================================== This table classifies by *wire format* alone, over the eleven codes -:class:`~pcapkit.const.ipv6.extension_header.ExtensionHeader` enumerates -- -matching IANA's own *IPv6 Extension Header Types* registry exactly, as of -GitHub issue :issue:`925`. Before that fix this package generated the enumeration -out of the *Protocol Numbers* registry's extension-header column instead, -which carried a twelfth code, ``BIT_EMU`` (147), that the authoritative -registry does not; 147 was never reachable through this class either way -(it had no dedicated parser and was never generic-dispatched here), so -fixing the enumeration's source changed nothing this table classifies. - -``Shim6`` conforms to it (:rfc:`5533`), but this package has never -had a dedicated parser class for it to begin with -- see "Two entry paths" -below for how it reaches this class regardless, by direct dispatch rather -than by a parser of its own failing. - -:rfc:`8200#section-4.5` sets Encapsulating Security Payload aside -- -*"For this purpose,"* it writes, ESP *"is not considered an extension -header"*, and the sentence after it lists ESP among *"examples of -upper-layer headers"*; :rfc:`4303` puts its -Next Header inside the encrypted trailer, with no length field anywhere -in the cleartext part; and 253/254 are reserved for private -experimentation (:rfc:`3692`) with no wire format at all. None of the -three is reachable through this class, by construction -- see -:meth:`pcapkit.protocols.internet.ipv6.IPv6._import_next_layer`. Each of -them still enters :meth:`IPv6._decode_next_layer -`'s walk (it is a -real :class:`~pcapkit.const.ipv6.extension_header.ExtensionHeader` member), -but the three part ways there: ``ESP`` resolves to its own dedicated parser, -whose info carries a ``next`` that is simply :data:`None` -- so the walk -ends the ordinary way, ``ExtensionHeader(None)`` failing at the top of the -next iteration, exactly as it did before this class existed. ``253`` and -``254`` have no dedicated parser and resolve to plain +:class:`~pcapkit.const.ipv6.extension_header.ExtensionHeader` enumerates, +which match IANA's *IPv6 Extension Header Types* registry exactly (GitHub +issue :issue:`925`). + +``Shim6`` conforms to it (:rfc:`5533`) but has no dedicated parser class; see +"Two entry paths" below for how it reaches this class by direct dispatch. + +:rfc:`8200#section-4.5` sets Encapsulating Security Payload aside: ESP is not +an extension header, and the RFC lists it among upper-layer headers. +:rfc:`4303` puts its Next Header inside the encrypted trailer, with no length +field in the cleartext part. 253 and 254 are reserved for private +experimentation (:rfc:`3692`) with no wire format at all. None of the three +is reachable through this class, by construction; see +:meth:`pcapkit.protocols.internet.ipv6.IPv6._import_next_layer`. Each still +enters :meth:`IPv6._decode_next_layer +`'s walk, being a +real :class:`~pcapkit.const.ipv6.extension_header.ExtensionHeader` member, but +they part ways there. ``ESP`` resolves to its own dedicated parser, whose info +carries a ``next`` of :data:`None`, so the walk ends the ordinary way with +``ExtensionHeader(None)`` failing at the top of the next iteration. ``253`` +and ``254`` have no dedicated parser and resolve to plain :class:`~pcapkit.protocols.misc.raw.Raw`, whose info has no ``next`` -*attribute* at all; for these two (and any future IANA code nobody has -implemented yet) the walk stops on a *structural* check -- does the parsed -layer carry a ``next`` at all? -- rather than on a list of codes. +*attribute*. For these two, and any future IANA code without an +implementation, the walk stops on a *structural* check (does the parsed layer +carry a ``next`` at all?) rather than on a list of codes. Two entry paths ---------------- @@ -104,62 +92,53 @@ This class is reached two different ways: 1. **An unrecognised protocol number.** :class:`~pcapkit.const.ipv6.extension_header.ExtensionHeader.Shim6` - is a real IANA extension header (:rfc:`5533`) that this package has never - had a dedicated parser for, so + is a real IANA extension header (:rfc:`5533`) with no dedicated parser, so :meth:`pcapkit.protocols.internet.internet.Internet._lookup_next_layer` - used to default it to plain :class:`~pcapkit.protocols.misc.raw.Raw` -- - which has no ``next`` field, so the walk in + 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 ` - crashed on it (GitHub issue :issue:`891`). This module registers itself for that - code instead (see the bottom of the module), so dispatch reaches a - working generic parser directly. That 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 happens to carry - protocol number 140; see its docstring for what that would otherwise do. -2. **A recognised header whose own parser raises.** This is the actual :issue:`891` - defect: ``HOPOPT``, ``IPv6-Route``, ``IPv6-Opts``, ``MH``, ``HIP``, - ``IPv6-Frag`` and ``AH`` all have dedicated classes, and when one of - *those* raises, :func:`~pcapkit.utilities.decorators.beholder` + would crash on it (GitHub issue :issue:`891`). This module registers itself + 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.utilities.decorators.beholder` (:meth:`Protocol._import_next_layer `) - would ordinarily catch it and substitute plain - :class:`~pcapkit.protocols.misc.raw.Raw` -- which loses the ``next`` - field the same way, and crashes the walk exactly as Shim6 did. + 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 that failure itself, one layer in from ``beholder``, and - substitutes this class instead: the bad header costs only itself, not - the rest of the chain. + 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. The overrun guard ------------------- The house convention for a declared length that does not fit what remains -is warn-and-clip: emit a :class:`~pcapkit.utilities.warnings.SchemaWarning` -naming what was declared against what is left, then read only what is -left -- - -* :func:`pcapkit.protocols.schema.misc.pcapng.bounded_option` - (``pcapkit/protocols/schema/misc/pcapng.py:373``) -* :func:`pcapkit.protocols.schema.misc.pcapng.bounded_area` - (``pcapkit/protocols/schema/misc/pcapng.py:443``) -* :meth:`pcapkit.corekit.fields.field.FieldBase.pack ` - (``pcapkit/corekit/fields/field.py:507``) -* :meth:`pcapkit.protocols.misc.pcapng.PCAPNG.read_frame` - (``pcapkit/protocols/misc/pcapng.py:1119``) - -This class follows that convention's *warning* and deliberately diverges on -the *action*. Clipping is right when the declared length only governs how -much of the current object to read -- there is always a well-defined "what -is left" to fall back to. Here, the declared length also decides *where the -next header starts*; a clipped skip distance points at whatever bytes -happen to be at the end of the buffer, which are not a header. Continuing -the walk from there would fabricate a layer and record a false entry in -:class:`~pcapkit.corekit.protochain.ProtoChain`, which reads as a parsed -fact rather than as the guess it would be. So :meth:`read` below warns in -the same wording as the sites above, then **stops the walk**: this instance -absorbs every remaining octet and reports :data:`None` for ``next``, which -is what the caller's loop already reads as "no more extension headers" and -ends on honestly, at the bad header, instead of inventing what follows it. +is warn-and-clip: emit a warning naming what was declared against what is +left, then read only what is left. See +:func:`pcapkit.protocols.schema.misc.pcapng.bounded_option`, +:func:`pcapkit.protocols.schema.misc.pcapng.bounded_area` and +:meth:`pcapkit.protocols.misc.pcapng.PCAPNG.read`. + +This class follows that convention's *warning* +(:class:`~pcapkit.utilities.warnings.SchemaWarning`) and deliberately diverges +on the *action*. Clipping is right when the declared length only governs how +much of the current object to read, since there is always a well-defined +"what is left" to fall back to. Here the declared length also decides *where +the next header starts*; a clipped skip distance points at whatever bytes +end the buffer, which are not a header. Continuing from there would +fabricate a layer and record a false entry in +:class:`~pcapkit.corekit.protochain.ProtoChain`, which reads as a parsed fact +rather than a guess. So :meth:`read` warns in the same wording as the sites +above, then **stops the walk**: this instance absorbs every remaining octet +and reports :data:`None` for ``next``, which the caller's loop already reads +as "no more extension headers". The chain ends honestly at the bad header +instead of inventing what follows it. """ from typing import TYPE_CHECKING, Generic, cast, overload @@ -207,19 +186,19 @@ class IPv6_Ext(Internet[_PT, _ST], Generic[_PT, _ST], """This class implements a generic IPv6 extension header parser, and is the shared base of every IPv6 extension header in this package. - See the module docstring for the RFC citations backing the length - rules below, the two ways this class gets dispatched to, and the - reasoning for stopping rather than clipping on an overrun. + The module docstring has the RFC citations behind the length rules, the two + ways this class is dispatched to, and why an overrun stops the walk rather + than clipping. The two roles -------------- - This one class plays both, on the owner's ruling for GitHub issue :issue:`917`: + 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 - this package has no dedicated class for, or whose dedicated class - raised -- which is what :meth:`read`, :meth:`make`, :attr:`name`, - :attr:`alias` and the ``schema=``/``data=`` above implement. + with no dedicated class, or whose dedicated class raised. :meth:`read`, + :meth:`make`, :attr:`name`, :attr:`alias` and the ``schema=``/``data=`` + above implement it. 2. **The base class** of the eight implemented extension headers (:class:`~pcapkit.protocols.internet.hopopt.HOPOPT`, :class:`~pcapkit.protocols.internet.ipv6_route.IPv6_Route`, @@ -232,9 +211,9 @@ class IPv6_Ext(Internet[_PT, _ST], Generic[_PT, _ST], ``_extf`` guards on :attr:`payload`, :attr:`protocol` and :attr:`protochain` are for. - It is generic in its data and schema types -- exactly like + It is generic in its data and schema types, like :class:`~pcapkit.protocols.internet.ipsec.IPsec`, the other base in this - package -- so that a subclass keeps its *own* ``_PT``/``_ST`` instead of + package, so that a subclass keeps its *own* ``_PT``/``_ST`` instead of inheriting this class's. ``AH`` and ``ESP`` therefore double-inherit two identically-parameterised generic bases, ``IPsec[…]`` and ``IPv6_Ext[…]``. @@ -242,12 +221,12 @@ class IPv6_Ext(Internet[_PT, _ST], Generic[_PT, _ST], A subclass **must** define :attr:`name`, :attr:`alias`, :attr:`protocol`, :attr:`length` and :meth:`__index__` itself. All five carry this class's *fallback-role* answers, which are wrong for a - header that has an identity of its own: it would report itself as + header with an identity of its own: it would report itself as ``IPv6 Extension Header`` / ``IPv6-Ext``, read its length off a data model that is not its own, and :meth:`__index__` would raise rather - than return its IANA number. Nothing in the language enforces the - override, so ``tests/protocols/internet/test_ipv6_ext_unit.py`` - enforces it instead, over every subclass discovered at runtime. + than return its IANA number. The language cannot enforce the override, + so ``tests/protocols/internet/test_ipv6_ext_unit.py`` does, over every + subclass discovered at runtime. """ @@ -259,11 +238,10 @@ class IPv6_Ext(Internet[_PT, _ST], Generic[_PT, _ST], def name(self) -> 'str': """Name of current protocol. - Annotated ``str`` rather than as the ``Literal`` every *leaf* protocol - in this package uses, because this one is also a base: a ``Literal`` - here makes each subclass's own ``Literal`` an incompatible override - (measured: eight ``[override]`` errors from mypy, one per subclass). - The value returned is still the single fallback-role constant. + Annotated ``str`` rather than the ``Literal`` every *leaf* protocol in + this package uses, because this one is also a base: a ``Literal`` here + makes each subclass's own ``Literal`` an incompatible override (mypy + ``[override]``). The value is still the single fallback-role constant. """ return 'IPv6 Extension Header' @@ -278,14 +256,13 @@ def alias(self) -> 'str': Hyphenated, like :attr:`IPv6_Frag.alias ` and :attr:`IPv6_Opts.alias `, - and for the same reason: :meth:`IPv6._decode_next_layer + for the same reason: :meth:`IPv6._decode_next_layer ` builds the - packet-dict key by ``self.alias.lstrip('IPv6-').lower()`` -- - :meth:`str.lstrip` strips a character *set*, not a prefix, so the - default (class-name) alias ``'IPv6_Ext'`` would strip to ``'_Ext'`` - and key the dict as ``_ext``. The hyphen makes every leading - character (``I``, ``P``, ``v``, ``6``, ``-``) a member of the set - being stripped, same as the siblings, giving ``ext``. + packet-dict key with ``self.alias.lstrip('IPv6-').lower()``, and + :meth:`str.lstrip` strips a character *set*, not a prefix. The default + (class-name) alias ``'IPv6_Ext'`` would strip to ``'_Ext'`` and key the + dict as ``_ext``; the hyphen makes every leading character (``I``, + ``P``, ``v``, ``6``, ``-``) part of the stripped set, giving ``ext``. """ return 'IPv6-Ext' @@ -295,9 +272,9 @@ def length(self) -> 'int': """Header length of current protocol. Fallback-role member: it reads this module's *own* data model, so a - subclass whose data model records its length elsewhere -- or not at - all, as :class:`~pcapkit.protocols.data.internet.ipv6_frag.IPv6_Frag` - does not -- must override it. All eight implemented headers do, and + subclass whose data model records its length elsewhere, or not at all + (as :class:`~pcapkit.protocols.data.internet.ipv6_frag.IPv6_Frag`), must + override it. All eight implemented headers do, and ``tests/protocols/internet/test_ipv6_ext_unit.py`` holds them to it. """ @@ -307,43 +284,40 @@ def length(self) -> 'int': def protocol(self) -> 'Optional[Enum_ExtensionHeader] | Optional[str]': """The extension header this instance stands in for. - In the *fallback* role -- i.e. when this instance parsed this - module's own schema -- this is **not** the base + In the *fallback* role (this instance parsed this module's own schema) + this is **not** the base :attr:`Protocol.protocol ` - meaning ("name of next layer protocol"); it is deliberately - repointed, on the owner's ruling for GitHub issue :issue:`891`, at this - instance's *own* identity -- which header's format it parsed, e.g. + 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`. - That 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 holds before it dispatches (``ipv6.py:388``). See - :meth:`__index__` for why the *class-level* identity cannot be - made to work the same way. It is deliberately *not* ``_extf``-guarded - in this role: that same call passes ``extension=True`` for every - extension header in a chain, so guarding it would make the identity - unreadable in precisely the case it exists for. + 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 + holds before it dispatches. See :meth:`__index__` for why the + *class-level* identity cannot work the same way. It is deliberately + *not* ``_extf``-guarded in this role: that same call passes + ``extension=True`` for every extension header in a chain, so guarding + it would make the identity unreadable in exactly the case it exists for. In the *base* role it falls through to ``super()``, restoring the ordinary :class:`~pcapkit.protocols.protocol.ProtocolBase` meaning. - That fall-through is load-bearing rather than tidy: all eight - subclasses implement their own ``_extf``-guarded ``protocol`` as - ``return super().protocol``, and this class sits between them and - :class:`~pcapkit.protocols.protocol.ProtocolBase` in the MRO -- so - without the discriminator below, every one of those eight would end - up reading ``self._info.protocol`` off a data model that has no such - field. Measured: ``AttributeError`` on all eight. + That is load-bearing: all eight subclasses implement their own + ``_extf``-guarded ``protocol`` as ``return super().protocol``, and this + class sits between them and + :class:`~pcapkit.protocols.protocol.ProtocolBase` in the MRO, so without + the discriminator below each would read ``self._info.protocol`` off a + data model with no such field (``AttributeError``). The discriminator is the *class-level* - :attr:`~pcapkit.protocols.protocol.ProtocolBase.__data__` rather than - an :func:`isinstance` test on ``self._info``, because a ``make``-only - instance has no ``_info`` at all -- reading it here to decide which - role we are in turns + :attr:`~pcapkit.protocols.protocol.ProtocolBase.__data__` rather than an + :func:`isinstance` test on ``self._info``, because a ``make``-only + instance has no ``_info`` at all. Reading it would turn :attr:`ProtocolBase.protocol `, - which only ever needed ``self._protos``, into an ``AttributeError`` - (measured: ``tests/protocols/internet/test_ipv6_extension_unit.py`` - constructs exactly that instance). + which only needs ``self._protos``, into an ``AttributeError``; + ``tests/protocols/internet/test_ipv6_extension_unit.py`` constructs + exactly that instance. """ if self.__data__ is Data_IPv6_Ext: @@ -354,16 +328,12 @@ def protocol(self) -> 'Optional[Enum_ExtensionHeader] | Optional[str]': def next(self) -> 'Optional[Enum_TransType]': """Next header, as parsed off the wire. - Shared by every subclass rather than fallback-role-only -- see - :class:`_NextHeaderData` for why that is sound. Note this is an - *addition* for the eight implemented headers: none of them declared a - ``next`` property of its own before GitHub issue :issue:`917`, so reading one - raised :exc:`AttributeError`, and nothing could have depended on a - value it never returned. + Shared by every subclass rather than fallback-role-only; see + :class:`_NextHeaderData` for why that is sound. - :data:`None` when the declared length would have overrun what - remained of the chain and the walk stopped instead of trusting it - -- see the module docstring's "overrun guard" section. + :data:`None` when the declared length would have overrun what remained + of the chain and the walk stopped instead of trusting it; see the + module docstring's "overrun guard" section. """ return cast('_NextHeaderData', self._info).next @@ -406,7 +376,7 @@ def read(self, length: 'Optional[int]' = None, *, # pylint: disable=arguments-d **kwargs: Arbitrary keyword arguments, two of which are this class's own and are supplied by :meth:`IPv6._import_next_layer ` - (``ipv6.py:540,550``) rather than typed by a caller: + rather than typed by a caller: * ``alias`` -- the numeric extension header code this instance stands in for, e.g. ``0`` for ``HOPOPT`` or ``140`` for @@ -419,17 +389,15 @@ def read(self, length: 'Optional[int]' = None, *, # pylint: disable=arguments-d Parsed packet data. Note: - Those two are read out of ``**kwargs`` rather than declared as - parameters, and ``version`` is not declared either, because this - class is *also* the base of eight subclasses whose own ``read`` - accepts none of the three. Declaring them here asserts of the whole - family an interface only the fallback has, and both linters say so: - mypy reports eight ``Signature of "read" incompatible with - supertype`` ``[override]`` errors, pylint ``arguments-differ``. - ``version`` costs nothing to drop in any case -- this method never - read it (it carried ``# pylint: disable=unused-argument`` for - exactly that reason); the version gate lives in - :meth:`__post_init__`, which still takes it explicitly. + Those two are read out of ``**kwargs`` rather than declared, and + ``version`` is not declared either, because this class is *also* + the base of eight subclasses whose own ``read`` accepts none of the + three. Declaring them would assert of the whole family an interface + only the fallback has, which mypy (``Signature of "read" + incompatible with supertype``, ``[override]``) and pylint + (``arguments-differ``) both reject. This method never reads + ``version``; the version gate lives in :meth:`__post_init__`, which + still takes it explicitly. """ alias = kwargs.get('alias') # type: Optional[int] @@ -447,27 +415,24 @@ class is *also* the base of eight subclasses whose own ``read`` ext_code = None if ext_code == Enum_ExtensionHeader.IPv6_Frag: - # RFC 8200 §4.5: octet 1 is Reserved, not a length -- the - # fragment header is always exactly 8 octets, and RFC 6564 §5 - # says explicitly that it predates and does not follow the - # generic format. + # RFC 8200 §4.5: octet 1 is Reserved, not a length; the fragment + # header is always exactly 8 octets, and RFC 6564 §5 says it predates + # and does not follow the generic format. nominal = 8 elif ext_code == Enum_ExtensionHeader.AH: # RFC 4302 §2.2: Payload Len is in 4-octet units, excluding the - # first 8 octets -- a different unit and a different bias from - # RFC 6564's Hdr Ext Len. + # first 8 octets: a different unit and bias from RFC 6564's + # Hdr Ext Len. nominal = (schema.len + 2) * 4 else: - # RFC 6564 §4 (MUST): Hdr Ext Len is in 8-octet units, - # excluding the first 8 octets. Also the fallback when ``alias`` - # named no known extension header at all, which is the best - # generic guess available. + # RFC 6564 §4 (MUST): Hdr Ext Len is in 8-octet units, excluding + # the first 8 octets. Also the best generic guess when ``alias`` + # named no known extension header. nominal = (schema.len + 1) * 8 if nominal > length: - # See the module docstring's "overrun guard" section for why - # this warns like the house convention but stops rather than - # clips. + # Warns like the house convention but stops rather than clips; see + # the module docstring's "overrun guard" section. warn(f'IPv6: extension header declares a length of {nominal} octet(s) with ' f'{length} octet(s) left in the chain; stopping the walk instead of ' f'skipping past it', SchemaWarning, stacklevel=stacklevel()) @@ -497,10 +462,9 @@ def make(self, *, Args: next: Next header type. - len: Raw ``Hdr Ext Len`` octet to emit -- the caller's - responsibility to size correctly, since this class does not - know, at construction time, which per-protocol rule (see - the module docstring) the octet is meant to satisfy. + len: Raw ``Hdr Ext Len`` octet to emit. The caller must size it, + since this class cannot know at construction time which + per-protocol rule (see the module docstring) it must satisfy. payload: Payload of current instance. **kwargs: Arbitrary keyword arguments. @@ -508,35 +472,32 @@ def make(self, *, Constructed packet data. Note: - **Keyword-only**, and that matters in two directions at once. + **Keyword-only**, for two reasons. The three stay *declared*, unlike :meth:`read`'s own keywords, because :func:`~pcapkit.protocols.protocol._check_construction_keywords` builds its allowlist from :func:`inspect.signature` of ``make``. - Hiding them in ``**kwargs`` was tried and **breaks construction**: - measured as ``UnsupportedCall: IPv6_Ext: unexpected keyword(s): - 'len' (did you mean 'length'?), 'next', 'payload'`` on a plain - ``IPv6_Ext(next=..., len=..., payload=...)``. The ``__keywords__`` - escape hatch would cover that, but it is unioned down the MRO, so - it would widen the allowlist -- and weaken that misspelling check -- - for all eight subclasses too. - - They are keyword-*only* because this class is also a base, and each + Hiding them in ``**kwargs`` **breaks construction**: a plain + ``IPv6_Ext(next=..., len=..., payload=...)`` raises + ``UnsupportedCall: IPv6_Ext: unexpected keyword(s)``. The + ``__keywords__`` escape hatch would cover that, but it is unioned + down the MRO, so it would widen the allowlist, and weaken that + misspelling check, for all eight subclasses too. + + They are keyword-*only* because this class is also a base and each subclass's ``make`` puts different names in the same positions. As - positional parameters they drew 18 pylint ``arguments-renamed`` - warnings (``ESP.make``: ``next`` -> ``spi``, ``len`` -> ``seq``, - ``payload`` -> ``next``; ``IPv6_Route.make``: ``next`` -> ``dst``, - ``len`` -> ``next``, ``payload`` -> ``next_default``; and two each - for the other six) and eight mypy ``[override]`` errors. With no - positional parameters there is no position to disagree about: - both counts drop to zero, and the allowlist is unaffected because + positional parameters they draw pylint ``arguments-renamed`` + warnings (``ESP.make`` has ``spi``, ``seq`` and ``next`` where + these have ``next``, ``len`` and ``payload``) and mypy + ``[override]`` errors. With no positional parameters there is no + position to disagree about, and the allowlist is unaffected because ``_declared_keywords`` collects ``KEYWORD_ONLY`` parameters too. - Nothing calls a protocol's ``make`` positionally -- ``__init__`` - spreads ``**kwargs`` into it (``protocol.py:607``). + Nothing calls a protocol's ``make`` positionally; ``__init__`` + spreads ``**kwargs`` into it. - ``read`` needs none of this: its ``alias``/``error`` only ever - arrive on the *parse* path, which ``__init__`` exempts from the - construction check outright. + ``read`` needs none of this: its ``alias``/``error`` only arrive on + the *parse* path, which ``__init__`` exempts from the construction + check. """ return cast('_ST', Schema_IPv6_Ext( @@ -576,34 +537,32 @@ def __post_init__(self, file: 'Optional[IO[bytes] | bytes]' = None, length: 'Opt For construction argument, please refer to :meth:`make`. Note: - The version gate below is scoped to the fallback role, by the same + The version gate is scoped to the fallback role by the same ``__data__`` discriminator :attr:`protocol` uses. Applying it to subclasses would break the two that are *not* IPv6-only: :class:`~pcapkit.protocols.internet.ah.AH` and :class:`~pcapkit.protocols.internet.esp.ESP` both default to - ``version=4`` and are perfectly valid under IPv4, so an - unconditional gate here rejects them outright (measured: - ``ProtocolError: ESP: only valid for IPv6, got version=4`` from - ``tests/protocols/internet/test_esp_unit.py``). - - The gate itself is unchanged in effect for the fallback, and is - there because this class is registered into :attr:`Internet.__proto__ - ` -- - shared by *every* :class:`~pcapkit.protocols.internet.internet.Internet` - subclass, :class:`~pcapkit.protocols.internet.ipv4.IPv4` included -- - so that :attr:`~pcapkit.const.ipv6.extension_header.ExtensionHeader.Shim6` - resolves to a working parser instead of defaulting to - :class:`~pcapkit.protocols.misc.raw.Raw`. Without this guard, an - IPv4 packet whose protocol byte happens to be 140 would reach - this class too and walk an IPv6-style extension-header chain out - of an IPv4 payload -- verified: it would read - ``IPv4:IPv6-Ext:...`` instead of the ``IPv4:Shim6`` a - plain, non-continuing ``Raw`` gives today. Rejecting here sends - construction back through :func:`~pcapkit.utilities.decorators.beholder` - at the *caller's* layer, which substitutes that same ``Raw`` -- - i.e. this restores exactly the pre-existing, version-agnostic - behaviour rather than inventing a new one, and needs no - IPv4-specific code of its own. + ``version=4`` and are valid under IPv4, so an unconditional gate + would reject them (``ProtocolError: ESP: only valid for IPv6, got + version=4``). + + The gate exists because this class is registered into + :attr:`Internet.__proto__ + `, which + *every* :class:`~pcapkit.protocols.internet.internet.Internet` + subclass shares, :class:`~pcapkit.protocols.internet.ipv4.IPv4` + included, so that + :attr:`~pcapkit.const.ipv6.extension_header.ExtensionHeader.Shim6` + resolves to a working parser instead of + :class:`~pcapkit.protocols.misc.raw.Raw`. Without it, an IPv4 + packet whose protocol byte is 140 would reach this class and walk + an IPv6-style extension-header chain out of an IPv4 payload, + reading ``IPv4:IPv6-Ext:...`` instead of the ``IPv4:Shim6`` that a + plain, non-continuing ``Raw`` gives. Rejecting here sends + construction back through + :func:`~pcapkit.utilities.decorators.beholder` at the *caller's* + layer, which substitutes that same ``Raw``: the version-agnostic + behaviour, with no IPv4-specific code. """ if self.__data__ is Data_IPv6_Ext and version != 6: @@ -683,18 +642,16 @@ def _make_data(cls, data: 'Data_IPv6_Ext') -> 'dict[str, Any]': # type: ignore[ } -# NOTE: Registered by direct assignment into ``Internet.__proto__`` -- the -# same mechanism ``pcapkit.protocols.internet.internet`` uses to pre-populate -# every other entry -- rather than via the ``code=`` keyword of -# ``ProtocolBase.__init_subclass__``. The two are equivalent in effect, but -# ``code=`` resolves through ``register_protocol_code``, which imports -# ``pcapkit.foundation.registry.protocols`` *at class-definition time*, i.e. -# while ``pcapkit.protocols.internet`` (which imports this module) is still -# being built. Nothing else in this package's ``code=`` usage does that from -# inside ``pcapkit.protocols.internet`` itself, and doing so here completed a -# cycle back into a not-yet-finished ``pcapkit.protocols.internet`` through -# ``pcapkit.foundation.extraction`` -- measured as ``ImportError: cannot -# import name 'Extractor' from partially initialized module -# 'pcapkit.foundation.extraction'`` on a bare ``import pcapkit``. Plain -# dict assignment carries no import of its own, so it cannot re-trigger that. +# NOTE: Registered by direct assignment into ``Internet.__proto__``, as +# ``pcapkit.protocols.internet.internet`` does for every other entry, rather +# than via the ``code=`` keyword of ``ProtocolBase.__init_subclass__``. The two +# are equivalent in effect, but ``code=`` resolves through +# ``register_protocol_code``, which imports +# ``pcapkit.foundation.registry.protocols`` *at class-definition time*, while +# ``pcapkit.protocols.internet`` (which imports this module) is still being +# built. From here that completes a cycle back into the unfinished +# ``pcapkit.protocols.internet`` through ``pcapkit.foundation.extraction``: +# ``ImportError: cannot import name 'Extractor' from partially initialized +# module 'pcapkit.foundation.extraction'`` on a bare ``import pcapkit``. Plain +# dict assignment carries no import of its own. Internet.__proto__[Enum_TransType.Shim6] = IPv6_Ext diff --git a/pcapkit/protocols/internet/ipv6_opts.py b/pcapkit/protocols/internet/ipv6_opts.py index b301f2a45..39501d38a 100644 --- a/pcapkit/protocols/internet/ipv6_opts.py +++ b/pcapkit/protocols/internet/ipv6_opts.py @@ -128,8 +128,8 @@ class IPv6_Opts(IPv6_Ext[Data_IPv6_Opts, Schema_IPv6_Opts], schema=Schema_IPv6_Opts, data=Data_IPv6_Opts): """This class implements Destination Options for IPv6. - This class currently supports parsing of the following IPv6 destination - options, which are registered in the + This class parses the following IPv6 destination options, which are + registered in the :attr:`self.__option__ ` attribute: @@ -345,17 +345,15 @@ def make(self, next_value = self._make_index(next, next_default, namespace=next_namespace, reversed=next_reversed, pack=False) - # NOTE: No options at all is not the same thing as no options area: the - # header is at least 8 octets, 2 of which are the fixed part, so the - # remaining 6 have to be padding options rather than nothing. Passing the - # empty list through the same path is what produces them -- returning - # ``[], 0`` here instead declared an 8-octet header and emitted 2. + # NOTE: No options is not the same as no options area. The header is at + # least 8 octets, 2 of them the fixed part, so the remaining 6 must be + # padding options. Passing the empty list through the same path produces + # them; returning ``[], 0`` would declare an 8-octet header and emit 2. options_value, total_length = self._make_ipv6_opts( options if options is not None else []) # NOTE: ``_make_ipv6_opts`` has aligned the header, so this division is - # exact; rounding up here used to hide the 6-octet shortfall that the - # per-option alignment left behind. + # exact and rounding up is unnecessary. length = (total_length - 6) // 8 return Schema_IPv6_Opts( @@ -470,20 +468,16 @@ def _ipv6_opts_option_length(schema_len: 'int') -> 'int': excludes the Option Type and Opt Data Len fields themselves, 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 ``Data_PadOption.length`` vs. ``Schema_PadOption.length`` - below, at the surviving explanation of that fix); 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. - - Section 4.2 is the citation because it is what defines the TLV option - format, and it is where the sentence quoted above actually appears. - IPv6-Opts itself is the Destination Options header of - :rfc:`8200#section-4.6`, which carries those TLVs but says nothing - about their internal length arithmetic. This cited - :rfc:`8200#section-4.3` until :issue:`517` -- that is the Hop-by-Hop Options - header, which is a different header and not the one this class + the ``+2``/``-2`` mismatch between the two (see ``Data_PadOption.length`` + vs. ``Schema_PadOption.length`` below). The read-side half lives in this + one helper so the arithmetic has a single home. Do NOT drop the + ``+ 2``. + + Section 4.2 is the citation because it defines the TLV option format and + contains the sentence quoted above. IPv6-Opts itself is the Destination + Options header of :rfc:`8200#section-4.6`, which carries those TLVs but + says nothing about their internal length arithmetic. Section 4.3 is the + Hop-by-Hop Options header, a different header from the one this class implements. Note that only the *stored-length* read-side call sites are @@ -798,10 +792,10 @@ def _read_opt_smf_dpd(self, schema: 'Schema_SMFDPDOption', *, options: 'Option') if TYPE_CHECKING: schema = cast('Schema_SMFIdentificationBasedDPDOption', schema) - # NOTE: #775 tier 1 made an unresolvable *string* key raise KeyError - # instead of minting a member; only a hand-constructed schema hits - # this guard. An unassigned *wire* value still mints via _missing_ - # (tier 2, deferred), so it never reaches here. + # NOTE: An unresolvable *string* key raises KeyError rather than + # minting a member, so only a hand-constructed schema hits this + # guard. An unassigned *wire* value still mints via _missing_ and + # never reaches here. tid_key = None try: tid_key = schema.info['type'] @@ -1032,10 +1026,10 @@ def _read_opt_mpl(self, schema: 'Schema_MPLOption', *, options: 'Option') -> 'Da ProtocolError: If the option is malformed. """ - # NOTE: #775 tier 1 made an unresolvable *string* key raise KeyError - # instead of minting a member; only a hand-constructed schema hits - # this guard. An unassigned *wire* value still mints via _missing_ - # (tier 2, deferred), so it never reaches here. + # NOTE: An unresolvable *string* key raises KeyError rather than + # minting a member, so only a hand-constructed schema hits this guard. + # An unassigned *wire* value still mints via _missing_ and never + # reaches here. seed_key = None try: seed_key = schema.flags['type'] @@ -1415,7 +1409,7 @@ def _make_opt_none(self, code: 'Enum_Option', opt: 'Optional[Data_UnassignedOpti **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if opt is not None: @@ -1440,16 +1434,16 @@ def _make_opt_pad(self, code: 'Enum_Option', opt: 'Optional[Data_PadOption]' = N **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. Note: :attr:`Data_PadOption.length ` counts the *whole* option, whereas :attr:`Schema_PadOption.len ` is the - ``Opt Data Len`` field -- two octets fewer, and absent altogether for - a ``Pad1``. ``opt`` used to be ignored here, so re-making a parsed - padding option silently collapsed it to a single ``Pad1``. + ``Opt Data Len`` field: two octets fewer, and absent altogether for a + ``Pad1``. ``opt`` is honoured so that re-making a parsed padding + option keeps its size instead of collapsing to a single ``Pad1``. """ if opt is not None: @@ -1481,7 +1475,7 @@ def _make_opt_tun(self, code: 'Enum_Option', opt: 'Optional[Data_TunnelEncapsula **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if opt is not None: @@ -1511,7 +1505,7 @@ def _make_opt_ra(self, code: 'Enum_Option', opt: 'Optional[Data_RouterAlertOptio **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if opt is not None: @@ -1544,7 +1538,7 @@ def _make_opt_calipso(self, code: 'Enum_Option', opt: 'Optional[Data_CALIPSOOpti **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if opt is not None: @@ -1588,7 +1582,7 @@ def _make_opt_smf_dpd(self, code: 'Enum_Option', opt: 'Optional[Data_SMFIdentifi **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if opt is not None: @@ -1693,7 +1687,7 @@ def _make_opt_pdm(self, code: 'Enum_Option', opt: 'Optional[Data_PDMOption]' = N **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if opt is not None: @@ -1749,7 +1743,7 @@ def _make_opt_qs(self, code: 'Enum_Option', opt: 'Optional[Data_QuickStartOption **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if opt is not None: @@ -1811,7 +1805,7 @@ def _make_opt_rpl(self, code: 'Enum_Option', opt: 'Optional[Data_RPLOption]' = N **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if opt is not None: @@ -1851,7 +1845,7 @@ def _make_opt_mpl(self, code: 'Enum_Option', opt: 'Optional[Data_MPLOption]' = N **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if opt is not None: @@ -1901,7 +1895,7 @@ def _make_opt_ilnp(self, code: 'Enum_Option', opt: 'Optional[Data_ILNPOption]' = **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if opt is not None: @@ -1909,16 +1903,15 @@ def _make_opt_ilnp(self, code: 'Enum_Option', opt: 'Optional[Data_ILNPOption]' = # NOTE: ``nonce`` is packed by a NumberField whose width is this very # ``len`` (c.f. pcapkit.protocols.schema.internet.ipv6_opts.ILNPOption), - # so the declared octet count has to be the ceiling of the bit length over - # eight -- ``bit_length() // 8`` floors instead, and wrapping a float-free - # floor division in ``math.ceil`` is a no-op, so every nonce whose bit - # length is not a multiple of eight used to be sized short and silently - # truncated on the wire (a nonce below 256 was declared as *zero* octets - # and vanished outright). ``bit_length()`` is 0 for 0 itself, which would - # likewise declare a zero-octet nonce -- collapsing "the nonce is 0" into - # "there is no nonce", when RFC 6744 gives the option a Nonce Value field - # -- so the width is floored at one octet, matching - # pcapkit.protocols.internet.mh.MH._make_opt_mn_id (c.f. #601). + # so the declared octet count must be the ceiling of the bit length over + # eight. ``bit_length() // 8`` floors, and ``math.ceil`` around an + # integer division is a no-op, so any nonce whose bit length is not a + # multiple of eight would be sized short and truncated on the wire (a + # nonce below 256 as *zero* octets). ``bit_length()`` is 0 for 0, which + # would collapse "the nonce is 0" into "there is no nonce" although + # RFC 6744 gives the option a Nonce Value field, so the width is + # floored at one octet, as in + # pcapkit.protocols.internet.mh.MH._make_opt_mn_id. return Schema_ILNPOption( type=code, len=max(1, math.ceil(nonce.bit_length() / 8)), @@ -1937,7 +1930,7 @@ def _make_opt_lio(self, code: 'Enum_Option', opt: 'Optional[Data_LineIdentificat **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if opt is not None: @@ -1962,7 +1955,7 @@ def _make_opt_jumbo(self, code: 'Enum_Option', opt: 'Optional[Data_JumboPayloadO **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if opt is not None: @@ -1986,7 +1979,7 @@ def _make_opt_home(self, code: 'Enum_Option', opt: 'Optional[Data_HomeAddressOpt **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if opt is not None: @@ -2016,7 +2009,7 @@ def _make_opt_ip_dff(self, code: 'Enum_Option', opt: 'Optional[Data_IPDFFOption] **kwargs: arbitrary keyword arguments Returns: - Constructured option schema. + Constructed option schema. """ if opt is not None: diff --git a/pcapkit/protocols/internet/ipv6_route.py b/pcapkit/protocols/internet/ipv6_route.py index c86fdb0a1..fd7396e3f 100644 --- a/pcapkit/protocols/internet/ipv6_route.py +++ b/pcapkit/protocols/internet/ipv6_route.py @@ -71,8 +71,8 @@ class IPv6_Route(IPv6_Ext[Data_IPv6_Route, Schema_IPv6_Route], schema=Schema_IPv6_Route, data=Data_IPv6_Route): """This class implements Routing Header for IPv6. - This class currently supports parsing of the following Routing Header for IPv6 - routing data types, which are registered in the + This class parses the following Routing Header for IPv6 routing data types, + which are registered in the :attr:`self.__routing__ ` attribute: @@ -220,25 +220,20 @@ def read(self, length: 'Optional[int]' = None, *, extension: 'bool' = False, # def _make_hdr_ext_len(data_length: 'int') -> 'int': """Compute ``Hdr Ext Len`` for a type-specific data payload of ``data_length`` octets. - Per :rfc:`8200#section-4.4`, ``Hdr Ext Len`` is *"the length of the - Routing header in 8-octet units, not including the first 8 - octets"*. The routing header's fixed part (``next``/``length``/ - ``type``/``seg_left``) is 4 octets, so the total on-the-wire header - is ``4 + data_length`` octets; equating that to ``8 + 8 * - hdr_ext_len`` and solving gives ``hdr_ext_len = (data_length - 4) / - 8``. ``ipv6_route_data_length`` in - :mod:`pcapkit.protocols.schema.internet.ipv6_route` is this - expression's inverse, used on the read side. - - This is the *only* place ``Hdr Ext Len`` is computed on the write - side: :meth:`make` used to compute it twice, once per branch, in two - different (and both wrong) units -- that duplication, not either - expression individually, is what let the two drift and is why there - is one helper now rather than two call sites. Do NOT "simplify" the - ``- 4`` / ``/ 8`` away: the units either side of it differ (octets - vs. 8-octet units), and dropping the offset silently reinterprets - the field, which is exactly the defect :issue:`487` fixed (compare the - ``* 8`` unit bug behind :issue:`483` in the scapy adapter). + Per :rfc:`8200#section-4.4`, ``Hdr Ext Len`` is the length of the + Routing header in 8-octet units, not including the first 8 octets. The + fixed part (``next``/``length``/``type``/``seg_left``) is 4 octets, so + the on-the-wire header is ``4 + data_length`` octets; equating that to + ``8 + 8 * hdr_ext_len`` gives ``hdr_ext_len = (data_length - 4) / 8``. + ``ipv6_route_data_length`` in + :mod:`pcapkit.protocols.schema.internet.ipv6_route` is the inverse, + used on the read side. + + This is the *only* place ``Hdr Ext Len`` is computed on the write side. + Computing it separately per branch in :meth:`make` let the copies + drift apart, so keep one helper. Do NOT "simplify" the ``- 4`` / ``/ 8`` + away: the units either side differ (octets vs. 8-octet units), and + dropping the offset silently reinterprets the field. Args: data_length: packed length, in octets, of the type-specific data @@ -300,10 +295,9 @@ def make(self, if isinstance(data, bytes): # ``data`` here *is* the type-specific data (no per-type - # constructor involved), so pad it out to the next octet count - # ``_make_hdr_ext_len`` can express exactly, then use that same - # helper -- rather than a third, separate expression -- to derive - # ``Hdr Ext Len`` from the now-aligned length. + # constructor involved), so pad it to the next octet count + # ``_make_hdr_ext_len`` can express exactly, then derive + # ``Hdr Ext Len`` from the aligned length with that same helper. length = self._make_hdr_ext_len(len(data)) data_val = data.ljust(ipv6_route_data_length(length), b'\x00') # type: bytes | Schema_RoutingType elif isinstance(data, (dict, Data_IPv6_Route)): @@ -316,10 +310,9 @@ def make(self, meth = name[1] # NOTE: Through ``parse_ip_address`` rather than - # ``ipaddress.ip_address`` directly, because ``bool`` is an - # ``int`` subclass the latter accepts without complaint. Before - # this, ``dst=True`` converted to ``::1`` with no exception at - # all (c.f. #508, #540). + # ``ipaddress.ip_address`` directly, because ``bool`` is an ``int`` + # subclass the latter accepts silently: ``dst=True`` would become + # ``::1``. dst_val = cast('IPv6Address', parse_ip_address( dst, f'{self.alias}: invalid destination address', version=6)) if dst is not None else None if isinstance(data, dict): @@ -379,6 +372,13 @@ def __post_init__(self, file: 'Optional[IO[bytes] | bytes]' = None, length: 'Opt dst_ip: destination IP address **kwargs: Arbitrary keyword arguments. + Note: + ``src_ip`` and ``dst_ip`` are declared only in the ``@overload`` + stub above, which types them as optional keyword parameters. The + runtime accepts them through ``**kwargs`` without defaulting them. + The discrepancy with the implementation signature is deliberate + (GitHub issue :issue:`505`). + See Also: For construction argument, please refer to :meth:`self.make `. @@ -512,11 +512,10 @@ def _read_data_type_src(self, schema: 'Schema_SourceRoute', *, header: 'Schema_I """ # ``header.length`` is ``Hdr Ext Len``, in 8-octet units (:rfc:`8200 - # #section-4.4`), not octets -- each 16-octet address costs 2 of - # those units, so a well-formed Source Route header always carries - # an even ``Hdr Ext Len``. The previous ``(header.length - 8) % 16`` - # check assumed ``header.length`` was already a total octet count, - # which is never true of this field; see #487. + # #section-4.4`), not octets. Each 16-octet address costs 2 of those + # units, so a well-formed Source Route header always carries an even + # ``Hdr Ext Len``; a check that assumes it is a total octet count + # rejects most well-formed headers. if header.length % 2 != 0: raise ProtocolError(f'{self.alias}: [TypeNo {header.type}] invalid format') @@ -561,10 +560,10 @@ def _read_data_type_2(self, schema: 'Schema_Type2', *, header: 'Schema_IPv6_Rout ProtocolError: If ``Hdr Ext Len`` is **NOT** ``2``. """ - # A Type 2 Routing header is fixed at 24 octets total (4 fixed + 4 - # reserved + 16-octet home address), so its ``Hdr Ext Len`` -- in - # 8-octet units, not octets (:rfc:`8200#section-4.4`) -- is always - # ``2``; a literal ``24`` here could never match. See #487. + # A Type 2 Routing header is fixed at 24 octets (4 fixed + 4 reserved + # + 16-octet home address), so its ``Hdr Ext Len``, in 8-octet units + # rather than octets (:rfc:`8200#section-4.4`), is always ``2``; a + # literal ``24`` could never match. if header.length != 2: raise ProtocolError(f'{self.alias}: [TypeNo {header.type}] invalid format') @@ -606,31 +605,24 @@ def _read_data_type_rpl(self, schema: 'Schema_RPL', *, header: 'Schema_IPv6_Rout Parsed route data. """ - # NOTE: the ``% 16`` bound that stood here had the same surface shape - # as the Source Route and Type 2 unit confusion #487 fixed above -- - # ``header.length`` is ``Hdr Ext Len``, in the 8-octet units - # :rfc:`6554#section-3` specifies, not octets -- and it additionally - # assumed 16-octet addresses, which an SRH only carries when ``CmprI`` - # and ``CmprE`` are both 0. #487 called it out but deliberately left - # it alone, because ``RPL.post_process`` raised on every pack back - # then so there was no round trip to validate a replacement against. - # #556 removed that blocker and #564 the mis-sized fixed area behind - # it, so the replacement is written here rather than guessed at. + # NOTE: ``header.length`` is ``Hdr Ext Len``, in the 8-octet units + # :rfc:`6554#section-3` specifies, not octets, and the addresses are + # not always 16 octets: an SRH carries full addresses only when + # ``CmprI`` and ``CmprE`` are both 0. A plain ``% 16`` bound on the + # length is therefore wrong on both counts. # - # :rfc:`6554#section-4.2` derives the address count from the same - # fields this reader has to hand: + # :rfc:`6554#section-4.2` derives the address count from the fields + # this reader has to hand: # # n = (((Hdr Ext Len * 8) - Pad - (16 - CmprE)) / (16 - CmprI)) + 1 # - # so the invariant actually available is that the division closes -- - # non-negative, and whole. ``16 - cmpr_i`` cannot be zero: ``CmprI`` - # is a *"4-bit unsigned integer"* per :rfc:`6554#section-3`, hence at - # most 15. Note this is a well-formedness check the library needs in - # order to walk ``Addresses[1..n]`` at all; :rfc:`6554#section-4.2` - # itself specifies no malformed-header drop condition, only - # ``Segments Left > n``, so nothing stricter is imposed here. Still - # not checked against a real RPL capture, which is the caveat the - # ``% 16`` bound carried too. + # so the invariant available is that the division closes: non-negative + # and whole. ``16 - cmpr_i`` cannot be zero, since ``CmprI`` is a + # 4-bit unsigned integer (:rfc:`6554#section-3`), at most 15. This is + # the well-formedness check the library needs to walk + # ``Addresses[1..n]`` at all; the RFC specifies no malformed-header + # drop condition beyond ``Segments Left > n``, so nothing stricter is + # imposed. Not checked against a real RPL capture. cmpr_i = schema.cmpr['cmpr_i'] cmpr_e = schema.cmpr['cmpr_e'] pad_len = schema.pad['pad_len'] @@ -761,13 +753,11 @@ def _make_data_type_rpl(self, type: 'Enum_Routing', route: 'Optional[Data_RPL]' ip = [] if ip is None else ip # NOTE: Through ``parse_ip_address`` rather than - # ``ipaddress.ip_address`` directly, because ``bool`` is an - # ``int`` subclass the latter accepts without complaint -- and - # here the laundered value would not just pack wrong, it would - # feed ``cmpr_i``/``cmpr_e`` below, corrupting the compression - # metadata alongside the address list. Before this, - # ``ip=[True]`` packed with ``cmpr_e=0`` and an address of - # ``00000001`` instead of raising (c.f. #508, #540). + # ``ipaddress.ip_address`` directly, because ``bool`` is an ``int`` + # subclass the latter accepts silently. Here the value would not + # just pack wrong but also feed ``cmpr_i``/``cmpr_e`` below, + # corrupting the compression metadata: ``ip=[True]`` would pack an + # address of ``00000001`` with ``cmpr_e=0``. descr = f'{self.alias}: invalid RPL source address' if dst is None: @@ -795,15 +785,13 @@ def _make_data_type_rpl(self, type: 'Enum_Routing', route: 'Optional[Data_RPL]' prefix_e = os_path.commonprefix(test_list) cmpr_e = len(prefix_e) - # NOTE: the outer ``% 8`` is what keeps a vector that is - # already 8-octet aligned from being handed a *full* 8 octets - # of padding -- ``8 - 0`` is 8, not 0. That is reachable - # whenever ``dst`` shares no prefix with the addresses, which - # makes ``cmpr_i`` and ``cmpr_e`` both 0 and the vector a - # multiple of 16, and it contradicts :rfc:`6554#section-3`: - # *"Note that when CmprI and CmprE are both 0, Pad MUST carry - # a value of 0."* ``_make_data_type_none`` above already - # spells the idiom this way; this branch did not. + # NOTE: the outer ``% 8`` keeps an already 8-octet-aligned + # vector from being handed a *full* 8 octets of padding + # (``8 - 0`` is 8, not 0). That is reachable whenever ``dst`` + # shares no prefix with the addresses, making ``cmpr_i`` and + # ``cmpr_e`` both 0 and the vector a multiple of 16, and it + # contradicts :rfc:`6554#section-3`: when CmprI and CmprE are + # both 0, Pad MUST be 0. Same idiom as ``_make_data_type_none``. pad = (8 - ((len(ip) - 1) * (16 - cmpr_i) + (16 - cmpr_e)) % 8) % 8 ip_val = [] diff --git a/pcapkit/protocols/internet/ipx.py b/pcapkit/protocols/internet/ipx.py index 29b2182a2..0be03af49 100644 --- a/pcapkit/protocols/internet/ipx.py +++ b/pcapkit/protocols/internet/ipx.py @@ -13,7 +13,7 @@ ======= ========= ====================== ===================================== Octets Bits Name Description ======= ========= ====================== ===================================== - 0 0 ``ipx.cksum`` Checksum + 0 0 ``ipx.chksum`` Checksum 2 16 ``ipx.len`` Packet Length (header includes) 4 32 ``ipx.count`` Transport Control (hop count) 5 40 ``ipx.type`` Packet Type @@ -137,7 +137,7 @@ def make(self, **kwargs: Arbitrary keyword arguments. Returns: - bytes: Constructed packet data. + Constructed packet data. """ type_val = self._make_index(type, type_default, namespace=type_namespace,