Skip to content

fix(hip)!: parse and keep the two fixed header bits (#1459) - #1462

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1459-hip-fixed-bits
Oct 9, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1459-hip-fixed-bits

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 9, 2026 •

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • 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 #1459

RFC 7401 §5.1: the two fixed bits "MUST be ignored when receiving". read() no longer rejects them and keeps them as packet_fixed and version_fixed. make() writes them, defaulting to the RFC values 0/1.

Breaking: Data_HIP gains two required fields.

Before: 59 04 10 20 … and 59 04 90 21 … raised ProtocolError: HIP: invalid format. After: both parse and from_data rebuilds them byte-exact.

New test_hip_fixed_bits_unit.py fails 7 of 8 checks without the fix. Two cases in test_hip_unit.py that pinned the rejection now pin the parse. Leg protocols/internet: 604 tests, 0 failures. Capture round-trip: 4 passed. tests/project: 419 passed, 1 skipped. examples/captures info: 1567 frames unchanged; the 49 HIP frames differ only by the two new keys.

RFC 7401 section 5.1 reserves the leading bit of Packet Type and the
low-order bit of Version for SHIM6: "MUST be set as shown when sending and
MUST be ignored when receiving". HIP.read raised ProtocolError unless they
read 0 and 1.

- read() no longer validates them and keeps them as the new required
  Data_HIP fields packet_fixed and version_fixed.
- make() takes packet_fixed=0 and version_fixed=1, the RFC values, and
  _make_data passes them through, so from_data rebuilds byte-exact.
- test_hip_unit: the two cases that pinned the rejection now pin the parse.
@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) breaking Breaks public-facing behaviour or API (apply alongside the type label) review: running A cross-review is in flight against the current head - no verdict yet labels Oct 9, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at b4d07c2af (Sonnet cross-review; the author ran on Opus).

  • RFC 7401 §5.1 checked. Bit 0 is 0 and the last bit of the version octet is 1 when sending, and both must be ignored on receipt. The PR's make() defaults (packet_fixed=0, version_fixed=1) match. The make() output is byte-identical to main for six parameter sets.
  • All four fixed-bit combinations round-trip. Each one rebuilds exactly through from_data(info) and from_data(info.to_dict()), both standalone and inside IPv6. I re-ran the two issue repros myself, in both forms.
  • The existing-test edit is legitimate. test_hip_unit.py drops only the two ProtocolError expectations and adds the new keys; nothing else is weakened.
  • ! is justified. Data_HIP gains two required fields. Nothing in the repo constructs it directly.
  • Tests: the new module fails before the fix with 7 errors. I re-ran it alongside test_hip_unit: 37 passed, 173 subtests. The internet leg (604) and tests/project (606) are OK. The 49 HIP capture layers differ from main only by the two new keys.

Merge note: #1454's Gap(1459) entry goes stale when this lands. Whichever of the two merges second removes it.

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

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Coverage: 89.51% (unit tier, Python 3.14, b4d07c2af, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18795 968 2356 869 90.90%
pcapkit/corekit 2043 89 636 25 94.77%
pcapkit/dumpkit 242 2 88 2 98.79%
pcapkit/foundation 2648 98 930 48 94.80%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 16871 225 4550 189 98.03%
pcapkit/toolkit 539 74 168 3 84.58%
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
JarryShaw merged commit 5132e93 into main Oct 9, 2026
42 checks passed
@JarryShaw
JarryShaw deleted the fix/1459-hip-fixed-bits branch October 9, 2026 03:36
JarryShaw added a commit that referenced this pull request Oct 9, 2026
…fixed

#1454 merged before the three fixes, so its stale-entry check now fails on
main: #1455 (L2TPv2/OSPF trailer), #1456/#1457 (RPL) and #1459 (HIP fixed
bits) all come back OK. Drop those entries. The HIP fixed-bit cuts and two
RPL in-IPv6 cut55 cases now parse but zero-fill, so they move under the
#1451/#1458 short-read gaps. All four edge modules pass.
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking Breaks public-facing behaviour or API (apply alongside the type label) fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(hip): packets whose fixed bits are not as sent are rejected, though RFC 7401 §5.1 says receivers must ignore them

1 participant