Skip to content

fix(reassembly): a datagram too long for its length field never completes (#1585) - #1598

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1585-reassembly-length-limit
Oct 10, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1585-reassembly-length-limit

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

Please follow the guide below

What is the purpose of your pull request?

Tick the commit type your subject line carries.

  • fix — corrects a defect
  • feat — adds a feature
  • perf — changes performance, not behaviour
  • refactor — changes neither behaviour nor performance
  • test — tests only
  • docs — documentation only
  • ci — workflows or build tooling
  • release — bumps the version or rolls up a distribution
  • chore — anything else

Description of your pull request and other information

Closes #1585. The code change is in pcapkit/foundation/reassembly/ip.py and ipv6.py; the docs under docs/source/pcapkit/foundation/reassembly/ip/ and the Datagram.conflict comment are brought in line.

  • Length bound. IP._is_complete now bounds a datagram at 65,535 octets with its header, as the new IP._header_overhead counts it. IPv4's Total Length counts the whole header. IPv6's Payload Length counts every header except the fixed 40 octets, so ipv6.py overrides the hook. The issue's case 3c (20 + 65,523) now reads PARTIAL with every octet still reported. IPv6 with no extension header still completes at 65,535 octets of data.
  • Total Length 0 declares no length (fix(reassembly): IPv4 reassembly leaves a TSO datagram (total_length 0) incomplete #1555). If such a datagram is too long with its header, the buffer grows past 65,535 octets, and that lifts the bound. The 1555 and 1565 TSO pins are unchanged.
  • _detect_conflicts docstring. It now says that octets in an unmarked partial block are deliberately not compared (fix(reassembly): IPv6 reassembly zero-fills a snaplen-cut non-final fragment and reports it COMPLETE #1567). A new test pins this. The Datagram.conflict comment and the conflict notes in ipv4.rst and ipv6.rst had the same stale claim, and now say the same.
  • Docs. ip/index.rst lists the bound, with the Total Length 0 exemption, as a third departure from RFC 791.
  • Changed pin. One fix(reassembly): IPv4 reassembly raises IndexError for a fragment reaching octet 65,528 #1565 test asserted that an IPv4 final fragment at offset 65,528 completes. That is the defect, because 20 + 65,529 > 65,535, so the test now runs on IPv6 only.

The default, dpkt and scapy engines agree on crafted IPv4/IPv6 captures. Sample-capture reassembly is byte-identical to main: 7,998 rows across those engines, in both modes.

…etes (#1585)

- A datagram's length was bounded by the 65,535-octet data buffer alone, so
  an IPv4 one whose 20-octet header and 65,523 octets of data make 65,543
  read COMPLETE. IP._is_complete now bounds it by 65,535 octets with its
  header, as a new IP._header_overhead counts it: all of it for IPv4's Total
  Length, and for IPv6's Payload Length (IPv6 override) all but the fixed
  40-octet header.
- A Total Length of 0 declares no length (#1555), so a fragment whose header
  and data outgrow that grows the buffer past 65,535 octets, which lifts the
  bound. Growth is otherwise unchanged.
- The _detect_conflicts docstring said a non-final fragment's range is
  always block-aligned. It now documents that a cut, truncated or clipped
  fragment's unmarked partial block is not compared, deliberately; the
  Datagram.conflict comment and the ipv4/ipv6 conflict notes say the same.
- The IP reassembly docs list the bound as a third departure from RFC 791.
- A #1565 pin asserted that an IPv4 final fragment at offset 65,528
  completes; that is the defect, so the pin is now IPv6 only.

Sample-capture reassembly is unchanged (7,998 rows, default/dpkt/scapy).
@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) review: running A cross-review is in flight against the current head - no verdict yet labels Oct 10, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 10, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at 3fa86c900. This was a Sonnet cross-review; the author ran on Opus.

Non-blocking corner: a first fragment with options (IHL 60, real Total Length) followed by a TL=0 final fragment at IHL 20, with TDL 65,500, reads PARTIAL; on main it reads COMPLETE. The growth rule uses the arriving fragment's IHL, while the bound uses the first fragment's. TSO does not produce that mix, but it makes the "only the buffer bounds it" wording in ip/index.rst slightly too strong.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 10, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 89.70% (unit tier, Python 3.14, 3fa86c900, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 3 20 3 95.16%
pcapkit/const 18858 956 2366 870 91.04%
pcapkit/corekit 2366 67 760 35 96.48%
pcapkit/dumpkit 275 3 100 3 98.40%
pcapkit/foundation 2863 154 1006 42 93.59%
pcapkit/interface 138 7 52 5 93.68%
pcapkit/protocols 16996 227 4540 192 98.04%
pcapkit/toolkit 770 96 266 6 86.68%
pcapkit/utilities 494 4 146 4 98.75%
pcapkit/vendor 4409 2342 1006 157 43.25%

Per-file detail: the coverage-html artifact of this run.

@JarryShaw
JarryShaw merged commit d1c7bb5 into main Oct 10, 2026
40 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 10, 2026
@JarryShaw
JarryShaw deleted the fix/1585-reassembly-length-limit branch October 10, 2026 14:48
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 10, 2026
@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

fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(reassembly): a datagram whose header plus data exceeds 65,535 octets still reads COMPLETE

1 participant