From 329067a6b96a32463c03f9ff8cf61e3b43650941 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Tue, 29 Sep 2026 13:19:06 -0400 Subject: [PATCH] fix(vendor,const): source ExtensionHeader from the extension-header registry, drop BIT_EMU (#925) `pcapkit.vendor.ipv6.extension_header.ExtensionHeader` crawled the *Protocol Numbers* registry (`protocol-numbers-1.csv`), filtered on its `IPv6 Extension Header` column -- a derived signal, not the registry RFC 8200 s4 names authoritative for this enumeration. That registry disagreed with the authoritative *IPv6 Extension Header Types* registry (`ipv6-parameters/extension-header.csv`) on header 147 (`BIT-EMU`): the former flagged it `Y`, citing RFC 9801; the latter omits it entirely. - Point `LINK` at `extension-header.csv` and rewrite `count`/`process` for its 3-column shape (`Protocol Number,Description,Reference` -- no flag to filter on). - That registry has no short `Keyword` column, unlike the old one, so add a `NAMES` override for the 8 members the old registry gave a keyword-derived name (`HOPOPT`, `IPv6-Route`, `IPv6-Frag`, `ESP`, `AH`, `IPv6-Opts`, `HIP`, `Shim6`) -- deriving names straight from the verbose Description column instead would silently rename all eight, which is worse than the registry defect being fixed. - Drop `BIT_EMU` from `pcapkit.const.ipv6.extension_header`, taking it to 11 members -- hand-applied to match what the fixed crawler produces from the fixture below byte-for-byte (proven by a new test, not merely asserted). - `IPv6._decode_next_layer`'s walk resolves `ExtensionHeader(proto)` at the top of its loop and used to rely on 147 succeeding there. `test_ipv6_ext_unit.py`'s `test_unimplemented_terminal_code_stops_ the_walk_not_the_packet` picked 147 only as an *example* of an unimplemented extension-header code (its own docstring: "253/254 are the same code path"); switched it to 253, which is actually in the authoritative registry, so the test exercises the identical code path without depending on a member being removed. Updated its docstring and the prose in `ipv6.py`/`ipv6_ext.py` that described BIT_EMU as one of the codes reachable through the walk/generic extractor -- it no longer is, by construction, since it is not an `ExtensionHeader` member at all any more. - `pcapkit.const.reg.transtype.TransType.BIT_EMU` is untouched: a different, correctly-sourced enumeration (147 legitimately belongs in the Protocol Numbers registry). New unit tests: the vendor suite feeds the issue's fixture CSV directly to the crawler (no network) and pins LINK, all 11 names/ values, BIT_EMU's absence, which comments genuinely diverge from history because the two registries carry different Reference text, and -- the seam between the two halves of this fix -- that `ExtensionHeader.context()` run against the fixture reproduces the committed const file byte-for-byte. The const suite pins the member count, that `ExtensionHeader(147)` now raises, that `TransType.BIT_EMU` is unaffected, and that the IPv6 walk still stops cleanly (no crash, header fields intact, no extension header recorded) on a next-header byte of 147 now that nothing maps it. Build: `python -m unittest` green on both new suites, the full `test_ipv6_ext_unit`, `test_const_registry_protocol` and `test_isort_clean`. A real vendor regeneration was not run against the live registry -- network crawls are disallowed in this environment -- so the committed const file is a hand-applied stand-in for that regeneration's output, verified only against the fixture text this issue's own investigation fetched. The owner should run the crawler for real once to confirm the live registry still matches that fixture. --- .../pcapkit/protocols/internet/ipv6_ext.rst | 7 +- pcapkit/const/ipv6/extension_header.py | 13 +- pcapkit/protocols/internet/ipv6.py | 32 +- pcapkit/protocols/internet/ipv6_ext.py | 30 +- pcapkit/vendor/ipv6/extension_header.py | 94 +++-- ...st_const_ipv6_extension_header_925_unit.py | 120 +++++++ .../protocols/internet/test_ipv6_ext_unit.py | 30 +- ...t_vendor_ipv6_extension_header_925_unit.py | 332 ++++++++++++++++++ 8 files changed, 578 insertions(+), 80 deletions(-) create mode 100644 tests/const/test_const_ipv6_extension_header_925_unit.py create mode 100644 tests/vendor/test_vendor_ipv6_extension_header_925_unit.py diff --git a/docs/source/pcapkit/protocols/internet/ipv6_ext.rst b/docs/source/pcapkit/protocols/internet/ipv6_ext.rst index 85feba9861..70f1b0bc2b 100644 --- a/docs/source/pcapkit/protocols/internet/ipv6_ext.rst +++ b/docs/source/pcapkit/protocols/internet/ipv6_ext.rst @@ -31,10 +31,9 @@ so those two octets are parseable without knowing anything else about the header. See the module docstring below for the closed exception table (``IPv6-Frag`` and ``AH`` each use their own length rule; ``ESP`` has a dedicated parser whose own info reports no next header rather than -lacking one, and ``BIT-EMU``, ``253`` and ``254`` have no dedicated -parser at all -- none of the four ever reaches this class), the two ways -this class is dispatched to, and why an overrun stops the walk instead of -clipping it. +lacking one, and ``253`` and ``254`` have no dedicated parser at all -- +none of the three ever reaches this class), the two ways this class is +dispatched to, and why an overrun stops the walk instead of clipping it. .. autoclass:: pcapkit.protocols.internet.ipv6_ext.IPv6_Ext :no-members: diff --git a/pcapkit/const/ipv6/extension_header.py b/pcapkit/const/ipv6/extension_header.py index 5d24d1b9dd..ee954f0709 100644 --- a/pcapkit/const/ipv6/extension_header.py +++ b/pcapkit/const/ipv6/extension_header.py @@ -23,13 +23,13 @@ class ExtensionHeader(EnumRegistry, IntEnum): #: HOPOPT, IPv6 Hop-by-Hop Option [:rfc:`8200`] HOPOPT = 0 - #: IPv6-Route, Routing Header for IPv6 [Steve Deering] + #: IPv6-Route, Routing Header for IPv6 [:rfc:`8200`][:rfc:`5095`] IPv6_Route = 43 - #: IPv6-Frag, Fragment Header for IPv6 [Steve Deering] + #: IPv6-Frag, Fragment Header for IPv6 [:rfc:`8200`] IPv6_Frag = 44 - #: ESP, Encap Security Payload [:rfc:`4303`] + #: ESP, Encapsulating Security Payload [:rfc:`4303`] ESP = 50 #: AH, Authentication Header [:rfc:`4302`] @@ -47,11 +47,8 @@ class ExtensionHeader(EnumRegistry, IntEnum): #: Shim6, Shim6 Protocol [:rfc:`5533`] Shim6 = 140 - #: BIT-EMU, Bit-stream Emulation [:rfc:`9801`] - BIT_EMU = 147 - - #: Use for experimentation and testing [:rfc:`3692`] + #: Use for experimentation and testing [:rfc:`3692`][:rfc:`4727`] Use_for_experimentation_and_testing_253 = 253 - #: Use for experimentation and testing [:rfc:`3692`] + #: Use for experimentation and testing [:rfc:`3692`][:rfc:`4727`] Use_for_experimentation_and_testing_254 = 254 diff --git a/pcapkit/protocols/internet/ipv6.py b/pcapkit/protocols/internet/ipv6.py index a0bbed7a38..4a6a14c8e7 100644 --- a/pcapkit/protocols/internet/ipv6.py +++ b/pcapkit/protocols/internet/ipv6.py @@ -82,15 +82,15 @@ class IPv6(IP[Data_IPv6, Schema_IPv6], #: :class:`IPv6_Ext` by *direct* registration instead (see the #: bottom of :mod:`pcapkit.protocols.internet.ipv6_ext`), which #: already produces exactly this class without needing this set to name - #: it. ``ESP``, ``BIT-EMU``, ``253`` and ``254`` are absent, but not for - #: the same reason as each other, and not because a generic fallback - #: would help them: + #: it. ``ESP``, ``253`` and ``254`` are absent, but not for the same + #: reason as each other, and not because a generic fallback would help + #: them: #: #: * ``ESP`` *does* have a dedicated, registered parser #: (:class:`~pcapkit.protocols.internet.esp.ESP`) -- it is excluded #: because :rfc:`4303` places the real Next Header byte inside the #: encrypted trailer, so its own info always *carries* a ``next`` - #: attribute (unlike ``BIT-EMU``/``253``/``254`` below), just one that is + #: attribute (unlike ``253``/``254`` below), just one that is #: :data:`None` whenever the payload could not be decrypted -- which, #: with no key material available to a generic parse, is always. The #: :meth:`_decode_next_layer` walk below still ends there, one iteration @@ -98,10 +98,16 @@ class IPv6(IP[Data_IPv6, Schema_IPv6], #: :class:`~pcapkit.const.ipv6.extension_header.ExtensionHeader`'s #: constructor at the top of the loop -- the *existing* end-of-chain #: path, unrelated to the structural check this set exists for. - #: * ``BIT-EMU``, ``253`` and ``254`` have no dedicated parser at all, so - #: they resolve to plain :class:`~pcapkit.protocols.misc.raw.Raw`, whose - #: info has no ``next`` *attribute* -- this is what the structural check - #: catches. + #: * ``253`` and ``254`` have no dedicated parser at all, so they resolve + #: to plain :class:`~pcapkit.protocols.misc.raw.Raw`, whose info has no + #: ``next`` *attribute* -- this is what the structural check catches. + #: (``BIT-EMU``/147 used to sit here too, until GitHub issue #925 found + #: it was never in IANA's authoritative extension-header registry to + #: begin with; :class:`~pcapkit.const.ipv6.extension_header + #: .ExtensionHeader` no longer carries it, so a next-header byte of 147 + #: now fails that same constructor at the *top* of the loop instead -- + #: the ordinary end-of-chain path any unrecognised upper-layer protocol + #: code already takes, one step earlier than it used to.) #: #: :meth:`_decode_next_layer`'s walk stops cleanly at whichever of these #: (or any other IANA code this package has not implemented) it meets, @@ -421,8 +427,8 @@ def _decode_next_layer(self, ipv6: 'Data_IPv6', proto: 'Optional[int]' = None, # which also carries ``next`` (possibly :data:`None`, on an # overrun -- see its module docstring). Every IANA extension # header code this package has not implemented a dedicated - # parser for -- today that is ``BIT-EMU``, ``253`` and ``254``, - # and tomorrow it is whatever IANA assigns next -- has no + # parser for -- today that is ``253`` and ``254``, and tomorrow + # it is whatever IANA assigns next -- has no # generic fallback either (see :attr:`__generic_ext_codes__`'s # docstring for why), so :meth:`_import_next_layer` returns a # plain :class:`~pcapkit.protocols.misc.raw.Raw`, whose info @@ -502,9 +508,9 @@ def _import_next_layer(self, proto: 'int', length: 'Optional[int]' = None, *, # method's own* behaviour for anything outside that closed set is exactly what it was before this method learned the substitution: a plain ``Raw`` for that one layer. What changed - for ``BIT-EMU``, ``253`` and ``254`` -- which have no dedicated - parser at all, so they were *already* reaching plain ``Raw`` - with no exception involved -- is one level up: + for ``253`` and ``254`` -- which have no dedicated parser at + all, so they were *already* reaching plain ``Raw`` with no + exception involved -- is one level up: :meth:`_decode_next_layer` now stops its walk structurally on any layer whose info carries no ``next`` attribute, ``Raw`` included, instead of reading ``info.next`` unconditionally and diff --git a/pcapkit/protocols/internet/ipv6_ext.py b/pcapkit/protocols/internet/ipv6_ext.py index 82446ffd75..df19d1b851 100644 --- a/pcapkit/protocols/internet/ipv6_ext.py +++ b/pcapkit/protocols/internet/ipv6_ext.py @@ -57,19 +57,19 @@ encrypted trailer (:rfc:`4303`), so its own info's ``next`` is :data:`None` rather than a value to continue on -``BIT-EMU``, ``253``, ``254`` terminal -- no dedicated parser exists, so no +``253``, ``254`` terminal -- no dedicated parser exists, so no next header field is ever read at all, either ======================================================= =========================================== -This table classifies by *wire format* alone, over the twelve codes -:class:`~pcapkit.const.ipv6.extension_header.ExtensionHeader` enumerates. -Note that IANA's own *IPv6 Extension Header Types* registry has **eleven**: -the twelfth, ``BIT_EMU`` (147), comes from this package generating that -enumeration out of the *Protocol Numbers* registry's extension-header -column instead, where 147 is flagged ``Y`` while the extension-header -registry omits it. That discrepancy is GitHub issue #925 and is not this -class's to resolve; the classification below holds either way, since 147 -is not reachable through here. +This table classifies by *wire format* alone, over the eleven codes +:class:`~pcapkit.const.ipv6.extension_header.ExtensionHeader` enumerates -- +matching IANA's own *IPv6 Extension Header Types* registry exactly, as of +GitHub issue #925. Before that fix this package generated the enumeration +out of the *Protocol Numbers* registry's extension-header column instead, +which carried a twelfth code, ``BIT_EMU`` (147), that the authoritative +registry does not; 147 was never reachable through this class either way +(it had no dedicated parser and was never generic-dispatched here), so +fixing the enumeration's source changed nothing this table classifies. ``Shim6`` conforms to it (:rfc:`5533`), but this package has never had a dedicated parser class for it to begin with -- see "Two entry paths" @@ -83,18 +83,18 @@ Next Header inside the encrypted trailer, with no length field anywhere in the cleartext part; and 253/254 are reserved for private experimentation (:rfc:`3692`) with no wire format at all. None of the -four is reachable through this class, by construction -- see +three is reachable through this class, by construction -- see :meth:`pcapkit.protocols.internet.ipv6.IPv6._import_next_layer`. Each of them still enters :meth:`IPv6._decode_next_layer `'s walk (it is a real :class:`~pcapkit.const.ipv6.extension_header.ExtensionHeader` member), -but the four part ways there: ``ESP`` resolves to its own dedicated parser, +but the three part ways there: ``ESP`` resolves to its own dedicated parser, whose info carries a ``next`` that is simply :data:`None` -- so the walk ends the ordinary way, ``ExtensionHeader(None)`` failing at the top of the -next iteration, exactly as it did before this class existed. ``BIT-EMU``, -``253`` and ``254`` have no dedicated parser and resolve to plain +next iteration, exactly as it did before this class existed. ``253`` and +``254`` have no dedicated parser and resolve to plain :class:`~pcapkit.protocols.misc.raw.Raw`, whose info has no ``next`` -*attribute* at all; for these three (and any future IANA code nobody has +*attribute* at all; for these two (and any future IANA code nobody has implemented yet) the walk stops on a *structural* check -- does the parsed layer carry a ``next`` at all? -- rather than on a list of codes. diff --git a/pcapkit/vendor/ipv6/extension_header.py b/pcapkit/vendor/ipv6/extension_header.py index fe4246fb54..0fc5bb202d 100644 --- a/pcapkit/vendor/ipv6/extension_header.py +++ b/pcapkit/vendor/ipv6/extension_header.py @@ -53,8 +53,53 @@ class {NAME}(EnumRegistry, IntEnum): class ExtensionHeader(Vendor): """IPv6 Extension Header Types""" + #: Keyword-style names carried over from this crawler's *previous* data + #: source, keyed by protocol number. The Protocol Numbers registry + #: (``protocol-numbers-1.csv``, this crawler's :attr:`LINK` before GitHub + #: issue #925) paired each of these headers with a short ``Keyword`` + #: column value, e.g. ``IPv6-Route`` for header 43. The registry + #: :attr:`LINK` now points at -- IANA's authoritative *IPv6 Extension + #: Header Types* registry -- has no such column, only a verbose + #: ``Description`` (``Routing Header for IPv6`` for the same header), so + #: deriving names the same way :meth:`process` does for every other + #: 3-column registry (see e.g. :class:`~pcapkit.vendor.ipv6.router_alert. + #: RouterAlert`) would silently rename these members. This mapping keeps + #: the existing, shorter names instead; any header not listed here still + #: falls back to a name derived from its description, same as always. + NAMES = { + 0: 'HOPOPT', + 43: 'IPv6-Route', + 44: 'IPv6-Frag', + 50: 'ESP', + 51: 'AH', + 60: 'IPv6-Opts', + 139: 'HIP', + 140: 'Shim6', + } # type: dict[int, str] + #: Link to registry. - LINK = 'https://www.iana.org/assignments/protocol-numbers/protocol-numbers-1.csv' + #: + #: .. note:: + #: + #: Until GitHub issue #925, this pointed at the *Protocol Numbers* + #: registry (``protocol-numbers/protocol-numbers-1.csv``), filtered on + #: its ``IPv6 Extension Header`` column -- a derived signal, not the + #: registry :rfc:`8200#section-4` names as authoritative for this + #: enumeration. That registry disagreed with this one on header 147 + #: (``BIT-EMU``): it flagged 147 as an IPv6 extension header, citing + #: :rfc:`9801`, while this registry omits 147 entirely -- see the issue + #: for the reading of :rfc:`9801` that makes the omission look + #: intentional rather than an erratum. Fixing :attr:`LINK` also let + #: :class:`~pcapkit.const.ipv6.extension_header.ExtensionHeader` drop + #: ``BIT_EMU``: :attr:`pcapkit.protocols.internet.ipv6.IPv6 + #: ._decode_next_layer`'s walk used to rely on ``ExtensionHeader(147)`` + #: resolving, but its own test (``tests.protocols.internet + #: .test_ipv6_ext_unit.IPv6ExtUnitTests + #: .test_unimplemented_terminal_code_stops_the_walk_not_the_packet``) + #: now exercises the identical code path on 253 -- a code this + #: registry *does* list -- so nothing outside this package depends on + #: 147 resolving any more. + LINK = 'https://www.iana.org/assignments/ipv6-parameters/extension-header.csv' def count(self, data: 'list[str]') -> 'Counter[str]': """Count field records. @@ -68,8 +113,9 @@ def count(self, data: 'list[str]') -> 'Counter[str]': """ reader = csv.reader(data) next(reader) # header - return collections.Counter(map(lambda item: self.safe_name(item[1] or item[2]), - filter(lambda item: len(item[0].split('-')) != 2, reader))) + return collections.Counter( + map(lambda item: self.safe_name(self.NAMES.get(int(item[0]), item[1])), + filter(lambda item: len(item[0].split('-')) != 2, reader))) def process(self, data: 'list[str]') -> 'tuple[list[str], list[str]]': """Process CSV data. @@ -87,12 +133,12 @@ def process(self, data: 'list[str]') -> 'tuple[list[str], list[str]]': enum = [] # type: list[str] miss = [] # type: list[str] for item in reader: - flag = item[3] - if flag != 'Y': - continue + code_str = item[0] + desc = item[1] + rfcs = item[2] - name = item[1] - rfcs = item[4] + keyword = self.NAMES.get(int(code_str)) if code_str.isdigit() else None + name = keyword or desc temp = [] # type: list[str] for rfc in filter(None, re.split(r'\[|\]', rfcs)): @@ -101,31 +147,17 @@ def process(self, data: 'list[str]') -> 'tuple[list[str], list[str]]': temp.append(f'[:rfc:`{rfc[3:]}`]') else: temp.append(f'[{rfc}]'.replace('_', ' ')) - lrfc = re.sub(r'( )( )*', ' ', f" {''.join(temp)}".replace('\n', ' ')) if rfcs else '' - - subd = re.sub(r'( )( )*', ' ', item[2].replace('\n', ' ')) - tmp1 = f' {subd}' if item[2] else '' - - split = name.split(' (', 1) - if len(split) == 2: - name, cmmt = split[0], f" ({split[1]}" - else: - name, cmmt = name, '' # pylint: disable=self-assigning-variable - - if name: - tmp1 = f',{tmp1}' if tmp1 else '' - else: - name, tmp1 = item[2], '' - desc = self.wrap_comment(f'{name}{tmp1}{lrfc}{cmmt}') + name_part = f'{keyword}, {desc}' if keyword else desc + comment = self.wrap_comment(re.sub(r'\r*\n', ' ', '%s %s' % ( # pylint: disable=consider-using-f-string + name_part, ''.join(temp) if rfcs else '', + ), flags=re.MULTILINE)) try: - code, _ = item[0], int(item[0]) - if not name: - name, desc = item[2], '' - renm = self.rename(name, code, original=item[1]) + code, _ = code_str, int(code_str) + renm = self.rename(name, code, original=keyword) pres = f"{renm} = {code}" - sufs = f"#: {desc}" + sufs = f"#: {comment}" #if len(pres) > 74: # sufs = f"\n{' '*80}{sufs}" @@ -133,10 +165,10 @@ def process(self, data: 'list[str]') -> 'tuple[list[str], list[str]]': #enum.append(f'{pres.ljust(76)}{sufs}') enum.append(f'{sufs}\n {pres}') except ValueError: - start, stop = item[0].split('-') + start, stop = code_str.split('-') miss.append(f'if {start} <= value <= {stop}:') - miss.append(f' #: {desc}') + miss.append(f' #: {comment}') miss.append(f" return extend_enum(cls, '{self.safe_name(name)}_%d' % value, value)") return enum, miss diff --git a/tests/const/test_const_ipv6_extension_header_925_unit.py b/tests/const/test_const_ipv6_extension_header_925_unit.py new file mode 100644 index 0000000000..6ab0271956 --- /dev/null +++ b/tests/const/test_const_ipv6_extension_header_925_unit.py @@ -0,0 +1,120 @@ +# -*- coding: utf-8 -*- +"""``ExtensionHeader`` drops ``BIT_EMU``, per GitHub issue #925. + +Issue #925's own diagnosis: :mod:`pcapkit.const.ipv6.extension_header` had 12 +members, but IANA's authoritative *IPv6 Extension Header Types* registry +(``ipv6-parameters/extension-header.csv``) has 11 -- the crawler used to be +sourced from the *Protocol Numbers* registry's ``IPv6 Extension Header`` +column instead, which disagrees with the authoritative registry on header +147 (``BIT-EMU``). :mod:`tests.vendor.test_vendor_ipv6_extension_header_925_unit` +fixes and pins that crawler, and its +``test_context_matches_the_committed_const_file_byte_for_byte`` proves this +file is exactly what the fixed crawler produces from the fixture the issue's +own investigation fetched. + +Removing ``BIT_EMU`` looked blocked at first: :meth:`pcapkit.protocols +.internet.ipv6.IPv6._decode_next_layer` resolves ``Enum_ExtensionHeader +(proto)`` at the top of its extension-header walk loop, wrapped in +``try/except ValueError: break``, and ``tests.protocols.internet +.test_ipv6_ext_unit.IPv6ExtUnitTests +.test_unimplemented_terminal_code_stops_the_walk_not_the_packet`` used to +build its packet around ``TransType.BIT_EMU``/``ExtensionHeader.BIT_EMU``. +But that test's own docstring says 147 was only ever the *example*: ``"253`` +/``254`` are the same code path (also unregistered, also resolve to +``Raw``)"``. Unlike 147, 253 *is* in the authoritative extension-header +registry, so that test now builds its packet around 253 instead -- +identical code path, still a real IANA extension-header code, and no longer +tied to a member this file is dropping. That is what frees ``BIT_EMU`` to +actually go. + +This suite is what is left to pin on the const side of that boundary: the +member count, ``BIT_EMU``'s absence, and that :meth:`pcapkit.protocols +.internet.ipv6.IPv6._decode_next_layer`'s walk still resolves a next-header +code no longer backed by any ``ExtensionHeader`` member -- 147 itself is +still a real :class:`~pcapkit.const.reg.transtype.TransType` value (it +legitimately belongs there; that enum is sourced from the still-correct +Protocol Numbers registry and is untouched by this fix) -- without raising +or losing the packet's own header fields. + +""" +from __future__ import annotations + +import io +import struct +import unittest + + +def _ipv6_bytes(next_code: 'int', ext_and_payload: 'bytes') -> 'bytes': + """A minimal IPv6 header (version 6, ``::1`` -> ``::1``) wrapping ``ext_and_payload``. + + Same construction as ``tests.protocols.internet.test_ipv6_ext_unit._ipv6_bytes``, + duplicated rather than imported so this suite does not depend on that + module's private helpers. + + """ + header = struct.pack('>IHBB', 6 << 28, len(ext_and_payload), next_code, 64) + header += (b'\x00' * 15 + b'\x01') * 2 # src = dst = ::1 + return header + ext_and_payload + + +class ExtensionHeaderBitEmuRemovedTests(unittest.TestCase): + """``BIT_EMU`` is gone, and the ``ipv6.py`` walk still handles 147 cleanly.""" + + def test_extension_header_now_has_exactly_eleven_members(self) -> None: + from pcapkit.const.ipv6.extension_header import ExtensionHeader + + names = [member.name for member in ExtensionHeader] + self.assertEqual(len(names), 11) + self.assertNotIn('BIT_EMU', names) + + def test_bit_emu_value_no_longer_resolves(self) -> None: + # The exact call pcapkit/protocols/internet/ipv6.py:378 makes at the + # top of its extension-header walk loop. With BIT_EMU gone and no + # _missing_ defined on this generated file (neither EnumLookup nor + # EnumRegistry in pcapkit.corekit.enum defines one either), 147 now + # raises plain ValueError instead of resolving. + from pcapkit.const.ipv6.extension_header import ExtensionHeader + + with self.assertRaises(ValueError): + ExtensionHeader(147) + + def test_transtype_still_carries_bit_emu(self) -> None: + # Confirms the scope of the fix: this is a different enumeration, + # correctly sourced from the (still valid) Protocol Numbers registry, + # and issue #925 never asked for it to change. + from pcapkit.const.reg.transtype import TransType + + self.assertEqual(int(TransType.BIT_EMU), 147) + + def test_ipv6_walk_stops_cleanly_on_the_code_bit_emu_used_to_occupy(self) -> None: + # The behaviour actually worth protecting, restated for 147 now that + # it is no longer an ExtensionHeader member: IPv6._decode_next_layer + # must not raise or lose the packet's own header just because a + # next-header code does not resolve as an extension header at all. + # ExtensionHeader(147) raising breaks the walk loop on its very + # first iteration -- the same graceful termination an ordinary + # upper-layer protocol code (UDP, TCP, ...) already takes -- so this + # packet ends up dispatched by TransType instead of being walked as + # an extension-header chain, and still parses without error. + from pcapkit.const.reg.transtype import TransType + from pcapkit.protocols.internet.ipv6 import IPv6 + from pcapkit.protocols.misc.raw import Raw + + raw = _ipv6_bytes(int(TransType.BIT_EMU), b'\x11\x01' + b'\x00' * 14) + ipv6 = IPv6(io.BytesIO(raw), len(raw)) + + # the header this whole #891 lineage of fixes exists to protect. + self.assertEqual(str(ipv6.src), '::1') + self.assertEqual(str(ipv6.dst), '::1') + + # no longer recorded as an extension header at all -- 147 is not one + # any more, so the walk loop's own top-of-loop check breaks + # immediately rather than ever reaching self._exthdr.add(...). + exthdrs = list(ipv6.extension_headers.items(multi=True)) + self.assertEqual(exthdrs, []) + + self.assertIsInstance(ipv6.payload, Raw) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/protocols/internet/test_ipv6_ext_unit.py b/tests/protocols/internet/test_ipv6_ext_unit.py index 260a51eedb..cfb28cba34 100644 --- a/tests/protocols/internet/test_ipv6_ext_unit.py +++ b/tests/protocols/internet/test_ipv6_ext_unit.py @@ -333,24 +333,36 @@ def test_shim6_registration_does_not_leak_into_ipv4_parsing(self) -> None: # -- collapse the whole packet ------------------------------------------- def test_unimplemented_terminal_code_stops_the_walk_not_the_packet(self) -> None: - """``BIT-EMU`` (147) has no dedicated parser and is not one of the - RFC 6564 conformers, so it resolves to plain :class:`Raw`, whose - info carries no ``next`` -- the exact #891 signature if the walk - read ``info.next`` on it unconditionally. The structural check in + """``253`` (``Use for experimentation and testing``, :rfc:`3692`) has + no dedicated parser in this package and is not one of the RFC 6564 + conformers, so it resolves to plain :class:`Raw`, whose info carries + no ``next`` -- the exact #891 signature if the walk read + ``info.next`` on it unconditionally. The structural check in :meth:`IPv6._decode_next_layer ` must stop there instead, keeping this packet's own header (source, destination, hop limit) intact rather than losing it to a further-out - ``@beholder``. ``253``/``254`` are the same code path (also - unregistered, also resolve to ``Raw``); this file covers one to keep - the test proportionate, per the review's own framing. + ``@beholder``. ``254`` is the same code path (also has no dedicated + parser, also resolves to ``Raw``); this file covers one to keep the + test proportionate, per the review's own framing. + + GitHub issue #925: this used to pick ``BIT-EMU`` (147) for the + example. Unlike 253/254, 147 turned out not to be in IANA's + authoritative *IPv6 Extension Header Types* registry at all -- it + leaked in from a stale cross-reference in the *Protocol Numbers* + registry -- and :class:`~pcapkit.const.ipv6.extension_header. + ExtensionHeader` no longer carries it. 253 exercises the identical + code path (an extension-header code this package has not + implemented a dedicated parser for) while remaining a code IANA + actually recognises as one, which 147 no longer is. """ from pcapkit.const.ipv6.extension_header import ExtensionHeader from pcapkit.const.reg.transtype import TransType from pcapkit.protocols.internet.ipv6 import IPv6 from pcapkit.protocols.misc.raw import Raw - raw = _ipv6_bytes(int(TransType.BIT_EMU), b'\x11\x01' + b'\x00' * 14) + code_253 = TransType.Use_for_experimentation_and_testing_253 + raw = _ipv6_bytes(int(code_253), b'\x11\x01' + b'\x00' * 14) ipv6 = IPv6(io.BytesIO(raw), len(raw)) # the header this class exists to protect -- lost entirely under the @@ -361,7 +373,7 @@ def test_unimplemented_terminal_code_stops_the_walk_not_the_packet(self) -> None exthdrs = list(ipv6.extension_headers.items(multi=True)) self.assertEqual(len(exthdrs), 1) code, terminal = exthdrs[0] - self.assertEqual(code, ExtensionHeader.BIT_EMU) + self.assertEqual(code, ExtensionHeader.Use_for_experimentation_and_testing_253) self.assertIsInstance(terminal, Raw) self.assertIsInstance(ipv6.payload, Raw) diff --git a/tests/vendor/test_vendor_ipv6_extension_header_925_unit.py b/tests/vendor/test_vendor_ipv6_extension_header_925_unit.py new file mode 100644 index 0000000000..273a27a025 --- /dev/null +++ b/tests/vendor/test_vendor_ipv6_extension_header_925_unit.py @@ -0,0 +1,332 @@ +# -*- coding: utf-8 -*- +"""Regression tests for GitHub issue #925. + +:mod:`pcapkit.vendor.ipv6.extension_header` used to crawl the *Protocol +Numbers* registry (``protocol-numbers/protocol-numbers-1.csv``), filtered on +its ``IPv6 Extension Header`` column -- a derived signal, not the registry +:rfc:`8200#section-4` names as authoritative for this enumeration. That +registry disagrees with the authoritative one on header 147 (``BIT-EMU``): +the Protocol Numbers registry flags it as an IPv6 extension header (citing +:rfc:`9801`), while IANA's *IPv6 Extension Header Types* registry +(``ipv6-parameters/extension-header.csv``) omits 147 entirely. The fix moves +:attr:`~pcapkit.vendor.ipv6.extension_header.ExtensionHeader.LINK` to the +authoritative registry and rewrites the parser for its 3-column shape +(``Protocol Number,Description,Reference`` -- no ``IPv6 Extension Header`` +flag to filter on). + +That registry's ``Description`` column is verbose (``Routing Header for +IPv6``), unlike the old registry's short ``Keyword`` column (``IPv6-Route``) +that :meth:`~pcapkit.vendor.ipv6.extension_header.ExtensionHeader.process` +used to derive most of the current member names from. Naively deriving names +from the new column would silently rename eight of the eleven surviving +members -- a breaking change far worse than the defect being fixed -- so the +crawler now carries an explicit +:attr:`~pcapkit.vendor.ipv6.extension_header.ExtensionHeader.NAMES` override +for exactly those eight, and :data:`EXPECTED_MEMBERS` below pins that no name +moves. + +This suite feeds the fixture CSV text directly to the crawler's own +:meth:`~pcapkit.vendor.default.Vendor.request`, :meth:`count`, :meth:`process` +and :meth:`context` -- the same pipeline :meth:`~pcapkit.vendor.default. +Vendor.__init__` would drive from a live fetch -- so the fix is exercised +without ever calling :mod:`requests`. Per the standing rule on this package, +no test here (or anywhere) invokes the crawler against the network. + +**The const half of the fix is now applied too.** Removing ``BIT_EMU`` looked +blocked at first: :meth:`pcapkit.protocols.internet.ipv6.IPv6 +._decode_next_layer`'s walk resolves ``Enum_ExtensionHeader(proto)`` at the +top of its loop and used to rely on that succeeding for 147. But that test's +own docstring says 147 was only ever the *example* -- ``"253``/``254`` are +the same code path (also unregistered, also resolve to ``Raw``)"`` -- and 253 +*is* in the authoritative registry where 147 never was, so +``tests.protocols.internet.test_ipv6_ext_unit +.IPv6ExtUnitTests.test_unimplemented_terminal_code_stops_the_walk_not_the_packet`` +now exercises the identical code path on 253 instead, which frees +:mod:`pcapkit.const.ipv6.extension_header` to drop ``BIT_EMU`` for real. +:mod:`tests.const.test_const_ipv6_extension_header_925_unit` pins that side; +:func:`test_context_matches_the_committed_const_file_byte_for_byte` below is +the seam between the two -- it proves the *committed* const file is exactly +what this fixture, run through the fixed crawler, produces. + +A real crawl against the live registry was still never run here -- the +standing rule on this repository disallows it. What this suite proves is +narrower and machine-checkable without one: feeding it the fixture text this +issue's own investigation fetched reproduces the committed const file +byte-for-byte. The owner should still run the crawler for real once, to +confirm the *live* registry matches the fixture -- IANA could have edited +the registry between the fetch and this PR landing. + +""" +from __future__ import annotations + +import importlib.util +import pathlib +import re +import unittest +from typing import TYPE_CHECKING + +from tests._support import purge_modules + +if TYPE_CHECKING: + from typing import Any + +#: Repository root, i.e. the grandparent of the directory holding this file. +ROOT = pathlib.Path(__file__).resolve().parents[2] + +#: Every distribution importing :mod:`pcapkit.vendor` needs -- see +#: :mod:`tests.vendor.test_ipx_packet_unit` for why all three are checked +#: rather than only :mod:`requests`. +VENDOR_DEPS = ('requests', 'bs4', 'html5lib') + +#: Whether the crawlers are importable at all; see +#: :mod:`tests.vendor.test_ipx_packet_unit` for the full rationale. +HAS_VENDOR_DEPS = all(importlib.util.find_spec(name) is not None for name in VENDOR_DEPS) + +#: The fixture the issue's own investigation fetched from +#: ``https://www.iana.org/assignments/ipv6-parameters/extension-header.csv`` +#: on 2026-09-29, with the header's line ending kept ``\\r\\n`` to match the +#: shape :meth:`~pcapkit.vendor.default.Vendor.request` splits a real HTTP +#: response on. +FIXTURE_CSV = ( + 'Protocol Number,Description,Reference\r\n' + '0,IPv6 Hop-by-Hop Option,[RFC8200]\r\n' + '43,Routing Header for IPv6,[RFC8200][RFC5095]\r\n' + '44,Fragment Header for IPv6,[RFC8200]\r\n' + '50,Encapsulating Security Payload,[RFC4303]\r\n' + '51,Authentication Header,[RFC4302]\r\n' + '60,Destination Options for IPv6,[RFC8200]\r\n' + '135,Mobility Header,[RFC6275]\r\n' + '139,Host Identity Protocol,[RFC7401]\r\n' + '140,Shim6 Protocol,[RFC5533]\r\n' + '253,Use for experimentation and testing,[RFC3692][RFC4727]\r\n' + '254,Use for experimentation and testing,[RFC3692][RFC4727]' +) + +#: Every member the fixed crawler is expected to produce from +#: :data:`FIXTURE_CSV`, as ``(name, value)`` in registry order -- exactly the +#: 11 non-``BIT_EMU`` members the committed +#: :class:`pcapkit.const.ipv6.extension_header.ExtensionHeader` now carries, +#: spelled out so a future change to :meth:`process` that renames one +#: silently fails here instead of shipping. +EXPECTED_MEMBERS = ( + ('HOPOPT', 0), + ('IPv6_Route', 43), + ('IPv6_Frag', 44), + ('ESP', 50), + ('AH', 51), + ('IPv6_Opts', 60), + ('Mobility_Header', 135), + ('HIP', 139), + ('Shim6', 140), + ('Use_for_experimentation_and_testing_253', 253), + ('Use_for_experimentation_and_testing_254', 254), +) + +#: Comments :data:`EXPECTED_MEMBERS` carried in +#: :mod:`pcapkit.const.ipv6.extension_header` *before* this fix (i.e. sourced +#: from the Protocol Numbers registry), for the six members whose text this +#: registry change does not disturb -- the other five (``IPv6_Route``, +#: ``IPv6_Frag``, ``ESP``, and both ``Use_for_experimentation_and_testing`` +#: members) draw genuinely different reference text from the new registry +#: (real RFC citations where the old one cited an author's name, or an extra +#: RFC 4727 citation), which is a known, reported divergence rather than a +#: parsing defect -- see the module docstring and this suite's +#: ``test_five_comments_genuinely_diverge_from_the_historical_registry``. The +#: committed const file now carries the *new* text for all 11, which is what +#: ``test_context_matches_the_committed_const_file_byte_for_byte`` pins. +UNCHANGED_COMMENTS = { + 'HOPOPT': 'HOPOPT, IPv6 Hop-by-Hop Option [:rfc:`8200`]', + 'AH': 'AH, Authentication Header [:rfc:`4302`]', + 'IPv6_Opts': 'IPv6-Opts, Destination Options for IPv6 [:rfc:`8200`]', + 'Mobility_Header': 'Mobility Header [:rfc:`6275`]', + 'HIP': 'HIP, Host Identity Protocol [:rfc:`7401`]', + 'Shim6': 'Shim6, Shim6 Protocol [:rfc:`5533`]', +} + + +def _members_from_enum(enum: 'list[str]') -> 'tuple[tuple[str, int], ...]': + """Extract ``(name, value)`` pairs from :meth:`Vendor.process`'s ``enum`` list.""" + out = [] # type: list[tuple[str, int]] + for entry in enum: + match = re.search(r'(\w+) = (\d+)', entry) + assert match is not None, f'could not parse enum entry: {entry!r}' + out.append((match.group(1), int(match.group(2)))) + return tuple(out) + + +def _comment_from_enum(enum: 'list[str]', name: 'str') -> 'str': + """Extract the ``#:`` comment text preceding the member named ``name``.""" + for entry in enum: + if re.search(rf'\b{re.escape(name)} = \d+', entry): + comment_line = entry.splitlines()[0] + assert comment_line.startswith('#: ') + return comment_line[len('#: '):] + raise AssertionError(f'no enum entry named {name!r}') + + +def _normalize(context: 'str') -> 'str': + """Apply the whitespace normalisation :meth:`Vendor.__init__` writes files + through, so a byte comparison against the committed file does the same + the generator itself would have -- see + :mod:`tests.vendor.test_ipx_packet_unit` for the same helper. + + Args: + context: Return value of :meth:`~pcapkit.vendor.default.Vendor.context`. + + Returns: + The text as the generator would have written it to disk. + + """ + lines = [] # type: list[str] + for line in context.splitlines(): + if line: + if line.strip(): + lines.append(line.rstrip()) + else: + lines.append(line) + return '\n'.join(lines) + '\n' + + +@unittest.skipUnless(HAS_VENDOR_DEPS, f'vendor extra not installed ({", ".join(VENDOR_DEPS)})') +class ExtensionHeaderVendorTests(unittest.TestCase): + """The crawler fix: correct registry, 3-column parser, names preserved.""" + + if TYPE_CHECKING: + vendor_module: 'Any' + + def setUp(self) -> None: + purge_modules(['pcapkit']) + + import pcapkit.vendor.ipv6.extension_header as vendor_module + + resolved = pathlib.Path(vendor_module.__file__).resolve() + if ROOT not in resolved.parents: + self.skipTest(f'{vendor_module.__name__} was imported from {resolved}, which is ' + f'outside {ROOT}; install this checkout with `pip install -e .` to ' + f'run this suite against it') + + self.vendor_module = vendor_module + + def _vendor(self) -> 'Any': + """A crawler instance with the attributes ``__init__`` would have set, + but without ``__init__``'s network fetch and file write -- see + :mod:`tests.vendor.test_ipx_packet_unit` for the same technique. + + """ + cls = self.vendor_module.ExtensionHeader + vendor = cls.__new__(cls) + vendor.NAME = cls.__name__ + vendor.DOCS = cls.__doc__ + lines = vendor.request(FIXTURE_CSV) + vendor.record = vendor.count(lines) + return vendor, lines + + def test_link_points_at_the_authoritative_registry(self) -> None: + # The root-cause fix, stated as an assertion: no longer the Protocol + # Numbers registry filtered on a derived flag. + link = self.vendor_module.ExtensionHeader.LINK + self.assertEqual(link, 'https://www.iana.org/assignments/ipv6-parameters/extension-header.csv') + self.assertNotIn('protocol-numbers', link) + + def test_fixture_splits_into_twelve_lines(self) -> None: + # Twelve lines: the header plus the eleven data rows -- confirms + # request()'s ``\r\n`` split is doing something on this fixture at + # all, before trusting count()/process() downstream of it. + vendor, lines = self._vendor() + self.assertEqual(len(lines), 12) + + def test_no_extension_header_type_is_lost_or_renamed(self) -> None: + # The count, and every surviving name, in one assertion: losing 147 + # (BIT_EMU) is the fix; losing or renaming any of the other 11 is not. + vendor, lines = self._vendor() + enum, miss = vendor.process(lines) + self.assertEqual(len(enum), 11) + self.assertEqual(_members_from_enum(enum), EXPECTED_MEMBERS) + self.assertEqual(miss, []) + + def test_bit_emu_is_absent_from_the_authoritative_registry_output(self) -> None: + vendor, lines = self._vendor() + enum, _ = vendor.process(lines) + names, values = zip(*_members_from_enum(enum)) + self.assertNotIn('BIT_EMU', names) + self.assertNotIn(147, values) + + def test_keyword_style_names_are_not_derived_from_the_verbose_description(self) -> None: + # Sanity check on the *mechanism*, not just the outcome: feeding the + # fixture's Description column through safe_name() directly (i.e. + # without the NAMES override) would NOT reproduce these names -- + # proving the override is doing real work rather than being a no-op + # that happens to agree with the default derivation. + vendor, lines = self._vendor() + naive = vendor.safe_name('Routing Header for IPv6') + self.assertNotEqual(naive, 'IPv6_Route') + self.assertEqual(naive, 'Routing_Header_for_IPv6') + + def test_six_comments_are_unchanged_from_the_historical_registry(self) -> None: + vendor, lines = self._vendor() + enum, _ = vendor.process(lines) + for name, expected_comment in UNCHANGED_COMMENTS.items(): + with self.subTest(name=name): + self.assertEqual(_comment_from_enum(enum, name), expected_comment) + + def test_five_comments_genuinely_diverge_from_the_historical_registry(self) -> None: + # Documents the divergence rather than hiding it: these five members' + # *comments* differ from the committed const file because the two + # registries carry different Reference/Description text for the same + # header -- not because the new parser mishandles them. Names and + # values still match EXPECTED_MEMBERS exactly (see the sibling test). + vendor, lines = self._vendor() + enum, _ = vendor.process(lines) + + self.assertEqual(_comment_from_enum(enum, 'IPv6_Route'), + 'IPv6-Route, Routing Header for IPv6 [:rfc:`8200`][:rfc:`5095`]') + self.assertNotEqual(_comment_from_enum(enum, 'IPv6_Route'), + 'IPv6-Route, Routing Header for IPv6 [Steve Deering]') + + self.assertEqual(_comment_from_enum(enum, 'IPv6_Frag'), + 'IPv6-Frag, Fragment Header for IPv6 [:rfc:`8200`]') + self.assertNotEqual(_comment_from_enum(enum, 'IPv6_Frag'), + 'IPv6-Frag, Fragment Header for IPv6 [Steve Deering]') + + self.assertEqual(_comment_from_enum(enum, 'ESP'), + 'ESP, Encapsulating Security Payload [:rfc:`4303`]') + self.assertNotEqual(_comment_from_enum(enum, 'ESP'), + 'ESP, Encap Security Payload [:rfc:`4303`]') + + for name in ('Use_for_experimentation_and_testing_253', 'Use_for_experimentation_and_testing_254'): + with self.subTest(name=name): + self.assertEqual(_comment_from_enum(enum, name), + 'Use for experimentation and testing [:rfc:`3692`][:rfc:`4727`]') + self.assertNotEqual(_comment_from_enum(enum, name), + 'Use for experimentation and testing [:rfc:`3692`]') + + def test_context_round_trips_through_the_full_pipeline(self) -> None: + # request() -> count() -> process() -> context(), the same sequence + # Vendor.__init__ drives from a live fetch, exercised end to end + # against the fixture with no network call anywhere in it. + vendor, lines = self._vendor() + generated = vendor.context(lines) + self.assertIn('class ExtensionHeader(EnumRegistry, IntEnum):', generated) + self.assertIn('HOPOPT = 0', generated) + self.assertNotIn('BIT_EMU', generated) + self.assertNotIn('147', generated) + + def test_context_matches_the_committed_const_file_byte_for_byte(self) -> None: + # The seam this fix rests on: pcapkit/const/ipv6/extension_header.py + # was hand-applied rather than produced by a live crawl (disallowed + # in this environment), so this is what stands in for "regenerating + # is a no-op" -- see tests.vendor.test_ipx_packet_unit for the same + # technique against a crawler with no LINK at all. Passing here does + # not prove the *live* registry still matches FIXTURE_CSV -- only + # that the committed file is exactly what this fixture produces. + vendor, lines = self._vendor() + generated = _normalize(vendor.context(lines)) + committed = (ROOT / 'pcapkit' / 'const' / 'ipv6' / 'extension_header.py').read_text(encoding='utf-8') + self.assertEqual(generated, committed, + 'pcapkit/const/ipv6/extension_header.py no longer matches what ' + 'ExtensionHeader.context() produces from FIXTURE_CSV; either the ' + 'hand-applied const file or this fixture is stale') + + +if __name__ == '__main__': + unittest.main()