Repository navigation
fix(ipv6-route): compute Hdr Ext Len in 8-octet units on both sides - #489
Conversation
IPv6_Route.make() emitted a Source-Route header its own parser rejected, for every address count, and a hand-built spec-correct header failed too -- the parse path was broken independently of anything make() produced. Root cause, per RFC 8200 4.4: Hdr Ext Len counts the header in 8-octet units "not including the first 8 octets" (4 fixed + the 4-octet Reserved field every routing type's data starts with), so a header's total size is 8 + 8*HdrExtLen octets and its type-specific data is 4 + 8*HdrExtLen. - Write side: make()'s dict branch wrote raw octets into `length`; its Schema branch used the wrong sign on the offset. For a 24-octet, one-address header they emitted 20 and 3 where 2 is correct. Replaced both (and the bytes branch) with one shared helper, IPv6_Route._make_hdr_ext_len, so the field is computed once. - Read side (not previously isolated): ipv6_route_data_selector sized the nested routing-data schema at `hdr_ext_len * 8` octets, 4 short of the wire -- missing the Reserved field -- so even a correct hand-built header's `ip` ListField saw an invalid length. Added the inverse helper, ipv6_route_data_length, and used it in the selector. - Fixed the two read-side guards the issue flagged as suspect for the same unit confusion: Source Route's now checks Hdr Ext Len is even (each 16-octet address costs 2 units), Type 2's now checks it equals 2 (fixed 24-octet header). RPL's guard has the same surface shape but is left as-is: its construction already fails from an unrelated, separately tracked defect (#476/#480), so there is no working round trip to validate a replacement against. Measured before: make()+pack() for 0/1/2/3 addresses produced Hdr Ext Len bytes 4/20/36/52 (octets, wrong); reading any of them, or a correct hand-built 24-octet header (Hdr Ext Len=2), raised FieldValueError: Field ip has invalid length. After: bytes are 0/2/4/6 and all four round-trip, and the hand-built header parses. Extended the #485/#480 tuple-vs-list test to go through the full IPv6_Route round trip instead of the schema alone (its prior reason for not doing so no longer applies), added a dedicated construct-then-parse test over 0-3 addresses, and a hand-built-wire-form test independent of make(). Removed the now-stale ipv6-route-type/Source_Route and Type_2_Routing_Header entries from EXPECTED_FAILURES in test_option_roundtrip_unit.py; RPL_Source_Route_Header's entry stays. Build: tests/protocols/internet/ (185 passed, 631 subtests), tests/protocols/schema/ (25 passed), tests/protocols/test_option_roundtrip_unit.py (6 passed, 358 subtests). Closes #487
|
Full unit-test suite finished (matching CI's No failures anywhere in the tree, not just in |
|
Reviewed at 1. Is The formula is correct for all four, and it's correct by construction rather than by luck: per RFC 8200 §4.4, the total on-wire Routing header is
2. Are One thing not self-consistent, though pre-existing and not introduced or worsened by this PR: the 3. Read-side guards. raised from 4. Test adequacy. Ran the actual suites (PYTHONPATH forced onto this worktree,
The #485 tuple-vs-list byte-identity assertion ( Still uncovered, worth flagging rather than a blocker: Type 2 has no dedicated hand-built-wire-form parse test analogous to Source Route's new 5. 6. Docs. CI, polled to 0 pending on No blocking findings. The unit-conversion fix is correct and provably general across the whole registry, the two helper functions are genuine inverses, the RPL non-fix is independently justified, and the test and doc changes hold up under an adversarial re-check. GOOD TO MERGE |
Closes #487
Root cause (both halves, confirmed with measurements)
Per RFC 8200 §4.4,
Hdr Ext Lencounts the Routing header in 8-octet units, not including the first 8 octets (4 fixed octets + the 4-octetReservedfield every routing type's data starts with). So total header size =8 + 8*HdrExtLen, and type-specific data size =4 + 8*HdrExtLen.IPv6_Route.make()'sdictbranch wrote raw octets intolength(len(data_val.pack())); itsSchemabranch used the wrong sign ((len+4)/8instead of(len-4)/8). For a 24-octet, one-address header they emitted20and3where2is correct.ipv6_route_data_selector(schema module) sized the nested routing-data schema athdr_ext_len * 8octets — 4 octets short of the wire, missing theReservedfield. This is why a hand-built, spec-correct 24-octet header (Hdr Ext Len=2) also failed to parse, independent of anythingmake()produced — confirmed by hand-building that exact header and walking the selector by hand before touching any code.Measured before fix:
What changed
IPv6_Route._make_hdr_ext_len, and used it on all threemake()branches (bytes/dict/Schema) instead of two separate (both wrong) expressions.ipv6_route_data_length, and used it inipv6_route_data_selector._read_data_type_srcnow checksHdr Ext Lenis even (each 16-octet address costs 2 units);_read_data_type_2now checks it equals2(a Type 2 header is fixed at 24 octets)._read_data_type_rpl's% 16guard has the same surface shape but is left as-is: RPL's construction already fails earlier from an unrelated, separately tracked defect (RPL.post_processassumesbyteson a pathSchema.packalso runs — schema: Schema.pack rejects the tuple its own data models declare, breaking parse-then-reconstruct for 16 HIP parameters #476/schema: let ListField.pack accept the tuple its own data models declare #480), so there's no working round trip to validate a replacement guard against. Flagged in a code comment for follow-up rather than guessed at.Tests
IPv6_Routeround trip (previously it could only unpack the nested schema directly, because the full path could never pass) — kept the tuple-vs-list assertion, updated the expectedHdr Ext Lenbytes.ipv6-route-type/Source_RouteandType_2_Routing_Headerentries fromEXPECTED_FAILURESintests/protocols/test_option_roundtrip_unit.py(verified they now come back'OK');RPL_Source_Route_Header's entry stays, for the unrelated reason above.Verified the new/updated tests actually catch the regression: reverted just the two production files to
main's version, confirmed all three tests fail withFieldValueError: Field ip has invalid length., restored, confirmed they pass.Test plan
tests/protocols/internet/— 185 passed, 631 subteststests/protocols/schema/— 25 passed, 5 subteststests/protocols/test_option_roundtrip_unit.py— 6 passed, 358 subtests (up from 356; two stale gap entries removed)FieldValueError, pass post-fixtests/(excluding*_runtime.py/*_regression.py, matching CI's unit-tests job) — 917 passed, 5 skipped, 1784 subtests passed, no failuresOut of scope, deliberately not touched
pcapkit/foundation/reassembly/*,pcapkit/toolkit/*,pcapkit/__init__.py(per open version-bump PR)post_processbytes-vs-list defect and its 5-vs-4-octet fixed-field layout (schema: Schema.pack rejects the tuple its own data models declare, breaking parse-then-reconstruct for 16 HIP parameters #476/schema: let ListField.pack accept the tuple its own data models declare #480 territory)