Skip to content

fix(extraction): extraction with format='pcap' fails (#1127) - #1164

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1127-pcap-output
Oct 7, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1127-pcap-output

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner
  • Searched for similar pull requests
  • Followed the coding style (make pylint, make mypy, make isort) — ran the three tools with the Makefile flags on the changed pcapkit/ files: isort and mypy clean; pylint reports only protected-access, as the engines already do
  • make test passes, and a test case covers the change — ran the modules listed below, not the full suite
  • Added a changelog entry — N/A, added centrally after the wave

What is the purpose of your pull request?

  • fix — corrects a defect

Description of your pull request and other information

Closes #1127. Extractor.__init__ created the writer before any header was read, so PCAPIO had no link type and every engine failed (extraction.py:1202). Now the header step creates it through Extractor._open_output(protocol=, byteorder=, nanosecond=), and run() opens it for engines with no header step. A PCAP writer gets the frame itself and skips the dict Global Header record. Third-party engines have no frame octets, so they fall back to JSON with a FormatWarning, as their flow tracing already does. PCAP-NG input raises FormatError.

Probe (in.pcap / dhcp.pcapng, format='pcap'): before, all 8 engine × input runs fail with TypeError: PCAPIO.__init__() missing ... 'protocol'. After: default + in.pcap writes a byte-identical 605-byte copy, and split / zero-frame output works. dpkt, scapy and pyshark write JSON with a warning. Default + pcapng raises FormatError.

Tests: new tests/foundation/test_extraction_pcap_output_unit.py passes 7 and fails 7 without the fix. Passing: #1149's test_extraction_record_header_unit (10), test_extraction (22), engines/test_runtime_engines (5), engines/test_pcapng_engine (14), the other engine modules, interface/test_core and test_misc, and tests/project (389 passed, 1 skipped). Fixture-only edits: _open_output on the mock extractor and byteorder on FakeHeader.

Extractor.__init__ created the output writer before any engine had read
the global header, so PCAPIO was built without the link type it needs
and every engine failed with a TypeError.

- Extractor keeps the writer class until the engine's header step, which
  now creates the writer via Extractor._open_output with the header's
  link type, byte order and timestamp resolution; run() opens it for
  engines with no header step.
- The PCAP engine hands a PCAP writer the frame itself and skips the
  "Global Header" record the writer has already written, so in.pcap
  round-trips byte-identically.
- The third-party engines produce frames without octets, so PCAP output
  from them falls back to JSON with a FormatWarning, as their flow
  tracing already does; PCAP-NG input raises FormatError.

Closes #1127
@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 6, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review verdict on a52057071: GOOD TO GO (ran on Sonnet; author Opus)

  • Root cause confirmed: on 2c7a97c84, all 8 engine × input runs raise PCAPIO.__init__() missing … 'protocol' at extraction.py:1202.
  • Output: default + in.pcap is byte-identical, which is exactly what the PR body claims. Split mode, zero-frame captures and stream input work. json/plist/tree output, split mode and trace/reassembly give identical diff -r against main. _ofile cannot be left unset: every run() route ends in _open_output().
  • Design choices:
    • The JSON fallback with FormatWarning copies the flow-tracing guard (~extraction.py:1188).
    • FormatError for PCAP-NG input is a conservative choice. A single-interface section could map to PCAP, but it is unimplemented, and the error fires before any output file exists.
  • Fails without the fix: 7 failed. The fixture edits only add stubs.
  • Follow-ups, not blocking:
    • Across the other 16 examples/captures/*.pcap, the written snaplen is always 262144. The input's snaplen, thiszone and sigfigs are not carried over, because PCAPIO._dump_header builds Header from byteorder and nanosecond only. Filed separately.
    • A requested engine that is not installed still forces JSON after falling back to the default engine, because the guard matches the requested name before resolution. The trace guard behaves the same way.

@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 6, 2026
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Coverage: 88.88% (unit tier, Python 3.14, a52057071, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18757 1035 2342 846 90.23%
pcapkit/corekit 1878 91 580 22 94.34%
pcapkit/dumpkit 142 0 42 0 100.00%
pcapkit/foundation 2442 102 852 39 94.38%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 15688 188 3960 163 98.18%
pcapkit/toolkit 487 65 144 1 85.42%
pcapkit/utilities 429 4 122 4 98.55%
pcapkit/vendor 4409 2359 1006 158 42.84%

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

@JarryShaw
JarryShaw merged commit b7ced07 into main Oct 7, 2026
41 checks passed
@JarryShaw
JarryShaw deleted the fix/1127-pcap-output branch October 7, 2026 01:45
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 7, 2026
JarryShaw added a commit that referenced this pull request Oct 7, 2026
- 4 new 1.5.0 entries: HTTP/2 sample stream identifiers (#1161),
  TCP option re-padding (#1163, breaking), format='pcap' output
  (#1164), declared lengths on truncated captures (#1166, breaking)
- extend the PCAP Header (#1159), HIP (#1160) and from_data
  round-trip (#1162) entries rather than duplicating them
- regenerate CHANGELOG.md with util/changelog_md.py
- process.rst: entry-count pin re-measured, 189 -> 193

tests/project: conventions_doc_claims, changelog_md and
documentation_claims pass; changelog_md.py --check is in step.
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(scapy): the scapy engine fails with format='pcap'

1 participant