Skip to content

fix(protocols): Quick-Start option readers use schema.func, which exists only under TYPE_CHECKING - #1117

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1091-quick-start-func
Oct 6, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1091-quick-start-func

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 6, 2026 •

Copy link
Copy Markdown
Owner
  • Searched for similar pull requests
  • Followed the coding style (make pylint, make mypy, make isort) — run on the 5 touched pcapkit/ files with the Makefile flags; no findings on changed lines (2 mypy unused-ignore at hopopt.py:1634/ipv6_opts.py:1623 predate this PR)
  • make test passes, and a test case covers the change — the modules listed below, not the full suite
  • 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 #1091. _read_opt_qs (IPv4, HOPOPT, IPv6-Opts) now reads flags['func'], matching how it already reads flags['rate']; schema.func is set only by _QSOption.post_process, so maker-built schemas raised AttributeError. HOPOPT/IPv6-Opts quick_start_data_selector also used SchemaField(length=5) for an 8-octet option (the second defect the EXPECTED_FAILURES entries recorded), now 8.

Probe (make_samples.py): before, 8 left out incl. hopopt-option/Quick_Start, ipv4-option/QS, ipv6-opts-option/Quick_Start (no attribute 'func'). After, 5 left out, none Quick-Start; 256 → 259 capturable. A built HOPOPT/IPv6-Opts option with nonce 19088743 decoded as 16777216 with packet length < 0: -3; now 19088743, no warning.

New tests/protocols/internet/test_quick_start_func_unit.py (round trip ×3 families ×2 functions, plus HOPOPT/IPv6-Opts wire nonce). Reverting the reader fix: 6 subtests fail (AttributeError); reverting the length fix: 6 fail (SchemaWarning: packet length < 0: -3). With both: 2 passed, 8 subtests passed. Removed the three Quick-Start EXPECTED_FAILURES entries; test_ipv4_unit and test_ipv6_extension_unit injected schema.func to reach the unknown-function branch, now set flags['func'].

Passing: test_option_roundtrip_unit (6), test_ipv4_unit (26), test_ipv6_extension_unit (61), test_ipv6_ext_unit (30), test_ipv6_extension_runtime (3), test_option_coverage_runtime (3), test_tcp_udp_unit (18), tests/project (379 passed, 1 skipped).

…sts only under TYPE_CHECKING

- _read_opt_qs in IPv4, HOPOPT and IPv6-Opts now reads the function from
  flags['func'], as it already reads rate. schema.func is set only by
  _QSOption.post_process, so a schema built by _make_opt_qs raised
  AttributeError when re-read.
- quick_start_data_selector in schema/internet/hopopt.py and ipv6_opts.py
  sized the nested suboption as 5 octets against the 8-octet option, which
  mis-decoded the nonce; it now uses 8.
- Drop the three Quick-Start EXPECTED_FAILURES entries; two reader tests
  that injected schema.func now set flags['func'] instead.

Closes #1091
@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 6, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review verdict on c2f928879: GOOD TO GO (ran on Sonnet; author Opus)

  • Length fix. Checked against RFC 4782 §3.2. The IPv6 Quick-Start format matches IPv4's, except that the Length field excludes type and length (6, against 8 for IPv4). Both the Request and Report schemas cover the full 8-octet option, so length=8 is right for HOPOPT and IPv6-Opts. IPv4 already derives 8.
  • func is read from flags['func']. An unknown function value still reaches the ProtocolError branch.
  • Fails without either fix.
    • Reverting the reader change raises the original AttributeError.
    • Reverting the schema length change fails exactly 6 subtests.
  • Tests.

@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 6, 2026
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Coverage: 88.74% (unit tier, Python 3.14, c2f928879, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18757 1035 2342 846 90.23%
pcapkit/corekit 1874 91 578 22 94.33%
pcapkit/dumpkit 136 0 40 0 100.00%
pcapkit/foundation 2422 143 842 34 92.62%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 15653 187 3942 162 98.19%
pcapkit/toolkit 487 71 144 3 84.15%
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 8521a14 into main Oct 6, 2026
41 checks passed
@JarryShaw
JarryShaw deleted the fix/1091-quick-start-func branch October 6, 2026 20:37
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 6, 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): Quick-Start option readers use schema.func, which exists only under TYPE_CHECKING

1 participant