LOCATOR_SET is conformant only by accident: nested Locator.len shadows the parameter in padding, and len is in 4-octet units where RFC Length is bytes

[JarryShaw]

Two pre-existing defects in LOCATOR_SET cancel each other exactly, so the parameter is RFC-conformant on main by coincidence rather than by construction. Fixing either one alone makes it non-conformant. #664 fixes the padding half and the parameter is now four octets short at every locator count.

Measured on both trees, independently of the PR author

CPython 3.14.7, __editable__* stripped from sys.meta_path, pcapkit.__file__ asserted to each worktree before any other import, HIP._make_param_locator_set(Parameter.LOCATOR_SET, version=2, locator_set=[...]) then .pack():

locators	contents	schema.len	main @ 0c7f2b7c9	#664 @ 819dbbd6a	RFC 7401 for Length = 24n
1	24	4	32 OK	28 short by 4	32
2	48	8	56 OK	52 short by 4	56
5	120	20	128 OK	124 short by 4	128

RFC total from §5.2.1's Total Length = 11 + Length - (Length + 3) % 8.

The two defects, and why they cancel

1. LocatorSetParameter.padding reads the nested Locator.len, not the parameter's. ListField packs each Locator into the shared packet context, and the nested len shadows the parameter's own. An IPv6 locator always has Locator.len == 4, so the old padding expression always appended exactly 4 octets regardless of locator count.

2. The parameter's len is sum(Locator.len) — in 4-octet units — where the RFC's Length is a byte count. Visible in the table above: contents of 24, 48 and 120 octets give schema.len of 4, 8 and 20, i.e. 4n rather than 24n.

Old total: 4 + 24n + 4 = 24n + 8. And because 24n ≡ 0 (mod 8), the RFC total for Length = 24n is also 24n + 8. Identical for every n — which is why no test and no round trip has ever caught either defect.

Why this is filed rather than folded into #664

The repair is not a one-expression change like EncryptedParameter.data was. It changes a Length field on the wire, changes the public Data_LocatorSetParameter.length, and requires re-checking the reader's ListField(length=lambda pkt: pkt['len']), which consumes the same quantity. That is the same class of change as #672, not a coupled fix that #664 can absorb safely.

Consequence for #664 as it stands

#664 must not merge in its current state. It would take LOCATOR_SET from conformant to four octets short, which is a regression in on-wire output even though every individual change in it is correct. Two ways out, and the choice is the owner's:

• (a) Fix len inside fix(hip): pad HIP parameters to the record length RFC 7401 gives, not the contents (#651) #664 — correct on both counts, but widens an already-breaking PR into a wire Length change plus a reader re-check.
• (b) Narrow fix(hip): pad HIP parameters to the record length RFC 7401 gives, not the contents (#651) #664 to leave LOCATOR_SET's padding alone — keeps the accidental conformance, keeps the PR small, and this issue then carries both defects. My recommendation, because it separates a 46-parameter padding correction from a single parameter's length-unit bug, and because the cancellation means (b) ships nothing wrong.

Found by the #664 worker while fixing a stale test pin, disclosed rather than absorbed, and reproduced here independently before filing.

[JarryShaw]

Owner decision: option (b) — narrow #664, leave LOCATOR_SET's padding alone

The owner's call, on the recommendation above: narrow #664 so it does not change LOCATOR_SET's padding, and let this issue carry both defects.

Why this is the safe answer, restated so the reasoning survives

The two defects cancel exactly — old total 4 + 24n + 4 = 24n + 8, and because 24n ≡ 0 (mod 8) the RFC total for Length = 24n is also 24n + 8, identical for every n. So leaving both in place ships nothing wrong on the wire: LOCATOR_SET stays byte-for-byte what main emits today, which is what RFC 7401 §5.2.1 asks for.

Option (a) — repairing len inside #664 — was rejected not because it is wrong but because it is a different change: it alters a Length field on the wire, alters the public Data_LocatorSetParameter.length, and needs the reader's ListField(length=lambda pkt: pkt['len']) re-verified, since that consumes the same quantity. Folding that into an already-breaking 46-parameter padding correction would make one PR carry two unrelated wire-format changes, and the cancellation means there is no urgency forcing them together.

What #664 becomes

A padding correction for 45 of 46 HIP parameters, with LOCATOR_SET explicitly excluded and the exclusion documented in the code rather than left as an accident. The docs/source/changelog/1.5.0.rst entry on #657 needs its claim narrowed too — it currently says the change makes HIP records conformant, which is true of the 45 and not of LOCATOR_SET.

What this issue now owns

Both defects, together, because neither is fixable alone:

1. LocatorSetParameter.padding reading the nested Locator.len instead of the parameter's, via ListField's shared packet context.
2. The parameter's len being sum(Locator.len) in 4-octet units where the RFC's Length is a byte count.

Whoever takes this must fix both in one change and re-verify the reader, and must assert the packed total against 11 + Length - (Length + 3) % 8 for several locator counts rather than against a single pinned literal — a pinned literal is what let the cancellation hide for as long as it did.

A worker is narrowing #664 now. This issue stays open.

[JarryShaw]

The narrowing has landed, and one measured caveat for whoever takes this

#664 is narrowed as decided: LOCATOR_SET keeps the pre-#651 padding expression at
both of its sites — LocatorSetParameter.padding and the reported record length in
_read_param_locator_set — so that parameter is unchanged in its octets and its
data model. Verified byte-for-byte against a18846c8f over 15 construction cases and
3 parse cases, not just the three lengths the decision quoted. #664 is now a padding
correction for 45 of 46 parameters, guarded by a test that asserts the exclusion is
exactly one parameter and which one.

The caveat: the cancellation is narrower than 24n + 8

The decision's arithmetic — old total 4 + 24n + 4 = 24n + 8, RFC total for
Length = 24n also 24n + 8 — is correct, and it is what makes leaving the line
alone safe. But it holds only for a set of plain IPv6 locators with n ≥ 1. A
locator carrying an SPI is 28 octets rather than 24, so 24n ≡ 0 (mod 8) stops
applying; and an empty set has no nested locator to shadow len at all. Measured on
this branch and on a18846c8f, byte-identically on both:

case	declared len	packed	multiple of 8?	RFC total for a byte-count Length
empty, n=0	0	4	no	8
plain, n=1	4	32	yes	32
plain, n=2	8	56	yes	56
SPI, n=1	5	35	no	32
SPI, n=2	10	63	no	64
plain then SPI	9	59	no	56
SPI then plain	9	60	no	56

So four of those seven shapes are not 8-aligned today, and one of them — the
empty set — is the single remaining violation in the RFC-only fixture walk over
options-internet.pcap, which goes 43 → 1 on #664. That frame is worth naming
precisely because it is easy to misread, and I did misread it at first: frame 102's
parameter area is 00 c1 00 00 00 c1 00 00, two empty LOCATOR_SET records of four
octets each where 11 + 0 - (0 + 3) % 8 = 8 is required. A Length-driven reader
advances 8 and consumes the second copy's header as padding.

What this means for the fix here. Asserting the packed total against
11 + Length - (Length + 3) % 8 for several locator counts, as this issue already
requires, is necessary but not sufficient — a sweep over n = 1..5 of plain locators
passes today by cancellation and would pass a half-fix too. The cases that
discriminate are the ones above: n = 0, the SPI/LocatorData variant, and a mixed
set in both orders, since those are the shapes where the two defects do not cancel
and where a fix to one of them alone shows up immediately.

Two further things found while measuring, both pre-existing and neither fixed:

• The SPI locator path is hard to reach through the public maker at all:
  _make_param_locator_set sets length = 5 when spi is given but leaves type
  at its default 0, and locator_value_selector accepts only type == 0, len == 4
  or type == 1, len == 5 — so spi= without an explicit type=1 raises
  FieldValueError: invalid locator type or length. Worth knowing before writing the
  tests for this issue, since the discriminating cases need that path.
• _read_param_locator_set's reported length is derived from the same wrong len,
  so it moves when len does. It is excluded from fix(hip): pad HIP parameters to the record length RFC 7401 gives, not the contents (#651) #664 for exactly that reason and
  should move with the pair rather than separately.

Credit where due: the narrowness of the cancellation was found by the cross-review on
#664, which went looking for locator shapes the 24n argument does not cover rather
than re-checking the ones it does.

[JarryShaw]

Correction to my own analysis above: the cancellation is not universal

I wrote that the two defects cancel "exactly" and that the totals are "identical for every n". That is overstated, and it changes how this issue should be read. The correction came from #664's worker via its cross-review, and I have re-measured the decisive case myself rather than relaying it.

What I verified directly

On origin/main, CPython 3.14.7, editable finder stripped via getattr(f, '__module__', ''), pcapkit.__file__ asserted to a throwaway worktree before any other import:

empty LOCATOR_SET        packed =   4   ** NOT 8-ALIGNED **   (RFC wants 8)
plain IPv6, n = 1        packed =  32   aligned
plain IPv6, n = 2        packed =  56   aligned

An empty locator set packs 4 octets where the RFC wants 8, on main, today. So the cancellation does not hold for every shape — it holds for homogeneous plain-IPv6 sets, which is what my n = 1, 2, 5 measurements happened to sample.

What I could not verify, stated as such

#664's worker additionally reports, measured identically on both trees: SPI n=1 → 35, SPI n=2 → 63, mixed → 59/60 — four of seven shapes not 8-aligned, before and after. I could not reproduce those: my attempt to build an SPI locator used the wrong key and raised FieldValueError: invalid locator type or length. Those three figures are its measurement, not mine. The empty case above is mine and is sufficient on its own to retract the universal claim.

Why this matters for the decision, which does not change

Option (b) — leave LOCATOR_SET's padding alone — remains the right call, but for a weaker and more honest reason than I gave.

I argued it ships "nothing wrong on the wire". That is true only of the homogeneous plain-IPv6 shapes. For the empty set and the SPI shapes, main is already non-conformant and stays so. So leaving the line alone is the safe choice — it changes nothing and breaks nothing — rather than the correct one. The parameter is not conformant today and will not be until both defects here are fixed together.

The practical consequence for whoever takes this issue: do not assume a passing round trip or an aligned total means the parameter is right. Assert against 11 + Length - (Length + 3) % 8 across several locator shapes — empty, plain, SPI-bearing, mixed — not merely several locator counts. Sampling counts within one shape is exactly how I reached the wrong conclusion, and how the cancellation stayed hidden for as long as it did.

#664's code comment at the excluded site and its PR body both now carry this narrower framing.