From c8e3cd875e715ac8b3f1adbb3fff86918d306c37 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Mon, 5 Oct 2026 14:37:07 -0400 Subject: [PATCH] docs(protocols): tighten the schema prose and cut timed context (#719) - schema.py: drop issue/PR history from the registrar, finaliser and packing notes; keep the rationale for each guard. State that EnumSchema.registry answers on an instance and EnumMeta.registry on the class. - transport/tcp.py: rewrite the DSS Data ACK/DSN selector notes around the SwitchField-versus-ConditionalField reason that still holds; cut the PortEnumField, MPTCP FastClose and MP_CAPABLE change narratives. - internet/{ipv4,ipv6_route,ipv6_opts,hopopt,hip,mh}.py and misc/{pcapng,pcap/frame}.py: cut "used to"/"until #NNN" prose and measured-before-the-fix figures; HIP padding, Quick-Start lengths and MPTCP widths re-checked against the declared fields. - application/httpv2.py: fix the HEADERS Flags docstring, which named DATA frames. - misc/pcapng.py: fix the "improt" typo in the Option example. Docstrings and comments only; AST identical to main once docstrings are stripped. Sphinx -n warnings for these files unchanged. --- .../protocols/schema/application/httpv2.py | 30 +- pcapkit/protocols/schema/internet/hip.py | 133 +++----- pcapkit/protocols/schema/internet/hopopt.py | 9 +- pcapkit/protocols/schema/internet/ipv4.py | 87 ++--- .../protocols/schema/internet/ipv6_opts.py | 9 +- .../protocols/schema/internet/ipv6_route.py | 53 ++- pcapkit/protocols/schema/internet/mh.py | 8 +- pcapkit/protocols/schema/misc/pcap/frame.py | 11 +- pcapkit/protocols/schema/misc/pcapng.py | 316 ++++++++---------- pcapkit/protocols/schema/schema.py | 273 +++++++-------- pcapkit/protocols/schema/transport/sctp.py | 3 +- pcapkit/protocols/schema/transport/tcp.py | 145 +++----- pcapkit/protocols/schema/transport/udp.py | 3 +- 13 files changed, 422 insertions(+), 658 deletions(-) diff --git a/pcapkit/protocols/schema/application/httpv2.py b/pcapkit/protocols/schema/application/httpv2.py index f0358c3043..9fb6ae17f1 100644 --- a/pcapkit/protocols/schema/application/httpv2.py +++ b/pcapkit/protocols/schema/application/httpv2.py @@ -127,14 +127,11 @@ class FrameType(EnumSchema[Enum_Frame]): class Flags(EnumLookup, enum.IntFlag): """Flags enumeration for HTTP/2 frames. - Re-parented onto :class:`~pcapkit.corekit.enum.EnumLookup` per - GitHub issue :issue:`877`'s ruling that every non-registry enumeration - shares that lookup contract. The six concrete per-frame subclasses - below each declare ``class Flags(FrameType.Flags):`` with no base - list of their own, so they inherit :class:`EnumLookup` transitively - through this one re-parent rather than needing it repeated -- - verified at runtime for GitHub issue :issue:`877` (see the session report), - not merely assumed from the MRO rules. + Built on :class:`~pcapkit.corekit.enum.EnumLookup`, the lookup contract + shared by every non-registry enumeration. The six concrete per-frame + subclasses below each declare ``class Flags(FrameType.Flags):`` with no + base list of their own, so they inherit :class:`EnumLookup` + transitively rather than needing it repeated. """ @@ -155,7 +152,7 @@ def post_process(self, packet: 'dict[str, Any]') -> 'Schema': # ``0`` only as a side effect, so without this a flags octet of ``0x00`` # -- routine in HTTP/2, not an edge case -- leaves an ``int`` behind and # a membership test against it raises :exc:`TypeError` rather than - # answering. This mirrors the construct path, which already seeds + # answering. This mirrors the construct path, which seeds # ``Flags(0)`` at every one of its six sites. # # ``FrameType.Flags`` itself declares no members, and from Python 3.11 @@ -213,12 +210,12 @@ class Flags(FrameType.Flags): # -- "read no payload at all" -- for every frame without the ``PADDED`` # flag. ``__length__`` is the *remaining* declared length at this field, so # the unpadded arm wants ``__length__`` itself, which is what subtracting a - # zero padding length gives. Padding is rare in HTTP/2, so the broken arm - # was the common one: an unpadded ``DATA`` frame parsed with ``data`` as - # ``b''`` and nothing raised or warned. The parentheses also keep + # zero padding length gives. Padding is rare in HTTP/2, so that arm would be + # the common one: an unpadded ``DATA`` frame would parse with ``data`` as + # ``b''`` and nothing would raise or warn. The parentheses also keep # ``pkt['pad_len']`` from being read at all when ``PADDED`` is clear, where # the :class:`~pcapkit.corekit.fields.misc.ConditionalField` above has left - # it as :data:`~pcapkit.corekit.fields.field.NO_VALUE`. See #668. + # it as :data:`~pcapkit.corekit.fields.field.NO_VALUE`. data: 'bytes' = BytesField(length=lambda pkt: pkt['__length__'] - ( pkt['pad_len'] if pkt['flags']['bit_3'] else 0 )) @@ -237,7 +234,7 @@ class HeadersFrame(FrameType, code=Enum_Frame.HEADERS): """Header schema for HTTP/2 ``HEADERS`` frames.""" class Flags(FrameType.Flags): - """Flags enumeration for HTTP/2 ``DATA`` frames.""" + """Flags enumeration for HTTP/2 ``HEADERS`` frames.""" END_STREAM = BIT_0 = 0x1 END_HEADERS = BIT_2 = 0x4 @@ -267,8 +264,7 @@ class Flags(FrameType.Flags): # NOTE: the conditional subtracts the padding length rather than replacing # the whole expression -- see :attr:`DataFrame.data` for why the outer # parentheses are load-bearing. Without them an unpadded ``HEADERS`` frame - # read its header block fragment as ``b''``, which is exactly what HPACK - # decoding would need. See #668. + # would read its header block fragment as ``b''``. fragment: 'bytes' = BytesField(length=lambda pkt: pkt['__length__'] - ( pkt['pad_len'] if pkt['flags']['bit_3'] else 0 )) @@ -365,7 +361,7 @@ class Flags(FrameType.Flags): # NOTE: the conditional subtracts the padding length rather than replacing # the whole expression -- see :attr:`DataFrame.data` for why the outer # parentheses are load-bearing. Without them an unpadded ``PUSH_PROMISE`` - # frame read its header block fragment as ``b''``. See #668. + # frame would read its header block fragment as ``b''``. fragment: 'bytes' = BytesField(length=lambda pkt: pkt['__length__'] - ( pkt['pad_len'] if pkt['flags']['bit_3'] else 0 )) diff --git a/pcapkit/protocols/schema/internet/hip.py b/pcapkit/protocols/schema/internet/hip.py index 5e52a2fe1e..3b78b6ec6a 100644 --- a/pcapkit/protocols/schema/internet/hip.py +++ b/pcapkit/protocols/schema/internet/hip.py @@ -326,27 +326,6 @@ def encrypted_data_len(pkt: 'dict[str, Any]') -> 'int': :meth:`~pcapkit.protocols.internet.hip.HIP._make_param_encrypted` writes ``len=4 + len(iv) + len(data)`` to match. - This subtracted the ``iv`` but not the ``reserved``, so the field claimed - four octets more than the parameter holds: on unpack it read four octets - of the *next* parameter into ``data``, and on pack it zero-extended the - ciphertext by four. That was recorded as the second half of - ``hip-parameter/ENCRYPTED`` in the round-trip suite's expected-failure - table, and left alone -- because while the padding rule was also four - octets out (:issue:`651`), the two errors cancelled at some residues of ``Length`` - and not others. Measured by packing through the public maker at every - residue, the old record total agreed with :rfc:`7401` Section 5.2.1 at - ``Length % 8`` in ``{0, 5, 6, 7}`` and was eight octets over at - ``{1, 2, 3, 4}`` -- so ``Length = 8``, which the unit suite happened to - use, is one of the four where the module emitted RFC-conformant - ``ENCRYPTED`` octets while getting both halves wrong. - - Fixing the padding without fixing this would therefore have *regressed* - ``ENCRYPTED``, from right-by-accident at four of the eight residues to four - octets too long at all eight: with only the padding corrected the total - becomes ``8 + Length + pad`` against a correct ``4 + Length + pad``, which - is a uniform four-octet surplus with no residue left where it cancels. So - the two go together. - Args: pkt: Parameter unpacked schema. @@ -387,16 +366,15 @@ def parameter_total_len(length: 'int') -> 'int': is "Length of the Contents, in bytes, excluding Type, Length, and Padding", and it is the **total** -- contents plus the four octets of ``Type`` and ``Length`` plus padding -- that must land on a multiple of - eight. Aligning the contents alone instead, as every padding site in this - module and in :mod:`pcapkit.protocols.internet.hip` did before :issue:`651`, puts - every parameter at ``4 (mod 8)`` for every possible ``Length``: never a - multiple of eight and never the length the RFC gives. It is not even a - consistent offset, because the two formulas disagree in both directions -- - at ``Length = 4`` (a whole ``SEQ``) the contents are already 8-aligned - with the header, so the RFC requires no padding at all and aligning the - contents appends four octets that must not be there; at ``Length = 8`` the - contents are 8-aligned on their own, so aligning them appends nothing and - the record is left four octets short. + eight. Aligning the contents alone would put every parameter at + ``4 (mod 8)`` for every possible ``Length``: never a multiple of eight and + never the length the RFC gives. It is not even a consistent offset, + because the two formulas disagree in both directions -- at ``Length = 4`` + (a whole ``SEQ``) the contents are already 8-aligned with the header, so + the RFC requires no padding at all and aligning the contents would append + four octets that must not be there; at ``Length = 8`` the contents are + 8-aligned on their own, so aligning them would append nothing and leave + the record four octets short. Args: length: The parameter's ``Length`` field, i.e. its contents length in @@ -431,11 +409,7 @@ def parameter_padding_len(pkt: 'dict[str, Any]') -> 'int': "0-7 bytes". This is one function shared by every parameter rather than a lambda - repeated per class because it was previously the latter -- 46 copies of - the same wrong expression here and 49 of its counterpart in - :mod:`pcapkit.protocols.internet.hip`, which is 95 places for the - arithmetic to be wrong in and one place too few to state the RFC's reason - for it. + repeated per class, so the RFC's arithmetic is stated in one place. Args: pkt: Parameter unpacked schema. @@ -463,9 +437,8 @@ def parameter_padding_len(pkt: 'dict[str, Any]') -> 'int': #: ``packet.update(self.__dict__)``, and :class:`Locator` declares a ``len`` of #: its own. So by the time ``padding`` is evaluated -- after the list, since #: fields are packed in declaration order -- ``pkt['len']`` is the *last -#: locator's* ``len``, not the parameter's. That is 4 for any IPv6 locator -#: whatever the locator count, which is how the pre-:issue:`679` padding expression came -#: to append exactly four octets to every ``LOCATOR_SET`` regardless of size. +#: locator's* ``len``, not the parameter's -- 4 for any IPv6 locator, +#: whatever the locator count. #: #: The shadowing is specific to the packing path. :meth:`Schema.unpack #: ` hands each field @@ -572,17 +545,15 @@ class R1CounterParameter(Parameter, code=[Enum_Parameter.R1_Counter, layout to code 128 (``R1_Counter``) and code 129 (``R1_COUNTER``) -- one parameter under two numbers, the difference being HIP's own C-bit rather than an unrelated code -- and the field list below is that layout - exactly, since :issue:`672` widened :attr:`counter` to eight octets. - :attr:`~pcapkit.protocols.internet.hip.HIP.__parameter__` already carries + exactly. :attr:`~pcapkit.protocols.internet.hip.HIP.__parameter__` carries two hand-written entries -- not a name-normalisation rule; ``R1_Counter`` - and ``R1_COUNTER`` differ only in case, and each needed its own line -- + and ``R1_COUNTER`` differ only in case, and each needs its own line -- mapping both codes to the same ``_read_param_r1_counter``, and - ``_make_param_r1_counter`` already built this class for 128; it was only - the schema lookup this class's own ``code=`` feeds -- consulted on the - parse path by :class:`~pcapkit.corekit.fields.collections.OptionField` - -- that fell back to :class:`UnassignedParameter` for 128, since a - single-code ``code=`` registered 129 alone and left 128 unclaimed. - See :issue:`690`. + ``_make_param_r1_counter`` builds this class for 128. The schema lookup + this class's own ``code=`` feeds -- consulted on the parse path by + :class:`~pcapkit.corekit.fields.collections.OptionField` -- would fall + back to :class:`UnassignedParameter` for 128 if a single-code ``code=`` + registered 129 alone and left 128 unclaimed. """ @@ -597,12 +568,8 @@ class R1CounterParameter(Parameter, code=[Enum_Parameter.R1_Counter, #: :rfc:`5201#section-5.2.3` gives the same 4 + 8 layout, so there is no #: version under which four octets is right, and both codes that reach this #: class -- ``R1_Counter`` (128, HIPv1) and ``R1_COUNTER`` (129) -- are - #: affected. It was a :class:`~pcapkit.corekit.fields.numbers.UInt32Field` - #: until :issue:`672`: the parameter declared the correct ``len=12`` and packed 12 - #: octets in total where :rfc:`7401` Section 5.2.1's arithmetic makes the - #: record 16, leaving it four short at ``4 (mod 8)``. Measured before the - #: fix, at ``counter=1``: ``00 80 00 0c 00 00 00 00 00 00 00 01`` for code - #: 128 and the same twelve octets under ``00 81`` for code 129. + #: affected. With the four ``reserved`` octets the contents are 12 octets, + #: which :rfc:`7401` Section 5.2.1's arithmetic pads to a 16-octet record. counter: 'int' = UInt64Field() #: Padding. padding: 'bytes' = PaddingField(length=parameter_padding_len) @@ -648,24 +615,21 @@ class LocatorSetParameter(Parameter, code=Enum_Parameter.LOCATOR_SET): #: The ``length`` callback is the parameter's own ``Length``, and that is #: correct on the unpacking path for the reason :data:`LOCATOR_SET_LEN` #: gives -- nothing has shadowed ``len`` yet when this field is resolved, - #: since ``type`` and ``len`` are the only fields ahead of it. What was - #: wrong until :issue:`679` is the *quantity* it was being handed: - #: :meth:`~pcapkit.protocols.internet.hip.HIP._make_param_locator_set` wrote - #: ``Length`` as ``sum(Locator.len)``, in the 4-octet units - #: :rfc:`8046#section-4` gives ``Locator Length``, where :rfc:`7401` - #: Section 5.2.1's ``Length`` is a byte count. :meth:`Schema.unpack + #: since ``type`` and ``len`` are the only fields ahead of it. What matters + #: is the *quantity* it is handed: :rfc:`7401` Section 5.2.1's ``Length`` is + #: a byte count, not the 4-octet units :rfc:`8046#section-4` gives + #: ``Locator Length``, and + #: :meth:`~pcapkit.protocols.internet.hip.HIP._make_param_locator_set` + #: writes it as one. :meth:`Schema.unpack #: ` reads exactly #: ``field.length`` octets off the stream and hands only those to #: :meth:`ListField.unpack - #: `, so a set of *n* - #: plain IPv6 locators offered ``4n`` octets of a ``24n``-octet contents: - #: measured on ``f0999858e``, n = 2 and n = 5 both parsed **one** truncated - #: locator and left the rest of the record unconsumed, with - #: :exc:`~pcapkit.utilities.warnings.SchemaWarning` for the negative - #: remainder and a repack that did not match the octets read. With - #: ``Length`` a byte count the budget is the contents, each locator bills - #: the 8 + ``Locator Length`` * 4 octets it actually consumed, and the - #: count comes out exact. + #: `, so a ``Length`` + #: in locator units would offer a set of *n* plain IPv6 locators ``4n`` + #: octets of a ``24n``-octet contents, parse one truncated locator and leave + #: the rest of the record unconsumed. With ``Length`` a byte count the + #: budget is the contents, and each locator bills the 8 + ``Locator + #: Length`` * 4 octets it actually consumed. locators: 'list[Locator]' = ListField( length=lambda pkt: pkt['len'], item_type=SchemaField(schema=Locator), @@ -681,27 +645,9 @@ class LocatorSetParameter(Parameter, code=Enum_Parameter.LOCATOR_SET): #: parameter's packet context while they pack, and their own ``len`` #: overwrites it before ``padding`` is reached. #: - #: This is the site :issue:`651` deliberately left alone, and its fix - #: documented as an exclusion, because two defects in this parameter - #: cancelled at the shape its tests sampled and correcting either alone - #: made the wire output worse. - #: The shadowed ``len`` is 4 for any IPv6 locator, so the old expression - #: appended exactly four octets whatever the locator count; and the wrong - #: ``Length`` unit above made the declared ``Length`` ``4n`` where the - #: contents were ``24n``. ``4 + 24n + 4`` is ``24n + 8``, and since ``24n`` - #: is a multiple of eight the RFC total for a byte-count ``Length`` of - #: ``24n`` is ``11 + 24n - 3``, the same ``24n + 8``. Hence 32, 56 and 128 - #: octets at n = 1, 2, 5 -- conformant, by two wrongs. - #: - #: That cancellation was never general, which is why the pair had to move - #: together rather than one at a time. It needs every locator to be 24 - #: octets and there to be at least one, and measured on ``f0999858e`` the - #: shapes that break it were non-conformant before this fix: an empty set - #: packed 4 octets where :rfc:`7401` Section 5.2.1 wants 8, one SPI-bearing - #: locator packed 35, two packed 63, and a mixed plain-and-SPI pair packed - #: 59 or 60 depending on order. Afterwards all seven shapes are the RFC's - #: own total: 8, 32, 56, 128, 32, 64, 56 and 56 respectively, with the three - #: plain figures unchanged. + #: The shadowed ``len`` is 4 for any IPv6 locator, so a padding read off + #: ``pkt['len']`` directly would append exactly four octets whatever the + #: locator count. padding: 'bytes' = PaddingField(length=locator_set_padding_len) if TYPE_CHECKING: @@ -751,7 +697,7 @@ class SolutionParameter(Parameter, code=Enum_Parameter.SOLUTION): #: (:rfc:`7401#section-5.2.5`, and :rfc:`5201#section-5.2.5` identically). #: This octet is *not* a lifetime: only ``PUZZLE`` carries one, at the same #: offset, and only :rfc:`7401#section-5.2.4` defines the ``2^(value - 32)`` - #: seconds encoding that goes in it. See :issue:`654`. + #: seconds encoding that goes in it. reserved: 'int' = UInt8Field() #: Opaque data. opaque: 'bytes' = BytesField(length=2) @@ -924,8 +870,7 @@ def pre_unpack(cls, packet: 'dict[str, Any]') -> 'None': sets it as a plain attribute so ``pack()``'s own ``packet.update(self.__dict__)`` carries it in here -- that value is trusted over the ``HIP_CIPHER`` sibling lookup below, which a - parameter packed on its own has no ``options`` list for. See - :issue:`556`. + parameter packed on its own has no ``options`` list for. """ if 'cipher' in packet: @@ -984,7 +929,7 @@ def post_process(self, packet: 'dict[str, Any]') -> 'Schema': #: :meth:`HIP._make_param_encrypted #: ` before #: packing -- so it is documented here rather than accepted by - #: ``__init__``. See :issue:`556`. + #: ``__init__``. cipher: 'Enum_Cipher' def __init__(self, type: 'Enum_Parameter', len: 'int', diff --git a/pcapkit/protocols/schema/internet/hopopt.py b/pcapkit/protocols/schema/internet/hopopt.py index 7d5753fc30..16832867fe 100644 --- a/pcapkit/protocols/schema/internet/hopopt.py +++ b/pcapkit/protocols/schema/internet/hopopt.py @@ -181,10 +181,10 @@ def smf_dpd_data_selector(pkt: 'dict[str, Any]') -> 'Field': ``Opt Data Len`` counts only what follows the option header [:rfc:`8200#section-4.2`], while both schemas this may return inherit :attr:`Option.type` and :attr:`Option.len` and so parse those two octets - themselves. Sizing the field at ``Opt Data Len`` handed them an area two + themselves. Sizing the field at ``Opt Data Len`` would hand them an area two octets short of the option they read, which - :class:`~pcapkit.corekit.fields.collections.OptionField` then mis-counted - against the option area -- c.f. :issue:`431`. + :class:`~pcapkit.corekit.fields.collections.OptionField` would then + mis-count against the option area. """ mode = Enum_SMFDPDMode.get(pkt['test']['mode']) @@ -278,8 +278,7 @@ def pad_opt_data_len(pkt: 'dict[str, Any]') -> 'int': :obj:`~pcapkit.corekit.fields.field.NoValueType` straight to :class:`~pcapkit.corekit.fields.strings.PaddingField`, where it becomes an unusable :mod:`struct` template and surfaces much later as an opaque - :exc:`struct.error` -- which is exactly how a ``Pad1`` option used to - fail to parse. + :exc:`struct.error`, which is how a ``Pad1`` option would fail to parse. """ length = pkt.get('len', 0) diff --git a/pcapkit/protocols/schema/internet/ipv4.py b/pcapkit/protocols/schema/internet/ipv4.py index 59af1f6500..047726ca96 100644 --- a/pcapkit/protocols/schema/internet/ipv4.py +++ b/pcapkit/protocols/schema/internet/ipv4.py @@ -121,14 +121,11 @@ def quick_start_option_length(schema: 'Type[QSOption]') -> 'int': value in the ``length`` field outright. It is summed from the resolved schema's own fields rather than written as - that literal, for two reasons. The registry is open -- - :class:`QSOption` is an :class:`~pcapkit.protocols.schema.schema.EnumSchema`, - so a caller may register a further function code with a schema of its own - width -- and a number written here has to be kept in step by hand with every - field the suboptions declare, which is precisely how :issue:`552` arose: the length - was ``5``, the width of a Quick-Start Request's ``ttl`` and ``nonce`` alone, - with the ``type``, ``length`` and ``flags`` octets in front of them - unaccounted for. + that literal, because the registry is open -- :class:`QSOption` is an + :class:`~pcapkit.protocols.schema.schema.EnumSchema`, so a caller may + register a further function code with a schema of its own width -- and a + number written here would have to be kept in step by hand with every field + the suboptions declare. Args: schema: Quick-Start suboption schema, as resolved from the ``func`` @@ -143,8 +140,7 @@ def quick_start_option_length(schema: 'Type[QSOption]') -> 'int': because the option is, and a variable-width one cannot be summed here without a packet to size it against -- which is the one thing a selector does not have for the schema it is about to return. It - fails rather than guessing, since guessing is the defect being - fixed. + fails rather than guessing a width. """ length = 0 @@ -152,8 +148,7 @@ def quick_start_option_length(schema: 'Type[QSOption]') -> 'int': # NOTE: A forward match consumes nothing and contributes no octets to # ``bytes(schema)`` either, c.f. the ``ForwardMatchField`` branches of # :meth:`Schema.pack ` and - # :meth:`Schema.unpack ` - # and the double-count #441/#446 fixed. + # :meth:`Schema.unpack `. if isinstance(field, ForwardMatchField): continue @@ -203,14 +198,9 @@ def quick_start_data_selector(pkt: 'dict[str, Any]') -> 'Field': Notes: The length handed to the :class:`~pcapkit.corekit.fields.misc.SchemaField` comes from :func:`quick_start_option_length`, i.e. from the suboption that - was just resolved. It used to be the literal ``5`` for both, which is not - the width of either: a well-formed eight-octet Quick-Start Request - ``1908002adeadbee0`` parsed with ``SchemaWarning: packet length < 0: -3`` - and decoded its ``nonce`` as **55** instead of 933982136, then left three - octets to be read as a further, fabricated option -- which made the - enclosing datagram fail with ``ProtocolError: IPv4: invalid format``. That - is silent corruption on the way to a misleading failure, and it was logged - in review twice before :issue:`552` filed it. + was just resolved, rather than from a fixed literal: a wrong width + mis-sizes the nested read, so the ``nonce`` decodes wrongly and the + leftover octets are read as a further, fabricated option. """ func = Enum_QSFunction.get(pkt['flags']['func']) @@ -365,12 +355,9 @@ def post_process(self, packet: 'dict[str, Any]') -> 'Schema': caller-built option schema and packs it, and :attr:`ts_data`'s :class:`~pcapkit.corekit.fields.numbers.UInt32Field` item type takes a :obj:`bool` as the :class:`int` it is a subclass - of, so nothing downstream can question it. Measured before this - fix: ``ts_data=[True, 5]`` packed as ``0000000100000005`` and - reported ``IPv4Address('0.0.0.1')`` with no exception and no - warning. This was the fifth site of that defect -- :issue:`469`, :issue:`491`, - :issue:`508` and :issue:`540` are the first four -- and the reason it is the fifth - is that each of those fixed the sites it could see. See :issue:`552`. + of, so nothing downstream can question it: ``ts_data=[True, 5]`` + would otherwise pack as ``0000000100000005`` and report + ``IPv4Address('0.0.0.1')`` with no exception and no warning. """ ts_flag = Enum_TSFlag.get(self.flags['flag']) @@ -426,15 +413,15 @@ def post_process(self, packet: 'dict[str, Any]') -> 'Schema': # but set the timestamp to 0 # # NOTE: Through ``parse_ip_address`` like the two conversions above, - # even though #540 and #552 both judged this site unable to launder a - # :obj:`bool` -- ``remainder`` is a + # even though ``remainder`` is a # :class:`~pcapkit.corekit.fields.strings.PaddingField`, so what it - # holds is octets rather than anything the caller named. It is routed - # through anyway because a bare :func:`ipaddress.ip_address` here - # still raises a plain :exc:`ValueError` for a tail that is not a - # whole number of 8-octet pairs, which no ``except BaseError`` can - # catch, and because leaving one of this method's three conversions - # unguarded is exactly how #552 came to be the fifth site of #469. + # holds is octets rather than anything the caller named, and cannot + # launder a :obj:`bool`. It is routed through anyway because a bare + # :func:`ipaddress.ip_address` here still raises a plain + # :exc:`ValueError` for a tail that is not a whole number of 8-octet + # pairs, which no ``except BaseError`` can catch, and because leaving + # one of this method's three conversions unguarded invites the same + # defect back. pad = self.remainder for index in range(0, len(pad), 8): buf_ip = pad[index:index + 4] @@ -455,16 +442,15 @@ def post_process(self, packet: 'dict[str, Any]') -> 'Schema': data: 'list[int] | OrderedMultiDict[IPv4Address, int]' timestamp: 'tuple[int | timedelta] | OrderedMultiDict[IPv4Address, int | timedelta]' - # NOTE: The keyword is ``ts_data``, the name of the field above, not the - # ``data`` this signature used to advertise. ``data`` is the *derived* - # attribute :meth:`post_process` writes and is declared three lines up; - # naming it here as a constructor argument too is what - # :meth:`~pcapkit.protocols.internet.ipv4.IPv4._make_opt_ts` was written - # against, and because :meth:`Schema.__update__ + # NOTE: The keyword is ``ts_data``, the name of the field above, not + # ``data``. ``data`` is the *derived* attribute :meth:`post_process` + # writes and is declared three lines up; passing it as a constructor + # argument would drop the timestamps in silence, because + # :meth:`Schema.__update__ # ` answers an unknown # field name with an # :class:`~pcapkit.utilities.warnings.UnknownFieldWarning` rather than an - # error, the timestamps were dropped in silence. See #552. + # error. def __init__(self, type: 'Enum_OptionNumber', length: 'int', pointer: 'int', flags: 'TSFlags', ts_data: 'list[int]') -> 'None': ... @@ -511,12 +497,7 @@ class SIDOption(Option, code=Enum_OptionNumber.SID): #: Stream identifier. Two octets, per :rfc:`791` section 3.1, which gives the #: option as four octets in total: one of type, one of length, and a 16-bit - #: stream identifier. This was a :class:`~pcapkit.corekit.fields.numbers.UInt32Field`, - #: which over-read a well-formed option by two octets on the way in -- the - #: ``packet length < 0: -2`` the library warned about -- and re-emitted it two - #: octets too wide on the way out, against the ``length=4`` that - #: :meth:`~pcapkit.protocols.internet.ipv4.IPv4._make_opt_sid` had always - #: written. See :issue:`534`. + #: stream identifier. sid: 'int' = UInt16Field() if TYPE_CHECKING: @@ -667,18 +648,14 @@ class QuickStartReportOption(QSOption, code=Enum_QSFunction.Report_of_Approved_R #: used"*, and that *"bytes 5-8 contain a 30-bit QS Nonce and a 2-bit #: Reserved field"* -- so the nonce begins at the fifth octet for both #: functions, and figure 4 gives this option as ``Length=8`` like figure 3 - #: gives its sibling. The field was missing, so the schema was seven octets + #: gives its sibling. Without this field the schema would be seven octets #: wide against the eight #: :meth:`~pcapkit.protocols.internet.ipv4.IPv4._make_opt_qs` writes into #: ``length`` and the eight #: :meth:`~pcapkit.protocols.internet.ipv4.IPv4._read_opt_qs` demands of it, - #: which meant a spec-correct Report of Approved Rate read off the wire - #: decoded its ``nonce`` one octet early -- measured, with the selector - #: length fixed and this field still absent: ``19088100deadbee0`` warned - #: ``packet length < 0: -1`` and then died with a bare ``struct.error: bad - #: char in struct format``, the unconsumed octet having been read as another - #: option. Declared as padding rather than as data because :rfc:`4782` gives - #: it no meaning and no caller should be setting it. See :issue:`552`. + #: and the ``nonce`` would decode one octet early. Declared as padding rather + #: than as data because :rfc:`4782` gives it no meaning and no caller should + #: be setting it. reserved: 'bytes' = PaddingField(length=1) #: QS nonce. nonce: 'QSNonce' = BitField(length=4, namespace={ diff --git a/pcapkit/protocols/schema/internet/ipv6_opts.py b/pcapkit/protocols/schema/internet/ipv6_opts.py index bdf086f708..6ae3e68bbb 100644 --- a/pcapkit/protocols/schema/internet/ipv6_opts.py +++ b/pcapkit/protocols/schema/internet/ipv6_opts.py @@ -181,10 +181,10 @@ def smf_dpd_data_selector(pkt: 'dict[str, Any]') -> 'Field': ``Opt Data Len`` counts only what follows the option header [:rfc:`8200#section-4.2`], while both schemas this may return inherit :attr:`Option.type` and :attr:`Option.len` and so parse those two octets - themselves. Sizing the field at ``Opt Data Len`` handed them an area two + themselves. Sizing the field at ``Opt Data Len`` would hand them an area two octets short of the option they read, which - :class:`~pcapkit.corekit.fields.collections.OptionField` then mis-counted - against the option area -- c.f. :issue:`431`. + :class:`~pcapkit.corekit.fields.collections.OptionField` would then + mis-count against the option area. """ mode = Enum_SMFDPDMode.get(pkt['test']['mode']) @@ -278,8 +278,7 @@ def pad_opt_data_len(pkt: 'dict[str, Any]') -> 'int': :obj:`~pcapkit.corekit.fields.field.NoValueType` straight to :class:`~pcapkit.corekit.fields.strings.PaddingField`, where it becomes an unusable :mod:`struct` template and surfaces much later as an opaque - :exc:`struct.error` -- which is exactly how a ``Pad1`` option used to - fail to parse. + :exc:`struct.error`, which is how a ``Pad1`` option would fail to parse. """ length = pkt.get('len', 0) diff --git a/pcapkit/protocols/schema/internet/ipv6_route.py b/pcapkit/protocols/schema/internet/ipv6_route.py index 770392e7f9..ad4020410f 100644 --- a/pcapkit/protocols/schema/internet/ipv6_route.py +++ b/pcapkit/protocols/schema/internet/ipv6_route.py @@ -56,8 +56,8 @@ def ipv6_route_data_length(hdr_ext_len: 'int') -> 'int': schema -- is ``4 + 8 * hdr_ext_len`` octets. This is the single place that arithmetic is done on the read side; see :meth:`~pcapkit.protocols.internet.ipv6_route.IPv6_Route._make_hdr_ext_len` - for its inverse on the write side. Do NOT drop the ``4 +``: that turns - the field back into raw octets and is the exact defect :issue:`487` fixed. + for its inverse on the write side. Dropping the ``4 +`` would leave the data + length 4 octets short. Args: hdr_ext_len: raw ``Hdr Ext Len`` field value, as read off the wire. @@ -81,10 +81,9 @@ def ipv6_route_header_length(hdr_ext_len: 'int') -> 'int': :mod:`pcapkit.protocols.internet.ipv6_route` reports back as the parsed route data's own ``.length``, which :meth:`~pcapkit.protocols.internet. ipv6_route.IPv6_Route.read` then subtracts from the outer packet length - to find the next layer's length. :issue:`487` unified the write side - (:meth:`~pcapkit.protocols.internet.ipv6_route.IPv6_Route._make_hdr_ext_len`) - into one helper; this is the matching read-side helper for the total - header length, finishing that half of :issue:`487`. + to find the next layer's length. It is the read-side counterpart of + :meth:`~pcapkit.protocols.internet.ipv6_route.IPv6_Route._make_hdr_ext_len`, + which does the inverse arithmetic on the write side. Args: hdr_ext_len: raw ``Hdr Ext Len`` field value, as read off the wire. @@ -192,14 +191,13 @@ class RPL(RoutingType, code=Enum_Routing.RPL_Source_Route_Header): #: NOTE: :rfc:`6554#section-3` gives ``CmprI`` and ``CmprE`` as *"4-bit #: unsigned integer"*, i.e. the high and low nibble of a single octet, so #: they cannot be two :class:`~pcapkit.corekit.fields.numbers.UInt8Field` - #: as they were before :issue:`564`. Together with :attr:`pad` below -- ``Pad`` + #: (one octet each). Together with :attr:`pad` below -- ``Pad`` #: (4 bits) plus ``Reserved`` (20 bits) -- this is the one 32-bit word the #: diagram in :rfc:`6554#section-3` draws, and the same word #: :meth:`IPv6_Route._read_data_type_rpl #: `'s - #: own docstring already drew correctly. The split between the two fields - #: falls on the octet boundary between ``CmprE`` and ``Pad``, so neither - #: straddles an octet. + #: own docstring draws. The split between the two fields falls on the octet + #: boundary between ``CmprE`` and ``Pad``, so neither straddles an octet. cmpr: 'CmprInfo' = BitField(length=1, namespace={ 'cmpr_i': (0, 4), 'cmpr_e': (4, 4), @@ -238,9 +236,7 @@ def post_process(self, packet: 'dict[str, Any]') -> 'Schema': # :class:`~pcapkit.corekit.fields.collections.ListField` handling # packs that list directly. The SRH prefix-decompression below # only makes sense against the raw octets a real parse hands - # here -- treating the list as ``bytes`` (as a bare ``cast`` - # used to, without a runtime check) raised trying to slice and - # re-join it. See #556. + # here. # # NOTE: ``ip`` still has to be *set*, though, rather than merely # left alone. :meth:`Protocol.__post_init__ @@ -249,19 +245,14 @@ def post_process(self, packet: 'dict[str, Any]') -> 'Schema': # ` hands # :meth:`~pcapkit.protocols.internet.ipv6_route.IPv6_Route._read_data_type_rpl` # *this* schema on that path, not a re-parsed one -- so returning - # early without ``ip`` raised a bare ``AttributeError: 'RPL' - # object has no attribute 'ip'`` from the reader. That was masked - # for as long as the reader's ``% 16`` guard rejected every - # constructed header first; fixing the guard alongside #564 - # exposed it, so it is fixed in the same pass. Each item is one - # whole element of ``Addresses[1..n]``, so a full-width (16-octet) - # item is decoded the way the parse path below decodes an - # uncompressed one, and a compressed suffix is left as - # :obj:`bytes` -- which is exactly what that path does too. The - # :func:`isinstance` test keeps anything that is not :obj:`bytes` - # passing through untouched, as it did when this branch returned - # without setting ``ip`` at all, rather than failing here on a - # :func:`len` the item may not support. + # early without ``ip`` would raise a bare ``AttributeError`` from + # the reader. Each item is one whole element of + # ``Addresses[1..n]``, so a full-width (16-octet) item is decoded + # the way the parse path below decodes an uncompressed one, and a + # compressed suffix is left as :obj:`bytes` -- which is exactly what + # that path does too. The :func:`isinstance` test keeps anything + # that is not :obj:`bytes` passing through untouched, rather than + # failing here on a :func:`len` the item may not support. self.ip = [ cast('IPv6Address', ipaddress.ip_address(item)) if isinstance(item, bytes) and len(item) == 16 else item @@ -285,15 +276,9 @@ def post_process(self, packet: 'dict[str, Any]') -> 'Schema': # NOTE: ``buffer`` is ``self.addresses``, whose own ``length`` callback # above already subtracted ``pad_len`` -- the trailing padding octets # are read by :attr:`padding`, not by this field. Subtracting - # ``pad_len`` a *second* time here dropped one address for every + # ``pad_len`` a *second* time here would drop one address for every # ``ilen`` octets of padding, so a padded (i.e. compressed) header - # parsed one address short; measured with ``cmpr_i=cmpr_e=4`` and three - # addresses, which yields ``pad_len=4`` and walked one element instead - # of two. Only reachable once the ``% 16`` guard in - # :meth:`IPv6_Route._read_data_type_rpl - # ` - # stopped rejecting every such header, which is why it is fixed in the - # same pass as #564. + # would parse one address short. for _ in range((len(buffer) - elen) // ilen): buf = buffer[counter:counter + ilen] if dst is None: diff --git a/pcapkit/protocols/schema/internet/mh.py b/pcapkit/protocols/schema/internet/mh.py index 995b8c534e..200122f03b 100644 --- a/pcapkit/protocols/schema/internet/mh.py +++ b/pcapkit/protocols/schema/internet/mh.py @@ -380,8 +380,7 @@ def pad_opt_data_len(pkt: 'dict[str, Any]') -> 'int': :class:`~pcapkit.corekit.fields.field.NoValueType` straight to :class:`~pcapkit.corekit.fields.strings.PaddingField`, where it becomes an unusable :mod:`struct` template and surfaces much later as an opaque - :exc:`struct.error` -- which is exactly how a ``Pad1`` option used to - fail to parse. + :exc:`struct.error`, which is how a ``Pad1`` option would fail to parse. """ length = pkt.get('length', 0) @@ -740,8 +739,7 @@ class MNIDOption(Option, code=Enum_Option.MN_ID_OPTION_TYPE): # ``IPv6_Address`` to a # :class:`~pcapkit.corekit.fields.strings.StringField` or # :class:`~pcapkit.corekit.fields.strings.BytesField`, and handing either - # a raw ``int`` is precisely the #467 defect -- ``struct.pack()`` cannot - # consume it (c.f. #467). + # a raw ``int`` would fail, since ``struct.pack()`` cannot consume it. def __init__(self, type: 'Enum_Option', length: 'int', subtype: 'Enum_MNIDSubtype', identifier: 'bytes | str | IPv6Address') -> 'None': ... @@ -1857,7 +1855,7 @@ class ANIMAGGroupIdentifierSuboption(ANISuboption, code=Enum_ANISuboption.MAG_Gr prose fixes ``ANI Length`` at ``2``, but the field description then calls it "a 3-octet unsigned integer value". Two of the three statements agree on **two** octets and are the ones that constrain the wire, so that is - what is parsed here. No erratum has been filed against the RFC. + what is parsed here. """ diff --git a/pcapkit/protocols/schema/misc/pcap/frame.py b/pcapkit/protocols/schema/misc/pcap/frame.py index 688935cc8c..f3d6edc616 100644 --- a/pcapkit/protocols/schema/misc/pcap/frame.py +++ b/pcapkit/protocols/schema/misc/pcap/frame.py @@ -32,12 +32,11 @@ def byteorder_callback(field: 'Field', packet: 'dict[str, Any]') -> 'None': both write it from the global header's magic number. The fallback to :data:`sys.byteorder` is for a schema packed or unpacked on its own, with no global header to ask -- which also means a *misspelled* key looks - exactly like an absent one and reports nothing. That is what hid GitHub - issue :issue:`605`: ``unpack`` wrote ``bytesorder``, so every field here was read - in the host's order rather than the file's, which is right by coincidence - on a little-endian capture and byte-swapped on a big-endian one. See - :file:`tests/protocols/misc/pcap/test_frame_endian_runtime.py` for the - fixtures that now take the other side of the branch. + exactly like an absent one and reports nothing: every field here would be + read in the host's order rather than the file's, which is right by + coincidence on a little-endian capture and byte-swapped on a big-endian + one. See :file:`tests/protocols/misc/pcap/test_frame_endian_runtime.py` + for the fixtures that take the other side of the branch. """ field._byteorder = packet.get('byteorder', sys.byteorder) diff --git a/pcapkit/protocols/schema/misc/pcapng.py b/pcapkit/protocols/schema/misc/pcapng.py index 8bce310fff..61a13f59b2 100644 --- a/pcapkit/protocols/schema/misc/pcapng.py +++ b/pcapkit/protocols/schema/misc/pcapng.py @@ -221,8 +221,8 @@ def nonnegative(length: 'Callable[[dict[str, Any]], int]') -> 'Callable[[dict[st Length less its fixed fields, an option's declared length less the part of itself it describes, an option area's leftover -- and every operand of those subtractions is a wire field that a malformed or truncated capture is free to - set to anything. Nothing made the difference non-negative, and the field - layer does not do it either: :meth:`_TextField.__call__ + set to anything. Nothing else makes the difference non-negative, and the field + layer does not either: :meth:`_TextField.__call__ ` builds its :mod:`struct` template as ``f'{length}s'`` unconditionally, so a negative length becomes the format ``'-8s'`` and :func:`struct.calcsize` raises a bare @@ -230,8 +230,8 @@ def nonnegative(length: 'Callable[[dict[str, Any]], int]') -> 'Callable[[dict[st length reaches :meth:`io.RawIOBase.read` and raises a bare :exc:`ValueError`. Neither is one of :mod:`pcapkit.utilities.exceptions`, so a caller cannot tell either from a bug in its own code, and neither is an :exc:`EOFError`, so - neither is caught by the frame loop -- one malformed block therefore cost the - whole extraction. See :issue:`678`. + neither is caught by the frame loop -- one malformed block would therefore + cost the whole extraction. Two shapes reach here. A Block Total Length below the block's own fixed-field floor -- 28 octets for a Section Header Block, 20 for an Interface @@ -248,8 +248,8 @@ def nonnegative(length: 'Callable[[dict[str, Any]], int]') -> 'Callable[[dict[st :func:`bounded_area` already made, for the reason their docstrings give: a block read has no catch point above :meth:`FieldBase.unpack `, so one refusal aborts the - whole extraction rather than one block, which is what the :issue:`431` accommodation exists - to prevent. The end of the file is the one case that is *not* a clamp, since + whole extraction rather than one block, so a capture cut short by its + snapshot length must keep parsing. The end of the file is the one case that is *not* a clamp, since there no block is being read at all -- see :meth:`PCAPNG._check_block_floor `, which reports it as the :exc:`~pcapkit.utilities.exceptions.StreamEOFError` the frame loop @@ -267,10 +267,8 @@ def nonnegative(length: 'Callable[[dict[str, Any]], int]') -> 'Callable[[dict[st :attr:`UnknownSecrets.data`, :attr:`TLSKeyLog.data` and :attr:`WireGuardKeyLog.data` -- out of this. They read ``__length__`` *whole* rather than subtracting from it, and ``Schema.pack`` leaves it at - ``-1`` for "unknown", so flooring them packs nothing at all. Measured: it - emptied both secrets payloads, and the two ``EXPECTED_FAILURES`` entries - recording their round-trip mismatch then came back ``OK``, since an empty - payload compares equal to an empty payload. On the parsing path their + ``-1`` for "unknown", so flooring them packs nothing at all. On the + parsing path their ``__length__`` is the length the enclosing :class:`~pcapkit.corekit.fields.misc.SchemaField` declared from a 32-bit ``secrets_length``, which cannot be negative, so there is nothing there to @@ -313,13 +311,12 @@ def bounded_option(length: 'Callable[[dict[str, Any]], int]') -> 'Callable[[dict padding without limit: 2,000 Enhanced Packet Blocks in 80,048 octets, each with one option declaring 65,535 against none present, produced 131,070,000 octets of zero padding, an amplification of 1,637x linear in the block - count. See :issue:`594`, and :issue:`573` for the - 32-bit band the field layer's own budget already covers. + count. The field layer's own budget already covers the 32-bit band. The bound has to come from this layer because the field layer cannot see it. What distinguishes the crafted case from the legitimate one is not the - shortfall's size -- both are inside a 16-bit length, which is why :issue:`554`'s - ``len(buffer) < length`` rejection was declined -- but whether the option + shortfall's size -- both are inside a 16-bit length, which is why a plain + ``len(buffer) < length`` rejection does not work -- but whether the option is inconsistent with the framing the block itself declares. Block Total Length is authoritative and cross-checked against its own trailing copy, so the area is ``length`` less the fixed fields, ``captured_len`` and its @@ -328,11 +325,12 @@ def bounded_option(length: 'Callable[[dict[str, Any]], int]') -> 'Callable[[dict capture says so through ``captured_len`` instead, and leaves its options whole, so it never trips this. - Clamping rather than refusing is what keeps the :issue:`431` accommodation: a - block read has no catch point above :meth:`FieldBase.unpack - `, so one refusal aborts the - whole extraction rather than one block, and a truncated capture would stop - parsing at the cut instead of reporting the frames before it. The clamp is + Clamping rather than refusing keeps a capture cut short by its snapshot + length parsing: a block read has no catch point above + :meth:`FieldBase.unpack `, + so one refusal aborts the whole extraction rather than one block, and a + truncated capture would stop parsing at the cut instead of reporting the + frames before it. The clamp is also history-independent -- it reads only this block's own declared framing -- which a running threshold would not be. @@ -408,9 +406,8 @@ def bounded_area(length: 'Callable[[dict[str, Any]], int]') -> 'Callable[[dict[s Secrets -- are deliberately left unclamped *against the block* here, since each computes its span with a different offset and the equality above has to be re-established per block rather than assumed. They do go - through :func:`nonnegative`, which is the part of :issue:`678` that stops a - declared length reaching a read at all; the per-block equality is still - open. + through :func:`nonnegative`, which stops a negative declared length + reaching a read at all; the per-block equality is not enforced for them. The nominal span goes through :func:`nonnegative` first here too. That closes a hole in this function's own arithmetic: a ``captured_len`` past @@ -463,8 +460,8 @@ def pcapng_block_selector(packet: 'dict[str, Any]') -> 'Field': or not those four octets were there to read -- :meth:`FieldBase.unpack ` zero-pads a short read rather than refusing it. A tail of one, two or - three octets therefore arrived here negative and - :meth:`io.RawIOBase.read` raised a bare ``ValueError``, which is :issue:`678`. The floor is a + three octets therefore arrives here negative, and + :meth:`io.RawIOBase.read` would raise a bare ``ValueError``. The floor is a backstop: :meth:`PCAPNG._check_block_floor ` reports that tail as end-of-stream before it gets here, and on the packing path @@ -563,74 +560,49 @@ def post_process(self, value: 'int | bytes', packet: 'dict[str, Any]') -> 'Enum_ Processed field value -- the registry member declared for the option code in this namespace (or in the shared ``opt`` namespace), or an unregistered member of the same registry, - carrying the code itself, when neither declares one. See GitHub - issue :issue:`575`. + carrying the code itself, when neither declares one. Notes: - Until GitHub issue :issue:`860`, - :meth:`~pcapkit.const.pcapng.option_type.OptionType.get` minted a - fresh member -- via :func:`aenum.extend_enum` -- for any code - neither namespace's row covers, unconditionally on a miss. :issue:`860` - converted that miss path to - :meth:`~pcapkit.corekit.enum.EnumRegistry._unregistered_member` - instead, so calling it now would no longer grow the registry - either. This still does not call it unconditionally, though, for - a reason that survives that fix: :meth:`EnumField. - _unregistered_member` (used below) carries its own - ``__reduce_ex__``, so a member built this way still round-trips - through :mod:`pickle` -- see that method's own docstring -- - which :meth:`~pcapkit.const.pcapng.option_type.OptionType. - _unregistered_member`'s own override does not: its - :attr:`~pcapkit.const.pcapng.option_type.OptionType._value_` is - the *formatted* display string (``'opt_unknown [8888]'``, not - ``8888``), so ``pickle.dumps`` on one succeeds but - ``pickle.loads`` of the result raises ``ValueError`` (measured - on Python 3.14.7) -- the default reduction reconstructs through - ``cls(self._value_)``, and ``_missing_``'s own int-only guard - rejects that formatted string outright. The base - :meth:`~pcapkit.corekit.enum.EnumRegistry._unregistered_member` - it overrides fails differently and earlier instead of not at - all: ``pickle.loads`` of its plain-built member's raw-code - ``_value_`` does succeed, but only by reconstructing through - ``_missing_`` -- and so through this override, not the base -- - which is why the result's own ``_value_`` comes back as the - *formatted* string rather than the original code, and comparing - the two directly raises ``AttributeError`` on ``opt_value``. - What actually fails immediately on the base path is ``repr()`` - itself, since + This replicates the read-only membership test + :meth:`~pcapkit.const.pcapng.option_type.OptionType.get` itself runs + first, and calls it only once that test finds the code declared. An + undeclared option type gets :meth:`EnumField._unregistered_member` + instead of ``get()``'s own miss path, for pickle-safety: that method + carries its own ``__reduce_ex__``, so a member built this way + round-trips through :mod:`pickle` -- see its own docstring -- which + :meth:`~pcapkit.const.pcapng.option_type.OptionType. + _unregistered_member`'s override, the miss path, does not. The override's + :attr:`~pcapkit.const.pcapng.option_type.OptionType._value_` is the + *formatted* display string (``'opt_unknown [8888]'``, not ``8888``), + so ``pickle.dumps`` on one succeeds but ``pickle.loads`` of the result + raises ``ValueError``: the default reduction reconstructs through + ``cls(self._value_)``, and ``_missing_``'s own int-only guard rejects + that formatted string outright. + + The base :meth:`~pcapkit.corekit.enum.EnumRegistry._unregistered_member` + that override replaces fails earlier, on ``repr()`` itself, since :attr:`~pcapkit.const.pcapng.option_type.OptionType.opt_name`/ - :attr:`~pcapkit.const.pcapng.option_type.OptionType.opt_value` - are never set (measured) -- which is *why* the override exists - at all, choosing to render correctly over round-tripping - correctly. This method still reaches for - :meth:`EnumField._unregistered_member` rather than either of - :class:`OptionType`'s own two paths, base or override, because - it is the only one of the three that gets both right at once. - :func:`copy.deepcopy` is not the difference -- it succeeds on - both, unaffected by any of that: :class:`aenum.Enum` subclasses - the standard library's :class:`enum.Enum`, and it is *that* - stdlib base (not :class:`aenum.Enum` itself) that defines + :attr:`~pcapkit.const.pcapng.option_type.OptionType.opt_value` are + never set -- which is *why* the override exists, choosing to render + correctly over round-tripping correctly. + :meth:`EnumField._unregistered_member` is the only one of the three + that gets both right. :func:`copy.deepcopy` is not the difference: it + succeeds on both kinds, since the standard library's + :class:`enum.Enum` (which :class:`aenum.Enum` subclasses) defines ``__copy__``/``__deepcopy__`` to return ``self`` outright, so - deep-copying any member of either kind never reaches - ``__reduce_ex__`` in the first place. So this replicates the read-only - membership test :meth:`~pcapkit.const.pcapng.option_type. - OptionType.get` itself runs first, and only calls it once that - test finds the code already declared, so an undeclared option - type still gets :meth:`EnumField._unregistered_member` instead of - a registry row -- now for pickle-safety, not to avoid minting. + deep-copying any member never reaches ``__reduce_ex__``. """ value = super(EnumField, self).post_process(value, packet) namespace = self._opt_ns members_ns = self._namespace.__members_ns__ if value not in members_ns.get('opt', {}) and value not in members_ns.get(namespace, {}): - # NOTE: an unregistered member of self._namespace itself, per - # GitHub issue #575's owner ruling. ``.opt_name`` and + # NOTE: an unregistered member of self._namespace itself, per the + # ruling in EnumField._unregistered_member. ``.opt_name`` and # ``.opt_value`` are what a real OptionType member carries -- # read unconditionally by e.g. pcapng's own ``_option_key`` -- - # and the ``_unknown`` name matches what the mint this - # replaces used to call it, so a rendered or re-keyed member - # reads the same either way. They are passed in the order + # and ``_unknown`` names an undeclared code. They are + # passed in the order # OptionType.__new__ sets them, because DictDumper.object_hook # renders a member's addon keys straight out of its ``__dict__`` # in insertion order, so any other order here would dump an @@ -738,7 +710,6 @@ def __init_subclass__(cls, /, code: 'Optional[Enum_OptionType | Iterable[Enum_Op own fourth parameter ``namespace``, and before Python 3.11 that parameter is positional-or-keyword rather than positional-only -- so a class keyword literally called ``namespace`` bound it twice. - See GitHub issue :issue:`439`. *args: Arbitrary positional arguments. **kwargs: Arbitrary keyword arguments. @@ -751,7 +722,7 @@ def __init_subclass__(cls, /, code: 'Optional[Enum_OptionType | Iterable[Enum_Op .. code-block:: python from pcapkit.const.pcapng.option_type import OptionType as Enum_OptionType - from pcapkit.protocols.schema.misc.pcapng improt Option + from pcapkit.protocols.schema.misc.pcapng import Option class NewOption(Option, ns='opt', code=Enum_OptionType.opt_new): ... @@ -765,9 +736,9 @@ class NewOption(Option, ns='opt', code=Enum_OptionType.opt_new): # move for the same reason. :meth:`Schema.__init_subclass__` is what # refuses to derive from a finalised schema, and a refusal has to land # before :meth:`Option.register` writes ``cls`` into ``__enum__``: calling - # this last, as this method used to, let a raise discard the class object - # while leaving the registry pointing at it, so a rejected declaration - # still displaced a built-in option schema for the rest of the process. + # this last would let a raise discard the class object while leaving the + # registry pointing at it, so a rejected declaration would still + # displace a built-in option schema for the rest of the process. super().__init_subclass__() if ns is not None: @@ -797,9 +768,7 @@ def register(code: 'Enum_OptionType', cls: 'Type[Option]', ns: 'Optional[str]' = reported as a :exc:`~pcapkit.utilities.warnings.RegistryWarning`, as every other registry in the package does -- the lookup that follows cannot tell a deliberate replacement from an accidental one, so an - unreported overwrite is a parser silently swapped out for another. See - :issue:`675` for the - guard ``register_protocol`` added first, which this one now matches. + unreported overwrite is a parser silently swapped out for another. The guard is identity-based: it fires only when the incumbent differs from ``cls``, so re-registering the exact same class under the same @@ -807,13 +776,8 @@ def register(code: 'Enum_OptionType', cls: 'Type[Option]', ns: 'Optional[str]' = rather than a warning about nothing displaced. That is what keeps :meth:`__init_subclass__` honest: it loops over a ``code`` list with no deduplication, so a repeated or aliased entry reaches this method twice - with the same class, and the second call now finds itself already the - incumbent. GitHub issue :issue:`718` corrected the previous presence-only - guard, which read every such repeat as a caller mistake whether or not - the value had actually changed -- the same fix the sibling - :meth:`register` methods on - :class:`~pcapkit.protocols.protocol.ProtocolBase` and friends already - received. + with the same class, and the second call finds itself already the + incumbent. Note: ``ns='opt'`` fans one registration out across every namespace, so the @@ -1807,107 +1771,93 @@ def post_process(self, packet: 'dict[str, Any]') -> 'Self': raised, for the reason :func:`nonnegative` gives: a bare :exc:`struct.error` is neither one of :mod:`pcapkit.utilities.exceptions` nor an :exc:`EOFError`, so it - aborted the whole extraction rather than this one entry. See :issue:`678`. + would abort the whole extraction rather than this one entry. A line of nothing but NUL octets is the block's own 32-bit padding and ends the entry. ``bytes.strip()`` takes only ASCII whitespace, - so those octets survived it and were read as the *name* of a binary - field -- which made every journal entry whose length is not a - multiple of four raise, valid or not, since the padding that follows - it has no 64-bit length prefix behind it to unpack. Measured on a - 14-octet ``MESSAGE=hello\\n`` entry, which is as ordinary as this - block gets. + so those octets would survive it and be read as the *name* of a + binary field -- which would make every journal entry whose length is + not a multiple of four raise, valid or not, since the padding that + follows it has no 64-bit length prefix behind it to unpack. A + 14-octet ``MESSAGE=hello\\n`` entry is as ordinary as this block + gets. A name line whose 64-bit length prefix is itself cut short ends the entry too. There is nothing to read past the end of the entry, so stopping at it is what keeps the truncated block parsing. A binary field's length is the widest declared length in the format, - and nothing bounded it against the entry holding it: at ``2**63`` and - above :meth:`io.BytesIO.read` refuses it outright with a bare + and nothing else bounds it against the entry holding it: at ``2**63`` + and above :meth:`io.BytesIO.read` refuses it outright with a bare :exc:`OverflowError` (``cannot fit 'int' into an index-sized - integer``), and below that it silently returned whatever was there -- - so the same malformed prefix was either fatal or invisible depending - only on its magnitude. It is clamped to the octets the entry has left - and reported, which is :func:`nonnegative`'s rule at the other end of - the same range. + integer``), and below that it silently returns whatever is there, so + the same malformed prefix would be either fatal or invisible + depending only on its magnitude. It is clamped to the octets the + entry has left and reported, which is :func:`nonnegative`'s rule at + the other end of the same range. Field names, keys and values are decoded with ``errors='replace'`` - rather than strictly. A non-UTF-8 octet in any of the three raised a - bare :exc:`UnicodeDecodeError` -- a :exc:`ValueError`, so foreign on - both counts, and fatal to the whole extraction over one bad octet in - one field. ``'replace'`` is the option this module's own + rather than strictly. A non-UTF-8 octet in any of the three would + raise a bare :exc:`UnicodeDecodeError` -- a :exc:`ValueError`, so + foreign on both counts, and fatal to the whole extraction over one + bad octet in one field. ``'replace'`` is the option this module's own :class:`~pcapkit.corekit.fields.strings.StringField` already takes for the same problem, and a value that is not text is a value the writer should have emitted as a *binary* field, so the entry is malformed however it is read. - Entries used to be split apart with ``self.entry.split(b'\\n\\n')`` - before a single field was read -- delimiting a *length-prefixed* - format by content, which a binary field's own bytes need no - escaping to defeat. A value that itself contains ``b'\\n\\n'`` was - cut in the middle of its own data, turning what followed it into a - bogus field in a fabricated second entry; a value 2,570 octets long - is worse, since ``struct.pack(' 'Self': # always NUL, never whitespace, so a `raw_line` ending in # anything else -- ordinary trailing whitespace included # -- proves the true padding is zero, and nothing here - # may be stripped. See #794. + # may be stripped. # the outer gate above already requires `raw_line` -- # and therefore `line`, since `bytes.strip()` cannot # remove a trailing NUL -- to end in one, so this loop @@ -2006,11 +1956,9 @@ def post_process(self, packet: 'dict[str, Any]') -> 'Self': # the reader's position is well defined either way -- # exactly length + 1 octets past where the field name # started -- so a bad octet here ends only this - # entry's field collection, matching #704: it does not - # abort the walk, which keeps looking for the next - # entry's separator from here. See #723 for why an - # outer abort was considered and rejected as - # the default. + # entry's field collection: it does not abort the + # walk, which keeps looking for the next entry's + # separator from here. terminator = entry_data.read(1) if terminator != b'\n': warn(f'PCAP-NG: [systemd Journal Export] binary field ' diff --git a/pcapkit/protocols/schema/schema.py b/pcapkit/protocols/schema/schema.py index 834628b345..53f0f8cf42 100644 --- a/pcapkit/protocols/schema/schema.py +++ b/pcapkit/protocols/schema/schema.py @@ -75,9 +75,9 @@ class marked with a user's own ``from typing import final`` on 3.10 is :meta decorator: """ # NOTE: keyed on ``__finalised__`` rather than on ``final``'s ``__final__``, - # and read out of ``cls.__dict__`` rather than through ``getattr``. See - # :func:`pcapkit.corekit.infoclass.info_final`, which makes both choices for - # the same two reasons: only ``__finalised__`` records *this function* having + # and read out of ``cls.__dict__`` rather than through ``getattr``, for the + # two reasons given in :func:`pcapkit.corekit.infoclass.info_final`, which + # makes both choices: only ``__finalised__`` records *this function* having # run, so a ``__final__`` test would make ``@schema_final`` over ``@final`` # skip the generation while the opposite order did the work; and # ``__finalised__`` is inherited, so ``getattr`` would skip a subclass @@ -87,11 +87,11 @@ class marked with a user's own ``from typing import final`` on 3.10 is SchemaWarning, stacklevel=stacklevel()) return cls - # NOTE: short-circuit for ``Schema`` itself -- see the identical NOTE in - # :func:`pcapkit.corekit.infoclass.info_final`, which makes the same fix for - # the same reason: ``Schema`` never reaches ``FinalisedState.BASE`` now, so a - # bare ``Schema()`` would otherwise re-enter this function, and redo the - # ``dir()``-over-the-MRO scan below, on *every* call. ``__base_ready__`` is + # NOTE: short-circuit for ``Schema`` itself, as in + # :func:`pcapkit.corekit.infoclass.info_final`: ``Schema`` never reaches + # ``FinalisedState.BASE``, so a bare ``Schema()`` would otherwise re-enter + # this function, and redo the ``dir()``-over-the-MRO scan below, on *every* + # call. ``__base_ready__`` is # its own marker, own-``__dict__`` only, so it never touches ``__finalised__``. if cls is Schema and cls.__dict__.get('__base_ready__'): return cls @@ -108,16 +108,15 @@ class marked with a user's own ``from typing import final`` on 3.10 is args_ = [f'{key}=NO_VALUE' for key in cls.__fields__] dict_ = [f'{key}={key}' for key in cls.__fields__] - # NOTE: We shall only attempt to generate ``__init__`` method if the class - # does not define such method -- which is a test on ``cls.__dict__``, not on - # ``hasattr``: every class inherits ``__init__`` from :obj:`object`, so - # ``hasattr(cls, '__init__')`` is unconditionally true and the generated - # method was never installed. ``Schema(...)`` therefore ran - # :meth:`Schema.__update__` alone and never reached + # NOTE: We shall only generate ``__init__`` if the class does not define one, + # which is a test on ``cls.__dict__``, not on ``hasattr``: every class + # inherits ``__init__`` from :obj:`object`, so ``hasattr(cls, '__init__')`` + # is always true. Without the generated method, ``Schema(...)`` would run + # :meth:`Schema.__update__` alone and never reach # :meth:`Schema.__post_init__`, leaving a schema built from a subset of its # fields holding :class:`~pcapkit.corekit.fields.field.FieldBase` objects in # place of the omitted values, so that it could not be packed at all and - # failed with an error naming a field class rather than a field. See #422. + # failed with an error naming a field class rather than a field. # # :class:`~pcapkit.protocols.schema.misc.null.NoPayload` is what the test # protects: it declares an argument-less ``__init__`` of its own so that no @@ -130,15 +129,13 @@ class marked with a user's own ``from typing import final`` on 3.10 is # It is inspired from the :func:`dataclasses._create_fn` function. # # ``**kwargs`` is forwarded rather than rejected, so that a keyword - # naming something other than a field keeps reaching - # :meth:`Schema.__update__` and drawing its - # :class:`~pcapkit.utilities.warnings.UnknownFieldWarning`, as it did - # while ``__init__`` *was* ``__update__``. Several schemas are - # constructed that way on purpose -- the Multipath TCP options take a - # ``kind`` and a ``length`` that the enclosing option owns and that - # ``MPTCP`` declares only for the type checker -- so a strict - # signature here would turn a warning into a :exc:`TypeError` on a - # path that has nothing to do with the missing ``__post_init__``. + # naming something other than a field reaches + # :meth:`Schema.__update__` and draws its + # :class:`~pcapkit.utilities.warnings.UnknownFieldWarning`. Several + # schemas are constructed that way on purpose -- the Multipath TCP + # options take a ``kind`` and a ``length`` that the enclosing option + # owns and that ``MPTCP`` declares only for the type checker -- so a + # strict signature would turn a warning into a :exc:`TypeError`. init_ = ( f'def __create_fn__():\n' f' def __init__(self, {", ".join(args_)}, *, __packet__=None, **kwargs):\n' @@ -163,8 +160,8 @@ class marked with a user's own ``from typing import final`` on 3.10 is if not _finalised: # NOTE: ``Schema`` itself must never receive this promotion, for the same - # reason ``Info`` must not -- see :func:`pcapkit.corekit.infoclass.info_final`, - # which makes the identical fix. This branch binds ``cls`` to ``Schema`` + # reason ``Info`` must not -- see :func:`pcapkit.corekit.infoclass.info_final`. + # This branch binds ``cls`` to ``Schema`` # only when something bare-constructs ``Schema()`` directly, and writing # ``BASE`` onto ``Schema.__dict__`` there would make every subclass declared # afterwards inherit ``BASE`` and skip :meth:`Schema.__new__`'s own @@ -172,8 +169,8 @@ class marked with a user's own ``from typing import final`` on 3.10 is # guard nested inside it for the rest of the process. # # ``__base_ready__`` is set instead, for the short-circuit at the top of - # this function -- own-``__dict__`` only, so it neither inherits nor - # touches ``__finalised__``. + # this function -- own-``__dict__`` only, so it is not inherited and + # does not touch ``__finalised__``. if cls is not Schema: cls.__finalised__ = FinalisedState.BASE else: @@ -273,8 +270,7 @@ def __prepare__(cls, name: 'str', bases: 'tuple[type, ...]', /, **kwds: 'Any') - #: positional-only, ``def __new__(mcls, name, bases, namespace, /, #: **kwargs)``), so a class keyword spelled the same as any of them binds #: that parameter twice: ``TypeError: ABCMeta.__new__() got multiple - #: values for argument '...'``. That is GitHub issue :issue:`439`'s root cause -- - #: ``namespace`` collided this way, which is why + #: values for argument '...'``. ``namespace`` collided this way, which is why #: :mod:`pcapkit.protocols.schema.misc.pcapng`'s ``Option`` subclasses #: spell it ``ns=`` instead. #: @@ -349,17 +345,13 @@ def __new__(cls, name: 'str', bases: 'tuple[type, ...]', attrs: 'dict[str, Any]' if hasattr(base, '__excluded__'): attrs['__excluded__'].extend(name for name in base.__excluded__ if name not in attrs['__excluded__']) - # See #439: this used to branch on ``sys.version_info < (3, 11)`` and - # call ``type.__new__`` directly below that, to dodge the ``namespace`` - # collision described above. That branch skipped ``ABCMeta.__new__``'s - # call to ``abc._abc_init(cls)``, so no :class:`Schema` subclass ever - # got its own ``_abc_impl``, and every one of them fell through the - # MRO to :class:`collections.abc.Mapping`'s -- corrupting - # ``isinstance`` against *any* of them for as long as the process ran. - # The actual fix was renaming the one colliding class keyword that was - # actually in use, which means this can now call ``ABCMeta.__new__`` - # unconditionally, on every supported version, like any other - # metaclass would. + # NOTE: ``ABCMeta.__new__`` is called unconditionally, on every supported + # version. Dodging the ``namespace`` collision described above by calling + # ``type.__new__`` directly would skip ``ABCMeta.__new__``'s call to + # ``abc._abc_init(cls)``, so no :class:`Schema` subclass would get its + # own ``_abc_impl`` and every one of them would fall through the MRO to + # :class:`collections.abc.Mapping`'s -- corrupting ``isinstance`` against + # *any* of them for as long as the process ran. return super().__new__(cls, name, bases, attrs, **kwargs) # type: ignore[return-value] @@ -401,8 +393,7 @@ def __init_subclass__(cls, /, *args: 'Any', **kwargs: 'Any') -> 'None': :func:`schema_final` finalises carries a generated ``__init__`` built from the :attr:`__fields__` that were declared at that moment, so a subclass adding a field afterwards inherits a constructor that cannot - set it -- which is :issue:`422`'s failure shape, reached by a different route. - This turns that promise into a rule the interpreter keeps. + set it. This turns that promise into a rule the interpreter keeps. Args: *args: Arbitrary positional arguments. @@ -423,12 +414,11 @@ def __init_subclass__(cls, /, *args: 'Any', **kwargs: 'Any') -> 'None': 'which is final') # NOTE: forwarded rather than swallowed, so that a class keyword nobody - # accepts still reaches ``object`` and still fails there, as it did before - # this hook existed. :meth:`EnumSchema.__init_subclass__` is the one caller - # that reaches here with nothing: it consumes its own ``code`` keyword and - # discards the rest, which is why a stray class keyword is tolerated on an - # :class:`EnumSchema` subclass and rejected on a plain one. That asymmetry - # predates this method and is left alone. + # accepts still reaches ``object`` and fails there. + # :meth:`EnumSchema.__init_subclass__` is the one caller that reaches here + # with nothing: it consumes its own ``code`` keyword and discards the + # rest, which is why a stray class keyword is tolerated on an + # :class:`EnumSchema` subclass and rejected on a plain one. super().__init_subclass__(*args, **kwargs) def __new__(cls, *args: '_VT', **kwargs: '_VT') -> 'Self': # pylint: disable=unused-argument @@ -456,8 +446,8 @@ def __new__(cls, *args: '_VT', **kwargs: '_VT') -> 'Self': # pylint: disable=un :attr:`~pcapkit.corekit.infoclass.FinalisedState.BASE` ancestor inherits ``BASE`` rather than ``NONE`` and so never reaches the branch below at all, regardless of its own marker. No ``BASE``-state - schema in the tree is marked ``@final`` by hand today, so the escape - is theoretical rather than live. + schema in the tree is marked ``@final`` by hand, so the escape is + theoretical. """ # NOTE: first instantiation is the earliest point a bare ``@final`` can @@ -467,7 +457,7 @@ def __new__(cls, *args: '_VT', **kwargs: '_VT') -> 'Self': # pylint: disable=un # nothing for it. See :meth:`pcapkit.corekit.infoclass.Info.__new__`, # which carries the full reasoning and the measurement; on this side the # missing generated ``__init__`` is the one built from - # :attr:`__fields__`, so the damage is #422's shape. + # :attr:`__fields__`. if cls.__finalised__ == FinalisedState.NONE: if cls.__dict__.get('__final__'): raise SchemaError(f'{cls.__name__}: marked final but never finalised, so it has no generated ' @@ -502,8 +492,7 @@ def __post_init__(self, packet: 'Optional[dict[str, Any]]' = None) -> 'None': # generated ``__init__`` is not the only caller: :meth:`from_dict` # seeds only the keys its argument carries, so a field the caller left # out is missing from ``__dict__`` entirely rather than holding - # ``NO_VALUE``, and subscripting it raised :exc:`KeyError` naming the - # field. + # ``NO_VALUE``. # # What is tested is ``NO_VALUE`` alone, not ``NO_VALUE`` or ``None``. # This method fills in what the caller did not say, and a ``None`` the @@ -543,8 +532,7 @@ def __post_init__(self, packet: 'Optional[dict[str, Any]]' = None) -> 'None': # carries them, and packing without it raises rather than producing # octets. ``__updated__`` is still set, so a schema left unpacked here is # packed by :meth:`__bytes__` on first use -- by which time the enclosing - # layer has supplied the context, which is where the octets were produced - # before this method ran on construction at all. + # layer has supplied the context. if packet is not None: self.pack(packet) @@ -553,7 +541,7 @@ def __update__(self, dict_: 'Optional[Mapping[str, _VT] | Iterable[tuple[str, _V # NOTE: Keys with the same names as the class's builtin methods will be # renamed with the class name prefixed as mangled class variables # implicitly and internally. Such mapping information will be stored - # within: attr: `__map__` attribute. + # within the ``__map__`` attribute. __name__ = type(self).__name__ # pylint: disable=redefined-builtin @@ -648,10 +636,9 @@ def __setattr__(self, name: 'str', value: '_VT') -> 'None': key = self.__map__.get(name, name) self.__dict__[key] = value # NOTE: ``self.__updated__ = True`` would re-enter this method once - # per field assigned -- 180297 recursive calls per extraction of - # examples/captures/http.pcap -- only to miss the __fields__ test and - # fall through to object.__setattr__. ``__updated__`` is an instance - # attribute established in __new__, so the direct store is the same + # per field assigned, only to miss the ``__fields__`` test and fall + # through to ``object.__setattr__``. ``__updated__`` is an instance + # attribute established in ``__new__``, so the direct store is the same # write with none of the round trip. self.__dict__['__updated__'] = True return @@ -777,31 +764,25 @@ class will consider negative value as a placeholder. # finds the *class* attribute when the instance has none -- and a # schema's class attribute for a field is the # :class:`~pcapkit.corekit.fields.field.FieldBase` object itself. A - # field the caller never set therefore arrived below as the field - # rather than as a value: ``getattr(self, name, None)`` could not - # return its ``None`` for one, so the absent-value branches never - # fired, and what surfaced instead was a failure from inside the - # packing of a field object -- naming a field *class*, and so saying - # nothing about which field had been left out. See #422. + # field the caller never set would arrive below as the field rather + # than as a value: ``getattr(self, name, None)`` could not return its + # ``None``, so the absent-value branches would not fire, and packing + # would fail from inside a field object -- naming a field *class*, and + # so saying nothing about which field had been left out. data = self.__dict__.get(self.__map__.get(field.name, field.name)) if isinstance(field, PayloadField): - # NOTE: ``ProtocolBase``, not ``Protocol``. The two were one class - # until the metaclass revision split them, which renamed the base to - # ``ProtocolBase`` and kept ``Protocol`` as a thin subclass that adds - # auto-registration for externally defined engines. Every module under - # ``pcapkit.protocols.schema`` was updated to import the base under the - # old name; this one was missed, because its import is a runtime import - # inside a method rather than a ``TYPE_CHECKING`` one at module level. - # No protocol in the library subclasses ``Protocol``, so the branch - # below had been unreachable ever since: handing a payload field any - # protocol instance -- which is exactly what :meth:`ProtocolBase._make_payload - # ` returns, and - # what ``make``'s own ``bytes | Protocol | Schema`` signature advertises - # -- fell through to the ``ProtocolUnbound`` below instead of being - # packed. That is what stopped ``from_data`` reconstructing any parsed - # packet. An external ``Protocol`` subclass is still a ``ProtocolBase``, - # so nothing that worked before is affected. See #506. + # NOTE: ``ProtocolBase``, not ``Protocol``: ``Protocol`` is a thin + # subclass that only adds auto-registration for externally defined + # engines, and no protocol in the library subclasses it. Testing + # against it would send any protocol instance -- which is exactly + # what :meth:`ProtocolBase._make_payload + # ` returns, + # and what ``make``'s own ``bytes | Protocol | Schema`` signature + # advertises -- to the ``ProtocolUnbound`` below instead of packing + # it, and ``from_data`` could not reconstruct a parsed packet. An + # external ``Protocol`` subclass is still a ``ProtocolBase``. The + # import is inside the method rather than at module level. from pcapkit.protocols.protocol import \ ProtocolBase # pylint: disable=import-outside-toplevel @@ -828,11 +809,10 @@ class will consider negative value as a placeholder. # 'tuple[Group, ...]'`` in pcapkit/protocols/data/internet/ # hip.py -- and ``_read_*`` then hands one straight back # here on reconstruction. ``ListField.pack`` only ever - # iterates its argument, so it does not care which of the - # two it gets; rejecting the tuple broke every - # parse-then-reconstruct cycle for such a field. See #476. - # ``list(data)`` is a no-op for an actual list and keeps - # ``ListField.pack``'s own ``Optional[list[_TL]]`` + # iterates its argument, so it accepts either, and + # rejecting the tuple would break parse-then-reconstruct for + # such a field. ``list(data)`` is a no-op for an actual list + # and keeps ``ListField.pack``'s own ``Optional[list[_TL]]`` # signature honest rather than widening it too. self.__buffer__[field.name] = field.pack(list(data), packet) else: @@ -853,7 +833,7 @@ class will consider negative value as a placeholder. # NOTE: a forward match consumes nothing, so it contributes no # octets to ``bytes(self)``/``len(self)`` either. :meth:`unpack` # mirrors this for the same reason -- see the ``ForwardMatchField`` - # branch there. See #446. + # branch there. self.__buffer__[field.name] = b'' continue @@ -989,17 +969,15 @@ def unpack(cls, data: 'bytes | IO[bytes]', # can size itself from them, but consumes neither the stream # (the rewind below) nor ``__length__`` (no decrement in this # branch). ``self.__buffer__[field.name]`` above still holds the - # octets just read, though, and until here nothing undid that: - # ``__bytes__``/``__len__`` concatenate every slot in - # ``__buffer__``, so the schema over-reported its length by - # exactly the forward match's width -- the same octets are read - # again, for real, by whichever field actually needs them, so - # nothing is lost by dropping the duplicate here. ``pack()`` - # above already zeroes this slot for the same field type; - # zeroing it here as well is what makes a declared area checked - # against ``len(self)`` -- :class:`~pcapkit.corekit.fields.collections.OptionField` + # octets just read, so it is zeroed here, as :meth:`pack` does for + # the same field type: ``__bytes__``/``__len__`` concatenate every + # slot in ``__buffer__``, and the octets are read again, for real, + # by whichever field actually needs them, so keeping them would + # over-report the schema's length by the forward match's width. + # Zeroing is also what lets a declared area checked against + # ``len(self)`` -- :class:`~pcapkit.corekit.fields.collections.OptionField` # and :class:`~pcapkit.corekit.fields.collections.ListField` both - # do this -- see the octets actually consumed. See #446. + # do this -- see the octets actually consumed. data.seek(-length, io.SEEK_CUR) self.__buffer__[field.name] = b'' elif isinstance(field, OptionField) and field.option_padding > 0: @@ -1051,25 +1029,23 @@ class _EnumRegistry(collections.defaultdict): """A registry :class:`collections.defaultdict` that never inserts a miss. :attr:`EnumSchema.registry` (and its class-level twin, - :attr:`EnumMeta.registry`) is read with a bare ``registry[code]`` at dozens - of call sites across the schema layer, e.g. ``Option.registry[type]``. A + :attr:`EnumMeta.registry`) is read with a bare ``registry[code]`` at many + call sites across the schema layer, e.g. ``Option.registry[type]``. A plain :class:`collections.defaultdict` inserts whatever :attr:`~collections.defaultdict.default_factory` returns the *first time* an unregistered ``code`` is looked up -- and since the registry lives on the *class*, that insertion is permanent and shared by every instance of every subclass in the process. Parsing one packet carrying an unrecognised - code is therefore enough to grow the registry for the remainder of the - process, and to make a later, entirely legitimate - :meth:`EnumSchema.register` call report an overwrite that never happened. - - This is the schema-layer instance of the defect :meth:`ProtocolBase.\ + code would therefore grow the registry for the remainder of the process, + and make a later, entirely legitimate :meth:`EnumSchema.register` call + report an overwrite that never happened. + + The fallback itself is deliberate -- it is how an unknown option, chunk or + block falls back to its ``Unknown*``/``Unassigned*`` schema -- so this + subclass keeps returning it and just stops recording it. + :meth:`ProtocolBase.\ _lookup_registry ` - fixed for the protocol-layer ``__proto__`` family in GitHub issue - :issue:`421`, and for the option, chunk and block registries in - :issue:`425`; see GitHub issue :issue:`555`. The fallback itself is - deliberate -- it is how an unknown option, chunk or block falls back to its - ``Unknown*``/``Unassigned*`` schema -- so this subclass keeps returning it, - it just stops recording it. + does the same for the protocol-layer ``__proto__`` family. """ @@ -1169,8 +1145,8 @@ def registry(self) -> 'DefaultDict[_ET, Type[Self]]': """Mapping of enumeration numbers to schemas. Note: - This property is also available as a class - attribute. + This property answers on an instance; on the class, the same + mapping is served by ``EnumMeta.registry``. Important: See :attr:`EnumMeta.registry`: the returned mapping is a @@ -1197,12 +1173,10 @@ def __init_subclass__(cls, /, code: 'Optional[_ET | Iterable[_ET]]' = None, *arg Warns: pcapkit.utilities.warnings.RegistryWarning: If any of ``code`` is already registered, naming the displaced schema and its - replacement. This is the same guard :meth:`register` applies, - and it is here as well because a class declaration is the - *other* way into :attr:`__enum__` -- ``class MyOption(Option, - code=...)`` writes the registry without any call to - :meth:`register`, so guarding only the method would leave the - declaration path silently displacing a built-in schema. + replacement. This is the guard :meth:`register` applies, repeated + here because a class declaration is the *other* way into + :attr:`__enum__`: ``class MyOption(Option, code=...)`` writes the + registry without calling :meth:`register`. Notes: If :attr:`__enum__` is not yet defined at function call, @@ -1224,9 +1198,9 @@ def __init_subclass__(cls, /, code: 'Optional[_ET | Iterable[_ET]]' = None, *arg # finalised schema, and a refusal has to land before this method writes # ``cls`` into ``__enum__``: raising afterwards would discard the class # object while leaving the registry pointing at it, so a rejected - # declaration would still have displaced a built-in schema. It took no - # arguments when it was the last statement here and still takes none -- - # ``code`` is this method's own and the rest are deliberately dropped. + # declaration would still have displaced a built-in schema. It takes no + # arguments: ``code`` is this method's own and the rest are deliberately + # dropped. super().__init_subclass__() if not hasattr(cls, '__enum__'): @@ -1237,9 +1211,8 @@ def __init_subclass__(cls, /, code: 'Optional[_ET | Iterable[_ET]]' = None, *arg # variant now, while ``cls`` is still being constructed and no # external code has had a chance to capture a reference to the # original dict. Every later access, through :attr:`registry` or - # otherwise, then sees the same safe object -- so identity across - # repeated ``.registry`` reads (see ``EnumMeta.registry``) is - # preserved, and nothing but the retention behaviour changes. + # otherwise, then sees the same safe object, and identity across + # repeated ``.registry`` reads is preserved. manual = cls.__dict__['__enum__'] cls.__enum__ = _EnumRegistry(getattr(manual, 'default_factory', None), manual) @@ -1251,14 +1224,13 @@ def __init_subclass__(cls, /, code: 'Optional[_ET | Iterable[_ET]]' = None, *arg # does not mean the same thing for every subclass. codes = code if isinstance(code, collections.abc.Iterable) else (code,) for _code in codes: - # This loop visits every element of an iterable ``code``, so a - # repeated member -- or two members that are the same object, - # as an :class:`enum.Enum` alias makes possible -- reaches this - # twice for the same key with ``cls`` on both sides: incumbent - # on the first pass, replacement on the second. So ordinary - # subclassing syntax reaches this guard, e.g. + # A repeated member of an iterable ``code`` -- or two members that + # are the same object, as an :class:`enum.Enum` alias makes + # possible -- reaches this twice for the same key with ``cls`` on + # both sides: incumbent on the first pass, replacement on the + # second. So ordinary subclassing syntax reaches this guard, e.g. # ``class X(Base, code=[A, A])`` or ``code=[A, B]`` with - # ``A is B``, with no second call to this method needed. + # ``A is B``. incumbent = cls.__enum__.get(_code) # type: ignore[arg-type] if incumbent is not None and incumbent is not cls: warn(f'schema {_code} already registered, overwriting ' @@ -1283,31 +1255,22 @@ def register(cls, code: '_ET', schema: 'Type[Self]') -> 'None': ``schema`` registers two halves of one binding -- a parser class through e.g. :meth:`IPv4.register_option `, and a - schema class through this method. The parser half has warned on an - overwrite for as long as it has existed; this half assigned bare, so - one ``register_ipv4_option`` call replacing a built-in reported the - parser it displaced and said nothing about the schema. The guard - here closes that asymmetry. - - It fires only when the incumbent differs from the replacement, the - same guard :func:`register_protocol - ` applies, - even though ``code`` here -- unlike ``register_protocol``'s key -- - is supplied by the caller and independent of ``schema``. GitHub - issue :issue:`718` corrected the previous presence-only guard: a repeat - call that names the exact same schema object is a caller replaying - a registration, not a mistake, so it is now a silent no-op. + schema class through this method. Both warn on an overwrite, so one + ``register_ipv4_option`` call replacing a built-in reports the + schema it displaced as well as the parser. + + The warning fires only when the incumbent differs from the + replacement: a repeat call that names the exact same schema object + is a caller replaying a registration, not a mistake, so it is a + silent no-op. Presence is a faithful "was this really registered" test only because :class:`_EnumRegistry` returns a miss without recording it. A plain - :class:`collections.defaultdict` would have inserted - :attr:`__default__` the first time any unregistered ``code`` was - looked up, so parsing a single packet carrying an unknown code would - have made the next legitimate registration for that code warn about - an entry no caller ever asked for -- the defect fixed for this layer - in :issue:`555`, for the parser-layer ``__proto__`` family in - :issue:`421`, and for the option, chunk and block registries in - :issue:`425`. That fix is what makes this guard safe to add. + :class:`collections.defaultdict` would insert :attr:`__default__` the + first time any unregistered ``code`` was looked up, so parsing a + single packet carrying an unknown code would make the next legitimate + registration for that code warn about an entry no caller ever asked + for. :class:`pcapkit.protocols.schema.misc.pcapng.Option` overrides this method with a namespaced registry of its own and does not delegate diff --git a/pcapkit/protocols/schema/transport/sctp.py b/pcapkit/protocols/schema/transport/sctp.py index 5e8a4ef864..b4de7b503d 100644 --- a/pcapkit/protocols/schema/transport/sctp.py +++ b/pcapkit/protocols/schema/transport/sctp.py @@ -219,8 +219,7 @@ def post_process(self, value: 'int | bytes', packet: 'dict[str, Any]') -> 'Enum_ Returns: Processed field value -- the registry member declared for the port, or an unregistered member of the same registry, carrying - the port itself, when the registry declares none. See GitHub - issue :issue:`575`. + the port itself, when the registry declares none. Notes: See :meth:`pcapkit.protocols.schema.transport.tcp.PortEnumField.post_process`, diff --git a/pcapkit/protocols/schema/transport/tcp.py b/pcapkit/protocols/schema/transport/tcp.py index 3f50d9b953..1e08ba8f6a 100644 --- a/pcapkit/protocols/schema/transport/tcp.py +++ b/pcapkit/protocols/schema/transport/tcp.py @@ -255,41 +255,17 @@ def mptcp_dss_ack_selector(pkt: 'dict[str, Any]') -> 'Field': Note: This is a :class:`~pcapkit.corekit.fields.misc.SwitchField` selector rather than a :class:`~pcapkit.corekit.fields.misc.ConditionalField` - wrapping ``NumberField(length=lambda pkt: ...)``, which is what it was - until :issue:`576`. - - The width lambda read ``8 if pkt['flags']['a'] else 0`` -- **0**, not 4 -- - so an unextended Data ACK packed no octets at all while the ``length`` - octet still counted 4 for it. That is the defect :issue:`576` records: the option - went onto the wire 4 (or 8, with ``dsn`` too) octets shorter than it - declared, and the ``ack`` value the caller supplied was simply not - present. - - Correcting the lambda to ``8 if ... else 4`` would not have worked *at the - time*, because :class:`~pcapkit.corekit.fields.numbers.NumberField` could - not pack a callable length at all: it called ``build_template`` once at - ``__init__`` with the placeholder length ``-1``, which latched - ``_need_process = True``, and nothing cleared that flag when ``__call__`` - later resolved the real length and rebuilt the template as ``>I``/``>Q``. - ``pre_process`` then handed :func:`struct.pack` bytes for an integer - template and it raised ``struct.error: required argument is not an - integer``. Measured on the 8-octet form, which the old lambda did reach: - ``_make_mptcp_dss(DSS, ack=1 << 40)`` raised exactly that. - - That half is now history: :issue:`591` **fixed it**, in - :mod:`pcapkit.corekit.fields.numbers` where this note used to say the fix - belonged, by recomputing ``_need_process`` from the width actually in - force instead of once from the placeholder. A callable-length - ``NumberField`` packs and unpacks both the 4- and the 8-octet form today, - so ``ConditionalField(NumberField(length=...), lambda pkt: - pkt['flags']['A'])`` would express this field correctly. Nor was wire - *absence* ever the obstacle: :attr:`MPTCPDSS.ssn`, :attr:`MPTCPDSS.dl_len` - and :attr:`MPTCPDSS.checksum` are each a ``ConditionalField`` on the - sibling ``M`` flag, so this very class already leans on that wrapper to - keep a field off the wire. - - The ``SwitchField`` form is kept anyway, for a narrower reason about - composition rather than about absence. A + wrapping ``NumberField(length=lambda pkt: ...)``. An unextended Data ACK + is 4 octets, so a width lambda returning ``0`` for it would leave the + ``length`` octet counting 4 octets that were never packed. + + The ``ConditionalField`` form could express this field correctly, since a + callable-length ``NumberField`` packs and unpacks both the 4- and the + 8-octet form, and wire *absence* is no obstacle: + :attr:`MPTCPDSS.ssn`, :attr:`MPTCPDSS.dl_len` and + :attr:`MPTCPDSS.checksum` are each a ``ConditionalField`` on the sibling + ``M`` flag. The ``SwitchField`` form is kept for a narrower reason about + composition. A :class:`~pcapkit.corekit.fields.misc.ConditionalField`'s ``length`` forwards to the wrapped field unconditionally, never consulting the condition, so reading it while the condition is false -- the wrapped field @@ -302,7 +278,7 @@ def mptcp_dss_ack_selector(pkt: 'dict[str, Any]') -> 'Field': special case: its selector always hands back an already-concrete field, :class:`~pcapkit.corekit.fields.misc.NoValueField` included, so its ``length`` is safe wherever it is read. Swapping the two would be a - behaviour change, not a tidy-up, and :issue:`603` does not make it. + behaviour change, not a tidy-up. """ if not pkt['flags']['A']: @@ -330,12 +306,9 @@ def mptcp_dss_dsn_selector(pkt: 'dict[str, Any]') -> 'Field': :class:`~pcapkit.corekit.fields.numbers.UInt32Field` instance. Note: - Identical in shape to :func:`mptcp_dss_ack_selector`, and it replaces the - identical defect: ``NumberField(length=lambda pkt: 8 if pkt['flags']['m'] - else 0, ...)``. See that function's note for why the ``0`` was wrong, why a - corrected lambda would not have packed either *at the time*, and why the - ``SwitchField`` form is kept now that :issue:`591` has made a callable length work. - C.f. :issue:`576`, :issue:`591`. + Identical in shape to :func:`mptcp_dss_ack_selector`; see that function's + note for why a ``SwitchField`` is used in place of a + ``ConditionalField`` around a callable-length ``NumberField``. """ if not pkt['flags']['M']: @@ -389,16 +362,15 @@ def post_process(self, value: 'int | bytes', packet: 'dict[str, Any]') -> 'Enum_ Returns: Processed field value -- the registry member declared for the port, or an unregistered member of the same registry, carrying - the port itself, when the registry declares none. See GitHub - issue :issue:`575`. + the port itself, when the registry declares none. Notes: :meth:`~pcapkit.const.reg.apptype.AppType.get` mints a fresh member -- via :func:`aenum.extend_enum` -- for any port neither an existing row nor one of :meth:`_missing_`'s documented IANA spans accounts for, which in practice means the ephemeral/dynamic range. - Calling it unconditionally on every parsed port therefore grew the - registry without bound. This peeks at the registry + Calling it on every parsed port would therefore grow the registry + without bound. This peeks at the registry :meth:`~pcapkit.const.reg.apptype.AppType.get` itself would consult -- its per-port rows via ``__registry__.getlist``, then its documented spans via ``_missing_`` -- and only calls @@ -409,30 +381,24 @@ def post_process(self, value: 'int | bytes', packet: 'dict[str, Any]') -> 'Enum_ A port outside this field's own width is rejected *before* any of that, rather than being let through to :meth:`_missing_` and caught alongside a genuine miss. Both are a bare :exc:`ValueError` - with nothing to tell them apart by type, and GitHub issue :issue:`758` - gave the out-of-range case a deliberate, ``breaking``-tagged - rejection specifically so it would stop being minted over -- a + with nothing to tell them apart by type, and + :meth:`~pcapkit.const.reg.apptype.AppType.get` rejects the + out-of-range case deliberately, so that it is never minted over -- a catch keyed on exception type alone cannot see the difference between that and :mod:`aenum`'s own "no member has this value", - so it would absorb both and quietly revert :issue:`758` for these four - fields. Checking the width first needs no exception-based - distinction at all: it asks the same question :meth:`_missing_` - would eventually ask, and asks it in a way that never manufactures - the bare :exc:`ValueError` this method would otherwise have to - tell apart from a foreign one. ``self.length`` is the field's own - declared byte width (``2`` for every caller of this class, hence - ``0``-``65535``) rather than a hard-coded ``65535`` borrowed from + so it would absorb both and quietly turn the rejection into a + miss. Checking the width first needs no exception-based + distinction at all. ``self.length`` is the field's own declared byte + width (``2`` for every caller of this class, hence ``0``-``65535``) + rather than a hard-coded ``65535`` borrowed from :meth:`~pcapkit.const.reg.apptype.AppType._missing_`'s own guard, - so the two stay in lockstep by construction: whatever width this - field is ever given, the range checked here is exactly the range a - value of that width can hold, no more permissive and no more - restrictive. + so the two stay in lockstep by construction. """ value = super(EnumField, self).post_process(value, packet) proto = Enum_TransportProtocol.tcp if not (isinstance(value, int) and 0 <= value < (1 << (8 * self.length))): - # NOTE: lets AppType.get() -- unmodified -- raise #758's rejection + # NOTE: lets AppType.get() -- unmodified -- raise its rejection # for a port this field's own width cannot represent, rather than # risking it being absorbed below as a foreign miss. return self._namespace.get(value, proto=proto) @@ -444,21 +410,20 @@ def post_process(self, value: 'int | bytes', packet: 'dict[str, Any]') -> 'Enum_ # NOTE: value is already known to be in-width here, so this # ValueError is aenum's own "no member has this value" for an # in-range but unassigned port -- a foreign miss, absorbed -- - # never #758's out-of-range rejection, which never reaches + # never the out-of-range rejection, which never reaches # this branch. A pcapkit.utilities.exceptions error is still a # deliberate registry decision and propagates unchanged. if isinstance(error, BaseError): raise declared = None if declared is None: - # NOTE: an unregistered member of ``owner`` itself, per GitHub - # issue #575's owner ruling -- see EnumField._unregistered_member - # -- rather than a foreign pseudo-enum. ``.port``, ``.svc`` and - # ``.proto`` are what a real AppType member carries -- read - # unconditionally by e.g. Transport._decode_next_layer's - # ``srcport.port`` -- and ``svc='unknown'`` matches what the - # mint this replaces used to name it. They are passed in the - # order AppType.__new__ sets them, because + # NOTE: an unregistered member of ``owner`` itself, per the + # ruling in EnumField._unregistered_member, rather than a foreign + # pseudo-enum. ``.port``, ``.svc`` and ``.proto`` are what a real + # AppType member carries -- read unconditionally by e.g. + # Transport._decode_next_layer's ``srcport.port`` -- and + # ``svc='unknown'`` marks a port with no declared service. They + # are passed in the order AppType.__new__ sets them, because # DictDumper.object_hook renders a member's addon keys straight # out of its ``__dict__`` in insertion order -- so any other # order here would make an unassigned port dump ``port`` before @@ -766,9 +731,7 @@ class _MPTCP(Schema): #: itself. :rfc:`8684` section 3 therefore puts ``kind`` in bits 0-7, #: ``length`` in bits 8-15, and the subtype in bits 16-19 of this 3-octet #: window -- which is why ``subtype`` reads from bit 16 and ``length`` has - #: to read from bit 8. It read from bit 1 until :issue:`553`, straddling the low - #: seven bits of ``kind`` and the high bit of ``length``, so a 12-octet - #: MP_CAPABLE (``1e 0c 01``) decoded its length as 60. + #: to read from bit 8. test: 'MPTCPSubtypeTest' = ForwardMatchField(BitField(length=3, namespace={ 'length': (8, 8), 'subtype': (16, 4), @@ -819,12 +782,12 @@ class MPTCP(EnumSchema[Enum_MPTCPOption]): # octet as a result, which is where its ``test`` (subtype/flags) field # expects to read from. # - # Before this, both directions were broken: packing rejected ``kind=``/ - # ``length=`` from the ``_make_mptcp_*`` makers with ``UnknownFieldWarning`` - # and then ``KeyError: 'length'`` the moment a sibling field's condition - # (e.g. ``MPTCPAddAddress.port``, ``MPTCPCapable.rkey``) read - # ``pkt['length']``; unpacking silently misread the ``kind`` octet as the - # subtype/flags octet, since nothing had consumed it first. C.f. #541. + # Without them, packing would reject ``kind=``/``length=`` from the + # ``_make_mptcp_*`` makers with ``UnknownFieldWarning`` and then + # ``KeyError: 'length'`` the moment a sibling field's condition (e.g. + # ``MPTCPAddAddress.port``, ``MPTCPCapable.rkey``) read ``pkt['length']``; + # unpacking would misread the ``kind`` octet as the subtype/flags octet, + # since nothing had consumed it first. #: Option kind. kind: 'Enum_Option' = EnumField(length=1, namespace=Enum_Option) #: MPTCP length. @@ -844,9 +807,7 @@ class MPTCP(EnumSchema[Enum_MPTCPOption]): # have. So this attribute is populated by the *construction* path instead # -- :meth:`~pcapkit.protocols.transport.tcp.TCP._make_mode_mp` sets it # right after building the subtype-specific schema, mirroring exactly what - # :meth:`_MPTCP.post_process` already does for real unpacking. C.f. #566, - # the third and last ``TYPE_CHECKING``-only attribute this class had; the - # other two (``kind``, ``length``) were fixed in #541. + # :meth:`_MPTCP.post_process` already does for real unpacking. if TYPE_CHECKING: #: MPTCP subtype. subtype: 'Enum_MPTCPOption' @@ -888,9 +849,7 @@ class MPTCPCapable(MPTCP, code=Enum_MPTCPOption.MP_CAPABLE): #: Option receiver's key. #: #: :rfc:`8684` section 3.1 gives MP_CAPABLE as 12 octets without this key - #: and 20 octets with it, so the field is present only for the latter -- - #: not, as it read until :issue:`567`, for every length *except* 32, which is not - #: an MP_CAPABLE length either RFC form uses. + #: and 20 octets with it, so the field is present only for the latter. rkey: 'int' = ConditionalField( UInt64Field(), lambda pkt: pkt['length'] == 20, @@ -981,8 +940,8 @@ class MPTCPDSS(MPTCP, code=Enum_MPTCPOption.DSS): #: #: 4 octets when ``A`` is set, 8 when ``a`` is set as well, absent otherwise -- #: :rfc:`8684` section 3.3 figure 9. Both the presence test and the width live - #: in :func:`mptcp_dss_ack_selector`, whose note records what this field - #: declared until :issue:`576` and why the switch form is kept. + #: in :func:`mptcp_dss_ack_selector`, whose note explains why the switch form + #: is used. ack: 'int' = SwitchField( selector=mptcp_dss_ack_selector, ) @@ -1103,12 +1062,10 @@ class MPTCPFastclose(MPTCP, code=Enum_MPTCPOption.MP_FASTCLOSE): #: :rfc:`8684` section 3.5 figure 14 spends a whole 32-bit row on #: ``Kind``/``Length``/``Subtype``/``(reserved)``, i.e. the subtype's 4 bits #: are followed by **12** reserved bits, not 4 -- so the subtype-and-reserved - #: part is 2 octets and the option is 12 octets in total. Until :issue:`576` this - #: field did not exist and ``test`` was the only octet between ``length`` and - #: ``key``, so the schema packed **11** octets against a ``length`` of 12. - #: Declared the same way :class:`MPTCPJoinACK` declares its own reserved - #: octet, for the same reason: a wider ``test`` would make the reserved bits - #: look like part of the subtype namespace. + #: part is 2 octets and the option is 12 octets in total. Declared the same + #: way :class:`MPTCPJoinACK` declares its own reserved octet, for the same + #: reason: a wider ``test`` would make the reserved bits look like part of + #: the subtype namespace. reserved: 'bytes' = PaddingField(length=1) #: Option receiver's key. key: 'int' = UInt64Field() diff --git a/pcapkit/protocols/schema/transport/udp.py b/pcapkit/protocols/schema/transport/udp.py index fac262b219..22ae70aabc 100644 --- a/pcapkit/protocols/schema/transport/udp.py +++ b/pcapkit/protocols/schema/transport/udp.py @@ -66,8 +66,7 @@ def post_process(self, value: 'int | bytes', packet: 'dict[str, Any]') -> 'Enum_ Returns: Processed field value -- the registry member declared for the port, or an unregistered member of the same registry, carrying - the port itself, when the registry declares none. See GitHub - issue :issue:`575`. + the port itself, when the registry declares none. Notes: See :meth:`pcapkit.protocols.schema.transport.tcp.PortEnumField.post_process`,