Skip to content

fix(protocols): rebuilding a frame cut at the extension-chain bound warns once (#1619) - #1630

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1619-chain-bound-rebuild-warns-once
Oct 13, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1619-chain-bound-rebuild-warns-once

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

Please follow the guide below

What is the purpose of your pull request?

Tick the commit type your subject line carries.

  • 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

Closes #1619

  • Cause (confirmed). Each layer from_data rebuilds parses its own octets again. A constructed Internet layer started at _exthdr_depth = 0, so each rebuilt extension header counted its chain from itself, and each parse that reached the 32-header bound warned. A trace shows every nested HOPOPT warning with _rebuilding set.
  • Fix. Internet._make_payload (internet.py:336) hands the payload its place in the chain, one past this layer's, through _exthdr_rebuilt (:64). This mirrors how fix(protocols): bound a frame's dissection at 64 layers (#1610) #1620 carries _frame_depth. A constructed IPv6_Ext takes that place (:261, :315). _next_layer_class skips the warning while _rebuilding is set (:409), as _parse_next_layer already does for the frame bound.
  • Measured on Ethernet with HOPOPT or Destination Options chains, rebuilding via Ethernet.from_data(info.to_dict()). The cells are rebuild warnings, before → after; the parse gives 0 at 32 headers and 1 past that, both before and after:
Frame 32 33 100 1,000
IPv6 0 → 0 2 → 1 2 → 1 2 → 1
IPv4, protocol 0 0 → 0 2 → 1 33 → 1 33 → 1

In the mixed case the 33rd header is also the 65th layer: 31 IPv4 tunnels under Ethernet, or 30 in a PCAP frame. Its rebuild warnings drop from 33 to 1 for both carriers. On the IPv4 frame with 1,000 headers, the rebuild ran +123 Python frames deeper than the parse, built 1,121 Internet layers and took 155 ms. It now runs +2 deeper, builds 594 and takes 79 ms. Bytes and protocol chain are identical in every case.

…arns once (#1619)

- Each layer from_data rebuilds parses its own octets again, and a layer
  constructed rather than parsed started at _exthdr_depth 0, so each rebuilt
  extension header counted its chain afresh: its parse went up to 32 headers
  past the cut, and warned again on reaching EXTENSION_HEADER_LIMIT. A cut
  frame rebuilt with 2 warnings over IPv6 and up to 33 over IPv4, not 1.
- Internet._make_payload now hands the payload its place in the chain, one
  past this layer's, as #1620 hands it its place in the frame; a constructed
  IPv6_Ext takes it up, so the rebuild stops where the parse did.
- _next_layer_class does not warn of the bound inside a payload being
  rebuilt, as _parse_next_layer does not of FRAME_LAYER_LIMIT: the layer
  being rebuilt warns once, from its own parse.
@JarryShaw JarryShaw added the fix Pull requests that fix a defect (fix: subject prefix) label Oct 13, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at 16910f495.

Sonnet cross-review. It confirmed the cause: each rebuilt extension header restarted the chain count. It reproduced the fail-before (the new file fails with the fix reverted), and measured the IPv4 1,000-header rebuild at 155 ms → 76 ms, with 1,121 → 594 Internet layers built.

Non-blocking notes:

  • An explicit _exthdr_depth keyword on a constructed layer is dropped (internet.py:261). Nothing passes it.
  • The extension-header test is by class, so a user-registered non-IPv6_Ext class under an extension-header number costs extra rebuild work. The output does not change.

Not checked: ESP with a configured SA decrypting a chain of 33 or more headers, and the Sphinx build.

I re-ran the new test file at this head: 5 passed, 119 subtests.

@JarryShaw JarryShaw added the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 13, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 13, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 89.69% (unit tier, Python 3.14, 16910f495, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 3 20 3 95.16%
pcapkit/const 18858 954 2366 868 91.06%
pcapkit/corekit 2366 67 760 35 96.48%
pcapkit/dumpkit 275 3 100 3 98.40%
pcapkit/foundation 2884 145 1010 35 94.14%
pcapkit/interface 138 7 52 5 93.68%
pcapkit/protocols 17166 226 4580 191 98.06%
pcapkit/toolkit 1020 154 380 5 83.50%
pcapkit/utilities 494 4 146 4 98.75%
pcapkit/vendor 4409 2342 1006 157 43.25%

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

@JarryShaw
JarryShaw merged commit 4c5b057 into main Oct 13, 2026
38 checks passed
@JarryShaw
JarryShaw deleted the fix/1619-chain-bound-rebuild-warns-once branch October 13, 2026 01:41
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(protocols): rebuilding a frame cut at the extension-chain bound warns more than once

1 participant