Skip to content

fix(dumpkit): a PCAP trace writes the octets it has, not a truncated record's claimed length (#1541) - #1543

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1541-trace-truncated-incl-len
Oct 10, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1541-trace-truncated-incl-len

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

Both readers keep a short record as captured. That covers a PCAP record whose incl_len runs past EOF, and a PCAP-NG block whose captured length runs past the block (#1405). The declared length stays in frame_info/cap_len, and packet holds the octets that are actually there. PCAPIO._append_value copied that declared length into the trace. It now writes len(packet), so every trace record holds exactly incl_len octets. A record held whole is written byte-for-byte as before.

  • Frame.read now warns, in the same wording as the PCAP-NG reader. Until now, parsing such a record raised no warning at all. Parse then rebuild was already byte-exact, from both info and to_dict(), and stays so.
  • On /tmp/r1511/badlen.pcap the trace record now reads incl_len 0x249 (585) and ends at EOF.
  • None of the KNOWN_FAILURES rows change, and no sample capture holds a truncated record.

The two new *_1541_unit.py modules fail on main: 6 of 8 tests, with every subtest failing. Ran one at a time, all green: tests/dumpkit, tests/protocols/misc, tests/foundation/traceflow, the five tests/protocols/test_*roundtrip*, tests/project, tests/test_docstring_contract.py and tests/test_tier_guard.py.

Closes #1541

…record's claimed length (#1541)

PCAPIO._append_value copied frame_info.incl_len into the trace record
header. A PCAP record whose incl_len runs past the end of its file, or a
PCAP-NG block whose captured length runs past the block (#1405), is kept
as captured: the declared length stays in frame_info and the packet is
the octets present. The trace then claimed octets it did not hold. It
now writes len(packet) as incl_len.

The PCAP Frame reader also warns on such a record, as the PCAP-NG reader
does, and still keeps it as captured, so parse -> rebuild is byte-exact.

Closes #1541
@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
@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 90.09% (unit tier, Python 3.14, 8f0f504f0, 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 2202 67 710 34 96.26%
pcapkit/dumpkit 258 2 90 2 98.85%
pcapkit/foundation 2650 98 934 42 94.98%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 17012 229 4576 189 98.04%
pcapkit/toolkit 539 74 168 3 84.58%
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

Copy link
Copy Markdown
Owner Author

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

  • Fix confirmed: on main, badlen.pcap traces to a record claiming 0x7fffffff octets with 585 present, and tshark rejects it with exit 14. Here the record claims 585 and tshark reads it (exit 0). Over a 185-point truncation sweep (IPv4/IPv6 TCP, IPv4 UDP), main wrote 10 malformed traces and this PR writes none. PCAP-NG EPB overruns are fixed too.
  • Whole records are byte-identical: 496 trace files (23 captures × nanosecond × byte order) and 17 whole-capture fout dumps match main.
  • Warning: a ProtocolWarning fires exactly once per truncated extraction, and never on the samples, on rebuild or on from_data.
  • Merge-check: merged into current main (b6b95bc8e), the new tests plus tests/dumpkit and tests/protocols/misc/pcap give 107 passed. On main with the new tests, 10 fail. I ran both.

Not blocking:

@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 4992ab1 into main Oct 10, 2026
41 checks passed
@JarryShaw
JarryShaw deleted the fix/1541-trace-truncated-incl-len branch October 10, 2026 01:58
@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(dumpkit): a PCAP trace copies a truncated record's claimed incl_len, writing a malformed file

1 participant