fix(hip): register R1CounterParameter under HIPv1's code 128 too

examples/generators/options.py

@@ -974,32 +974,37 @@ def _mh_extension_build(code: 'Any', kwargs: 'dict[str, Any]') -> 'Any':
 #: ``Length`` had been counted in 4-octet units where the RFC counts bytes --
 #: which is what moved the one-copy figure from 44 to 46, level with two copies.
 #:
-#: Three remain, none of them helped by a second copy since each fails at two
-#: copies as well as at one:
+#: Two remain, neither helped by a second copy since each fails at two copies
+#: as well as at one:
 #:
 #: * ``HOST_ID`` declares ``len=8`` and packs 18. Recorded as
 #:   ``hip-parameter/HOST_ID``.
 #: * ``HIP_TRANSFORM`` is HIPv1-only -- ``_read_param_hip_transform`` raises for
 #:   any other version -- while this table builds it at version 2. Recorded as
 #:   ``hip-parameter/HIP_TRANSFORM``.
-#: * ``R1_Counter`` (128) parses as an ``UnassignedParameter``: ``_read_param_*``
-#:   and ``_make_param_*`` are found by enumeration member name so both exist for
-#:   code 128, but the *schema* registry is keyed on the ``code=`` of the class
-#:   statement and ``R1CounterParameter`` declares only 129. Recorded as
-#:   ``hip-parameter/R1_Counter``, filed as #690.
 #:
-#: So the pair was routing around nothing by the time #672 and #679 landed, and
-#: dropping to one copy does not change what round-trips. Measured over this
-#: table's 49 HIP codes on ``5f0a1aa90`` (after #696, which #689 itself waited
-#: on -- see the issue):
+#: A third gap here, ``R1_Counter`` (128), was closed separately by #690 and was
+#: never one of the pair's cancellations above: it parsed as an
+#: ``UnassignedParameter`` because the *schema* registry is keyed on the
+#: ``code=`` of the class statement and ``R1CounterParameter`` declared only
+#: 129 -- even though ``__parameter__``'s ``_read_param_*``/``_make_param_*``
+#: entries already existed for both codes, as two hand-written dict entries,
+#: not because of any name-normalisation rule. No longer recorded as of #690,
+#: which registered ``R1CounterParameter`` for 128 too.
+#:
+#: So the pair was routing around nothing by the time #672, #679 and #690 had
+#: all landed, and dropping to one copy does not change what round-trips.
+#: Measured over this table's 49 HIP codes on ``5f0a1aa90`` (after #696, which
+#: #689 itself waited on -- see the issue) and again after #690:
 #:
 #: ======================  ========  ==========
 #: tree                    one copy  two copies
 #: ======================  ========  ==========
 #: ``5f0a1aa90``           46 OK     46 OK
+#: after #690              47 OK     47 OK
 #: ======================  ========  ==========
 #:
-#: The three gaps above are the only cases either setting fails, and neither
+#: The two gaps above are the only cases either setting fails, and neither
 #: their ``status`` nor their ``defect`` moves between settings, so
 #: :data:`tests.protocols.test_option_roundtrip_unit.EXPECTED_FAILURES` needed no
 #: change to keep recording them accurately at one copy.

pcapkit/protocols/schema/internet/hip.py

@@ -563,8 +563,28 @@ def __init__(self, type: 'Enum_Parameter', len: 'int', index: 'int',
 
 
 @schema_final
-class R1CounterParameter(Parameter, code=Enum_Parameter.R1_COUNTER):
-    """Header schema for HIP ``R1_COUNTER`` parameters."""
+class R1CounterParameter(Parameter, code=[Enum_Parameter.R1_Counter,
+                                          Enum_Parameter.R1_COUNTER]):
+    """Header schema for HIP ``R1_COUNTER`` parameters.
+
+    Registered under both codes, not 129 alone. :rfc:`5201#section-5.2.3`
+    (HIPv1) and :rfc:`7401#section-5.2.3` (HIPv2) give the identical 4 + 8
+    layout to code 128 (``R1_Counter``) and code 129 (``R1_COUNTER``) --
+    one parameter under two numbers, the difference being HIP's own C-bit
+    rather than an unrelated code -- and the field list below is that layout
+    exactly, since #696 widened :attr:`counter` to eight octets.
+    :attr:`~pcapkit.protocols.internet.hip.HIP.__parameter__` already carries
+    two hand-written entries -- not a name-normalisation rule; ``R1_Counter``
+    and ``R1_COUNTER`` differ only in case, and each needed its own line --
+    mapping both codes to the same ``_read_param_r1_counter``, and
+    ``_make_param_r1_counter`` already built this class for 128; it was only
+    the schema lookup this class's own ``code=`` feeds -- consulted on the
+    parse path by :class:`~pcapkit.corekit.fields.collections.OptionField`
+    -- that fell back to :class:`UnassignedParameter` for 128, since a
+    single-code ``code=`` registered 129 alone and left 128 unclaimed.
+    See #690.
+
+    """
 
     #: Reserved.
     reserved: 'bytes' = PaddingField(length=4)

tests/protocols/internet/test_hip_r1_counter_width_unit.py

@@ -175,12 +175,12 @@ def test_a_lone_r1_counter_parameter_survives_a_hip_packet(self) -> None:
         both codes; the two-copy pairing in
         :mod:`examples.generators.options` is what concealed that.
 
-        Code 128 is deliberately not exercised here. It fails for an unrelated
-        reason that #672 does not touch -- the schema registry is keyed on the
-        ``code=`` of the class statement and ``R1CounterParameter`` declares only
-        129, so 128 parses as an ``UnassignedParameter`` -- which is #690, and
-        which ``hip-parameter/R1_Counter`` in
-        :mod:`tests.protocols.test_option_roundtrip_unit` records.
+        Code 128 is deliberately not exercised here. #690 registered
+        ``R1CounterParameter`` for 128 as well as 129, so it now reaches this
+        same class rather than parsing as an ``UnassignedParameter`` --
+        that coverage lives with the fix, in
+        :meth:`HIPUnitTests.test_hip_r1_counter_code_128_resolves_to_its_own_schema`,
+        not here; this module stays scoped to the width #672 fixed.
         """
         from pcapkit.const.hip.parameter import Parameter
         from pcapkit.protocols.internet.hip import HIP

tests/protocols/internet/test_hip_unit.py

@@ -2303,6 +2303,108 @@ def test_hip_nat_traversal_mode_and_esp_transform_survive_the_full_parser(self)
                 for copy in copies:
                     self.assertEqual(getattr(copy, attr), (1,))
 
+    def test_hip_r1_counter_code_128_resolves_to_its_own_schema(self) -> None:
+        """#690: HIPv1's ``R1_Counter`` (code 128) must not fall through to
+        ``UnassignedParameter`` on the parse path.
+
+        ``R1CounterParameter`` used to register itself with ``code=
+        Enum_Parameter.R1_COUNTER`` alone -- 129, the HIPv2 spelling.
+        :attr:`~pcapkit.protocols.internet.hip.HIP.__parameter__` already
+        carries two hand-written entries -- not a name-normalisation rule --
+        mapping both 128 and 129 to ``_read_param_r1_counter``, and
+        ``_make_param_r1_counter`` already built a real ``R1CounterParameter``
+        for 128 -- so construction worked and only parsing was broken. The
+        *schema* used to unpack a parameter off the wire is chosen by a
+        different mapping, :attr:`~pcapkit.protocols.schema.schema.EnumSchema.
+        registry`, wired into :class:`HIP`'s ``param`` field as
+        ``OptionField(..., registry=Parameter.registry)``, and that one is
+        keyed solely by the ``code=`` a schema class declares. So code 128
+        fell to ``Parameter.__default__`` -- ``UnassignedParameter``, whose
+        schema has ``value: bytes`` and no ``counter`` -- and
+        ``_read_param_r1_counter`` failed reading ``schema.counter`` off it
+        with ``AttributeError: 'UnassignedParameter' object has no attribute
+        'counter'``. Measured verbatim on the tree before this fix, with the
+        exact bytes this test builds.
+
+        The expected wire bytes below are derived from
+        ``R1CounterParameter.__fields__`` rather than a hex literal, on
+        purpose: a literal is exactly what made the first version of this
+        test fail the moment #696 widened ``counter`` from four octets to
+        eight (12-octet records became 16). Deriving from the field widths
+        keeps this test about the registry dispatch it names, rather than
+        about a wire width #696's own ``test_hip_r1_counter_width_unit.py``
+        already pins.
+
+        Two copies, matching
+        ``test_hip_nat_traversal_mode_and_esp_transform_survive_the_full_parser``'s
+        own reason: proving the reader's stride across repeated occurrences
+        of the same code, not working around any alignment defect. With #696
+        landed a lone ``R1_COUNTER`` record already packs to a self-aligned
+        16 octets and parses cleanly on its own -- measured directly, one
+        copy raises nothing and reads back the same counter -- so two copies
+        here are a stride check, not a workaround.
+
+        """
+        from pcapkit.const.hip.parameter import Parameter
+        from pcapkit.protocols.data.internet import hip as hip_data
+        from pcapkit.protocols.internet.hip import HIP
+        from pcapkit.protocols.schema.internet import hip as hip_schema
+
+        # The schema registry itself: both codes now resolve to the same class.
+        self.assertIs(hip_schema.Parameter.registry[Parameter.R1_Counter],
+                      hip_schema.R1CounterParameter)
+        self.assertIs(hip_schema.Parameter.registry[Parameter.R1_COUNTER],
+                      hip_schema.R1CounterParameter)
+
+        proto = object.__new__(HIP)
+
+        r1_schema = proto._make_param_r1_counter(Parameter.R1_Counter, version=1,
+                                                 counter=0xaabbccdd)
+        r1_one = bytes(r1_schema)
+
+        # Derived from the schema's own declared field widths -- see the
+        # docstring above for why this is not a hex literal. This only agrees
+        # with the actual packed record because _make_param_r1_counter hard-
+        # codes len=12 (below) rather than deriving it the same way; the two
+        # would diverge the moment reserved + counter stopped summing to 12.
+        reserved_len = hip_schema.R1CounterParameter.__fields__['reserved'].length
+        counter_len = hip_schema.R1CounterParameter.__fields__['counter'].length
+        content_len = reserved_len + counter_len
+        self.assertEqual(content_len, 12)  # matches _make_param_r1_counter's len=12
+        total_len = hip_schema.parameter_total_len(content_len)
+        padding_len = total_len - 4 - content_len
+        expected = (
+            int(Parameter.R1_Counter).to_bytes(2, 'big')
+            + content_len.to_bytes(2, 'big')
+            + bytes(reserved_len)
+            + (0xaabbccdd).to_bytes(counter_len, 'big')
+            + bytes(padding_len)
+        )
+        self.assertEqual(r1_one, expected)
+        self.assertEqual(len(r1_one), total_len)
+        self.assertEqual(len(r1_one) % 8, 0)
+
+        param_area = r1_one * 2
+        self.assertEqual(len(param_area) % 8, 0)
+        hdr_len_units = 4 + len(param_area) // 8
+
+        # next(1)=0x3b len(1) pkt(1)=0x00 ver(1)=0x11 (version nibble 1, plus
+        # the reserved bit that must be 1) checksum(2) control(2) shit(16)
+        # rhit(16) -- the fixed 40-octet header, declaring HIPv1 and the
+        # parameter area to follow.
+        fixed = (bytes([0x3b, hdr_len_units, 0x00, 0x11]) + bytes(2) +
+                  bytes(2) + bytes(16) + bytes(16))
+        self.assertEqual(len(fixed), 40)
+        raw = fixed + param_area
+
+        parsed = HIP(raw, len(raw), extension=True)
+        self.assertEqual(parsed.info.version, 1)
+        copies = parsed.info.parameters.getlist(Parameter.R1_Counter)
+        self.assertEqual(len(copies), 2)
+        for copy in copies:
+            self.assertIsInstance(copy, hip_data.R1CounterParameter)
+            self.assertEqual(copy.counter, 0xaabbccdd)
+
     def test_hip_schema_selectors_and_encrypted_parameter_branches(self) -> None:
         from pcapkit.const.hip.cipher import Cipher
         from pcapkit.const.hip.hi_algorithm import HIAlgorithm

tests/protocols/test_option_roundtrip_unit.py

@@ -388,14 +388,14 @@ class Gap(NamedTuple):
 
     # -- HIP ------------------------------------------------------------------
 
-    # ``_read_param_*`` and ``_make_param_*`` are found by enumeration member
-    # name, so both exist for code 128; but the *schema* registry is keyed by
-    # the ``code=`` of the class statement, and ``R1CounterParameter`` declares
-    # only 129. So code 128 parses as an ``UnassignedParameter``.
-    'hip-parameter/R1_Counter': Gap(
-        'PARSE', "no attribute 'counter'",
-        'pcapkit/protocols/internet/hip.py:822 -- Parameter.registry[128] is '
-        'UnassignedParameter, because R1CounterParameter declares code=129 only'),
+    # ``hip-parameter/R1_Counter`` used to be here: ``HIP.__parameter__``'s
+    # hand-written dict entries already mapped both 128 and 129 to
+    # ``_read_param_r1_counter``/``_make_param_r1_counter`` (not a
+    # name-normalisation rule -- each code has its own literal entry), so both
+    # existed for code 128. But the *schema* registry was keyed by the
+    # ``code=`` of the class statement, and ``R1CounterParameter`` declared
+    # only 129 -- so code 128 parsed as an ``UnassignedParameter``. Fixed by
+    # #690, registering ``R1CounterParameter`` under both codes.
 
     # ``ENCRYPTED`` used to have an entry here, for two defects at once: that
     # ``_make_param_encrypted`` passed ``cipher=``, a keyword