Skip to content

fix(protocols): understated __length_hint__ and an unguarded next-layer import still fabricate octets - #1176

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1173-length-hint-audit
Oct 7, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1173-length-hint-audit

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner
  • Searched for similar pull requests
  • Followed the coding style (make pylint, make mypy, make isort) — mypy on the 4 source files and pylint E/W on the new test only; no new messages, but not the full make targets
  • make test passes, and a test case covers the change — per module only, listed below; not the full make test
  • Added a changelog entry under docs/source/changelog/ and regenerated CHANGELOG.md, if the change is user-visible — N/A, added centrally after the wave

What is the purpose of your pull request?

  • fix — corrects a defect

Description of your pull request and other information

Closes #1173. IPv6_Route (4), MH (6), HOPOPT and IPv6_Opts (2) now hint 8, their minimum header (RFC 8200 §4.4/§4.3/§4.6, RFC 6275 §6.1.1). ApplicationBase._import_next_layer already reaches _parse_next_layer through super(), so it is unchanged; a test pins that route.

IPv4 + zero body before after
IPv6_Route, 4–7 octets IPv6-Route, rebuilds 8 Raw, rebuilds 4–7
MH, 6–7 octets MH, rebuilds 8 Raw, rebuilds 6–7

HOPOPT/IPv6_Opts were not fabricating (short streams already fail to parse), so their change is for correctness only. test_ipv6_extension_unit and test_mh_unit pinned the old hints and now pin 8.

from_data sweep over examples/captures/ (1612 frames) is the same before and after: IPv4 0/0, UDP 0/0, TCP 0/0, IPv6 0 / 183 (pre-existing), with byte-identical layer chains.

Tests: the new test_length_hint_minimum_header_unit.py gives 4 passed; without the fix, 10 subtests fail. ipv6_extension 61, mh 52, ipv6_ext 30, next_layer_short_header 7, ospf 3, option_roundtrip 6, tests/project 389 passed / 1 skipped. Ordering legs: protocols (rest) 838, 0F/0E before and after; protocols/internet 468 → 472, 0F/0E.

…er import still fabricate octets

IPv6_Route hinted 4 octets and MH 6, though both headers are at least 8
(RFC 8200 section 4.4, RFC 6275 section 6.1.1), so _parse_next_layer parsed
a 4-7 or 6-7 octet stream and rebuilt a full 8-octet header. HOPOPT and
IPv6_Opts hinted 2 for the same 8-octet minimum (RFC 8200 sections 4.3 and
4.6). All four now hint 8. The two tests that pinned the old hints now
pin 8.

Application payloads already reach _parse_next_layer through
ProtocolBase._import_next_layer, so application.py is unchanged; the new
test pins that route.
@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
@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Coverage: 88.89% (unit tier, Python 3.14, 222dd72a2, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18757 1034 2342 845 90.24%
pcapkit/corekit 1878 91 580 22 94.34%
pcapkit/dumpkit 142 0 42 0 100.00%
pcapkit/foundation 2442 102 852 39 94.38%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 15707 188 3974 163 98.19%
pcapkit/toolkit 487 65 144 1 85.42%
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

Copy link
Copy Markdown
Owner Author

Cross-review verdict on 222dd72a2: GOOD TO GO (ran on Sonnet; author Opus). Labelled review: good-to-go.

  • Hints: the RFC 8200 §4.3/§4.4/§4.6 and RFC 6275 §6.1.1 minimums were checked against the RFC text. An 8-octet header and any longer one still parse as IPv6_Route, MH, HOPOPT or IPv6_Opts, whether over IPv4 or in the IPv6 chain.
  • Behaviour change: the only change against main is Route at 4–7 octets and MH at 6–7 octets, which now become Raw and rebuild exactly.
  • Fails without the fix: with the four source files reverted, 10 subtests fail.
  • Ordering legs: protocols (rest) ran 838 tests and protocols/internet ran 472, both 0 failures and 0 errors.
  • Capture sweep: the from_data output is byte-identical between main and this PR.
  • Application path: application payloads reach the guard (application.py:132-154).
  • Nit: the application-path test and the complete-headers test pass on main as well. That is correct for guards, but neither one proves the fix.

Two follow-ups, both reproduced on main as well and neither blocking: #1177 (from_data drops extension headers, which I re-measured as 48→40 octets) and #1178 (whether the IPv6_Ext fallback hint stays at 2, needs: decision). CI: 0 failed, 0 incomplete.

@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
@JarryShaw
JarryShaw merged commit 20438e3 into main Oct 7, 2026
40 checks passed
@JarryShaw
JarryShaw deleted the fix/1173-length-hint-audit branch October 7, 2026 04:01
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 7, 2026
@JarryShaw JarryShaw added this to the PyPCAPKit v1.5 milestone Oct 25, 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(protocols): understated __length_hint__ and an unguarded next-layer import still fabricate octets

1 participant