Repository navigation
The #431 truncated-option short read has no test, only its empty-tail case does #572
Description
The short-read behaviour #431 established is load-bearing — a change that breaks it stops real captures parsing — but only its empty-tail case is tested. The truncated-data case has no coverage at all, and that gap let a proposed change regress it while passing the whole suite.
What is covered, and what is not
The only test guarding this is tests/protocols/internet/test_ipv4_unit.py:1977, and it exercises the path where the option area's tail is empty: the type byte decodes as 0 and the option loop breaks, reading it as end-of-option-list.
Not covered: an option whose declared length exceeds the data actually present — the "capture cut short by the snapshot length" shape that pcapkit/corekit/fields/collections.py's own #431 comment names. That path runs through OptionField.unpack, which rewinds and calls schema.unpack(file, length, packet) at collections.py:420, unpacking the whole option schema including its callable-length data field: BytesField(length=lambda pkt: pkt['length'] - 2) at pcapkit/protocols/schema/transport/tcp.py:321 and pcapkit/protocols/schema/internet/ipv4.py:259.
The concrete case that should be pinned
A TCP segment with data offset 7 (8 octets of option area) carrying unassigned option kind 0x4f, declaring length 12 with only 6 data octets present. On main this parses, yielding options=[('79', 12)]. A candidate fix for #554 (PR #571) turned it into
FieldValueError: Field data requires 10 octets, but only 6 are available.
escaping TCP(...) unwrapped — and the full suite stayed green, because nothing looks at this path. The same holds at declared length 32 against 6 octets.
Why this is worth its own test regardless of which #554 fix lands
The #431 accommodation is the constraint every bound on FieldBase.unpack has to respect, and it is currently protected by one test covering the easier half. Anyone tightening that function — and #554 is precisely such a change — needs a test that fails when the truncated-data path breaks, rather than discovering it from a user's capture.
Coverage wanted
A test constructing the segment above and asserting it parses, with the option's declared length and data preserved as main produces them. Ideally one for IPv4 options too, since the same callable-length data field exists there. Proven to fail against a deliberately over-strict guard, with the exit code read from a file.
Metadata
Metadata
Assignees
Labels
Projects
- StatusShow more project fieldsDone
Activity
test: pin the truncated-data half of the #431 short-read accommodation (
test: correct the docstrings on the #572 truncated-option tests (#578) (
JarryShaw commentedon Sep 21, 2026
Fixed, and ready to close.
f6721fe66(#578) added the missing truncated-data coverage — this issue's point was that only the empty-tail half of the #431 short-read accommodation was pinned, and the truncated-data half had no test at all.tests/protocols/transport/test_tcp_udp_unit.pyinmainatfa6d18e31now carries those cases.0503e6f0a(#580) followed up with docstring corrections to the tests #578 added. Worth recording why that follow-up existed: #578 merged before its cross-review finished, the review then found prose errors in the new tests' docstrings, and those corrections became their own pull request.Open only because #578's body referenced this issue as a bare
#572rather thanFixes #572.JarryShaw commentedon Sep 21, 2026
Closing: the missing truncated-data coverage shipped in
f6721fe66(#578), with docstring corrections in0503e6f0a(#580). Verified present inmainatfa6d18e31.fix(protocols)!: a declared length running past the data is accepted …
fix(protocols)!: a declared length running past the data is accepted …
fix(protocols)!: a declared length running past the data is accepted …