Skip to content

fix(engines): take a scapy packet's link type and clock from its interface (#1503, #1517, #1549) - #1563

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1503-1517-1549-scapy-reader
Oct 10, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1503-1517-1549-scapy-reader

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 10, 2026 •

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

All three fixes are in the scapy engine's reader subclasses (_reader_type(), pcapkit/foundation/engines/scapy.py), the one place that sees each packet's interface:

Deletes the Gap(1503, ...) row. Tests: tests/foundation/engines/test_scapy_reader_interfaces_runtime.py (SPB, if_tsoffset over two byte orders with EPB and obsolete Packet Blocks, raw-IPv4 PCAP-NG and PCAP traces compared byte for byte with the default engine), tests/foundation/engines/test_scapy_reader_hooks_unit.py (the hook contracts against a stand-in parent, whatever scapy is installed) and tests/toolkit/test_scapy_linktype_unit.py. All fail on main. Also documents packet2frame, attach_resolution and attach_linktype.

Closes #1503
Closes #1517
Closes #1549

@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 10, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 10, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: NEEDS CHANGES at 17e00001b. I cross-reviewed this on Sonnet; the author ran Opus.

Blocking: under scapy 2.8.0 every PCAP-NG packet is dropped, with no error. CI installs 2.8.0 because scapy is unpinned; the local venv has 2.7.0.

  • In 2.8.0, RawPcapNgReader._check_interface_id returns a bool (scapy/utils.py:1887-1894). The EPB/SPB/PKT readers then do if not self._check_interface_id(intid): return None (:1949, :1971, :2004).
  • The override at pcapkit/foundation/engines/scapy.py:147-151 calls super() and returns None. So test.pcapng reads 0 frames instead of 5, and dhcp.pcapng 0 instead of 4. This is the cause of the red Integration 3.10–3.13 and parity legs (Lists differ: [] != [...]).
  • Fix: return the parent's result, and add a pass-through test that doesn't depend on the installed scapy version.
  • main doesn't override this method; fix(engines): reset the scapy reader's interfaces at each PCAP-NG section (#1522) #1546's sections test passes on main under 2.8.0.

Also needed:

  • scapy 2.5.0: interfaces[-1][2] is an int there, so scapy.py:142 raises TypeError on the first IDB, where base reads fine. Keep the tsoffsets in a reader-side table instead.
  • Docs (nit): attach_linktype is missing from docs/source/pcapkit/toolkit/3rdparty.rst.

Confirmed on 2.7.0:

The author is fixing all three.

@JarryShaw JarryShaw added review: needs-changes Cross-review at the current head says changes are required; see the verdict comment and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 10, 2026
@github-actions

github-actions Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Coverage: 89.57% (unit tier, Python 3.14, 71df9d433, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18858 957 2366 871 91.03%
pcapkit/corekit 2240 67 722 34 96.32%
pcapkit/dumpkit 258 2 90 2 98.85%
pcapkit/foundation 2796 156 992 47 93.27%
pcapkit/interface 108 7 36 5 91.67%
pcapkit/protocols 16993 230 4546 191 98.02%
pcapkit/toolkit 703 104 234 7 84.10%
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.

…rface (#1503, #1517, #1549)

- A Simple Packet Block has no timestamp, and scapy left the packet with the
  time it was built; the engine's PCAP-NG reader now sets it to 0, as the
  default engine reads it (#1503).
- The flow tracer looked the link type up by the first layer's name, so a raw
  IPv4 packet (`IP`) raised MissingKeyError. Both readers now attach the
  interface's link type with `attach_linktype`, and `tcp_traceflow` uses it;
  only a packet built by hand is still looked up by name, with no default (#1517).
- scapy ignores `if_tsoffset`; the PCAP-NG reader now reads option 14 of each
  interface itself and adds it to every timestamp on it (#1549).
- The overrides hold on scapy 2.5, 2.7 and 2.8: `_check_interface_id` passes
  its result through (2.8 skips a block on a false one), the offsets are kept
  by the reader rather than in scapy's interface tuple, and a missing hook
  raises VersionError.
- Document `packet2frame`, `attach_resolution` and `attach_linktype`; delete
  the #1503 Gap row of the engine agreement table.

New runtime and unit tests fail on main; scapy, trace and agreement selections
pass on scapy 2.7.0 and 2.8.0.
@JarryShaw
JarryShaw force-pushed the fix/1503-1517-1549-scapy-reader branch from 17e0000 to 71df9d4 Compare October 10, 2026 04:40
@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment labels Oct 10, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at 71df9d433. This was a Sonnet delta review; the author ran Opus. Every point below was measured in fresh venvs for scapy 2.5.0, 2.6.1, 2.7.0 and 2.8.0. I confirmed _check_interface_id now returns super()'s result (scapy.py:189-196).

  • Frames: on 2.6.1, 2.7.0 and 2.8.0, frame counts, octets and exact timestamps match the default engine. That covers 6 sample PCAP-NGs and synthetic captures: mixed byte-order sections, if_tsoffset on and off, and SPBs inside an offset section. Flow-trace bytes are identical as well.
  • Bad interface ID: it behaves like stock scapy. 2.8 skips the block, and earlier versions raise EOFError. The next valid block never inherits a stale offset or link type.
  • Layouts: both scapy interface layouts work. On 2.5.0 the earlier TypeError is gone, and the offset list stays aligned across IDBs and SHBs.
  • _read_tsoffset: it matches the default engine for both byte orders, signed extremes, option order and padding. It is more lenient than the default engine on malformed option 14: a wrong length or a duplicate gives 0 or the first value, where the default engine raises.
  • Version guard: it runs on first use, so import pcapkit never imports scapy.
  • Fail-before: the old head drops every PCAP-NG frame on 2.8.0, and the new head passes.

Non-blocking:

  • VersionError is documented as "Unknown IP version" (utilities/exceptions.py:567), so raising it for a scapy without the hooks stretches its meaning.
  • On scapy 2.5.0, three failures reproduce identically on main, so they are not this PR's. That includes profile.pcapng's nanosecond interface read at the wrong resolution: scapy 2.5's utils.py:1544-1549 stops parsing IDB options at a comment without a newline. The Scapy extra is unpinned, so I'm checking these separately.

@JarryShaw JarryShaw added 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 10, 2026
@JarryShaw
JarryShaw merged commit e5bfacb into main Oct 10, 2026
39 checks passed
@JarryShaw
JarryShaw deleted the fix/1503-1517-1549-scapy-reader branch October 10, 2026 05:17
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 10, 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

1 participant