fix(ipv6-route): size the RPL fixed area at 4 octets per RFC 6554 (#564)

[JarryShaw]

Fixes #564

Root cause

RFC 6554 Section 3, "Format of the RPL Routing Header", draws the routing header as two 32-bit words plus the address vector. The second word — the part pcapkit's RPL routing-data schema owns, the first being the generic next/Hdr Ext Len/Routing Type/Segments Left — is:

     | CmprI | CmprE |  Pad  |               Reserved                |

and the field definitions give CmprI, CmprE and Pad each as a "4-bit unsigned integer", with Reserved taking the remaining 20 bits. That is 32 bits — one 4-octet word.

A citation note, since a previous change in this programme misattributed an RFC figure number: RFC 6554 carries no numbered figure captions at all. grep -i figure over the RFC returns one hit, lowercase prose in Section 2 introducing a different sketch. The diagram above is introduced only by "The Source Routing Header has the following format:". The only correct citation is Section 3 — there is no "Figure 1". Note also that Section 3's title is "Format of the RPL Routing Header"; "RPL Source Route Header" is the document title and running page header, not the section title.

pcapkit/protocols/schema/internet/ipv6_route.py declared the fixed area as UInt8Field() + UInt8Field() + BitField(length=3) — 5 octets. IPv6_Route._make_hdr_ext_len then derived Hdr Ext Len from that inflated data area, so a built header was narrower than its own declaration. Measured on the two addresses the round-trip table uses:

len(bytes(made)) == 41      # 4 + (5 + 32)
made.length      == 5       # ceil((37 - 4) / 8)
8 + 8 * 5        == 48      # what that Hdr Ext Len declares

_read_data_type_rpl's own docstring diagram had the layout right all along; only the schema disagreed with it.

Three more defects behind it, taken off in the same pass

#564's scope note asks for the Hdr Ext Len arithmetic and the % 16 guard to be reconciled together, "the guard cannot be validated against a header whose width is still wrong". Fixing the width made the next layer reachable, and so on; each of these is required for the one in front of it to be verifiable.

1. The reader's length guard (pcapkit/protocols/internet/ipv6_route.py:638-639). It read if header.length % 16 != 0, wrong twice over: header.length is Hdr Ext Len, "the length of the Routing header in 8-octet units" (Section 3), not an octet count; and a fixed multiple-of-16 bound assumes 16-octet addresses, which an SRH only carries when CmprI and CmprE are both 0. Together they admitted only Hdr Ext Len ∈ {0, 16, 32, …} — nothing under 136 octets. This is the unit confusion IPv6_Route Source-Route headers cannot round-trip: make() emits a wrong Hdr Ext Len and the parse path rejects even a correct wire form #487 fixed for Source Route and Type 2, which fix(ipv6-route): compute Hdr Ext Len in 8-octet units on both sides #489 flagged and deliberately left for want of a working RPL round trip. Replaced with Section 4.2's own arithmetic:

   n = (((Hdr Ext Len * 8) - Pad - (16 - CmprE)) / (16 - CmprI)) + 1
   

   The reader now requires that division to close — non-negative and whole. Nothing stricter is imposed: Section 4.2 specifies no malformed-header drop condition, only Segments Left > n.

2. RPL.post_process double-subtracted pad_len (schema/internet/ipv6_route.py:296). addresses' own length callback already takes pad_len off — the trailing padding is read by the separate padding field — so subtracting it again lost one address per 16 - CmprI octets of padding. Only reachable once the guard stopped rejecting every padded header.

3. RPL.post_process never set ip on the construction path (schema/internet/ipv6_route.py:264). Protocol.__post_init__ packs and then unpacks, and IPv6_Route.read hands _read_data_type_rpl the schema make just built, not a re-parsed one. On that path addresses is the list[bytes] of Two dropped-keyword/wrong-cast defects flagged in review and never filed (hip.py:3533, ipv6_route.py:207) #556, so post_process returned early and the reader raised a bare, out-of-library AttributeError: 'RPL' object has no attribute 'ip'. Masked for as long as the % 16 guard rejected constructed headers first — verified by neutralising only the guard on an otherwise unfixed tree (evidence below).

Plus one independent RFC MUST violation in the same arithmetic:

4. _make_data_type_rpl computed Pad as 8 - length % 8 (protocols/internet/ipv6_route.py:807), missing the outer % 8, so an already-8-octet-aligned vector was handed a full 8 octets of padding — 8 - 0 is 8, not 0. Reachable through the public API whenever dst shares no prefix with the addresses, which is exactly the case that forces CmprI and CmprE to 0, and Section 3 says: "Note that when CmprI and CmprE are both 0, Pad MUST carry a value of 0." _make_data_type_none in the same module already spelled the idiom with the outer modulo.

Breaking changes

• Wire format. A built RPL header is one octet narrower at the fixed area, and its Hdr Ext Len changes accordingly (2 addresses: 41 octets/Hdr Ext Len 5 → 40 octets/Hdr Ext Len 4). Headers built by the old code were self-inconsistent, so nothing conforming is lost.
• Schema constructor. RPL(cmpr_i=..., cmpr_e=...) is now RPL(cmpr={'cmpr_i': ..., 'cmpr_e': ...}), beside the pad={'pad_len': ...} that was already there. Data_RPL is unchanged — info.cmpr_i and info.cmpr_e still read as plain ints.
• A dst equal to one of the addresses yields CmprI of 16, which no longer fits a 4-bit field and now raises FieldValueError at pack time instead of silently packing a nonsense header. See "not fixed" below.

Hardening side-effect

cmpr_i was a whole octet, so a hostile packet could set it to 16 and make 16 - CmprI zero — a ZeroDivisionError in post_process. A 4-bit field tops out at 15, so the divisor is now structurally at least 1. Pinned by a test parsing 0xff into the cmpr octet.

Test evidence — failing first, then passing

Four new tests in tests/protocols/internet/test_ipv6_extension_unit.py. Proved against the unfixed tree via a throwaway git worktree at the pre-fix commit with only the new test file copied in (pcapkit/ pristine; since removed). Interpreter and tree pinned:

PRE-FIX TREE CONFIRMED: /tmp/pypcapkit-prefix564/pcapkit/__init__.py
pre-fix RPL fields: ['cmpr_i', 'cmpr_e', 'pad', 'addresses', 'padding']

Exit codes read from files, never from the printed summary (this pytest has no pytest-subtests, so a failing subtest's parent still prints PASSED).

Before — all four fail, EXIT=1 each:

test_ipv6_route_rpl_fixed_area_is_four_octets
>       self.assertEqual(len(bytes(bare)), 4)
E       AssertionError: 5 != 4

test_ipv6_route_rpl_length_guard_follows_rfc6554_address_arithmetic
>       info = IPv6_Route(io.BytesIO(uncompressed), len(uncompressed), extension=True).info
E           pcapkit.utilities.exceptions.ProtocolError: IPv6-Route: [TypeNo 3] invalid format

test_ipv6_route_rpl_pad_is_zero_when_nothing_is_elided
>       self.assertEqual(schema.pad['pad_len'], 0)
E       AssertionError: 8 != 0

test_ipv6_route_rpl_construction_path_decodes_its_own_addresses
>       built = IPv6_Route(type=Routing.RPL_Source_Route_Header,
E           pcapkit.utilities.exceptions.ProtocolError: IPv6-Route: [TypeNo 3] invalid format

The last one fails at the guard, which is what masked defect 3. Neutralising only the guard on the otherwise-unfixed tree exposes it, confirming the ordering claim rather than asserting it:

EXIT=1
>       built = IPv6_Route(type=Routing.RPL_Source_Route_Header,
>           ip=tuple(schema.ip),
E       AttributeError: 'RPL' object has no attribute 'ip'

After — EXIT=0:

7 passed, 52 deselected, 1 warning, 6 subtests passed in 5.87s

(7 = the 4 new tests plus the 3 pre-existing -k rpl ones.)

The round-trip gate proves it independently

ipv6-route-type/RPL_Source_Route_Header had an EXPECTED_FAILURES entry. With the fix applied and the entry still present, the harness failed because the case had started passing — its own words:

E  : ipv6-route-type/RPL_Source_Route_Header was recorded as failing with CONSTRUCT (...)
     but came back OK: . If the defect is fixed, delete its EXPECTED_FAILURES entry.
EXIT=1

Entry deleted, per the table's own "entries deleted rather than left behind" convention, with the four-defect stack recorded in the comment that replaces it:

6 passed, 1 warning, 358 subtests passed in 0.92s        EXIT=0

Suite

coverage run -m pytest (never pytest-cov). tests/protocols/ + tests/foundation/ — everything this change touches — 783 passed, 11 skipped, 1796 subtests passed, EXIT=0. Coverage, from a run scoped with --include to the two changed source files: both at 100% statements and 100% branches -- protocols/internet/ipv6_route.py 179 statements / 54 branches and schema/internet/ipv6_route.py 78 / 12, zero missing, EXIT=0. No tree-wide run was completed: one was started and terminated for host memory, so its result is unknown and nothing above rests on it. CI's own matrix is the full-suite gate.

Found and deliberately not fixed

• CmprI of 16 from make. If dst equals one of the addresses, os.path.commonprefix returns 16 octets and _make_data_type_rpl sets cmpr_i/cmpr_e to 16, which a 4-bit field rejects with FieldValueError. Eliding all 16 octets is meaningless (a zero-length element), so refusing is right, but the message comes from BitField rather than from a check that explains itself. Clamping or validating in _make_data_type_rpl is a separate behaviour decision and belongs in its own change.
• A zero-address RPL header raises a bare ValueError during parse. Hdr Ext Len of 0 gives an empty address buffer; post_process then calls ipaddress.ip_address(b''), which raises ValueError, not an in-library exception. It happens before the guard, which runs in _read_data_type_rpl after the schema has been unpacked, so the new guard cannot catch it either. Pre-existing and orthogonal to the octet width; fixing it means restructuring where validation happens.
• _make_data_type_rpl with dst set and ip=[] raises IndexError from ip[-1]. Pre-existing, unrelated.
• ipv6_route_header_length is undocumented — docs/source/pcapkit/protocols/internet/ipv6_route.rst lists ipv6_route_data_selector and ipv6_route_data_length but not it. Left alone to keep this diff to the defect; only CmprInfo was added there, which this change requires.
• A GH-556 reference in test_ipv6_route_rpl_packs_a_multi_address_list's docstring, where house style is #556. Not touched, to avoid widening an already sizeable test diff.

None of this has been checked against a real RPL capture — the same caveat #489 recorded against the guard it replaces, and #564 repeats. Every measurement here is against RFC 6554's text and hand-built wire forms.

[JarryShaw]

✅ GOOD TO GO at head sha 2947b80517924331b517550a87a905b4fd561e43 (branch fix/564-rpl-fixed-area-four-octets). This PR was force-pushed while I was reviewing it — my first pass (against the earlier head a25ccff59f79f26b7cdcae224de4b9f07228f86a) had independently found a real, reproducible Changelog drift failure and diagnosed its root cause before I noticed the head had moved; the new head's diff against the old one is changelog-only and fixes exactly that. Full derivation in the appendix.

[JarryShaw]

Reviewer: Sonnet; PR authored on Opus 5.

Falsify-not-bless pass on PR #590 (fix/564-rpl-fixed-area-four-octets), closing #564. Final head 2947b80517924331b517550a87a905b4fd561e43; a force-push moved it there from a25ccff59f79f26b7cdcae224de4b9f07228f86a partway through this review, and both heads are covered below.

1. RFC 6554 re-derivation, from https://www.rfc-editor.org/rfc/rfc6554.txt directly

• Citation-accuracy claim: the PR notes RFC 6554 has no numbered figures and that "Section 3" (not "Figure 1") is the only correct citation, and that "RPL Source Route Header" is the document's running title, not the section 3 heading. Checked with grep -in figure on the downloaded plaintext: one hit total, lowercase prose in Section 2 ("...as shown in the following figure:"), unrelated to the fixed-area diagram. Section headers confirm 3. Format of the RPL Routing Header at line 304, and the running page header reads RFC 6554 RPL Source Route Header March 2012 — exactly as the PR describes. Confirmed independently, not taken on the PR's word.
• The fixed-area width: RFC 6554 §3's own diagram, read directly, gives CmprI/CmprE/Pad as three 4-bit fields plus a 20-bit Reserved in one 32-bit row = 4 octets. The field definitions below the diagram state each of CmprI, CmprE, Pad as "4-bit unsigned integer" verbatim. Matches the PR's claim exactly.
• The §4.2 arithmetic: RFC 6554 §4.2's algorithm listing gives, verbatim: n = (((Hdr Ext Len * 8) - Pad - (16 - CmprE)) / (16 - CmprI)) + 1 and if Segments Left is greater than n { ... } as the only malformed-header condition specified. The PR's guard formula and its claim that "nothing stricter is imposed" both match this verbatim.
• The Pad-must-be-zero rule: RFC 6554 §3 states "Note that when CmprI and CmprE are both 0, Pad MUST carry a value of 0." — matches the PR's cited justification for the _make_data_type_rpl fix exactly.

2. Code-level claims, verified against the diff

Read pcapkit/protocols/schema/internet/ipv6_route.py and pcapkit/protocols/internet/ipv6_route.py diffs directly (unchanged by the later force-push, which touched only the changelog):

• cmpr_i/cmpr_e were two full UInt8Field()s (2 octets) before this PR — confirmed in the diff's removed lines — replaced by one BitField(length=1, namespace={'cmpr_i': (0,4), 'cmpr_e': (4,4)}), matching RFC 6554 §3 exactly and dropping the fixed area from 5 to 4 octets.
• The reader's old guard was header.length % 16 != 0 — confirmed in the diff — replaced by the §4.2 arithmetic (remainder = header.length*8 - pad_len - (16-cmpr_e); if remainder<0 or remainder % (16-cmpr_i) != 0).
• RPL.post_process's construction-path early return self previously skipped setting self.ip at all — confirmed in the diff (the new code adds an explicit self.ip = [...] list comprehension before returning) — and the parse path's double-subtraction of pad_len ((len(buffer) - self.pad['pad_len'] - elen) // ilen → (len(buffer) - elen) // ilen, since buffer's own length callback already removes pad_len) is visible in the same diff.
• _make_data_type_rpl's pad computation changed from 8 - (...) % 8 to (8 - (...) % 8) % 8 — confirmed in the diff — which only changes behavior when the inner remainder is exactly 0 (old: pad=8, new: pad=0), matching the RFC's "MUST carry a value of 0" requirement precisely.

3. Tests: proven to fail without the fix, by revert-and-rerun

Checked out the (then-current) head into a throwaway worktree and ran the RPL-keyed tests: 7 passed, 52 deselected, 6 subtests passed in 6.20s, exit 0 — matches the PR's own reported 7 passed, 52 deselected, 6 subtests passed in 5.87s.

Reverted only pcapkit/protocols/internet/ipv6_route.py and pcapkit/protocols/schema/internet/ipv6_route.py to origin/main (keeping the PR's updated tests) and reran: 6 failed, 1 passed, 52 deselected, 3 subtests passed in 6.85s, exit 1. All 4 of the PR's own named new tests (test_ipv6_route_rpl_fixed_area_is_four_octets, test_ipv6_route_rpl_length_guard_follows_rfc6554_address_arithmetic, test_ipv6_route_rpl_pad_is_zero_when_nothing_is_elided, test_ipv6_route_rpl_construction_path_decodes_its_own_addresses) are among the 6 failures. The other 2 (test_ipv6_route_rpl_source_addresses_reject_a_bool, test_ipv6_route_schema_selector_and_rpl_post_process_branches) fail too, but for an explained, non-defect reason: the PR's own "Breaking Changes" section discloses that the schema constructor moved from RPL(cmpr_i=..., cmpr_e=...) to RPL(cmpr={'cmpr_i': ..., 'cmpr_e': ...}), and these two pre-existing tests were updated in this same PR to use the new shape — reverting only the source (not the tests) leaves them calling a constructor argument (cmpr=) the old schema doesn't recognize (UnknownFieldWarning: 'cmpr' is not a valid field name, then TypeError: unsupported operand type(s) for -: 'int' and 'UInt8Field'). This is an artifact of my revert method interacting with a disclosed breaking change, not evidence against the PR.

4. The round-trip stale-entry deletion, reproduced exactly

Restored the fixed source and ran the full option round-trip suite: 6 passed, 1 warning, 358 subtests passed in 0.95s, exit 0 — matches the PR's claim exactly. Read the diff of tests/protocols/test_option_roundtrip_unit.py: the ipv6-route-type/RPL_Source_Route_Header Gap entry is deleted and replaced with a comment recording the four-defect stack, consistent with the table's own "delete rather than leave behind" convention (same mechanism I verified for PR #585's MP_FASTCLOSE entry).

5. Fixes #564 literal line

Present, verbatim, first line of the PR body: Fixes #564.

6. CI — the one real finding, now resolved by a force-push during this review

At the original head a25ccff59f79f26b7cdcae224de4b9f07228f86a, Changelog drift was FAILURE while every other CheckRun was SUCCESS/IN_PROGRESS/QUEUED/SKIPPED. Independently reproduced (not taken from the job log) by running python util/changelog_md.py --check in a throwaway worktree:

ResidualMarkupError: docs/.../1.5.0.rst uses reStructuredText the six conversion rules do not cover:
  line 46: an unconverted `` literal: "- **Fixed** -- IPv6-Route's RPL Source Route Header routing data was f"
  line 47: an unconverted `` literal: '- 8) - Pad - (16 - CmprE)) / (16 - CmprI)) + 1``, which the reader now'
  line 46: an unconverted interpreted-text role: ':rfc:`...`'  (x2)
  line 47: an unconverted hyperlink reference (the link target would be lost): '`...`_'

Diagnosed the root cause myself from util/changelog_md.py's actual source rather than guessing:

• **Rule 2's regex is r':rfc:\(\d+)`'** — digits only. The new entry used :rfc:`6554#section-3`and:rfc:`6554#section-4.2` (anchor fragments), which that regex cannot match at all, so both roles passed through unconverted. Confirmed by editing a scratch copy to drop the anchors (:rfc:`6554`, section 3) — the two :rfc:` residual hits disappeared.
• **Rule 3's regex is re.sub(r'\`([^\`]+)``', r'`\1`', line), applied per line** (the conversion loop processes one source line at a time). The entry's RFC §4.2 formula was written as a double-backtick literal split across a line wrap (`` ``n = (((Hdr Ext Len`` on one line, `` * 8) - ... + 1`` `` on the next) — since neither line contains both the opening and closing `` `` `` pair, rule 3 fires on neither, leaving literal backticks that rule 6 then joins into the surrounding prose, producing the garbled "unconverted `` literal" reports and (as a downstream artifact of the same broken span colliding with a later legitimate `` _make_data_type_rpl`` term after unwrapping) the phantom "hyperlink reference" report. Confirmed by rejoining the formula onto one line in the same scratch copy: the:rfc:`-anchor fix alone left 2 residual hits; adding this fix cleared the remaining 2 (the "unconverted `` literal" and the hyperlink-reference report both disappeared in combination).

While I was mid-diagnosis, the PR was force-pushed to 2947b80517924331b517550a87a905b4fd561e43. Diffing the two heads shows the amend is changelog-only (CHANGELOG.md and docs/source/changelog/1.5.0.rst, nothing else), and the new text does exactly what my scratch-copy experiment predicted: :rfc:\6554` section 3/:rfc:`6554`'s ... section 4.2's(anchors dropped, section named in prose) and the §4.2 formula rejoined onto a single line. Independently verified at the new head:grep -c ':rfc:`[0-9]*#'on the new1.5.0.rstreturns0, and python util/changelog_md.py --checkin a fresh worktree at2947b80printsCHANGELOG.md is in step with docs/source/changelog/1.5.0.rst, exit 0. CI's Changelog driftCheckRun re-queued on the new commit with no FAILURE anywhere in the 24-CheckRun tally as of this writing (6 SUCCESS, 5 IN_PROGRESS, 10 QUEUED, 2 SKIPPED, 0 FAILURE —Changelog drift` itself QUEUED, not yet re-run, but the source-level fix is independently confirmed regardless of when that specific job re-executes).

Disagreements / open items

• None on the RFC 6554 derivation or the code fix — both check out precisely against the primary source and by direct revert-and-rerun.
• The changelog defect was real and I found it independently before the author/coordinator's fix landed; it is now independently confirmed resolved at the source level (grep + --check exit 0), not merely inferred from the CI conclusion re-queuing.
• Changelog drift's CheckRun had not finished re-running at time of posting; flagging that the source fix is confirmed rather than claiming the CheckRun itself has gone green, since those are different things and conflating them is exactly the kind of premature-green claim this review process exists to catch.