Skip to content

fix(ipv6): parse unrecognised extension headers generically so the chain survives #891

Description

@JarryShaw

Describe the bug

When an IPv6 extension header fails to parse, @beholder substitutes a Raw — but IPv6's own chain-walk then reads .next off it, which Raw does not carry:

ipv6.py:329   info = next_.info      # next_ is whatever _import_next_layer returned
ipv6.py:338   proto = info.next      # AttributeError when next_ is the Raw fallback

The consequence is not confined to the extension header. The AttributeError escapes IPv6.read before IPv6's own info is returned, so the @beholder one level up turns the whole IPv6 packet into Raw — source, destination, hop limit and flow label lost along with the extension header.

Reproduction

Measured on main at 21b121c0e, and on PR #889's head, with PYTHONSAFEPATH=1 and pcapkit.__file__ asserted to the tree under test. A well-formed Mobility Header (an IPv6 extension header — Mobility_Header = 135 is in pcapkit/const/ipv6/extension_header.py:42) whose status byte is unassigned:

Ethernet/IPv6  assigned status   -> payload=IPv6 protochain=Ethernet:IPv6:MH
Ethernet/IPv6  unassigned status -> payload=Raw  protochain=Ethernet:Internet_Protocol_version_6
IPv6 alone     unassigned status -> AttributeError: 'Raw' object has no attribute 'next'

Note both IPv6 and MH are absent from the degraded protochain.

It is not specific to that enum. On unmodified main, two unrelated MH failure modes raise the identical AttributeError: a truncated MH (6 bytes) and an absurd Header Len of 0xff. So any extension-header parse failure reaches it.

Expected behavior

A failed extension header degrades to Raw in place, leaving the IPv6 header's own parsed fields intact and the rest of the chain walked as far as it can be — which is what @beholder exists to provide. At minimum the walk should stop cleanly on a Raw rather than raising AttributeError, since Raw is a value _import_next_layer is documented to return.

System information

  • Python 3.14.7, CPython, checkout at 21b121c0e.

Additional context

Surfaced while reviewing #889, which stops four mh.py helper enums from minting on an unassigned byte. #889 does not introduce this — it makes it far more reachable, turning a malformed-packet-only path into one a well-formed packet with an unrecognised status byte takes. It is out of scope there and #889 documents the real cost instead.

This also corrects a claim I made on #877 when recommending those enums raise: I said the cost was one MH message's parse. On the IPv6 path it is the whole IPv6 packet. See issues/877 for the correction.

Activity

  1. added
    bugIssues reporting a defect (set by the bug report template; a default, not an assessment)
    fixPull requests that fix a defect (fix: subject prefix)
    on Sep 28, 2026
  2. JarryShaw commented on Sep 29, 2026

    @JarryShaw
    OwnerAuthor

    One design decision needed before this is implemented, because "degrade in place" can mean three different things and they are not equivalent.

    The mechanism is settled and measured: ipv6.py:329 takes info = next_.info, :338 does proto = info.next, and a Raw fallback's info carries no .next, so the AttributeError escapes IPv6.read before IPv6's own info is returned and the outer @beholder replaces the whole packet. Confirmed pre-existing — a truncated extension header and an absurd Header Len both reach it on unmodified main.

    What is not settled is what should happen instead. Three candidates:

    (1) Stop the chain walk cleanly on a Raw. Treat a Raw from _import_next_layer as end-of-chain: keep IPv6's own parsed fields, attach the Raw as the final layer, stop walking. A capture with a bad Mobility Header would then read Ethernet:IPv6:Raw instead of Ethernet:Internet_Protocol_version_6, with source, destination, hop limit and flow label intact.

    (2) Let the walk continue past it. Would need Raw to carry a usable next, which it cannot — the whole reason it is a Raw is that the header did not parse, so the next-header byte is not trustworthy. I do not think this is viable and mention it only to rule it out explicitly.

    (3) Leave the wholesale degrade, and only stop it being an AttributeError. Catch it, log it, and let the packet become Raw as it does today — cheaper, but it keeps losing the IPv6 header fields, which is the actual damage.

    My recommendation is (1). It is what @beholder exists to provide — a bad layer costs that layer — and it is the only option where an operator inspecting a capture still sees the addresses. It does change observable output for malformed captures (protochain gains a Raw tail where the packet used to vanish into one), so it wants a breaking label and a test pinning the new shape.

    One consequence you should weigh, because it cuts the other way: #889 added a test asserting today's wholesale-degrade behaviour precisely so that fixing this would break it and force a look. Taking (1) means that test gets re-pointed rather than deleted — which is correct, but it means the shape lands in two places and the changelog entry for #889 will describe behaviour the library no longer has by then.

    Labelled needs: decision. Nothing dispatched until you rule.

  3. added
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Sep 29, 2026
  4. JarryShaw commented on Sep 29, 2026

    @JarryShaw
    OwnerAuthor

    But i think IPv6 extension headers share the same first few fields (including length) formats? Then we can actually have a RawExtension fallback that carries the unparsed payload as if a Raw protocol then next to the actual next one.

  5. JarryShaw commented on Sep 29, 2026

    @JarryShaw
    OwnerAuthor

    Good idea, and mostly right — but I measured the layouts and there are three exceptions that decide the design. Field order taken from the schemas on main:

    header proto first fields
    HOPOPT 0 next, len, options
    IPv6_Opts 60 next, len, options
    IPv6_Route 43 next, length, type, seg_left
    MH 135 next, length, type, reserved, chksum
    HIP 139 next, len, pkt, ver, …
    AH 51 next, len, reserved, spi, seq
    IPv6_Frag 44 next, reserved, flags, id — no length at all
    ESP 50 no next, no length — has neither
    Shim6 140 no schema in the tree

    So next at offset 0 holds for seven of the nine, which is what makes your RawExtension idea work — the walk can keep going, which is strictly better than my option (1) of stopping. But the length needed to skip to that next header is not uniform:

    • IPv6_Frag is fixed at 8 octets — byte 1 is reserved, not a length. A generic reader must special-case 44.
    • AH's len is in 4-octet units minus 2, not the 8-octet-units-minus-1 that HOPOPT/Opts/Route/MH/HIP use. Same field position, different arithmetic.
    • ESP has neither field — its next-header byte is inside the encrypted payload, so the chain genuinely ends there and no fallback can continue it.

    So the shape I would build, if you agree: RawExtension parses next at offset 0 and computes its own length from a per-protocol rule (8-octet units for the five, fixed 8 for Frag, 4-octet units for AH), carries the remainder as opaque bytes like Raw, and hands next back so ipv6.py:338's walk continues to the real next header. ESP stays a terminal — it already is, and cannot be otherwise. That gets an operator the IPv6 header, a marker for the unparsable extension, and every layer after it, which none of my three options delivered.

    Two things to decide, since they are yours not mine:

    1. Should RawExtension trust a length it cannot validate? If the bad header is bad because its length field is garbage, skipping by that length lands mid-packet and the next "header" is noise. Safer is to stop the walk when the computed length would overrun the remaining payload — so it continues where it can and stops where it cannot, rather than fabricating layers.
    2. protochain output changes either way — a bad Mobility Header currently makes the whole packet Raw, and would become something like Ethernet:IPv6:RawExtension:UDP. That is the point, but it is breaking for anyone matching on today's shape, and it means re-pointing the test fix(mh): stop the four RFC-inline helper enums from minting on unassigned bytes #889 added to pin the current behaviour.

    Confirm the shape and I will dispatch it. Keeping needs: decision until you do, since point 1 changes the implementation materially.

  6. JarryShaw commented on Sep 29, 2026

    @JarryShaw
    OwnerAuthor

    Let's re-read IPv6 extension header's RFC documentation? Maybe some of these values are not considered as extension headers? Maybe there have been a standardised format of how legit extension headers should contain/look like so that we can have our RawExtension (name subject to your recommendation based on the library's naming convention and future facing considerations).

    But if we really cannot find the trustworthy source or shared format for them, then we fall back to original recommendation on (1) - stop with a Raw containing whatever is remained.

  7. JarryShaw commented on Sep 29, 2026

    @JarryShaw
    OwnerAuthor

    Ruling recorded, verbatim:

    Let's re-read IPv6 extension header's RFC documentation? Maybe some of these values are not considered as extension headers? Maybe there have been a standardised format of how legit extension headers should contain/look like so that we can have our RawExtension (name subject to your recommendation based on the library's naming convention and future facing considerations).

    But if we really cannot find the trustworthy source or shared format for them, then we fall back to original recommendation on (1) - stop with a Raw containing whatever is remained.

    Right question to ask — my table was a census of this library's schemas, which is evidence about our code rather than about what a legitimate extension header is. Investigating against the specifications, with the fallback you named as the default if the answer comes back "no trustworthy shared format".

    Four things being established, each to be answered with quoted text rather than summary:

    • The authoritative list. IANA's Protocol Numbers registry carries an explicit IPv6 Extension Header column, so the question "are some of these not extension headers?" has a published answer rather than a judgement. That also catches anything the library is missing, which the census could not.
    • Whether a standardised format exists. RFC 8200 §4.8, Defining New Extension Headers and Options, is the section most likely to settle it — whether the layout requirement is normative, and whether it binds the headers defined before RFC 8200 or only new ones.
    • The three non-conformers individually, from their own specs: IPv6_Frag's fixed 8 octets (RFC 8200 §4.5), AH's Payload Length unit and bias (RFC 4302, to be read rather than recalled), and ESP's next-header byte living inside the encrypted payload (RFC 4303). Plus Mobility_Header, HIP, Shim6, BIT_EMU and whether 253/254 are extension headers at all or merely reserved protocol numbers.
    • The decisive test, stated so it can come back negative: can a generic reader handed an unparsable extension header reliably determine both the next-header value and how many octets to skip? If that is "yes for the conforming set, with a small closed set of special cases", RawExtension is buildable. If not, your fallback applies.

    Name deferred to the same investigation, as you asked — against the library's own conventions and where it should live (protocols/internet/ beside the other extension headers, or protocols/misc/ beside Raw).

    Keeping needs: decision until the findings are in, since they decide which design you are choosing between.

  8. JarryShaw commented on Sep 29, 2026

    @JarryShaw
    OwnerAuthor

    Findings in, and your instinct was right — there is a trustworthy shared format, so the fallback is stricter than the evidence requires. It is RFC 6564, not RFC 8200 §4.8.

    1. IANA says all twelve are extension headers, so that question is settled and not in our favour. The Protocol Numbers registry has an explicit column, and the Y rows are an exact element-for-element match with pcapkit/const/ipv6/extension_header.py — nothing extra, nothing missing. Unsurprising once you look: the module is generated from that column. (The CSV is protocol-numbers-1.csv; the unsuffixed URL 404s.)

    0 HOPOPT   43 IPv6-Route   44 IPv6-Frag   50 ESP   51 AH   60 IPv6-Opts
    135 Mobility Header   139 HIP   140 Shim6   147 BIT-EMU   253   254
    

    2. But the RFCs do narrow it, and the format requirement is a real MUST. RFC 6564, A Uniform Format for IPv6 Extension Headers, Standards Track, Updates 2460, RFC 2119 keywords declared. §4, verbatim:

    Any IPv6 extension headers defined in the future … MUST use the consistent format defined in Figure 1.

    Hdr Ext Len 8-bit unsigned integer. Length of the extension header in 8-octet units, not including the first 8 octets.

    And §5 answers the retroactivity question outright, verbatim:

    The scheme proposed in this document is not intended to be backward compatible with all the currently defined IPv6 extension headers. It applies only to newly defined extension headers. Specifically, the fragment header predates this document and does not follow the format proposed in this document.

    Cite RFC 6564 §4, not RFC 8200 §4.8 — I had pointed at the wrong section. RFC 8200 §4.8 restates the layout without an RFC 2119 keyword ("they need to use the following format") and its field description carries a copy-paste defect, describing the generic length field as "Length of the Destination Options header".

    3. So the answer to your decisive question is "yes, with a closed six-entry exception table":

    next header skip length
    HOPOPT 0, IPv6_Route 43, IPv6_Opts 60, MH 135, HIP 139, Shim6 140 octet 0 (octet1 + 1) * 8
    IPv6_Frag 44 octet 0 constant 8 — octet 1 is Reserved (RFC 8200 §4.5)
    AH 51 octet 0 (octet1 + 2) * 4 — RFC 4302 §2.2 flags its own deviation
    ESP 50, BIT_EMU 147, 253, 254 no no — terminal

    The four terminals are terminal for stated reasons, not by inference: RFC 8200 §4.5 says "the Encapsulating Security Payload (ESP) is not considered an extension header" and RFC 4303 puts its Next Header in a trailer after a variable-length encrypted payload with no length field anywhere; RFC 9801 defines 147 as a payload type ("the payload is an emulated bit-stream") with no header layout at all, making IANA's Y a filing artefact; and RFC 3692 assigns 253/254 for local experiment with no wire format by design.

    Recommendation: build it. Six conformers today, plus every extension header standardised from April 2012 onward by the MUST — which is precisely the population a generic fallback exists to serve. Two need a documented per-value length rule, four terminate into a plain Raw. That is enumerable and spec-cited, not heuristic. So: stop the walk, but only at those four values.

    Name: RawExtension, in pcapkit/protocols/internet/raw_extension.py, subclassing Internet — your name, kept. The argument for the location is that Raw is defined by refusing to continue the chain: protocol raises UnsupportedCall, length is hardcoded 0, _next is always NoPayload(). RawExtension's whole purpose is the opposite — to yield a real next-header value and a real octet length. It belongs beside the extension headers it stands in for. It should still follow Raw's precedent of raising UnsupportedCall from __index__, having no single protocol number.

    Two things still yours to say before I dispatch: whether it should refuse to skip when the computed length would overrun the remaining payload (I would — a garbage length field is the likeliest reason the header failed to parse, and skipping by it fabricates layers), and confirmation that protochain changing from Ethernet:Internet_Protocol_version_6 to Ethernet:IPv6:RawExtension:UDP is breaking and re-points the test #889 added to pin today's behaviour.

  9. JarryShaw commented on Sep 29, 2026

    @JarryShaw
    OwnerAuthor

    whether it should refuse to skip when the computed length would overrun the remaining payload

    yes, it should, or actually i think the current existing workflow or house convention already handles/covers that.

    confirmation that protochain changing from Ethernet:Internet_Protocol_version_6 to Ethernet:IPv6:RawExtension:UDP is breaking

    yes confirmed - the RawExtension name is still up to your recommendations, as well as where it should be housed.

  10. JarryShaw commented on Sep 29, 2026

    @JarryShaw
    OwnerAuthor

    Ruling recorded, verbatim:

    whether it should refuse to skip when the computed length would overrun the remaining payload

    yes, it should, or actually i think the current existing workflow or house convention already handles/covers that.

    confirmation that protochain changing from Ethernet:Internet_Protocol_version_6 to Ethernet:IPv6:RawExtension:UDP is breaking

    yes confirmed - the RawExtension name is still up to your recommendations, as well as where it should be housed.

    You are right that a convention already exists, and it is worth naming precisely because it does not quite cover this case. The house pattern is warn-and-clip: emit a SchemaWarning saying what was declared against what remains, then read what remains.

    pcapkit/protocols/schema/misc/pcapng.py:373   "option declares {nominal} octet(s) of payload with
                                                   {remaining} octet(s) left in its area; reading {remaining}"
    pcapkit/protocols/schema/misc/pcapng.py:443   the same shape for a block's option area
    pcapkit/corekit/fields/field.py:507           "Field {name} declares a length of {length} octet(s), ..."
    pcapkit/protocols/misc/pcapng.py:1119         "{length} exceeds the {read} octet(s) left in ..."
    

    The gap: clipping a payload and clipping a skip-distance are different operations. Clipping is right when the declared length only governs how much of this header to read — you take what is there and carry on. But RawExtension's length also decides where the next header starts, and a clipped skip-distance points at whatever happens to sit at the end of the buffer, which is not a header. Continuing from there fabricates a layer out of trailing bytes, which is worse than stopping: a wrong protochain entry reads as a parsed fact.

    So the implementation will follow the convention's warning and diverge on the action: emit a SchemaWarning in the established wording, then stop the walk rather than clip-and-continue — RawExtension absorbs the remaining octets and reports no next header. The chain ends honestly at the bad header instead of inventing what follows it. I will say so in the docstring, citing the convention so the divergence reads as deliberate rather than as an oversight.

    Name and location, per your deferral: RawExtension, in pcapkit/protocols/internet/raw_extension.py, subclassing Internet. Raw is defined by refusing to continue a chain — protocol raises UnsupportedCall, length is hardcoded 0, _next is always NoPayload(). RawExtension exists to do the opposite for a conforming header, so it belongs beside the extension headers it substitutes for rather than beside Raw. It will follow Raw's precedent of raising UnsupportedCall from __index__, having no single protocol number of its own.

    needs: decision removed, breaking and wip applied, worker dispatched. Scope: the six RFC 6564 conformers walk generically, IPv6_Frag uses a constant 8 and AH uses (len + 2) * 4, and ESP/BIT_EMU/253/254 terminate into a plain Raw as they do today.

  11. added
    breakingBreaks public-facing behaviour or API (apply alongside the type label)
    wipWork in flight - a covering PR is open or an agent is actively on it
    and removed
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Sep 29, 2026
  12. JarryShaw commented on Sep 29, 2026

    @JarryShaw
    OwnerAuthor

    Confused tho. Now that we confirmed that IPv6 extension headers should have the uniformed format, why dont we make RawExtension dynamic, meaning, it reads and parses according to the shared format then its property is still able to return the necessary information as index, next, length, protocol (derived from the actual Enum value), etc.?

    RawExtension name may sound ambiguous - it's not directly pointing to IPv6 extension headers.

  13. 1 remaining item

  14. changed the title [-]fix(ipv6): a Raw fallback for a failed extension header makes the whole IPv6 packet degrade to Raw[/-] [+]fix(ipv6): parse unrecognised extension headers generically so the chain survives[/+] on Sep 29, 2026
  15. JarryShaw commented on Sep 29, 2026

    @JarryShaw
    OwnerAuthor

    What if we make it something like IPv6_Ext and have every other known implemented extension headers subclass it. So that we can sort of have the shared logic and common methodology that's being used in the library's house convention.

  16. JarryShaw commented on Sep 29, 2026

    @JarryShaw
    OwnerAuthor

    Measured your IPv6_Ext proposal. Build it — but the rationale you gave for it is the weakest one available, and that changes what the base should contain.

    What if we make it something like IPv6_Ext and have every other known implemented extension headers subclass it.

    There is substantial shared logic: ~348 duplicated source lines collapsing to ~55, a net ~290 removed. But it is not the RFC 6564 header format. It is the extension-mode contract — the extension= keyword, the self._extf flag, the three UnsupportedCall guard properties, and the if extension: return info short-circuit. AST-normalised comparison of the class bodies:

    protocol      identical 7/7   70 lines -> 10     usable unchanged by 7/7
    protochain    identical 7/7   70 -> 10                             7/7
    payload       identical 7/7   74 -> 11                             7/7
    __post_init__ identical 6/7  116 -> 21                             7/7
    length        identical 6/7   18 ->  3            6/7 (IPv6_Frag overrides)
    _make_data / make / name / alias / __index__      hoistable: 0/7
    

    The format you named is shared by only 5 of 7, and it is the one thing I would not build the base around. IPv6_Frag has no length field at all (constant 8), AH uses (len + 2) * 4 per RFC 4302, and ESP is terminal. Worse, the 5 that do share it disagree on the schema field name — len in HOPOPT/IPv6_Opts/HIP, length in IPv6_Route/MH. A base designed around a length field two members do not have starts by papering over a rename. A base designed around the extension-mode contract covers all 7 and fixes three live inconsistencies.

    Two corrections to things I told you earlier, both mine:

    1. Shim6 is not implemented at all. pcapkit/protocols/internet/shim6.py does not exist; there is a 0-byte NotImplemented/shim6.py, excluded from the wheel by MANIFEST.in:90, with a TODO at internet/__init__.py:12. So there are 7 implemented extension headers, not 8, and the generic-conformer set is 5, not 6. I had listed Shim6 in the exception table I gave the worker; corrected. It is actually a good test case for the generic parser — a registered IANA header with no class behind it.
    2. AH subclassing IPsec is not an obstacle, as I implied it was. IPsec is 48 lines providing exactly one member, id(), which both AH and ESP override anyway. All four parenting shapes linearise with zero MRO conflict, verified live. AH(IPsec, IPv6_Ext) is the right one — re-parenting IPsec itself would drag ESP into a base whose contract it cannot honour.

    Breakage is near-nil and I checked rather than assumed: 0 MRO conflicts, 0 protochain string changes (chains derive from alias → concrete class name, which does not move), 0 dispatch-table changes (Internet.__proto__ holds ModuleDescriptor string pairs, resolved lazily), 0 pickling impact, 0 real isinstance/issubclass call sites in pcapkit/. One concrete casualty: tests/protocols/internet/test_ah_unit.py:111 patches IPsec.__post_init__, which stops intercepting once __post_init__ hoists — that test needs rewriting. None of the 7 classes, schemas or data models is generated, so regeneration is unaffected.

    Two PRs, and the ordering matters more than the split. The #891 fix creates the 8th member of this family — IPv6_GenericExt is precisely a subclass of the base you are proposing. Designing IPv6_Ext against 7 members and then retrofitting the 8th is worse than letting the 8th exist first. Also the proof obligations are opposite: #891 must ship a test that fails without it, while the refactor's correctness argument is "the diff only moves lines and the suite is unchanged" — merge them and that argument is unavailable. I will file the refactor as its own issue once #891 lands, unless you would rather have it first, in which case refactor-only goes first and the generic parser lands as a subclass afterwards. What I would argue against is one merged PR either way.

    Name clash to settle: IPv6_Ext collides with the in-flight ipv6_generic_ext.py and with the ExtensionHeader enum in pcapkit/const/ipv6/extension_header.py. Suggest IPv6_Ext for the base and keeping IPv6_GenericExt for the concrete generic parser — base names the family, subclass names the mechanism. Unverified and worth settling before either lands: whether mypy/pylint in CI accept two identically-parameterised generic bases on AH.

  17. JarryShaw commented on Sep 29, 2026

    @JarryShaw
    OwnerAuthor

    If we double inherit AH, we should do it for ESP as well i think. Like all IPv6 extension headers should get the same base class of IPv6_Ext which contains the shared contracts for extension headers - despite some of these headers can be standalone protocols by themeless. Or to make it simpler, we only use IPv6_Ext as base class for those headers that cannot stand alone?

  18. JarryShaw commented on Sep 29, 2026

    @JarryShaw
    OwnerAuthor

    Measured both options. Neither boundary is "standalone vs not" — and ESP turns out to be a different case than you or I assumed.

    The contract markers, counted per file:

                    _extf   UnsupportedCall   if extension: return
    HOPOPT            4            7                   1
    IPv6_Route        4            7                   1
    IPv6_Opts         4            7                   1
    IPv6_Frag         4            7                   1
    MH                4            7                   1
    HIP               4            7                   1
    AH                4            7                   1
    ESP               1            0                   0
    

    7 of 8 implement the contract in full. ESP stores the flag and then ignores it. esp.py:1276 does self._extf = extension and nothing ever reads it — compare ah.py:75,87,99, three if self._extf: guards. ESP also has zero UnsupportedCall guard properties and no if extension: return short-circuit. So ESP(..., extension=True) today records the flag and behaves exactly as if it were False. That is a latent defect independent of this refactor, and I am filing it separately.

    So your option 2 — "only headers that cannot stand alone" — is the weaker one, which is the opposite of what I would have guessed. It would exclude AH, MH and HIP, all three of which are arguably standalone protocols and all three of which implement the contract completely. That cuts the base's coverage from 7 to 4 while removing nothing structural. The line you are reaching for is not "can it stand alone" but "does it participate in extension mode", and those two do not coincide.

    On the symmetry instinct — right in principle, and the facts do not support it here.

    If we double inherit AH, we should do it for ESP as well i think

    They are not symmetric: AH honours the contract and ESP does not. Giving ESP the base would inherit the three UnsupportedCall guards and the if extension: return it currently lacks — which is to say it would fix ESP, not merely re-parent it. That is a behaviour change, and smuggling it inside a refactor whose whole correctness argument is "the diff only moves lines" destroys that argument. Same reason I argued against merging #891 with the refactor.

    Recommendation: IPv6_Ext covers the 7 that implement the contract today, AH via AH(IPsec, IPv6_Ext) as you suggested. ESP stays out of the refactor and gets its own decision: should ESP support extension mode? If yes, it joins the base in that change and the dead _extf is fixed by construction. If no, the extension= keyword should be removed from its signature rather than accepted and discarded.

    That keeps the refactor provably behaviour-preserving and puts the ESP question where it can be answered on its merits instead of as a side effect.

  19. JarryShaw commented on Sep 29, 2026

    @JarryShaw
    OwnerAuthor

    If per IANA, ESP is an extension header, then it MUST honour the same contracts - that's an implementation defect. And thus we should have ESP double inherit as AH then.

  20. JarryShaw commented on Sep 29, 2026

    @JarryShaw
    OwnerAuthor

    Ruling recorded, verbatim, and the condition is satisfied — I checked IANA directly:

    If per IANA, ESP is an extension header, then it MUST honour the same contracts - that's an implementation defect. And thus we should have ESP double inherit as AH then.

    IANA's protocol-numbers-1.csv, the IPv6 Extension Header column:

    Decimal,Keyword,Protocol,IPv6 Extension Header,Reference
    50,ESP,Encap Security Payload,Y,[RFC4303]
    51,AH,Authentication Header,Y,[RFC4302]
    

    Y for both. And this library already agrees — pcapkit/const/ipv6/extension_header.py:33 lists ESP = 50 in its own IANA-derived extension-header registry. So the inconsistency is internal: the registry says ESP is an extension header while the ESP class accepts extension=, stores it at esp.py:1276, and never reads it. That is a sharper statement of the defect than I made earlier.

    So your ruling stands and it resolves the tension I raised. Consequences:

    Unchanged: #891 itself stays the narrow fix (the generic parser so the chain survives), and the IPv6_Ext base remains a separate follow-up filed after #891 lands.

    One incidental from the same CSV, consistent with the earlier Shim6 finding: 140,Shim6,Shim6 Protocol,Y,[RFC5533] is a registered extension header with no implementation here, which is exactly the case the generic parser is for.

  21. JarryShaw commented on Sep 29, 2026

    @JarryShaw
    OwnerAuthor

    Correcting the record on this issue, because a ruling you gave rests on a premise I got wrong.

    The exception table I gave you and the implementer said:

    ESP, BIT_EMU, 253, 254 — terminal, no next header

    and described those as keeping "degrading to a plain Raw exactly as they do today". You then ruled, verbatim:

    ESP/BIT_EMU/253/254 terminate into a plain Raw as they do today.

    That is correct about the wire format and wrong about this codebase. Measured on origin/main: only ESP is implemented terminally — it parses cleanly to Ethernet:IPv6:ESP. For 147 (BIT_EMU), 253 and 254, "as they do today" means the whole IPv6 packet collapses — which is the defect this issue exists to fix, not a benign terminal state.

    The mechanism, verified myself rather than relayed:

    • All three are Enum_ExtensionHeader members — BIT_EMU: 147, Use_for_experimentation_and_testing_253, …_254 — so they enter the extension-header walk at pcapkit/protocols/internet/ipv6.py:315.
    • None has an Internet.__proto__ entry (measured None for each), so _lookup_next_layer returns Raw.
    • Raw has no next, so proto = info.next at :338 raises AttributeError: 'Raw' object has no attribute 'next'.

    Result on PR #904 as it stands: code 140 is fixed, and 147/253/254 still produce payload=Raw, protochain=Ethernet:Internet_Protocol_version_6 — the exact #891 signature quoted in this issue's own body. Worse, my framing led the implementer to document that as intended, at ipv6_generic_ext.py:51-54 and ipv6.py:445-446.

    So the table had a real consequence: it told the implementer to preserve the bug for three of the twelve codes, and they faithfully did. This issue's own Expected behavior — "At minimum the walk should stop cleanly on a Raw rather than raising AttributeError" — is unmet for those three.

    What I am briefing unless you rule otherwise: make the walk break when the layer it just decoded has no next, rather than special-casing a code list. That satisfies the terminal semantics you ruled for, fixes all three codes, and needs no table at all — which is the right shape, since a table of codes is exactly what was wrong here. The two docstrings get corrected to say the walk stops at these headers rather than that the packet degrades.

    Corrected table for the record, with the implemented-versus-conforming distinction that mine elided:

    header(s) wire format implemented behaviour on main
    HOPOPT, IPv6_Route, IPv6_Opts, MH, HIP (octet1 + 1) * 8 dedicated parser
    IPv6_Frag constant 8 dedicated parser
    AH (octet1 + 2) * 4 (RFC 4302) dedicated parser
    ESP terminal dedicated parser, terminates cleanly
    Shim6 (140) conforms (RFC 5533) no parser — now handled generically by #904
    BIT_EMU (147), 253, 254 terminal no parser — currently collapses the packet
  22. removed
    wipWork in flight - a covering PR is open or an agent is actively on it
    on Sep 29, 2026
  23. added this to the 1.5 milestone on Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    breakingBreaks public-facing behaviour or API (apply alongside the type label)bugIssues reporting a defect (set by the bug report template; a default, not an assessment)fixPull requests that fix a defect (fix: subject prefix)

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions