Skip to content

fix(toolkit): the default engine reads TCP under an IPv6 header of Version 4 (#1666) - #1675

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1666-tcp-segment-picks-branch-by-layer
Oct 13, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1666-tcp-segment-picks-branch-by-layer

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

Please follow the guide below

  • You will be asked some questions, please read them carefully and answer honestly

  • Put an x into all the boxes [ ] relevant to your pull request (like that [x])

  • Use Preview tab to see how your pull request will actually look like

  • Searched for similar pull requests

  • Followed the coding style (make pylint, make mypy, make isort)

  • make test passes, and a test case covers the change (the toolkit, traceflow and project tests near the change, file by file; not the whole suite)

  • Added a changelog entry under docs/source/changelog/ and regenerated CHANGELOG.md, if the change is user-visible: N/A — centralised in docs(changelog): shared 1.5.0 changelog — long-lived, merges last (#610, #616, #617, #618, #620) #657

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

tcp_segment (pcapkit/toolkit/pcap.py) chose the IPv4 branch of its later-fragment check by the header's Version field. IPv6.read keeps any Version, so an IPv6 layer with Version 4 carrying a rejected TCP header read IPv4's offset, and the AttributeError ended the extraction. The check now uses isinstance(ip, IPv4). The PCAP-NG adapters call the same function, so pcapng.py is unchanged. The default engine's toolkit has no other .version == dispatch.

Measured on 3.14.8 and 3.11.15. The cases are the issue's 16 rows with every variant, plus 15 controls, giving 38 frame cases. Each was read as PCAP and as PCAP-NG with every engine that runs, and every TCP-reassembly, trace, IPv4-reassembly and IPv6-reassembly input was recorded:

  • The only cells that change are the default engine's on rows 1-6 and on three more shapes: an H4 inside an H4, an H4 first fragment, and an H4 followed by a Hop-by-Hop header (H4 = an IPv6 header with Version 4). Each goes from AttributeError to inputs equal to dpkt's and scapy's, field for field.
  • Rows 7-16 and every other control are unchanged for every engine. Applying fix(toolkit): scapy walks link types, layer bounds and IPv6 extension headers as the default engine does (#1609, #1626, #1650) #1663 on top changes no cell (measured on 3.14).
  • These disagreements remain and are not fixed here. pypcapfile has no IPv6 decoder, and tshark stops with "Bogus IPv6 version", so neither engine reads these frames. They already behaved the same way on rows 7-8.

IPv6.read is left alone. I patched it at runtime only, to reject a Version other than 6. The default engine then hands over no inputs on rows 1-8 and 15, or on the H4 controls, while dpkt and scapy still hand them over.

The new test fails on main (2 failed, AttributeError) and passes with the fix (2 passed, 24 subtests), on both interpreters.

Closes #1666

…rsion 4 (#1666)

tcp_segment took the IPv4 branch of its later-fragment check by the
header's Version field, but IPv6.read keeps whatever Version it reads.
An IPv6 layer with Version 4 carrying a TCP header the parser rejected
therefore read IPv4's `offset`, and the AttributeError ended the
extraction, PCAP and PCAP-NG alike. The branch now goes by the layer's
class, so those frames hand over the inputs dpkt and scapy already do.

Closes #1666
@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) review: good-to-go Cross-review at the current head says ready; CI state is separate labels Oct 13, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at 1533afb43. A Sonnet cross-review checked the Opus worker's change. I re-ran the new test and test_pcap_tcp_segment_unit.py on 3.14 and 3.11, and both pass.

tcp_segment now picks the later-fragment branch with isinstance(ip, IPv4) instead of the header's Version (pcapkit/toolkit/pcap.py:167). pcapng.py calls the same function, so it needed no change.

The worker's survey covered 38 cases, as PCAP and PCAP-NG, across every engine. The 32 cells that changed all went from AttributeError to inputs identical to dpkt's and scapy's, and every other cell is byte-identical.

Still open for you on #1666: should IPv6.read reject a Version other than 6? Measured with a runtime patch, that would drop the default engine's inputs on rows 1-8 and 15. dpkt and scapy would then need the same change.

@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 89.46% (unit tier, Python 3.14, 1533afb43, 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 3031 152 1074 35 94.23%
pcapkit/interface 138 7 52 5 93.68%
pcapkit/protocols 17166 226 4580 190 98.07%
pcapkit/toolkit 1351 275 558 6 77.21%
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 dcbff86 into main Oct 13, 2026
49 of 50 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 13, 2026
@JarryShaw
JarryShaw deleted the fix/1666-tcp-segment-picks-branch-by-layer branch October 13, 2026 20:53
@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(toolkit): the default engine ends the extraction on an IPv6 header whose Version is 4

1 participant