Skip to content

test(protocols): edge-case round trips for every layer family (#1202) - #1454

Merged
JarryShaw merged 1 commit into
mainfrom
test/1202-protocol-edge-cases
Oct 9, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
test/1202-protocol-edge-cases

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 9, 2026 •

Copy link
Copy Markdown
Owner

Refs #1202: the protocol edge-case slice. Unit tier, no fixture reads; tests/protocols/_edge_roundtrip.py is the shared harness.

Module Subtests Time / RSS
link/test_link_edge_roundtrip_unit.py (Ethernet, 802.1Q/ad, ARP/RARP, L2TPv2, OSPF) 143 0.8 s / 121 MiB
internet/test_internet_edge_roundtrip_unit.py (IPv4, IPX, IPv6 + every ext header, HIP, MH, AH, ESP) 581 1.2 s / 125 MiB
transport/test_transport_edge_roundtrip_unit.py (TCP, UDP, SCTP) 128 0.8 s / 120 MiB
application/test_application_edge_roundtrip_unit.py (FTP, HTTP/1, HTTP/2) 140 0.9 s / 145 MiB

Each case must parse to itself and rebuild from info. Its one-short and half-length cuts must either be refused with an in-library error or round-trip. make() must be stable through a re-parse. KNOWN_FAILURES is grouped by root cause and checked in both directions: #1455 (L2TPv2/OSPF drop octets past their length), #1456 (RPL compression lost inside IPv6), #1457 (truncated RPL address list), #1458 (a header cut inside a fixed-width field is zero-filled, every family), #1451 (its IPv6 extension-header case) and #1459 (HIP rejects packets whose fixed bits it must ignore). Each entry is pinned by status and a failure fragment. The #1443 and #1446 cases pass on main and are ordinary cases.

@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 9, 2026
@JarryShaw JarryShaw added test Pull requests that add or correct tests (test: subject prefix) review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Oct 9, 2026
@JarryShaw
JarryShaw force-pushed the test/1202-protocol-edge-cases branch from 53b9d24 to d0bbf31 Compare October 9, 2026 02:31
@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet and removed review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Oct 9, 2026
@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Coverage: 89.56% (unit tier, Python 3.14, 9106d4cde, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18795 957 2356 870 91.00%
pcapkit/corekit 2043 89 636 25 94.77%
pcapkit/dumpkit 242 2 88 2 98.79%
pcapkit/foundation 2648 98 930 48 94.80%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 16875 222 4554 187 98.05%
pcapkit/toolkit 539 74 168 3 84.58%
pcapkit/utilities 429 4 122 4 98.55%
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: NEEDS CHANGES at d0bbf31f3 (Sonnet cross-review; the author ran on Opus).

Blocking: hip/fixed-bit-clear and hip/leading-bit-set are listed in REJECTED in tests/protocols/internet/test_internet_edge_roundtrip_unit.py, but they are valid packets. RFC 7401 §5.1, which I re-read, says receivers MUST ignore the two fixed bits. I confirmed on main that both raise ProtocolError: HIP: invalid format, which makes this a round-trip defect, now filed as #1459. Move the two entries into a Gap(1459, ...) known failure.

Verified:

Nits:

@JarryShaw JarryShaw added review: needs-changes Cross-review at the current head says changes are required; see the verdict comment and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 9, 2026
Four unit-tier modules and a shared harness build minimum and maximum
lengths, reserved bits, zero-length values, padding variants and
unassigned codes for link, internet, transport and application
protocols. Each packet must parse back to itself and rebuild from its
info; one-short and half-length cuts must be refused in-library or
round-trip too; make() must be stable through a re-parse.

KNOWN_FAILURES groups what still fails by root cause, pinned by status
and failure fragment, and is asserted in both directions: #1455
(L2TPv2/OSPF length trailer), #1456 (RPL compression under IPv6), #1457
(truncated RPL address vectors), #1458 (zero-filled short reads), #1451
(its IPv6 extension-header case) and #1459 (HIP fixed bits rejected).
@JarryShaw
JarryShaw force-pushed the test/1202-protocol-edge-cases branch from d0bbf31 to 9106d4c Compare October 9, 2026 02:56
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at 9106d4cde (round 2; the round-1 Sonnet review was on d0bbf31f3).

Merge order: #1460 (#1455) and #1462 (#1459) will each make their gap entry here stale. Whichever lands second has to delete that entry.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment labels Oct 9, 2026
@JarryShaw
JarryShaw merged commit c61b20d into main Oct 9, 2026
38 checks passed
@JarryShaw
JarryShaw deleted the test/1202-protocol-edge-cases branch October 9, 2026 03:35
JarryShaw added a commit that referenced this pull request Oct 9, 2026
…fixed

#1454 merged before the three fixes, so its stale-entry check now fails on
main: #1455 (L2TPv2/OSPF trailer), #1456/#1457 (RPL) and #1459 (HIP fixed
bits) all come back OK. Drop those entries. The HIP fixed-bit cuts and two
RPL in-IPv6 cut55 cases now parse but zero-fill, so they move under the
#1451/#1458 short-read gaps. All four edge modules pass.
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 9, 2026
JarryShaw added a commit that referenced this pull request Oct 9, 2026
…ured (#1458, #1451)

When the data ended inside a field, Schema.unpack read the missing octets
as zeros and recorded nothing, so from_data(info) wrote that field, and
every field after it, at full width: Ethernet(bytes(12) + b'\x08')
rebuilt as 14 octets, and a Routing header with 23 of 24 octets as 24.

- Schema.unpack records the first field it reads short, and how many
  octets it got, as __short_read__; Schema.pack writes back only those,
  and Schema.to_dict/from_dict carry the record like __remainder__.
- keep_short_read/replay_short_read carry the record through info: the
  Link, Internet, Transport and Application bases add it after a parse
  and cut a from_data rebuild back to it. It is an ordinary info key, so
  Info.to_dict carries it and from_data(info.to_dict()) replays it too;
  only a truncated header has it.
- The #1454 edge tables drop the #1451/#1458 rows this closes; five SCTP
  last-chunk pad rows remain, under #1455.

Decoded values are unchanged: a truncated number still reads as
zero-filled.
JarryShaw added a commit that referenced this pull request Oct 9, 2026
…ured (#1458, #1451)

When the data ended inside a field, Schema.unpack read the missing octets
as zeros and recorded nothing, so from_data(info) wrote that field, and
every field after it, at full width: Ethernet(bytes(12) + b'\x08')
rebuilt as 14 octets, and a Routing header with 23 of 24 octets as 24.

- Schema.unpack records the first field it reads short, and how many
  octets it got, as __short_read__; Schema.pack writes back only those,
  and Schema.to_dict/from_dict carry the record like __remainder__.
- keep_short_read/replay_short_read carry the record through info: the
  Link, Internet, Transport and Application bases add it after a parse
  and cut a from_data rebuild back to it. It is an ordinary info key, so
  Info.to_dict carries it and from_data(info.to_dict()) replays it too;
  only a truncated header has it.
- The #1454 edge tables drop the #1451/#1458 rows this closes, and the
  harness its two now-unused defect strings; five SCTP last-chunk pad
  rows remain, under #1474.

Decoded values are unchanged: a truncated number still reads as
zero-filled.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test Pull requests that add or correct tests (test: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant