Fix five dpkt toolkit defects (#351, #370)

[JarryShaw]

Two filed issues, plus three more found while verifying them. Every one made the dpkt engine quietly wrong rather than loud, which is why nothing noticed.

#351 — TCP header split at a fixed 20 octets

dpkt.tcp.TCP serialises as pack_hdr() + opts + data, but the toolkit sliced at tcp.__hdr_len__, the fixed 20-octet struct size. With options present the option bytes landed at the head of the payload and len disagreed with payload.

The consequence is worse than a misplaced boundary: the reassembly buffer is sequence-indexed, so those bytes overwrote payload instead of lengthening it.

test.pcap	before	after
frame 6 len(header)	20	32 (= dataofs*4)
frame 6 len / len(payload)	269 / 281	269 / 269
frame 6 payload[:20]	b'\x01\x01\x08\nL\n\x9a\xfb\xd6\x17Q)GET /ind'	b'GET /index.html HTTP'
frames mismatching	32 / 32	0 / 32
dpkt datagrams	10 (6 spurious, 4 completed=False)	4, all complete
4587-octet datagram head	b'\x01\x01\x08\n\xd6\x17Q5L\n\x9a\xff'	b'HTTP/1.1 200'

Now tcp.pack()[:tcp.off * 4], with payload taken from tcp.data so len and payload cannot disagree even on a malformed Data Offset. pcapkit/toolkit/scapy.py already did this correctly and was the in-repo reference. dpkt output is now byte-for-byte identical to the default engine on all four datagrams.

#370 — IPv6 fragment API read wrongly

ipv6_frag.nh does not exist on dpkt 1.9.8 (the class exposes nxt), and frag_off was passed unscaled where the reassembler wants octets. The issue verified the AttributeError at attribute level and reasoned the engine path was unusable; measured end to end here, it propagates out of the engine uncaught — a hard crash, not a degraded result. After the fix the same capture reassembles to one complete 1536-octet datagram, byte-identical to the original body.

The units contract was checked rather than assumed: pcapkit/foundation/reassembly/ip.py:97-105 indexes RCVBT[FO // 8], and the native IPv4 parser scales * 8, so octets is right.

The IPv4 latent defect — real

Same fixed-length mistake on the IPv4 path. Not observable on the fixtures because none carries IP options (as the issue said), so it was filed as inferred; confirmed here with a constructed hl=6 packet — 20-octet header and payload starting b'\x01\x01\x01\x00YYYY' before, 24 and b'YYYYYYYY' after. Now ipv4.hl * 4.

Three more, found by measurement

• fo=ipv4.off used a property dpkt deprecates in 1.9.8, and it returns the raw flags-and-offset word: for a first fragment with MF set it returned 8192 instead of 0. Now ipv4.offset * 8; the deprecation warning goes away as a side effect.
• tl=len(ipv6) counted the fragment header, so TL − IHL overshot the payload by 8. Since the reassembler does datagram[start:stop] = payload, a length mismatch silently resized the 65535-octet buffer. Now hdr_len + len(payload).
• bufid[3] passed .name, a string, where BufferID declares TransType and submit() hands it to Protocol.analyze. Internet.__proto__ is a defaultdict, so it never crashed — it silently degraded every reassembled datagram's parsed packet to Raw. That quietness is exactly why it survived.

Why the old tests missed all of it

The fakes were unfaithful. The IPv6 mock manufactured nh = 6 — the very attribute whose absence is the bug — and asserted fo == 2 from nxt, pinning the wrong behaviour. The TCP mock had no off at all. The fakes now match the real dpkt API.

New tests pin invariants, not magic numbers: len(header) == tcp.off * 4, len == len(payload), header + payload == tcp.pack(), tl - ihl == len(payload), bufid[3] is a TransType — plus an end-to-end assertion that dpkt reassembly of test.pcap matches the default engine byte for byte, which is the check that would have caught this. Each fixture asserts it actually exercises the defect, so none can pass vacuously.

Fail-before: 42 failed across 10 methods. After: 14 passed, 36 subtests. Full suite 512 passed, 4 skipped, 321 subtests.

Reported, not fixed

The default engine mis-reassembles fragmented IPv6 — pcapkit/protocols/internet/ipv6_frag.py:141 passes the offset unscaled where IPv4 scales * 8 (that is #352). Measured on a 1536-octet two-fragment datagram: default gives 640 octets and does not match the original, dpkt after this change gives 1536 and does. toolkit/scapy.py passes unscaled offsets too. This is why there is no dpkt-vs-default IPv6 parity test here — the default engine is currently the wrong reference for IPv6. The TCP parity test is unaffected.

Also untouched: ipv6.flow (filed separately), and the pre-existing trace=True crash for dpkt/scapy.

Closes #351, closes #370.

[Copilot]

🟢 Approval recommended

The changes are narrowly scoped to verified dpkt API mismatches/units bugs and are backed by strong regression and end-to-end tests that assert the key invariants.

Pull request overview

This PR fixes multiple correctness defects in the engine='dpkt' toolkit that previously produced silently incorrect TCP/IP reassembly output (and, for IPv6 fragments, could crash), and adds regression/integration tests to lock in the corrected invariants.

Changes:

• Fix TCP header/payload split to use tcp.off * 4 and take payload from tcp.data (prevents TCP options bytes from corrupting reassembly payload).
• Fix dpkt IPv6 fragment handling (nxt vs missing nh, scale frag_off to octets, and correct payload/length calculations around the Fragment header).
• Fix dpkt IPv4 fragment handling (use hl * 4 for IHL, scale offset * 8 for fragment offset, and pass TransType instead of a string in the buffer identifier) and add comprehensive regression + engine parity tests.

File summaries

File	Description
pcapkit/toolkit/dpkt.py	Corrects TCP/IPv4/IPv6 reassembly field extraction (header lengths, fragment offsets, dpkt attribute usage, and buffer identifier protocol type).
tests/toolkit/test_dpkt_unit.py	Adds targeted regression fixtures plus end-to-end parity tests to ensure dpkt reassembly matches expected invariants and (for TCP) matches the default engine byte-for-byte.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.