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.
@JarryShaw JarryShaw added this to the PyPCAPKit v1.5 milestone Oct 26, 2026
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