Skip to content

fix(ipv6)!: rebuild a cut extension header chain byte-exact (#1471, #1473) - #1476

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1471-1473-ipv6-ext
Oct 9, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1471-1473-ipv6-ext

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 9, 2026 •

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • fix — corrects a defect
  • feat — adds a feature
  • perf — changes performance, not behaviour
  • refactor — changes neither behaviour nor performance
  • test — tests only
  • docs — documentation only
  • ci — workflows or build tooling
  • release — bumps the version or rolls up a distribution
  • chore — anything else

Description of your pull request and other information

Breaking (owner ruling on this PR): info.raw.packet changes when the upper layer is cut. Behind extension headers, it no longer repeats those headers: for a DST+TCP cut, it was 060001040000000000 and is now 00. For a decrypted ESP payload, the Raw fallback no longer includes the ESP trailer; it now matches Data_ESP.plaintext. The info keys, Raw.length and the protochain are unchanged.

tests/protocols/internet/test_ipv6_exthdr_cut_rebuild_unit.py covers every cut of 7 chains over IPv6 and Ethernet, checking the parse, from_data(info) and from_data(info.to_dict()). On origin/main sources: 429 failed, 2 passed, 1130 subtests passed. With the fix: 3 passed, 1558 subtests passed.

Also passing: the four edge modules, test_capture_roundtrip_runtime.py, run_unittest_leg.py protocols/internet (620 tests, 0 failures) and tests/project. The json and tree dumps of all 23 examples/captures are byte-identical to origin/main.

Closes #1471
Closes #1473

@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) review: running A cross-review is in flight against the current head - no verdict yet labels Oct 9, 2026
@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Coverage: 89.56% (unit tier, Python 3.14, fd77588a0, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18795 958 2356 871 90.99%
pcapkit/corekit 2107 93 672 27 94.60%
pcapkit/dumpkit 242 2 88 2 98.79%
pcapkit/foundation 2648 98 930 48 94.80%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 17056 239 4612 192 97.96%
pcapkit/toolkit 539 74 168 3 84.58%
pcapkit/utilities 431 4 124 4 98.56%
pcapkit/vendor 4409 2342 1006 157 43.25%

Per-file detail: the coverage-html artifact of this run.

@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at b59db0a3f. Cross-reviewed on Sonnet; the author ran on Opus. No defect found.

  • Cut chains: the reviewer built 25 chains of its own (Routing 0/2, SRH, Fragment, Mobility, repeated DST, ESP/AH, Shim6, stand-ins) and cut each at every offset, 1886 cases in all. Main has 1122 failures; the head has 0. Nothing that passed on main fails on the head.
  • Captures: 213,856 Ethernet cuts across the 23 captures give identical results on main and the head, with 0 failures. The to_dict() hashes per capture are identical too.

One visible value change, and the question it raises. info.raw.packet for a cut upper layer behind extension headers no longer repeats the headers. I measured a DST+TCP cut: 060001040000000000 on main, 00 on the head. Decrypted ESP is affected the same way: its Raw fallback drops the ESP trailer.

Not a blocker in its own right. Two follow-ups, neither needed for merge:

  • _exthdr_key relies on IPv6_Ext.alias.fget(None), which is safe only while that property ignores self. A module-level constant would remove that fragility.
  • An ESP test would pin the behaviour change.

@JarryShaw JarryShaw added needs: decision Waiting on the maintainer to decide — not blocked by other work review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 9, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Sounds like a breaking change? Yea update the body as well.

@JarryShaw JarryShaw changed the title fix(ipv6): rebuild a cut extension header chain byte-exact (#1471, #1473) fix(ipv6)!: rebuild a cut extension header chain byte-exact (#1471, #1473) Oct 9, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Recorded: the owner rules this a breaking change. I have added the ! to the title and the breaking label. The body now has a Breaking paragraph covering the raw.packet change behind extension headers and the decrypted-ESP case. The head is unchanged, so the GOOD TO GO at b59db0a3f stands.

@JarryShaw JarryShaw added breaking Breaks public-facing behaviour or API (apply alongside the type label) and removed needs: decision Waiting on the maintainer to decide — not blocked by other work labels Oct 9, 2026
)

- beholder: a Raw fallback re-reads the `payload` the method was given,
  not the whole layer payload, so an upper layer cut after IPv6
  extension headers no longer holds those headers a second time (#1471)
- IPv6._lookup_exthdr: an entry under IPv6_Ext's key rebuilds with
  IPv6_Ext rather than the dedicated parser for its code, so a header
  whose parser raised survives from_data(info.to_dict()) (#1473)
- IPv6._exthdr_key: one place for the info key of an extension header
- tests: every cut of seven chains over IPv6 and Ethernet, parse, info
  and dict rebuilds (424 of 1036 failed before)
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict carried to fd77588a0: GOOD TO GO. This is a pure rebase onto main at bbfd4bf64, after #1475 merged. range-diff shows the one commit as =, and the stable patch-id is unchanged (46c969fa378d).

@JarryShaw
JarryShaw force-pushed the fix/1471-1473-ipv6-ext branch from b59db0a to fd77588 Compare October 9, 2026 14:06
@JarryShaw
JarryShaw merged commit 62776b4 into main Oct 9, 2026
36 checks passed
@JarryShaw
JarryShaw deleted the fix/1471-1473-ipv6-ext branch October 9, 2026 17:39
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking Breaks public-facing behaviour or API (apply alongside the type label) fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

1 participant