Skip to content

fix(const): the Quick-Start function enums reject 9-15, though the field is 4 bits wide - #1353

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1350-qs-function-range
Oct 8, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1350-qs-function-range

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner
  • Searched for similar pull requests (fix(ipv4): rebuild option areas, route slots, timestamps and reserved bits byte-exactly #1344 touches the IPv4 QS option, not these enums)
  • Followed the coding style: ran pylint and isort --check on the touched files, not make mypy. pylint reports only the existing duplicate-code between the two templates, plus missing method docstrings and _dest_path access in the new test, which follows the house test style.
  • make test passes, and a test case covers the change (only the modules listed below)
  • Changelog entry — N/A, added centrally

What is the purpose of your pull request?

  • fix — corrects a defect

Description of your pull request and other information

Closes #1350

RFC 4782 §3.1 gives Quick-Start "a four-bit Function field". The vendor templates set FLAG to 0 <= value <= 8. This PR widens it to 0 <= value <= 15, the decimal form that hip/hit_suite, hip/di and mh/dsmip6_tls_packet already use, and regenerates both const files. Both templates read the local DATA dict (LINK is None), so I rendered them offline by running the vendor class. The unchanged templates reproduce main's const files with no diff, and the fixed ones produce exactly the 2-line diff here.

Tests:

  • New tests/const/test_const_qs_function_range_unit.py: 0–15 resolve, -1, 16 and 255 raise, and each const file is byte-identical to its template's rendered output.
  • Without the fix (the origin/main sources written over the four files), it fails 14 subtests (9–15 × 2) and passes 3 tests; with the fix, 3 passed and 40 subtests passed.
  • util/run_unittest_leg.py const: 487 tests, 0 failures, 0 errors.
  • tests/project: 389 passed, 1 skipped.
  • Also green: test_quick_start_func_unit, test_ipv4_unit, test_ipv6_extension_unit, test_option_roundtrip_unit, test_const_enum_no_mint, test_tcp_quick_start_response_unit and test_const_enum_get.

…eld is 4 bits wide

RFC 4782 section 3.1 gives the Quick-Start option a four-bit Function
field. The vendor templates bounded QSFunction at 0 <= value <= 8, so
9-15 raised ValueError instead of resolving to Unassigned. Widen the
bound to 0 <= value <= 15 in both templates, matching the other 4-bit
const modules (hip.hit_suite, hip.di, mh.dsmip6_tls_packet), and
regenerate both const files from the templates. 16 and up still raise.

Closes #1350
@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) const Regenerated IANA or vendor constant tables; members keep their numeric values 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 0a61b1021: GOOD TO GO (Sonnet reviewed; author Opus).

  • RFC 4782 §3.1: "The third byte includes a four-bit Function field". Only 0000 and 1000 are assigned, so 0–15 is the full wire range.
  • Both namespaces: 1, 9 and 15 return Unassigned. 0 and 8 are unchanged. 16 and −1 still raise through both QSFunction() and .get.
  • Rendering: the PR's templates render byte-identical (cmp) to its const files. The unchanged main templates reproduce main's const files, which validates the method. hip/hit_suite, hip/di and mh/dsmip6_tls_packet use the same 0 <= value <= 15 form.
  • Callers: every caller goes through Enum_QSFunction.get, and none uses except ValueError as a validity check. HOPOPT function 15 now gets past the enum and stops at the selector with FieldValueError. That part is fix(ipv4): an unassigned Quick-Start function code fails the whole header #1322, which the PR rightly does not close.
  • Tests: the revert proof gives 14 errors on main's sources. The const leg runs 487 tests with 0 failures, and test_ipv4_unit plus test_ipv6_extension_unit run 87, OK.

@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
@github-actions

github-actions Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Coverage: 88.96% (unit tier, Python 3.14, 0a61b1021, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18777 1033 2348 844 90.26%
pcapkit/corekit 1878 89 580 22 94.43%
pcapkit/dumpkit 189 1 68 1 99.22%
pcapkit/foundation 2442 102 852 39 94.38%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 15968 206 4158 177 98.06%
pcapkit/toolkit 495 73 150 1 83.57%
pcapkit/utilities 429 4 122 4 98.55%
pcapkit/vendor 4409 2342 1006 157 43.25%

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

@JarryShaw

Copy link
Copy Markdown
Owner Author

The red PyPCAP/PyPCAPFile parity Python 3.11 leg is not caused by this diff. It is a slow mirror hitting a bound that main has already widened.

  • Both apt attempts in job 113001716708 were killed after exactly 90 s (20:53:30→20:55:00 and 20:55:04→20:56:34). Each stalled on the Azure mirror, the second while fetching libwireshark17t64, a 23.7 MB package.
  • 0a61b1021 predates ci(unit-tests): widen the apt bounds so a slow mirror still installs tshark #1309 (08c874eff), which raised that bound to 180 s. This branch still runs the old timeout 90.
  • I re-ran the failed jobs, but a re-run keeps the old bound. The fix is to update the branch onto main, which the strict ruleset requires anyway. The Required checks passed failure follows from this leg.

@JarryShaw
JarryShaw merged commit 75d28cd into main Oct 8, 2026
64 of 66 checks passed
@JarryShaw
JarryShaw deleted the fix/1350-qs-function-range branch October 8, 2026 00:44
@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

const Regenerated IANA or vendor constant tables; members keep their numeric values fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(const): the Quick-Start function enums reject 9-15, though the field is 4 bits wide

1 participant