Skip to content

refactor(toolkit): revert scapy's agreement-only code (#1708) - #1724

Merged
JarryShaw merged 1 commit into
mainfrom
refactor/1708-scapy-revert-agreement
Oct 14, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
refactor/1708-scapy-revert-agreement

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

Phase 2 of #1708, for scapy, per the plan on #1687.

@JarryShaw JarryShaw added refactor Restructuring for its own sake — neither a fix nor a new capability (refactor: prefix) review: good-to-go Cross-review at the current head says ready; CI state is separate labels Oct 14, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at 6f1feb52f. A Sonnet cross-review checked it; the author was Opus.

@JarryShaw

Copy link
Copy Markdown
Owner Author

Heads-up: this conflicts with #1721 (pypcapfile) in toolkit/scapy.py, ext.rst, index.rst, internet.rst and protocol.rst, where both rewrite the same sentences about the engines. Either can merge first, and I will rebase the other onto it.

@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 14, 2026
Third-party engines are best effort (#1687), so the scapy code that
exists only to match the default engine comes out:

- revert #1683, #1663, #1652 and #1631, and the scapy hunks of #1614: the
  link-type and loopback-family walk, the frame and extension-header
  bounds, the IPv6 chain walk over the octets, the stops at GRE, VXLAN,
  PPPoE and the other layers the default engine leaves raw and at ICMP
  quotes, and the IPv4 header checks
- keep #1624's walk (TCP behind AH or an IPv6 extension header in IPv4,
  and the later-fragment stop), #1605's keying by the carrying IP header
  and #1589's trailer cut; the walk calls _walk_extension_headers, which
  the pypcapfile revert keeps
- re-land as guards: a zero-length frame is no IPv4 input and has no
  packet2dict octets (#1676); IPv4 in IPv6 ends at the Payload Length
  (#1623); a packet built by hand is read off its octets (#1676);
  ipv6_reassembly takes only its own IPv6 layer's Fragment header, and
  none in a later IPv4 fragment
- delete the four scapy agreement modules and test_scapy_unit's
  default-engine checks; the guards' tests assert constants
- docs: scapy applies the extension-header bound only behind IPv4 and the
  frame-layer bound nowhere, and its silent gaps are listed
  where gaps are said to be announced

The scapy toolkit and engine modules, the shared modules that run scapy,
and the project and tier guards pass on 3.14 (scapy 2.7.0) and 3.11
(scapy 2.8.0).
@JarryShaw

Copy link
Copy Markdown
Owner Author

Rebased onto main after #1721 as 53bf23eda.

Verdict: GOOD TO GO at 53bf23eda. A Sonnet cross-review (author: Opus) checked 41a149d8a. 53bf23eda differs from it only in one commit-body line, corrected because the toolkit applies the extension-header bound behind IPv4.

  • Unchanged code: toolkit/scapy.py is byte-identical to 6f1feb52f. The rebase changes only the five conflicted doc hunks, and no refactor(toolkit): revert pypcapfile's agreement-only code (#1708) #1721 file is in the diff.
  • Merged sentences: in ext.rst, index.rst, internet.rst and protocol.rst they are true of main's pypcapfile adapter and this scapy adapter.
  • Measured: behind IPv4, TCP is read after 32 extension headers and not after 33. In an IP-in-IP tunnel it is read 65 layers deep. pypcapfile drops TCP behind VLAN, IP-in-IP and AH without a warning.
  • Sphinx and tests: Sphinx warnings are the same as main's (63). tests/toolkit and tests/foundation/engines pass on 3.14 (363 passed) and on 3.11 with scapy 2.8.0 (378 passed).

@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 89.65% (unit tier, Python 3.15, 53bf23eda, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 3 20 3 95.16%
pcapkit/const 18860 954 2368 868 91.06%
pcapkit/corekit 2372 67 762 35 96.49%
pcapkit/dumpkit 275 3 100 3 98.40%
pcapkit/foundation 2885 144 1010 34 94.20%
pcapkit/interface 138 7 52 5 93.68%
pcapkit/protocols 17181 226 4590 190 98.07%
pcapkit/toolkit 1015 173 372 10 80.89%
pcapkit/utilities 508 4 154 4 98.79%
pcapkit/vendor 4417 2342 1010 157 43.38%

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

@JarryShaw
JarryShaw merged commit 51cd24c into main Oct 14, 2026
49 checks passed
@JarryShaw
JarryShaw deleted the refactor/1708-scapy-revert-agreement branch October 14, 2026 15:47
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 14, 2026
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Restructuring for its own sake — neither a fix nor a new capability (refactor: prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant