protocols: size HIP NAT_TRAVERSAL_MODE and ESP_TRANSFORM list entries at 16 bits, not 8

[JarryShaw]

Summary

• NATTraversalModeParameter.modes and ESPTransformParameter.suites (pcapkit/protocols/schema/internet/hip.py:567-570 and :939-942) declared their list item_type as EnumField(length=1) -- one octet per entry -- while RFC 5770 5.4 and RFC 7402 5.1.2 both specify a 16-bit Mode ID / Suite ID per entry. The makers (_make_param_nat_traversal_mode, _make_param_esp_transform) already compute len=2 + 2 * len(entries) on the 16-bit assumption, so the mismatch produced a phantom trailing entry on every round trip. This is the same defect class Four more HIP parameters underflow pkt['len'] - 2 and silently return an empty list #463/protocols: floor four more HIP list-length callbacks at zero #466 fixed for TRANSPORT_FORMAT_LIST, at two sites that fix did not cover.
• Fix: item_type=EnumField(length=2, ...) at both sites, matching HIPTransportModeParameter.mode, which already used the correct width.
• Added maker round-trip tests for both parameters (0-3 entries), plus a full-HIP()-parser test with two 8-aligned copies each.

Verification

Verified the RFC widths directly against the RFC text (both specify 16 bits) before changing anything, and reproduced the reported defect byte-for-byte on main:

modes=[1]  -> packed 0260000400000100000000 (11 octets, declared len=4) -> unpacked [1, 0]
suites=[1] -> packed 0fff000400000100000000 (11 octets, declared len=4) -> unpacked [1, 0]

I could not reproduce the issue's literal AttributeError: 'NATTraversalModeParameter' object has no attribute 'modes' through several full-HIP() round-trip constructions -- instead I hit ProtocolError: HIPv2: invalid format (via HIP.make() on two aligned copies, since the pre-fix per-parameter length isn't 8-aligned) and SchemaWarning: packet length < 0 with silent data loss (via a hand-built two-parameter packet). Both are consistent with "the full parser makes it worse," just not textually identical to the quoted error -- noting this plainly rather than claiming an exact repro.

Confirmed the fix does not change hip-parameter/NAT_TRAVERSAL_MODE / hip-parameter/ESP_TRANSFORM's EXPECTED_FAILURES entry in tests/protocols/test_option_roundtrip_unit.py -- both remain in the pre-existing tuple/list RECONSTRUCT gap group (an unrelated defect: _read_param_* returns a tuple where _make_param_* needs a list), which is orthogonal to this item-width fix.

Reverting either fix line fails the corresponding new regression test (checked directly).

Test plan

• pytest tests/protocols/internet/test_hip_unit.py -- 21 passed, 68 subtests passed
• pytest tests/protocols/test_option_roundtrip_unit.py -- 6 passed, 358 subtests passed (no change to HIP's recorded gaps)
• Full suite: pytest tests/ -- 1023 passed, 17 skipped, 0 failed
• Reverted each fix line individually and confirmed the corresponding new test fails

Closes #472

[JarryShaw]

Review of PR #475 at 07ef9db8b

1. RFC widths — read directly, not trusted from the issue

Downloaded and read the actual RFC text (curl https://www.rfc-editor.org/rfc/rfc5770.txt and rfc7402.txt):

• RFC 5770 §5.4 (NAT_TRAVERSAL_MODE), Figure 6: | Reserved | Mode ID #1 | on one 32-bit row, then | Mode ID #2 | Mode ID #3 | on the next — each Mode ID is unambiguously 16 bits.
• RFC 7402 §5.1.2 (ESP_TRANSFORM): | Reserved | Suite ID #1 |, then | Suite ID #2 | Suite ID #3 | — each Suite ID is 16 bits.

Both confirmed 16-bit. The fix (length=1 → length=2 on both item_type=EnumField(...)) is RFC-correct.

2. Is length=2 alone sufficient?

Yes, and here's why nothing else needed to change. two_octet_prefix_list_len (pcapkit/protocols/schema/internet/hip.py:219-248) already computes a byte length (pkt['len'] - 2), not an item count:

length = pkt['len'] - 2

ListField.unpack (pcapkit/corekit/fields/collections.py) decrements that byte-length by field.length per item consumed — so it was always going to derive the right item count once the item width matched the wire. The maker functions (_make_param_nat_traversal_mode, _make_param_esp_transform in pcapkit/protocols/internet/hip.py, untouched by this PR — confirmed via git diff origin/main..origin/fix-472-hip-16-bit-list-entries --stat, which shows only hip.py (schema, 4 lines) and the test file changed) already compute len=2 + 2 * len(mode_id) / len=2 + 2 * len(suite_id) — i.e. the maker was already assuming 2-octet entries. The defect was purely that the unpack side (item_type=EnumField(length=1,...)) disagreed with the write side. This PR brings the two into agreement; no changes to two_octet_prefix_list_len or the makers were needed or made.

3. Padding arithmetic — independently verified for 0..7 entries

Ran both makers through pack()/unpack() for entry counts 0–7 (RFC caps at 6, but the library doesn't enforce that — pre-existing, out of scope):

n=0 declared_len=  2 padding=6 total_packed=12  roundtrip_ok=True got=[]
n=1 declared_len=  4 padding=4 total_packed=12  roundtrip_ok=True got=[1]
n=2 declared_len=  6 padding=2 total_packed=12  roundtrip_ok=True got=[1, 2]
n=3 declared_len=  8 padding=0 total_packed=12  roundtrip_ok=True got=[1, 2, 3]
n=4 declared_len= 10 padding=6 total_packed=20  roundtrip_ok=True got=[1, 2, 3, 4]
n=5 declared_len= 12 padding=4 total_packed=20  roundtrip_ok=True got=[1, 2, 3, 4, 5]
n=6 declared_len= 14 padding=2 total_packed=20  roundtrip_ok=True got=[1, 2, 3, 4, 5, 6]
n=7 declared_len= 16 padding=0 total_packed=20  roundtrip_ok=True got=[1, 2, 3, 4, 5, 6, 7]

(identical results for both NATTraversalModeParameter and ESPTransformParameter). The empty-list case (n=0, which the maker permits) round-trips correctly, and padding never gets misread as a list entry at any count. The content region (reserved + list) always lands on an 8-octet boundary as intended.

4. Do the new tests actually catch the regression?

Reverted just the two length=2 edits back to length=1 (verified with git diff showing exactly those 2 lines) and re-ran pytest tests/protocols/internet/test_hip_unit.py -v:

7 failed, 20 passed, 6 warnings, 60 subtests passed
SUBFAILED(suites=[1])       test_hip_esp_transform_parameter_round_trips_through_the_maker
SUBFAILED(suites=[1, 2])    test_hip_esp_transform_parameter_round_trips_through_the_maker
SUBFAILED(suites=[1, 2, 3]) test_hip_esp_transform_parameter_round_trips_through_the_maker
FAILED test_hip_nat_traversal_mode_and_esp_transform_survive_the_full_parser
SUBFAILED(modes=[1])        test_hip_nat_traversal_mode_parameter_round_trips_through_the_maker
SUBFAILED(modes=[1, 2])     test_hip_nat_traversal_mode_parameter_round_trips_through_the_maker
SUBFAILED(modes=[1, 2, 3])  test_hip_nat_traversal_mode_parameter_round_trips_through_the_maker

The maker round-trip tests correctly SUBFAIL on every non-empty case (AssertionError: Lists differ: [1, 2, 3, 0, 0, 0] != [1, 2, 3] — the phantom-entry pattern), and the full-parser test fails outright with SchemaWarning: packet length < 0: -3. The [] case still passes both ways (nothing to mismatch), which is expected and not a weakness. Restored the two lines afterward (git checkout -- pcapkit/protocols/schema/internet/hip.py); confirmed clean at 07ef9db8b again. These are genuine regression tests, not tests that pass either way.

Also ran the full suite at 07ef9db8b independently: pytest tests/protocols/internet/test_hip_unit.py -v → 21 passed, 68 subtests, 0 failed — matches what was claimed. And tests/protocols/test_option_roundtrip_unit.py (untouched by this PR) → 6 passed, 358 subtests, confirming NAT_TRAVERSAL_MODE/ESP_TRANSFORM are still (and correctly) recorded as RECONSTRUCT in EXPECTED_FAILURES against pcapkit/protocols/schema/schema.py:624 (_read_param_* returns a tuple where _make_param_* needs a list) — a pre-existing, unrelated defect shared by 15 other parameter types, not touched or worsened by this PR.

5. Every other EnumField(length=1, ...) in the file — RFC width table

Read the actual RFC section for each. None of them are wrong; all are genuinely 8-bit fields:

Line	Class / field	Parameter	RFC · section	Field	Width per RFC	Verdict
501	DHGroupListParameter.groups (list item)	DH_GROUP_LIST	RFC 7401 §5.2.6	"DH GROUP ID"	8 bits ("Each DH Group ID is one octet long")	correct as-is
515	DiffieHellmanParameter.group	DIFFIE_HELLMAN	RFC 7401 §5.2.7	"Group ID"	8 bits (packed with 16-bit Public Value Length in the same 32-bit row)	correct as-is
750	HITSuiteListParameter.suites (list item)	HIT_SUITE_LIST	RFC 7401 §5.2.10	"ID"	8 bits, explicit: "The ID field in the HIT_SUITE_LIST is defined as an eight-bit field"	correct as-is
764	CertParameter.cert_group	CERT	RFC 8002 §... (Figure)	"CERT group"	8 bits (4 fields packed into one 32-bit row: group/count/ID/type)	correct as-is
770	CertParameter.cert_type	CERT	RFC 8002	"CERT type"	8 bits (same row)	correct as-is
822	RegInfoParameter.reg_info (list item)	REG_INFO	RFC 8003 §4.2	"Reg Type"	8 bits (4 per 32-bit row)	correct as-is
841	RegRequestParameter.reg_request (list item)	REG_REQUEST	RFC 8003 §4.3	"Reg Type"	8 bits	correct as-is
859	RegResponseParameter.reg_response (list item)	REG_RESPONSE	RFC 8003 §4.4	"Reg Type"	8 bits	correct as-is
877	RegFailedParameter.reg_failed (list item)	REG_FAILED	RFC 8003 §4.5	"Reg Type"	8 bits	correct as-is
893	RegFromParameter.protocol	REG_FROM	RFC 5770 §5.3/Fig.8	"Protocol"	8 bits (Port(16)+Protocol(8)+Reserved(8))	correct as-is
984	PayloadMICParameter.next	PAYLOAD_MIC	RFC 6078 §4.3	"Next Header"	8 bits (Next Header(8)+Reserved(24))	correct as-is
1154	RelayFromParameter.protocol	RELAY_FROM	RFC 5770 §5.3/Fig.8 (shares format with REG_FROM)	"Protocol"	8 bits	correct as-is
1171	RelayToParameter.protocol	RELAY_TO	RFC 5770 §5.3/Fig.8	"Protocol"	8 bits	correct as-is
1279	HIP.next (fixed header)	—	RFC 7401 §5.1	"Next Header"	8 bits, standard HIP fixed header	correct as-is

No follow-up issue is owed. Every other EnumField(length=1, ...) site in this file matches its RFC exactly; the two this PR touched (NAT_TRAVERSAL_MODE.modes, ESP_TRANSFORM.suites) were the only ones that were 16-bit-per-entry and had been left at 1. Note also that the hand-written wire diagrams in pcapkit/protocols/internet/hip.py's _read_param_nat_traversal_mode/_read_param_esp_transform docstrings (untouched by this PR) already correctly depicted the 16-bit Mode ID/Suite ID layout — the schema's item_type was the only place the width was wrong, and it's now fixed.

CI

gh pr view 475 --json statusCheckRollup (waited it out to completion): 21 CheckRun SUCCESS + 2 SKIPPED (Docs test gate, Gate (full suite, Python 3.14) — skip-by-design on pull_request) + 1 StatusContext (pyup.io/safety-ci) SUCCESS. Fully green.

Environment note

pcapkit.__file__ was asserted to resolve inside the worktree (/local/home/jarryx/GitHub/PyPCAPKit/.claude/worktrees/agent-ad427fe0d81e7dd4c/pcapkit/__init__.py) before every measurement, per the PYTHONPATH-forced-import discipline. Fixtures were regenerated with examples/generators/make_samples.py before any suite run (unrelated pre-existing warnings for MPTCP/TCP-option/IPv4-SEC/HIP-ENCRYPTED samples, none touching the two parameters this PR changes).

---

GOOD TO MERGE at 07ef9db8b

[JarryShaw]

Accepting this, with one of its claims independently re-run and one explicitly not re-derived.

I reproduced the revert-proof myself, because a review-only mandate cannot do it — #468's reviewer hit exactly that wall on the same kind of claim, defeated by setUp's purge_modules(['pcapkit']) reloading the package from disk before every test. Reverting both item_type=EnumField(length=2, …) back to length=1 in a scratch worktree (HEAD checked as 07ef9db8b first, not inferred from the branch name):

7 failed, 2 passed, 18 deselected, 2 subtests passed
SUBFAILED(modes=[1]) / (modes=[1, 2]) / (modes=[1, 2, 3])      nat_traversal_mode_parameter_round_trips_through_the_maker
SUBFAILED(suites=[1]) / (suites=[1, 2]) / (suites=[1, 2, 3])   esp_transform_parameter_round_trips_through_the_maker
FAILED                                                          nat_traversal_mode_and_esp_transform_survive_the_full_parser

Restored: 3 passed, 10 subtests passed. So all three tests are real evidence, not tests that pass either way.

Your claim that nothing besides item_type needed changing checks out, and this is the part that makes the fix minimal rather than lucky. two_octet_prefix_list_len returns a byte count (pkt['len'] - 2), which ListField.unpack divides by the item width — so it derives the right item count automatically once the width matches. And the makers already assumed two-octet entries all along: pcapkit/protocols/internet/hip.py has len=2 + 2 * len(mode_id) at :3446 and :4360 and len=2 + 2 * len(suite_id) at :4135, none of them touched by this PR. That confirms the defect was purely a read/write mismatch on item_type — the writer was already right.

What I did not re-derive: the 14-site RFC width table. Your finding that every other item_type=EnumField(length=1, …) in the file is genuinely 8-bit, checked against RFCs 7401, 8002, 8003, 5770 and 6078, is the most consequential thing in this review, because it closes off a whole line of follow-up work — it is the difference between "no follow-up issue is owed" and a sweep across a dozen parameters. I have not independently read those five RFCs, so that conclusion currently rests on your reading alone and is recorded that way rather than as settled. I did confirm the one adjacent case it implies: HIPTransportModeParameter.mode is already EnumField(length=2, …) and shares two_octet_prefix_list_len, so the three call sites that function documents are now consistent with each other.

Also noted and not re-reported: hip-parameter/NAT_TRAVERSAL_MODE and hip-parameter/ESP_TRANSFORM still return RECONSTRUCT from the option round-trip harness on this branch and on origin/main, as pre-existing EXPECTED_FAILURES entries for the unrelated pcapkit/protocols/schema/schema.py:624 tuple-versus-list defect. Your observation that 15 other parameter types share that same defect is new to me and worth its own issue rather than being folded in here; I will file it separately once this lands.