Skip to content

fix(dumpkit): pcap output drops the input's snaplen, thiszone and sigfigs (#1165) - #1168

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

JarryShaw merged 1 commit into
mainfrom
fix/1165-pcap-output-header-fields

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 7, 2026 •

Copy link
Copy Markdown
Owner
  • Searched for similar pull requests
  • Followed the coding style (make pylint, make mypy, make isort) — ran pylint/mypy on the two changed source files only: no new messages on changed lines, no mypy errors in them; not the make targets
  • make test passes, and a test case covers the change — ran only the modules listed below
  • Added a changelog entry under docs/source/changelog/ and regenerated CHANGELOG.md, if the change is user-visible — N/A, added centrally after the wave

What is the purpose of your pull request?

  • 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

Closes #1165. The PCAP engine passes the input header's thiszone, sigfigs and snaplen to Extractor._open_output, and PCAPIO._dump_header hands them to Header (which already accepts them as make() keywords). test_runtime_engines.py pinned the old _open_output arguments and is updated.

Sweep, every examples/captures/*.pcap + http6.cap, default engine, format='pcap': before 1/17 byte-identical (16 differ only at header bytes 16–19); after 17/17.

Tests: new tests/foundation/test_pcap_output_header_fields_unit.py (in.pcap rewritten to thiszone -3600 / sigfigs 7 / snaplen 1514: whole file, split mode, zero frames). Without the fix: 3 failed; with: 3 passed. Also test_runtime_engines.py 5 passed, test_extraction_pcap_output_unit.py 7 passed, test_extraction.py 22 passed, dumpkit/test_common_unit.py 14 passed, integration/test_files_output_naming.py 4 passed, tests/project 389 passed, 1 skipped.

…figs (#1165)

The PCAP engine now passes the input global header's thiszone, sigfigs and
snaplen to Extractor._open_output, and PCAPIO._dump_header writes them
instead of the Header defaults (snaplen 262144). The engine unit test that
pinned the old _open_output arguments is updated.
@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 7, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

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

  • Root cause confirmed: on main, PCAPIO._dump_header builds Header from network, byteorder and nanosecond only. I re-read this myself. Header.make already accepts thiszone=0, sigfigs=0 and snaplen=0x40000 (header.py:189-190).
  • Sweep: default engine, format='pcap', over all 17 pcap inputs. Byte-identical output was 1/17 before and is 17/17 now. All 16 earlier diffs were header bytes 16–19 (snaplen).
  • Split mode and zero-frame output both carry the input header. Big-endian nanosecond input with a negative thiszone round-trips byte-identically.
  • No other path breaks. PCAP-NG input still raises FormatError, the non-PCAP dumpers take the new keywords through **kwargs, and traceflow never calls _dump_header.
  • Fails without the fix: 3 failed. The edited test_runtime_engines.py pin adds non-default values, so swapping two fields would also fail.
  • Follow-ups, not blocking:
    • The input's version fields are still written as 2.4. Every capture in the repo is 2.4.
    • The new test covers only little-endian input. Big-endian was probed by hand but is not pinned.

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

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

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

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18757 1034 2342 845 90.24%
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 15696 188 3970 165 98.17%
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 91bd80e into main Oct 7, 2026
41 checks passed
@JarryShaw
JarryShaw deleted the fix/1165-pcap-output-header-fields branch October 7, 2026 02:24
@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
Seven new entries and one extension in docs/source/changelog/1.5.0.rst,
the regenerated CHANGELOG.md, and the process.rst entry-count pin
re-measured (193 -> 200). The opening snapshot now cites 319 issues and
pull requests up to #1195.
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(dumpkit): pcap output drops the input's snaplen, thiszone and sigfigs

1 participant