Skip to content

fix(toolkit): pyshark reads frames to the 64-layer bound and past tshark's 802.1Q limit, and keys its fallback by the header (#1609, #1671) - #1686

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1609-1671-pyshark-fallback-and-bound
Oct 14, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1609-1671-pyshark-fallback-and-bound

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

#1671. The fallback _from_layers runs where a frame's octets can't be had: tcp_traceflow gets no captured, or the engine's second tshark pass or capture-file read fails. It read ip.dst unguarded. Behind a route option whose data isn't a whole number of addresses, tshark gives no ip.dst, so extract() raised a bare AttributeError. Behind a live LSRR or SSRR, ip.dst is the last hop. _header_addresses (pcapkit/toolkit/pyshark.py:604) now keys by the header's destination, which is tshark's ip.cur_rt there. It returns no segment where tshark gives no address, since the default engine rejects that option (pcapkit/protocols/internet/ipv4.py:898).

#1609. _from_octets (:908) now starts at the depth _network_start gives and walks through #1667's _walk_upper_layer, so it stops at FRAME_LAYER_LIMIT. tshark dissects at most 20 802.1Q tags, so TCP behind 21–61 tags was never read. _stops_in_vlan_tags (:685) and vlan in OCTETS_FILTER now read those frames from their octets.

Measured on 3.11 with tshark 4.6.9, main (7aeeaf804) → this branch. Trace inputs agreeing with default:

Sweep main branch
#1609 cells (37 frames) 23 37
bound/VLAN sweep, DLTs 0, 1, 101, 108, 228, 229 (886 frames) 669 886
fallback, every LSRR/SSRR length × pointer (7,205 frames) 1,641 (540 raise, 183 last hop) 2,364
23 example captures (1,261 traced frames) 1,261 1,261

No new disagreements. dpkt, scapy and pypcapfile agree on all 37 #1609 cells on main.

Unchanged from main, not fixed here (pyshark 0, default 1): an IPv6 jumbogram whose Jumbo Payload Length is ≤ 65535, which tshark flags as malformed (a real one of 70,028 octets agrees), and TCP behind IPv6 Mobility or HIP (#1626). In the fallback sweep, the 4,841 frames whose route options the default engine rejects are still traced as tshark dissects them.

Tests. New tests/toolkit/test_pyshark_fallback_and_layer_bound_1609_1671_unit.py. All 6 tests fail on main (7 failures, 7 errors under unittest) and pass here. Pins in the #1625 and #1639 files are updated for the new filter, warning and _network_start shape.

Closes #1609
Closes #1671

…ark's 802.1Q limit, and keys its fallback by the header (#1609, #1671)

- #1671: where a frame's octets cannot be had, _from_layers read tshark's
  ip.dst unguarded. tshark reports none behind a route option whose data is
  no whole number of addresses, so AttributeError ended extract(); behind a
  live LSRR or SSRR it is the route's last hop. _header_addresses now takes
  the header's destination (ip.cur_rt there) and reads no segment where
  tshark reports no address, as the default engine rejects such an option.
- #1609: _from_octets followed IP-in-IP, AH and extension headers past layer
  64. _network_start now gives the IP layer's place in the frame, and the
  walk goes through pypcapfile's _walk_upper_layer, which stops at
  FRAME_LAYER_LIMIT.
- #1609: tshark dissects at most 20 802.1Q tags, so frames behind more were
  never read from their octets. Such a frame now asks for them
  (_stops_in_vlan_tags; OCTETS_FILTER gains vlan).

New tests/toolkit/test_pyshark_fallback_and_layer_bound_1609_1671_unit.py:
all 6 tests fail on main and pass here, on 3.11 with tshark 4.6.9.

Closes #1609
Closes #1671
@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 2e1a1c018. A Sonnet cross-review checked the Opus worker's change. I re-ran the new test, the 1639 test and the 1604 agreement test on 3.11 with tshark 4.6.9: 6, 11 and 3 passed.

Left, all already on main and unchanged here:

@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 89.46% (unit tier, Python 3.14, 2e1a1c018, 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 3034 152 1076 35 94.23%
pcapkit/interface 138 7 52 5 93.68%
pcapkit/protocols 17166 226 4580 190 98.07%
pcapkit/toolkit 1433 280 590 6 77.95%
pcapkit/utilities 494 4 146 4 98.75%
pcapkit/vendor 4417 2342 1010 157 43.38%

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

@JarryShaw
JarryShaw merged commit e86799b into main Oct 14, 2026
50 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 14, 2026
@JarryShaw
JarryShaw deleted the fix/1609-1671-pyshark-fallback-and-bound branch October 14, 2026 01:17
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 14, 2026
JarryShaw added a commit that referenced this pull request Oct 14, 2026
Third-party engines are best effort (#1687), so the pyshark code that
exists only to match the default engine comes out:

- revert #1686, #1670 and #1642: the second tshark pass that read a
  frame's octets, the link type read from the capture file, the 64-layer
  bound, reading past tshark's 802.1Q limit, and keying a source-routed
  segment by ip.cur_rt
- revert #1613's tshark preferences that turned IP reassembly off, with
  its FileCapture assertion in test_runtime_engines.py and its sentence
  in the engines index
- keep #1613's partial-TCP-header guard and #1605's keying by the IP
  header that carries the segment
- re-land #1671's crash fix: a TCP layer whose IP layer lacks ip.src or
  ip.dst is not traced, rather than raising AttributeError
- delete the three test modules those reverts added and the agreement
  checks in test_pyshark_unit.py; the #1611 and #1671 checks assert
  constants

The pyshark test modules, test_runtime_engines.py, test_engine_runtime.py
and the project guards pass on 3.11 (tshark 4.6.9) and 3.14.
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

1 participant