fix(hip): size the SOLUTION parameter as two whole-octet fields (#608)

CHANGELOG.md

@@ -77,6 +77,8 @@ This is the resolution of #548, which reported `TransType.L2TP` (115) as registe
 - **Fixed** -- the one assertion #604 left pinning the old padding side, which had been red on `mainline` since #621 merged. `TCPUDPUnitTests.test_a_truncated_option_still_parses_its_declared_length` expected a truncated TCP option's `data` as the synthesised zero octets *followed by* the real ones, which is what `rjust()` produced; #621 made the padding `ljust()` everywhere but could not retarget this file, since another change (#612) owned it at the time and editing it concurrently risked discarding work that has since landed. The real octets now come first for both parametrised widths, and the docstring above the assertion says tail-padding rather than left-padding. Test-only: no library code changes, and the sibling case in `tests/protocols/internet/test_ipv4_unit.py` was already retargeted in #621. Measured against `main` at `2221c2d8f`: two subtest failures before, none after (#604).
 - **Fixed** -- `main` went red the moment #604's `ljust()` landed, because `TCPUDPUnitTests.test_a_truncated_option_still_parses_its_declared_length` still pinned the head-padded short read that fix removed. The `Reserved_79` option declaring `length=12` over 6 real octets now reports `aabbccddeeff00000000` where the test expected `00000000aabbccddeeff`, so both subtests -- `declared_length=12` and `=32` -- failed on that one assertion while the 17 other cases in the file stayed green: the parse itself never changed, only which end the synthesised zeros sit at. The expectation is inverted, and the docstring above it -- which said the short read was *left*-padded and described the value as four zero octets followed by the six real ones -- is corrected to match, since a docstring that contradicts its own assertion is how the stale expectation survived in the first place. The inputs do discriminate: `trailing` is non-zero and the pad width is 4 and 24, so neither subtest would hold under the other order. #621 left this file alone deliberately, because #612 owned it at the time, and merged two minutes ahead of the cross-review verdict that named it (#604, #621).
 - **Fixed** -- `util/bump_version.py` left `CITATION.cff` naming the previous release. Nothing else in the repository maintains that file -- no workflow, hook or packaging file mentions it -- so every bump since it landed in #615 would have stranded the `version` and `date-released` it renders as GitHub's "Cite this repository" button and that citation managers, Zenodo and dependency inventories read directly. Both fields now move with `__version__`. They have the same standing, since the file's own header says both describe the newest *published* release, and moving only one would assert that 1.5.0b5 was released on the day 1.5.0b4 was; the date is taken in UTC, because seven of the thirty most recent bumps were made late evening in US-Eastern where a local date is a day behind the publish it describes. That the two are the same day at all is measured rather than assumed: the bump is what triggers `create-release.yml`, the median gap to the PyPI upload is three minutes, and the UTC calendar dates agree 30 times out of 30. The rewrite is line-oriented, so the comment header, key ordering and each field's existing quoting survive -- `cff-version` and a `references` entry's own `version` are anchored out at column zero -- and the result is checked with `cffconvert --validate`. An absent file is reported on stderr and skipped rather than failing the vendor cron before its `git commit`, which would discard the whole registry crawl for the sake of a documentation file; a file present with no `version` field raises instead, before anything is written, because rewriting nothing while reporting success is the staleness this fixes. Two things came with it. The script gains a `main()` guard, having previously run the entire bump at import, which is why it had no testable surface; and the `import pcapkit` fallback in its version reader, which returned `"1.5.0b4'\n"` -- closing quote and newline included, which `packaging` rejects -- is fixed, a path that had never worked and went unnoticed because the only caller installs the package first. A new gate asserts the committed file still names the packaged version, covering the version changes made by hand, which never run this script at all -- 40 of the 159 commits that have moved `__version__` on `main`, a quarter over the project's life and 11 of the most recent 25 (#625).
+- **Fixed** -- the HIP `SOLUTION` builder sized the parameter with `4 + math.ceil(max(random.bit_length(), solution.bit_length()) / 4)`, which is an invalid shorthand for the two fields it actually has to describe. [RFC 7401 Section 5.2.5](https://datatracker.ietf.org/doc/html/rfc7401#section-5.2.5) gives the parameter's `Length` as `4 + RHASH_len / 4` over a `Random #I` and a `Puzzle solution #J` of `RHASH_len / 8` octets **each**, and `/ 4` equals twice `/ 8` only because `RHASH_len` -- the natural output length of a hash function, in bits -- is a whole number of octets. Applied to an arbitrary `bit_length()` the identity fails, and the builder emitted an *odd* contents width, which its own reader then refused: `_read_param_solution`'s `(schema.len - 4) % 2` guard exists because `SolutionParameter` splits that width into two equal `(len - 4) // 2` halves, so an odd width cannot be framed at all. `random=0x1` with `solution=0xfff` declared `len=7` and raised `ProtocolError: HIPv2: [ParamNo 321] invalid format` on input the library had itself produced. The undersized length was the worse half of it: because both fields take their width from that same `len`, `solution=0xfff` was packed into the one octet it allowed and came back as `0xff` -- silent truncation, nothing raised. Fixed to `4 + 2 * math.ceil(max(...) / 8)`, the form the sibling `PUZZLE` builder already uses, which is even by construction and so can never trip the guard. Loosening the reader was rejected as the alternative: an odd contents width has no meaning in the wire format, since the RFC makes the two fields equal-width and says nothing about which would take an extra octet, and a laxer guard would still have truncated the value (#608).
+- **Added** -- `SOLUTION` parameter width coverage in `tests/protocols/internet/test_hip_unit.py`: eight widths asserting the declared length, the reader's acceptance and the construct-pack-parse cycle, plus a 57-bit case pinning the 20-octet length [RFC 5201 Section 5.2.5](https://datatracker.ietf.org/doc/html/rfc5201#section-5.2.5) requires of HIPv1. Five of the eight widths -- 1, 9, 12, 17 and 25 bits -- are deliberately *not* multiples of eight, because at a multiple of eight the defective `ceil(bits / 4)` coincides with the correct width, which is why every fixture that reached this builder passed through it unharmed; the round-trip case in `examples/generators/options.py` supplies no `random` or `solution` at all, so it exercised the formula at zero bits. 57 bits is what discriminates on the HIPv1 path for the same reason 64 does not: both formulas give 20 at a full-width value (#608).
 
 Preceded by `1.5.0a1` (2026-09-15), `1.5.0b1` and `1.5.0b2` (both 2026-09-18) and `1.5.0b3` (2026-09-19), all published as prereleases and so resolved only by `pip install --pre`. `1.5.0b1` half-shipped: the tag, the GitHub release and the Conda deployments landed, but PyPI rejected the wheel because `twine check` found a Sphinx-only `:mod:` role in `README.rst`, which `pyproject.toml` declares as the dynamic long description. `1.5.0b2` is what reshipped it -- the release workflow is version-driven, so an existing version cannot republish -- and `1.5.0b3` followed the CI change that stops a TestPyPI outage from costing a release its wheels (#497, #498).
 

docs/source/changelog/1.5.0.rst

@@ -1063,6 +1063,41 @@ pull requests between #326 and #509.
   version, covering the version changes made by hand, which never run this script at
   all -- 40 of the 159 commits that have moved ``__version__`` on ``main``, a quarter
   over the project's life and 11 of the most recent 25 (#625).
+* **Fixed** -- the HIP ``SOLUTION`` builder sized the parameter with
+  ``4 + math.ceil(max(random.bit_length(), solution.bit_length()) / 4)``, which is
+  an invalid shorthand for the two fields it actually has to describe.
+  :rfc:`7401#section-5.2.5` gives the parameter's ``Length`` as
+  ``4 + RHASH_len / 4`` over a ``Random #I`` and a ``Puzzle solution #J`` of
+  ``RHASH_len / 8`` octets **each**, and ``/ 4`` equals twice ``/ 8`` only because
+  ``RHASH_len`` -- the natural output length of a hash function, in bits -- is a
+  whole number of octets. Applied to an arbitrary ``bit_length()`` the identity
+  fails, and the builder emitted an *odd* contents width, which its own reader then
+  refused: ``_read_param_solution``'s ``(schema.len - 4) % 2`` guard exists because
+  ``SolutionParameter`` splits that width into two equal ``(len - 4) // 2`` halves,
+  so an odd width cannot be framed at all. ``random=0x1`` with ``solution=0xfff``
+  declared ``len=7`` and raised
+  ``ProtocolError: HIPv2: [ParamNo 321] invalid format`` on input the library had
+  itself produced. The undersized length was the
+  worse half of it: because both fields take their width from that same ``len``,
+  ``solution=0xfff`` was packed into the one octet it allowed and came back as
+  ``0xff`` -- silent truncation, nothing raised. Fixed to
+  ``4 + 2 * math.ceil(max(...) / 8)``, the form the sibling ``PUZZLE`` builder
+  already uses, which is even by construction and so can never trip the guard.
+  Loosening the reader was rejected as the alternative: an odd contents width has
+  no meaning in the wire format, since the RFC makes the two fields equal-width and
+  says nothing about which would take an extra octet, and a laxer guard would still
+  have truncated the value (#608).
+* **Added** -- ``SOLUTION`` parameter width coverage in
+  ``tests/protocols/internet/test_hip_unit.py``: eight widths asserting the declared
+  length, the reader's acceptance and the construct-pack-parse cycle, plus a 57-bit
+  case pinning the 20-octet length :rfc:`5201#section-5.2.5` requires of HIPv1. Five
+  of the eight widths -- 1, 9, 12, 17 and 25 bits -- are deliberately *not* multiples
+  of eight, because at a multiple of eight the defective ``ceil(bits / 4)`` coincides
+  with the correct width, which is why every fixture that reached this builder passed
+  through it unharmed; the round-trip case in ``examples/generators/options.py``
+  supplies no ``random`` or ``solution`` at all, so it exercised the formula at zero
+  bits. 57 bits is what discriminates on the HIPv1 path for the same reason 64 does
+  not: both formulas give 20 at a full-width value (#608).
 
 Preceded by ``1.5.0a1`` (2026-09-15), ``1.5.0b1`` and ``1.5.0b2`` (both
 2026-09-18) and ``1.5.0b3`` (2026-09-19), all published as prereleases and so

pcapkit/protocols/internet/hip.py

@@ -1087,7 +1087,11 @@ def _read_param_solution(self, schema: 'Schema_SolutionParameter', *, version: '
         _time = schema.lifetime
         _opak = schema.opaque
         _rand = schema.random
-        _solt = schema.solution  # Length (schema.len) = 4 + RHASH_len / 4
+        # ``schema.len`` is ``4 + RHASH_len / 4`` per :rfc:`7401#section-5.2.5`, which
+        # is the same quantity as ``4 + 2 * (RHASH_len / 8)`` -- two equal-width fields
+        # of ``RHASH_len / 8`` octets -- only because ``RHASH_len`` is a whole number of
+        # octets. Do not reuse the ``/ 4`` shorthand on a width that is not; see #608.
+        _solt = schema.solution
 
         solution = Data_SolutionParameter(
             type=schema.type,
@@ -3197,7 +3201,15 @@ def _make_param_solution(self, code: 'Enum_Parameter', param: 'Optional[Data_Sol
 
         return Schema_SolutionParameter(
             type=code,
-            len=4 + math.ceil(max(random.bit_length(), solution.bit_length()) / 4),
+            # Two equal-width fields, ``Random #I`` and ``Puzzle solution #J``, of
+            # ``RHASH_len / 8`` octets each -- so the contents length is
+            # ``4 + 2 * ceil(bits / 8)`` and is necessarily even after the 4-octet
+            # ``#K``/``Reserved``/``Opaque`` prefix. :rfc:`7401#section-5.2.5` spells
+            # the same quantity ``4 + RHASH_len / 4``, which is an identity only
+            # because a real ``RHASH_len`` is a whole number of octets; ``ceil(bits
+            # / 4)`` on an arbitrary :meth:`int.bit_length` is not that quantity and
+            # yields an odd width that :meth:`_read_param_solution` rejects. See #608.
+            len=4 + 2 * math.ceil(max(random.bit_length(), solution.bit_length()) / 8),
             index=index,
             lifetime=lifetime,
             opaque=opaque,

tests/protocols/internet/test_hip_unit.py

@@ -2386,6 +2386,133 @@ def test_hip_schema_selectors_and_encrypted_parameter_branches(self) -> None:
         self.assertIs(encrypted.post_process({'__cipher__': Cipher.NULL_ENCRYPT}), encrypted)
         self.assertIs(encrypted.cipher, Cipher.NULL_ENCRYPT)
 
+    def test_hip_solution_parameter_length_is_two_whole_octet_fields(self) -> None:
+        """#608: a ``SOLUTION`` parameter must be built with an even contents
+        width, because its own reader splits that width into two equal halves.
+
+        :rfc:`7401#section-5.2.5` gives the ``SOLUTION`` parameter's ``Length``
+        as ``4 + RHASH_len / 4``, over a ``Random #I`` and a ``Puzzle solution
+        #J`` of ``RHASH_len / 8`` octets *each*. The ``/ 4`` is shorthand for
+        twice ``/ 8`` and holds only because ``RHASH_len`` -- the natural output
+        length of a hash function, in bits -- is a whole number of octets.
+        :meth:`~pcapkit.protocols.internet.hip.HIP._make_param_solution` used to
+        apply that shorthand to an arbitrary :meth:`int.bit_length`, where the
+        identity fails, and so emitted an odd contents width that
+        :meth:`~pcapkit.protocols.internet.hip.HIP._read_param_solution`'s
+        ``(len - 4) % 2`` guard rejects -- the library refusing to parse what it
+        had just built.
+
+        Two independent symptoms, both asserted below. The odd ``len`` is
+        rejected outright; and because
+        :class:`~pcapkit.protocols.schema.internet.hip.SolutionParameter` sizes
+        each field from that same ``len``, an undersized ``len`` also silently
+        *truncates* the values on the way out -- ``solution=0xfff`` packed into
+        the one octet ``len=7`` allowed for it came back as ``0xff``.
+
+        On the widths chosen
+        --------------------
+        Only the widths that are **not** multiples of 8 can tell the right
+        formula from the wrong ones, which is why five of the eight cases below
+        are 1, 9, 12, 17 and 25 bits. At 12 bits, for instance, the four
+        plausible field-pair widths disagree: ``2 * ceil(12 / 8) == 4``
+        (correct), ``ceil(12 / 4) == 3`` (the defect), ``2 * floor(12 / 8) == 2``,
+        and ``ceil(12 / 8) == 2`` (one field's worth rather than two). At 1 bit
+        they are 2, 1, 0 and 1.
+
+        The 8-, 16- and 24-bit cases are deliberate controls rather than
+        discriminators: at a multiple of 8 the defect's ``ceil(bits / 4)`` and
+        the floor variant both coincide with the correct width, which is exactly
+        why every byte-aligned fixture this library ships passed through the
+        defect unharmed. They are asserted to confirm the repair changes nothing
+        on the path that already worked.
+
+        """
+        import math
+
+        from pcapkit.const.hip.parameter import Parameter
+        from pcapkit.corekit.multidict import OrderedMultiDict
+        from pcapkit.protocols.internet.hip import HIP
+        from pcapkit.protocols.schema.internet import hip as hip_schema
+
+        proto = object.__new__(HIP)
+        options = OrderedMultiDict()
+
+        # (random, solution) pairs, ordered by the width that governs the length.
+        #        random,   solution,  bits, expected len
+        cases = (
+            (0x1, 0x1, 1, 6),            # discriminates: /4 -> 5, floor -> 4, half -> 5
+            (0xff, 0xff, 8, 6),          # control: /4 and floor both agree here
+            (0x1, 0x1ff, 9, 8),          # discriminates: /4 -> 7, floor -> 6, half -> 6
+            (0x1, 0xfff, 12, 8),         # the issue's headline case; /4 -> 7
+            (0xffff, 0x1, 16, 8),        # control: /4 and floor both agree here
+            (0x1ffff, 0x3, 17, 10),      # discriminates: /4 -> 9, floor -> 8, half -> 7
+            (0xffffff, 0xffffff, 24, 10),  # control -- the width #601's fixture uses
+            (0x1ffffff, 0x0, 25, 12),    # discriminates: /4 -> 11, floor -> 10, half -> 8
+        )
+        for random, solution, bits, expected in cases:
+            with self.subTest(random=random, solution=solution, bits=bits):
+                self.assertEqual(max(random.bit_length(), solution.bit_length()), bits)
+                self.assertEqual(expected, 4 + 2 * math.ceil(bits / 8))
+
+                schema = proto._make_param_solution(
+                    Parameter.SOLUTION, version=2, index=1, lifetime=2,
+                    opaque=b'op', random=random, solution=solution,
+                )
+                self.assertEqual(schema.len, expected)
+
+                # The reader's own guard: an odd contents width cannot be split
+                # into the two equal fields the schema declares.
+                self.assertEqual((schema.len - 4) % 2, 0)
+                parsed = proto._read_param_solution(schema, version=2, options=options)
+                self.assertEqual(parsed.random, random)
+                self.assertEqual(parsed.solution, solution)
+
+                # ... and the values must survive the octets, not just the schema:
+                # too small a `len` narrows both fields and drops the high octets.
+                packed = bytes(schema)
+                reparsed = hip_schema.SolutionParameter.unpack(packed)
+                self.assertEqual(reparsed.len, expected)
+                self.assertEqual(reparsed.random, random)
+                self.assertEqual(reparsed.solution, solution)
+
+    def test_hip_solution_parameter_at_57_bits_stays_legal_for_hipv1(self) -> None:
+        """#608: a 57-bit puzzle value must still build the 20-octet
+        ``SOLUTION`` parameter that HIPv1 requires.
+
+        :rfc:`5201#section-5.2.5` fixes ``Random #I`` and ``Puzzle solution #J``
+        at 8 octets each and the parameter's ``Length`` at 20, and
+        :meth:`~pcapkit.protocols.internet.hip.HIP._read_param_solution`
+        enforces exactly that for ``version=1``. A 57-bit value is a perfectly
+        ordinary 8-octet field with seven leading zero bits, so it must produce
+        ``Length = 20``; ``4 + ceil(57 / 4)`` produces 19, which fails both the
+        HIPv1 equality check and the ``(len - 4) % 2`` parity check.
+
+        57 bits is the discriminating width here precisely because 64 is not:
+        ``4 + ceil(64 / 4)`` and ``4 + 2 * ceil(64 / 8)`` are both 20, so a test
+        written with a full-width 64-bit value would pass either way.
+
+        """
+        from pcapkit.const.hip.parameter import Parameter
+        from pcapkit.corekit.multidict import OrderedMultiDict
+        from pcapkit.protocols.internet.hip import HIP
+
+        proto = object.__new__(HIP)
+        options = OrderedMultiDict()
+
+        random = 1 << 56  # bit_length() == 57
+        solution = 0x1
+        self.assertEqual(random.bit_length(), 57)
+
+        schema = proto._make_param_solution(
+            Parameter.SOLUTION, version=1, index=1, lifetime=2,
+            opaque=b'op', random=random, solution=solution,
+        )
+        self.assertEqual(schema.len, 20)
+
+        parsed = proto._read_param_solution(schema, version=1, options=options)
+        self.assertEqual(parsed.random, random)
+        self.assertEqual(parsed.solution, solution)
+
 
 if __name__ == '__main__':
     unittest.main()