Skip to content

fix(reassembly): IP reassembly reports only the octets it holds (truncated fragments, strict runs, zero-length datagrams) - #1368

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1357-1359-ip-reassembly
Oct 8, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1357-1359-ip-reassembly

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 7, 2026 •

Copy link
Copy Markdown
Owner
  • Searched for similar pull requests

  • Followed the coding style (make pylint, make mypy, make isort) — pylint (Makefile flags), mypy and isort run on reassembly/ip.py: clean

  • make test passes, and a test case covers the change — the modules listed below only, not the full suite

  • Added a changelog entry — N/A, added centrally

  • fix — corrects a defect


Closes #1357
Closes #1358
Closes #1359

One root cause: IP.reassembly wrote and marked octets from the declared Total Length rather than the octets held, and tested TDL for truthiness. A fragment now writes min(len(payload), TL-IHL) octets, clipped to the buffer, which never changes length. A truncated fragment leaves its partial last block clear. Strict runs stop at TDL and the last run is flushed. TDL is tested against -1, on every fragment.

Behaviour change: a datagram now completes on whichever fragment fills the last hole. Before, it completed only on the fragment with MF clear, and otherwise waited for a flush.

Repro Before After
2000 B, middle fragment 500/800 B (3 orders) COMPLETE, 300 zeros, tail shifted PARTIAL; strict [1296, 400]; loose 2000 B, [0:1300] real (incl. 4 unmarked tail octets), [1300:1600] zero
FO 0 1480 B + FO 2960 43 B, strict [1480, 48] [1480, 43]
FO 0 8 B + FO 65520 7 B, strict [8] [8, 7]
proto 59, TL=20, unfragmented PARTIAL, () COMPLETE, b''

Loose mode and truncated tails (documented, not changed). Receipt is tracked in 8-octet blocks, so a truncated fragment's partial last block is not marked. Strict runs leave it out. While TDL is unknown, loose mode returns the prefix up to the first clear bit, so it is left out there too. Once TDL is known, loose mode keeps its real octets and zero-fills only the rest of the hole. The IP docstring and the submit comments now say this.

Tests: tests/foundation/reassembly/test_ip_reassembly_roundtrip_unit.py passes 6, with 8 subtests. With origin/main's ip.py written over the fix, all 6 fail (11 failed counting subtests). These also pass: test_ip, test_ipv6, test_timeout, test_reassembly_base, tests/integration/test_reassembly_end_to_end.py and test_reassembly_engine_parity.py, which include the ipv6.pcap 4-fragment fixture under all 3 engines. tests/project gives 382 passed, 8 skipped.

util/run_unittest_leg.py foundation gives 489 tests, 2 failures and 11 errors, none in reassembly. The 2 failures are test_runtime_engines PCAPEngineTests, which fail identically on origin/main's ip.py. The 11 errors come from #1372, a test on main that writes JSON without bound until the disk fills. Under a 200 MB file-size cap, the leg on this PR and on its base both give the same 2 failures and 2 Errno 27 errors.

@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 7, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review verdict on 1273d3265: GOOD TO GO (Sonnet reviewed; author Opus).

Two body/docstring corrections, not blocking:

@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 7, 2026
…cated fragments, strict runs, zero-length datagrams)

- A fragment writes and marks only the octets the capture holds, clipped to
  its declared length and to the data buffer, which never changes length; a
  truncated fragment's partial last block stays a hole (#1357).
- Strict runs stop at the total data length, and the last run is emitted
  even when it reaches the end of the bit table (#1358).
- TDL is tested against -1 rather than for truthiness, so a zero-length
  datagram completes; completion is checked against the buffer's TDL on
  every fragment, not only the one with MF clear (#1359).
@JarryShaw
JarryShaw force-pushed the fix/1357-1359-ip-reassembly branch from 1273d32 to f0dcf5d Compare October 7, 2026 21:15
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict re-pointed to f0dcf5d62: GOOD TO GO. I read the delta from 1273d3265 myself. It touches one file, reassembly/ip.py (+12/−3), and every changed line is a docstring or a comment. The IP docstring now says that the partial last block of a truncated fragment is left out of the strict runs. In loose mode it keeps its real octets and only the rest of the hole is zero-filled. The docstring and a comment also cover the unknown-TDL case. The PR body cites #1372 for the capped-leg errors in place of the ordering explanation. No code changed, so the round-1 verification still holds.

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Coverage: 88.98% (unit tier, Python 3.14, f0dcf5d62, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18777 1032 2348 843 90.27%
pcapkit/corekit 1949 84 606 20 94.91%
pcapkit/dumpkit 189 1 68 1 99.22%
pcapkit/foundation 2452 102 858 39 94.41%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 16013 206 4178 177 98.06%
pcapkit/toolkit 539 74 168 3 84.58%
pcapkit/utilities 429 4 122 4 98.55%
pcapkit/vendor 4409 2359 1006 158 42.84%

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

@JarryShaw
JarryShaw merged commit a96aea3 into main Oct 8, 2026
36 checks passed
@JarryShaw
JarryShaw deleted the fix/1357-1359-ip-reassembly branch October 8, 2026 00:53
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 8, 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

1 participant