fix(pcap): Frame.len is the on-wire length, cap_len the captured one (#618)

[JarryShaw]

Fixes #618.

⚠️ BREAKING CHANGE TO A PUBLIC ATTRIBUTE — READ THIS FIRST

Frame.len and Frame.cap_len change meaning when read from a .pcap file.
Before this PR, frame.len was the captured length and frame.cap_len the
on-wire length. After it, they are the other way round — len is on-wire,
cap_len is captured — which is what the PCAP-NG reader returns.

This is a decision, not the correction of a typo. The PCAP spelling is the
older of the two and was internally self-consistent, code and docstrings
agreeing. See "Seniority does not settle it" below for the dates and for what
actually breaks the tie.

For a frame the snapshot length cut short, that is a different number, not a
relabelling: big_endian.pcap's third frame goes from len=96, cap_len=1200
to len=1200, cap_len=96. Any consumer reading either attribute off a .pcap
is affected. This is your call to accept, and it is the reason this PR is
worth reading rather than rubber-stamping.

No compatibility alias is provided. One would have to lie about one of the two
names, since the whole defect is that the two names were swapped.

The defect, re-verified on current main

The two readers filled the same two attributes from opposite wire fields, so one
caller reading frame.len got different meanings depending on the container the
frame arrived in.

Site	On main (6c3d1b0d9)
pcapkit/protocols/misc/pcap/frame.py:261-262	len=_ilen (incl_len, captured), cap_len=_olen (orig_len, on-wire)
pcapkit/toolkit/pcapng.py:270-271	len=block.original_len (on-wire), cap_len=block.captured_len (captured)

Line numbers re-derived on 6c3d1b0d9, after #614 moved this file; they are
unchanged from the ones the issue quotes. Measured, both readers, one run:

PCAP reader -- examples/captures/big_endian.pcap
  frame 1: incl_len=   74 orig_len=   74  ->  len=   74 cap_len=   74
  frame 2: incl_len=   66 orig_len=   66  ->  len=   66 cap_len=   66
  frame 3: incl_len=   96 orig_len= 1200  ->  len=   96 cap_len= 1200   <-- captured in `len`

PCAP-NG reader -- examples/captures/test.pcapng (via block2frame)
  block 3: captured_len=   32 original_len=  314  ->  len=  314 cap_len=   32   <-- on-wire in `len`

Frames 1 and 2 satisfy either assignment, which is why this survived: the two
lengths are equal unless the snapshot length truncated the frame, so every
pre-#614 fixture compared a value against itself.

Which reader was wrong, and why — from the specifications, fetched

Both container formats define the underlying fields identically, so neither
format is the odd one out. Quoted from the text, not from memory:

• pcap-savefile(5) (tcpdump.org): incl_len is "a 4-byte value giving the
  number of bytes of captured data that follow the per-packet header"; orig_len
  gives "the number of bytes that would have been present had the packet not been
  truncated by the snapshot length". And: "The two lengths will be equal if the
  number of bytes of packet data are less than or equal to the snapshot length."
• draft-ietf-opsawg-pcapng (Enhanced Packet Block): Captured Packet Length is
  "an unsigned integer that indicates the number of octets captured from the
  packet (i.e., the length of the Packet Data field)", being "the minimum value
  among the Original Packet Length and the snapshot length for the interface";
  Original Packet Length is "the number of octets of packet data that would have
  been provided had the packet not been truncated", and "It SHOULD NOT be less
  than the Captured Packet Length."
• draft-ietf-opsawg-pcap gives the classic format the same two definitions.

So the specs fix what incl_len/orig_len mean, but not what len and
cap_len should mean — those names are not in either spec. They are
Wireshark's, and Wireshark's epan/dissectors/packet-frame.c settles it:

• frame.len — label "Frame Length", description "Frame length on the wire"
• frame.cap_len — label "Capture Length", description "Frame length stored into the capture file"
• and the expert info frame.len_lt_caplen, "Frame length is less than captured
  length", registered PI_MALFORMED/PI_ERROR, fires on if (frame_len < cap_len)
  where cap_len = tvb_captured_length(tvb) and frame_len = tvb_reported_length(tvb).

That last one is decisive on its own: if len were the captured length, then
len < cap_len would be the ordinary truncated case rather than a malformed
one.

One weaker corroboration, labelled weak because the cross-review rightly pushed
back on it: pcapkit/toolkit/pyshark.py's packet2dict copies tshark's
frame_info field names into its output dict, so a cap_len key in pcapkit's
output can already carry tshark's meaning of the word. But it does that
generically — getattr(frame, field) for field in frame.field_names — and never
reads Data_Frame.cap_len, so it is a naming coincidence rather than a code
dependency. It is not load-bearing here.

Seniority does not settle it, and this PR does not pretend otherwise

Raised by the cross-review, then verified here with a pickaxe search over each file:

Spelling	Introduced	Commit
PCAP: len=incl_len (captured)	2022-01-11	c43892af, with the data model's docstrings agreeing in f2a09794 a day later
PCAP-NG: len=original_len (on-wire)	2023-04-27	25f216f4, "implemented toolkit functions for PCAPNG"

Same author, 15 months apart, and both internally consistent — the PCAP
reader's code and its docstrings agreed with each other for four years. So this is
not a typo being repaired, and the older convention is not self-evidently the
intended one. #618 itself calls it "an owner decision".

What breaks the tie is that the two names are borrowed rather than invented.
They are Wireshark's, and Wireshark defines them the other way round — decisively
so via frame.len_lt_caplen. The later spelling is the one that fits the names
it uses, so that is the one kept.

Verdict: the PCAP reader is the one that moves. The PCAP-NG reader is left
alone apart from a comment recording that it is now the reference and that it is
the newer of the two, so that nobody reverses this later on seniority grounds.

Consumers whose behaviour changes

Every read of Data_Frame.len / .cap_len in the repository, traced:

Consumer	Changes?
pcapkit/protocols/misc/pcap/frame.py — _decode_next_layer(frame, network, …)	No. It needs the octets actually present. It read frame.len when that was the captured length; it now reads frame.cap_len, which is the same value. Rewired in this PR so the call site names what it means; dissection output is byte-identical (measured).
pcapkit/foundation/engines/pcap.py:166,168 — ofile(frame.info.to_dict(), …)	Yes. to_dict() copies every attribute, so len and cap_len are literal keys in the JSON/plist/tree output file. A consumer of that file sees the two swap. The committed fixtures examples/captures/out.{json,plist,txt} are unchanged, because they derive from in.pcap, which is not truncated.
pcapkit/foundation/traceflow/tcp.py:170 — output(packet.frame, …)	Yes, for trace_format in json/plist/tree, by the same to_dict() route.
pcapkit/dumpkit/pcap.py:190-195 — PCAPIO._append_value	No. It packs the record header from frame_info.incl_len / frame_info.orig_len, never from these two — and frame_info was already correct in both readers. So trace_format='pcap' and every PCAP write path are untouched, and a PCAP-NG frame dumped to PCAP still gets the right record header.
tests/protocols/misc/pcap/test_frame_runtime.py:29	No — in.pcap is untruncated, so the assertion held either way. Comment added saying so, since it reads like it pins #618 and does not.
tests/protocols/misc/pcap/test_frame_runtime.py:92	Corrected: it sliced the file's stored octets by frame.info.len, now by frame.info.cap_len. arp.pcap is untruncated so it passed either way — a latent wrong-field read rather than a live failure.

Checked and carrying no such read: foundation/extraction.py, the IP and TCP
reassemblers, traceflow/traceflow.py, dumpkit/{common,null}.py,
protocols/misc/pcapng.py, the other six engines and toolkits, and docs/**
(no .rst names either attribute). The hundreds of other .len / pkt['len']
hits belong to unrelated per-protocol schemas.

Which fixture has differing lengths, and by how much

This matters more than usual: with incl_len == orig_len a test cannot tell the
two fields apart, which is the same failure mode as inputs that are all multiples
of 8.

Fixture	Frame	Captured	On-wire	Difference
big_endian.pcap	3 of 3	96	1200	1104 octets
little_endian.pcap	3 of 3	96	1200	1104 octets
big_endian_nanosecond.pcap	3 of 3	96	1200	1104 octets
test.pcapng	3rd packet block	32	314	282 octets

The three .pcap ones are what #614 added (snaplen 96 against a 1200-octet
datagram); the PCAP-NG one already existed. test_the_fixture_really_holds_a_truncated_frame
and its PCAP-NG twin assert the truncation is present and that exactly one frame
carries it, so a fixture regenerated without it fails loudly instead of turning
the module green for the wrong reason.

Failing without the fix, passing with it

Run from immutable git archive snapshots, with pcapkit pinned by stripping the
editable install's MetaPathFinder and asserting on pcapkit.__file__, and the
exit code read from a file rather than off a pipeline. The test file is
byte-identical on both sides (md5 c74fe2bf35a45540d59d04bb385d6481); only
pcapkit/protocols/misc/pcap/frame.py differs — before
260cee48528f4382edb4b2d2d74e6c3b, after a025f5bda00f72c4222817ea52031fdf, each
hashed out of an immutable snapshot of its commit rather than out of a live
working tree.

Before — main at 6c3d1b0d9:

[guard] MEASURED AGAINST: /tmp/probe618/before-tree/pcapkit/__init__.py
...
E                   AssertionError: 96 != 1200
tests/protocols/misc/pcap/test_frame_length_runtime.py:250: AssertionError
...
E       AssertionError: Lists differ: [1200, 32] != [96, 32]
tests/protocols/misc/pcap/test_frame_length_runtime.py:406: AssertionError

SUBFAILED(fixture='big_endian.pcap', frame=3) …::test_len_is_the_on_wire_length_and_cap_len_the_captured_one
SUBFAILED(fixture='little_endian.pcap', frame=3) …::test_len_is_the_on_wire_length_and_cap_len_the_captured_one
SUBFAILED(fixture='big_endian_nanosecond.pcap', frame=3) …::test_len_is_the_on_wire_length_and_cap_len_the_captured_one
FAILED …::ReadersAgreeRuntimeTests::test_a_truncated_frame_reads_alike_whichever_container_it_came_from
========= 6 failed, 4 passed, 12 warnings, 16 subtests passed in 8.29s =========
pytest exit code: 1

Lists differ: [1200, 32] != [96, 32] is the disagreement itself, verbatim: the
cap_len of the two truncated frames, 1200 out of the .pcap against 32 out of
the .pcapng.

After — this branch:

[guard] pcapkit.__file__ = /local/home/jarryx/GitHub/PyPCAPKit/.claude/worktrees/agent-a5e0a6d575bd30616/pcapkit/__init__.py
…::PcapFrameLengthRuntimeTests::test_cap_len_counts_the_octets_the_frame_actually_carries PASSED [ 16%]
…::PcapFrameLengthRuntimeTests::test_len_is_the_on_wire_length_and_cap_len_the_captured_one PASSED [ 33%]
…::PcapFrameLengthRuntimeTests::test_the_fixture_really_holds_a_truncated_frame PASSED [ 50%]
…::PcapngFrameLengthRuntimeTests::test_block2frame_puts_the_on_wire_length_in_len PASSED [ 66%]
…::PcapngFrameLengthRuntimeTests::test_the_fixture_really_holds_a_truncated_block PASSED [ 83%]
…::ReadersAgreeRuntimeTests::test_a_truncated_frame_reads_alike_whichever_container_it_came_from PASSED [100%]
============== 6 passed, 10 warnings, 28 subtests passed in 4.16s ==============
pytest exit code: 0

Note that pytest 9.1.1 prints PASSED on the parent of a test whose subtests
failed — the before run's top line for test_len_is_the_on_wire_length_… reads
PASSED while three of its subtests failed. The exit code and the summary line
are the truth.

One mutant survives this test, and it is disclosed rather than papered over.
Reverting only the _decode_next_layer argument to frame.len while keeping the
attribute swap leaves the whole selection green — confirmed independently by the
cross-review and re-confirmed here (19 passed, exit 0). The reason: the truncated
frame dissects to Ethernet:IPv4:UDP:Raw, and Raw.read takes a length it never
uses (pylint: disable=unused-argument on its signature), so the over-long value
reaches nothing that checks it. The shipped argument is the correct one — it is
also the value _import_next_layer defaults to — but nothing here would catch a
regression of that line, and the comment at the call site says so. Closing it needs
a fixture whose truncation lands in a length-checked field instead of bottoming out
in Raw.

Both readers are covered, which is the point: PcapFrameLengthRuntimeTests
pins the one that was wrong, PcapngFrameLengthRuntimeTests is the control and
passes on main too — it exists so a later change cannot "fix" the disagreement
from the other end — and ReadersAgreeRuntimeTests asserts the cross-format
property the issue actually reports.

Regressions and coverage

tests/protocols/misc/ tests/toolkit/ tests/dumpkit/
  116 passed, 4 skipped, 234 subtests passed          exit 0
tests/foundation/ tests/integration/
  315 passed, 13 skipped, 489 subtests passed         exit 0

The full suite was deliberately not run: coverage run -m pytest tests/
reached 41.4 GB RSS on this host today and was killed.

Coverage cannot go backwards, because no executable statement was added to or
removed from pcapkit — the change is two swapped keyword values, one attribute
rename at a call site, and comments. Measured on both trees with the same
selection:

Module	Before	After
pcapkit/protocols/misc/pcap/frame.py	127 stmts, 30 branches, 100%	127 stmts, 30 branches, 100%
pcapkit/protocols/data/misc/pcap/frame.py	9 stmts, 100%	9 stmts, 100%
pcapkit/toolkit/pcapng.py	58 stmts, 12 branches, 100%	58 stmts, 12 branches, 100%

Since the changed lines already executed, the meaningful figure is the test count:
that selection goes from 17 tests / 14 subtests to 23 tests / 42 subtests,
the new module contributing 6 tests and 28 subtests.

Also clean: mypy on the three changed modules (Success: no issues found in 3 source files), pycodestyle on both test files, rstcheck on the changelog
entry, and python util/changelog_md.py --check (exit 0, CHANGELOG.md
regenerated rather than hand-edited). pylint reports the identical finding set
before and after, with only line numbers shifted — no new findings.

Found and deliberately not fixed

block2frame hands on a Data_Frame with no packet octets. Every packet
block read out of test.pcapng comes back with packet == b'' while declaring a
captured_len in the hundreds, so block2frame's frame.__update__(packet=block.packet)
copies nothing. The consequence is that dumping a PCAP-NG frame through PCAPIO
writes a record header declaring incl_len of 314 followed by zero octets of data.
That is a separate defect from #618, it is pre-existing and unaffected by this
change, and it is why the PCAP-NG test here asserts cap_len against the block's
own field rather than against len(frame.packet) as the PCAP test does. Worth its
own issue.

Files changed

• pcapkit/protocols/misc/pcap/frame.py — the swap, and the _decode_next_layer rewiring
• pcapkit/protocols/data/misc/pcap/frame.py — docstrings, which documented the inverted meanings
• pcapkit/toolkit/pcapng.py — comment only, pinning it as the reference
• tests/protocols/misc/pcap/test_frame_length_runtime.py — new
• tests/protocols/misc/pcap/test_frame_runtime.py — one corrected field read, two comments
• docs/source/changelog/1.5.0.rst, CHANGELOG.md — changelog entry, regenerated

[JarryShaw]

Cross-review verdict: GOOD TO GO

Independent cross-review by a Claude Sonnet subagent (this PR was authored by
Claude Opus, so the reviewing model differs from the authoring one by
construction). It was briefed to falsify rather than to bless, to give a verdict
per load-bearing claim with evidence it obtained itself, and told that
disagreements were the valuable output. It ran read-only and did not touch the
branch. No model substitution was needed.

It fanned out internally and reached 3–4x independent triangulation on most claims,
with no contradictions on the load-bearing facts.

Verdicts

Claim	Verdict
The PCAP reader is the one that should move	CONFIRMED, with a framing caveat — see below
The pcap-savefile(5) / pcapng / pcap-draft quotations are real, not paraphrased	CONFIRMED (all fetched fresh; one unmarked elision flagged)
Wireshark's packet-frame.c strings and the len_lt_caplen logic	CONFIRMED verbatim against wireshark/wireshark@master
_decode_next_layer receives an unchanged value, and cap_len is the right argument	CONFIRMED — and it read _import_next_layer to establish that cap_len is what that parameter already defaults to
The dumpkit / PCAP write path is unaffected	CONFIRMED
The consumer list is complete	CONFIRMED — repo-wide greps three times found nothing the table missed
Fails without the fix, passes with it	CONFIRMED — reproduced from its own snapshots three times: before 6 failed, 4 passed, exit 1; after exit 0
Statement counts 127 / 9 / 58 and 100% both sides	CONFIRMED exactly, three times
Fixture lengths 96/1200 and 32/314	CONFIRMED by three independent struct-only parsers with no pcapkit import
Commit hygiene, changelog honesty, house rules	CONFIRMED

What it disputed, and what was done about it

Three real concerns. All three were verified here rather than taken on trust, all
three held, and all three are now fixed in the branch and the description —
this comment is on the revised revision, not the original.

1. "The PCAP-NG reader was already right" overstated the certainty. The
   reviewer traced the history and found the PCAP convention is the older of the
   two and was internally self-consistent, code and docstrings agreeing. Verified
   here with a pickaxe search: PCAP len=incl_len at c43892af (2022-01-11) with
   docstrings following in f2a09794; PCAP-NG's opposite spelling at 25f216f4
   (2023-04-27), 15 months later, same author, no rationale recorded. So this is a
   tie broken by external authority, not a typo repaired.
   Fixed: the framing is corrected in the PR description (new section
   "Seniority does not settle it"), in the changelog entry, and in both code
   comments — the PCAP-NG one now records that it is the newer spelling, so
   nobody reverses this later on seniority grounds.

2. The _decode_next_layer rewiring is correct but untested. Three of its
   forks independently built the same mutant — revert only that argument to
   frame.len, keep the attribute swap — and it passed everything. Reproduced here
   from an isolated copy: 19 passed, exit 0. Root cause confirmed: the truncated
   frame dissects to Ethernet:IPv4:UDP:Raw and Raw.read never uses the length
   it is handed (pylint: disable=unused-argument on its signature), so the
   over-long value reaches nothing that checks it.
   Fixed, as disclosure: the gap is now recorded in the PR description and in a
   comment at the call site, naming what a fixture would need to close it — a
   truncation landing in a length-checked field rather than bottoming out in Raw.
   The shipped argument is still the correct one; only the test coverage of that
   line is missing.

3. The pyshark.py corroboration was overstated. packet2dict is a generic
   getattr copy over frame.field_names and never reads Data_Frame.cap_len, so
   it is a naming coincidence rather than a code dependency.
   Fixed: demoted to a labelled-weak aside in the description and dropped from
   the changelog's argument. The Wireshark registration is what carries the point.

One further inaccuracy, found by a reviewer fork as an unreconciled hash and traced
here to its cause: the "after" hash originally quoted (4cdaadfa…) was taken
before a comment-only reword, so it no longer matched the tree being shipped. Both
hashes are now re-measured from immutable snapshots of the two commits and quoted
in full — before 260cee48528f4382edb4b2d2d74e6c3b, after
a025f5bda00f72c4222817ea52031fdf.

What the review could not check

It did not re-run the tests/foundation/ tests/integration/ selection (315 tests /
489 subtests) that the description cites, substituting a narrower directly-relevant
selection instead, because the full suite reaches ~41 GB RSS on this host and gets
killed. That figure is therefore the author's measurement only, not independently
reproduced. It has been re-run here since the amend and still passes.

[JarryShaw]

Rewritten: changelog entry moved out, rebased onto current main

This branch was force-pushed. Head is now 210c68cb5, one commit on top of
7b0df4a9c (origin/main at the time of the push).

What was dropped: this PR's bullet in docs/source/changelog/1.5.0.rst, and the
CHANGELOG.md regeneration that came with it. Nothing else. The code patch is
byte-identical to what was reviewed — verified by diffing the old and the new
patch restricted to the non-changelog paths, which came back empty. The commit
message is byte-identical too, Fixes #618 included.

Where the entry went: #657, the shared long-lived changelog PR for the 1.5.0
cycle, verbatim — not reworded, reflowed or trimmed. It merges last.

Why: five open PRs each added a bullet at the same anchor in the entry file. Any
one of them merging re-conflicted the other four on the two changelog files, even
though none of their code files overlapped at all. That cycle had already cost this
wave thirteen rebases. With the changelog out, this PR reports MERGEABLE and stays
that way when the others land.

This PR now touches the two Frame modules, pcapkit/toolkit/pcapng.py and two test
files — 5 files, down from 7.

Heads-up on red CI, which this PR does not cause

Changelog drift and the matrix jobs fail here, and would fail on any branch cut from
current main. main itself is drifted: 375e9d411 (#638) hand-inserted three lines
into the generated CHANGELOG.md instead of running util/changelog_md.py, which left
a duplicated #630 bullet and a misplaced #631 one. python util/changelog_md.py --check
exits 1 on main and on this branch, and the two changelog files here are
byte-identical to main's — so the failure is inherited, not introduced.

The repair is the first commit of #657 and can be cherry-picked ahead of the rest of
that branch to turn main green.