Skip to content

fix(engines): reset the scapy reader's interfaces at each PCAP-NG section (#1522) - #1546

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1522-scapy-section-tsresol
Oct 10, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1522-scapy-section-tsresol

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 — test added; ran the new module, tests/toolkit/test_engine_pcap_trace_runtime.py, the scapy and runtime engine tests, the engine agreement harness, the isort/shard/tier/docstring guards, not the full 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

Scapy's RawPcapNgReader appends every section's IDBs to one interfaces list, so interface 0 of a later section resolved to section 1's, link type and if_tsresol included. The engine's _PcapNgReader (pcapkit/foundation/engines/scapy.py) now overrides _read_block_shb to start each section with no interfaces, as the PCAP-NG specification defines a section.

tests/foundation/engines/test_scapy_pcapng_sections_runtime.py builds two captures from tcp.pcap: the issue's (Ethernet if_tsresol=9, then Ethernet microseconds), and one whose section 2 has raw IPv6 (229) as interface 0 and Ethernet as interface 1. It checks each frame's layer, resolution and timestamp, and that every traced PCAP flow matches the default engine's byte for byte, both trace resolutions. On c3d412015 all 6 subtests fail (Decimal('1500000.000003318') vs 1500000000.003318; Dot3 vs IPv6). The agreement harness has no #1522 row: test.pcapng's section 2 interface 0 has the same link type and resolution as section 1's, so it could not see the bug.

Closes #1522

…tion (#1522)

Scapy's RawPcapNgReader appends every section's Interface Description
Blocks to one table, so an interface ID in a later section resolved to
the first section's interface, with its link type and if_tsresol: a
microsecond section after a nanosecond one was dated 1000x too small,
and its traced PCAP differed from the default engine's.

The engine's PCAP-NG reader subclass now starts each section with no
interfaces, as the PCAP-NG specification defines a section.

The new runtime test builds two two-section captures from tcp.pcap and
checks each frame's layer, resolution and timestamp, and that the
traced PCAP flows match the default engine's byte for byte.
@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

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at 7ba60b529. Cross-reviewed on Sonnet; the author ran Opus.

  • Root cause confirmed: in scapy 2.7.0, self.interfaces is set only in __init__ (utils.py:1631). _read_block_shb never clears it, and EPB, SPB and PKT blocks all resolve their link type and tsresol through it. 2.5.0 and 2.4.5 have the same shape, read statically.
  • Fix holds beyond the PR's own tests: the reviewer's own three-section capture (little/big/little-endian, uneven IDB counts, tsresol 9/6/3/7) matches the default engine on all 6 frames, and its PCAP traces are byte-identical at both resolutions; main fails frames 3–6.
  • Single-section captures unchanged: all six examples/captures/*.pcapng give identical frame bytes, times and resolutions to main, 1,088 fields in total.
  • Fail-before, my own run on the merged tree: 2 passed with the fix, 6 failed with main's engines/scapy.py. The PR merges cleanly onto 16fbea054.

Not blocking:

  • process_information: scapy also accumulates Apple PIB entries across sections. pcapkit never reads them, and whether PIB indexes are section-scoped is unverified.
  • EPB with no IDB: a section-2 EPB with no IDB now stops the read after 2 frames, where main read 4 frames with wrong times. The default and dpkt engines raise FormatError on this input.
  • fix(engines): the scapy engine ignores a PCAP-NG interface's if_tsoffset #1549 next: with the table now section-scoped, the if_tsoffset fix is a small override, about 25 lines in the reviewer's prototype.

@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
@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 90.01% (unit tier, Python 3.14, 7ba60b529, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18795 842 2356 756 92.09%
pcapkit/corekit 2204 67 712 34 96.26%
pcapkit/dumpkit 258 2 90 2 98.85%
pcapkit/foundation 2800 135 986 48 94.11%
pcapkit/interface 108 7 36 5 91.67%
pcapkit/protocols 16992 229 4546 190 98.03%
pcapkit/toolkit 635 98 198 5 83.31%
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.

@JarryShaw
JarryShaw merged commit 3be3fdd into main Oct 10, 2026
41 checks passed
@JarryShaw
JarryShaw deleted the fix/1522-scapy-section-tsresol branch October 10, 2026 03:35
@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

Development

Successfully merging this pull request may close these issues.

fix(engines): scapy engine keeps the first section's timestamp resolution after a second PCAP-NG section

1 participant