fix(ipv6-route): compute Hdr Ext Len in 8-octet units on both sides

docs/source/pcapkit/protocols/internet/ipv6_route.rst

@@ -35,6 +35,7 @@ Octets      Bits        Name                    Description
    .. automethod:: make
 
    .. automethod:: _make_data
+   .. automethod:: _make_hdr_ext_len
 
    .. automethod:: _read_data_type_none
    .. automethod:: _read_data_type_src
@@ -92,6 +93,7 @@ Auxiliary Functions
 ~~~~~~~~~~~~~~~~~~~
 
 .. autofunction:: pcapkit.protocols.schema.internet.ipv6_route.ipv6_route_data_selector
+.. autofunction:: pcapkit.protocols.schema.internet.ipv6_route.ipv6_route_data_length
 
 Data Models
 -----------

pcapkit/protocols/internet/ipv6_route.py

@@ -41,6 +41,7 @@
 from pcapkit.protocols.schema.internet.ipv6_route import SourceRoute as Schema_SourceRoute
 from pcapkit.protocols.schema.internet.ipv6_route import Type2 as Schema_Type2
 from pcapkit.protocols.schema.internet.ipv6_route import UnknownType as Schema_UnknownType
+from pcapkit.protocols.schema.internet.ipv6_route import ipv6_route_data_length
 from pcapkit.protocols.schema.schema import Schema
 from pcapkit.utilities.exceptions import ProtocolError, UnsupportedCall
 from pcapkit.utilities.warnings import RegistryWarning, warn
@@ -214,6 +215,42 @@ def read(self, length: 'Optional[int]' = None, *, extension: 'bool' = False,  #
             return ipv6_route
         return self._decode_next_layer(ipv6_route, schema.next, length - ipv6_route.length)
 
+    @staticmethod
+    def _make_hdr_ext_len(data_length: 'int') -> 'int':
+        """Compute ``Hdr Ext Len`` for a type-specific data payload of ``data_length`` octets.
+
+        Per :rfc:`8200#section-4.4`, ``Hdr Ext Len`` is *"the length of the
+        Routing header in 8-octet units, not including the first 8
+        octets"*. The routing header's fixed part (``next``/``length``/
+        ``type``/``seg_left``) is 4 octets, so the total on-the-wire header
+        is ``4 + data_length`` octets; equating that to ``8 + 8 *
+        hdr_ext_len`` and solving gives ``hdr_ext_len = (data_length - 4) /
+        8``. ``ipv6_route_data_length`` in
+        :mod:`pcapkit.protocols.schema.internet.ipv6_route` is this
+        expression's inverse, used on the read side.
+
+        This is the *only* place ``Hdr Ext Len`` is computed on the write
+        side: :meth:`make` used to compute it twice, once per branch, in two
+        different (and both wrong) units -- that duplication, not either
+        expression individually, is what let the two drift and is why there
+        is one helper now rather than two call sites. Do NOT "simplify" the
+        ``- 4`` / ``/ 8`` away: the units either side of it differ (octets
+        vs. 8-octet units), and dropping the offset silently reinterprets
+        the field, which is exactly the defect #487 fixed (compare the
+        ``* 8`` unit bug behind #483 in the scapy adapter).
+
+        Args:
+            data_length: packed length, in octets, of the type-specific data
+                (i.e. ``len(data_val.pack())`` for a :class:`~pcapkit.protocols.
+                schema.schema.Schema`-based payload, or the padded raw
+                :obj:`bytes` length for the unknown/raw-bytes case).
+
+        Returns:
+            The ``Hdr Ext Len`` field value.
+
+        """
+        return math.ceil(max(0, data_length - 4) / 8)
+
     def make(self,
              dst: 'Optional[IPv6Address | str | int| bytes]' = None,
              next: 'Enum_TransType | StdlibEnum | AenumEnum | str | int' = Enum_TransType.UDP,
@@ -257,8 +294,13 @@ def make(self,
                                          reversed=type_reversed, pack=False))
 
         if isinstance(data, bytes):
-            length = math.ceil((len(data) + 4) / 8)
-            data_val = data.ljust(length * 8 - 4, b'\x00')  # type: bytes | Schema_RoutingType
+            # ``data`` here *is* the type-specific data (no per-type
+            # constructor involved), so pad it out to the next octet count
+            # ``_make_hdr_ext_len`` can express exactly, then use that same
+            # helper -- rather than a third, separate expression -- to derive
+            # ``Hdr Ext Len`` from the now-aligned length.
+            length = self._make_hdr_ext_len(len(data))
+            data_val = data.ljust(ipv6_route_data_length(length), b'\x00')  # type: bytes | Schema_RoutingType
         elif isinstance(data, (dict, Data_IPv6_Route)):
             name = self._lookup_registry(self.__routing__, type_val)
             if isinstance(name, str):
@@ -273,9 +315,9 @@ def make(self,
                 data_val = meth(type_val, dst=dst_val, **data)
             else:
                 data_val = meth(type_val, data, dst=dst_val)
-            length = len(data_val.pack())
+            length = self._make_hdr_ext_len(len(data_val.pack()))
         elif isinstance(data, Schema):
-            length = math.ceil((len(data.pack()) + 4) / 8)
+            length = self._make_hdr_ext_len(len(data.pack()))
             data_val = data
         else:
             raise ProtocolError(f'{self.alias}: invalid routing data type: {data.__class__}')
@@ -458,7 +500,13 @@ def _read_data_type_src(self, schema: 'Schema_SourceRoute', *, header: 'Schema_I
             Parsed route data.
 
         """
-        if (header.length - 8) % 16 != 0:
+        # ``header.length`` is ``Hdr Ext Len``, in 8-octet units (:rfc:`8200
+        # #section-4.4`), not octets -- each 16-octet address costs 2 of
+        # those units, so a well-formed Source Route header always carries
+        # an even ``Hdr Ext Len``. The previous ``(header.length - 8) % 16``
+        # check assumed ``header.length`` was already a total octet count,
+        # which is never true of this field; see #487.
+        if header.length % 2 != 0:
             raise ProtocolError(f'{self.alias}: [TypeNo {header.type}] invalid format')
 
         ipv6_route = Data_SourceRoute(
@@ -499,10 +547,14 @@ def _read_data_type_2(self, schema: 'Schema_Type2', *, header: 'Schema_IPv6_Rout
             Parsed route data.
 
         Raises:
-            ProtocolError: If ``length`` is **NOT** ``24``.
+            ProtocolError: If ``Hdr Ext Len`` is **NOT** ``2``.
 
         """
-        if header.length != 24:
+        # A Type 2 Routing header is fixed at 24 octets total (4 fixed + 4
+        # reserved + 16-octet home address), so its ``Hdr Ext Len`` -- in
+        # 8-octet units, not octets (:rfc:`8200#section-4.4`) -- is always
+        # ``2``; a literal ``24`` here could never match. See #487.
+        if header.length != 2:
             raise ProtocolError(f'{self.alias}: [TypeNo {header.type}] invalid format')
 
         ipv6_route = Data_Type2(
@@ -542,6 +594,19 @@ def _read_data_type_rpl(self, schema: 'Schema_RPL', *, header: 'Schema_IPv6_Rout
             Parsed route data.
 
         """
+        # NOTE: this guard has the same surface shape as the Source Route and
+        # Type 2 unit confusion #487 fixed above -- ``header.length`` is
+        # ``Hdr Ext Len`` in 8-octet units, not octets, and ``% 16`` reads
+        # like a leftover assumption that it was already a total octet
+        # count. It is left as-is here: RPL addresses are variable-length
+        # (compressed by ``cmpr_i``/``cmpr_e``), so a fixed ``% 16`` bound is
+        # not obviously the right invariant even under correct units, and
+        # this module's RPL construction already fails before ever reaching
+        # this method, from an unrelated defect (``RPL.post_process`` in
+        # pcapkit/protocols/schema/internet/ipv6_route.py assumes ``bytes``
+        # on a path ``Schema.pack`` also runs, per #476/#480) -- so there is
+        # no working round trip here to validate a replacement against.
+        # Flagged for follow-up rather than guessed at.
         if header.length % 16 != 0:
             raise ProtocolError(f'{self.alias}: [TypeNo {header.type}] invalid format')
 

pcapkit/protocols/schema/internet/ipv6_route.py

@@ -37,6 +37,31 @@ class PadInfo(TypedDict):
         pad_len: int
 
 
+def ipv6_route_data_length(hdr_ext_len: 'int') -> 'int':
+    """Length, in octets, of the IPv6-Route type-specific data for a given ``Hdr Ext Len``.
+
+    Per :rfc:`8200#section-4.4`, ``Hdr Ext Len`` is *"the length of the
+    Routing header in 8-octet units, not including the first 8 octets"* --
+    i.e. the total on-the-wire header is ``8 + 8 * hdr_ext_len`` octets. Of
+    that, the 4 octets of ``next``/``length``/``type``/``seg_left`` are not
+    part of the type-specific data, so the data itself -- what
+    :func:`ipv6_route_data_selector` hands to the nested ``RoutingType``
+    schema -- is ``4 + 8 * hdr_ext_len`` octets. This is the single place
+    that arithmetic is done on the read side; see
+    :meth:`~pcapkit.protocols.internet.ipv6_route.IPv6_Route._make_hdr_ext_len`
+    for its inverse on the write side. Do NOT drop the ``4 +``: that turns
+    the field back into raw octets and is the exact defect #487 fixed.
+
+    Args:
+        hdr_ext_len: raw ``Hdr Ext Len`` field value, as read off the wire.
+
+    Returns:
+        Length, in octets, of the type-specific data.
+
+    """
+    return 4 + hdr_ext_len * 8
+
+
 def ipv6_route_data_selector(pkt: 'dict[str, Any]') -> 'Field':
     """Selector function for :attr:`IPv6_Route.data` field.
 
@@ -51,7 +76,7 @@ def ipv6_route_data_selector(pkt: 'dict[str, Any]') -> 'Field':
     """
     type = cast('Enum_Routing', pkt['type'])
     schema = RoutingType.registry[type]
-    return SchemaField(length=pkt['length'] * 8, schema=schema)
+    return SchemaField(length=ipv6_route_data_length(cast('int', pkt['length'])), schema=schema)
 
 
 @schema_final

tests/protocols/internet/test_ipv6_extension_unit.py

@@ -316,7 +316,9 @@ def test_ipv6_route_readers_and_constructors_cover_registered_types(self) -> Non
         with self.assertRaises(ProtocolError):
             proto._read_data_type_src(route_schema.SourceRoute(ip=[]), header=header)
 
-        type2_header = types.SimpleNamespace(next=TransType.TCP, length=24,
+        # Hdr Ext Len for a Type 2 header is fixed at 2 (24 total octets), in
+        # 8-octet units per :rfc:`8200#section-4.4` -- not 24. See #487.
+        type2_header = types.SimpleNamespace(next=TransType.TCP, length=2,
                                              type=Routing.Type_2_Routing_Header, seg_left=1)
         type2 = proto._read_data_type_2(route_schema.Type2(ip='2001:db8::2'), header=type2_header)
         self.assertEqual(str(type2.ip), '2001:db8::2')
@@ -461,7 +463,11 @@ def test_ipv6_route_read_make_registry_and_property_edges(self) -> None:
         decode.assert_called_once()
 
         made_bytes = proto.make(type=Routing.Source_Route, data=b'abcd')
-        self.assertEqual(made_bytes.length, 1)
+        # Hdr Ext Len in 8-octet units (:rfc:`8200#section-4.4`): 4 octets of
+        # raw data is exactly the 4-octet fixed part every routing type's
+        # data starts with, so no additional 8-octet units are needed. See
+        # #487 -- this used to assert ``1``, the pre-fix octet-count value.
+        self.assertEqual(made_bytes.length, 0)
         self.assertEqual(made_bytes.data, b'abcd')
         made_dict = proto.make(type=Routing.Source_Route, data={'ip': ['2001:db8::2']})
         self.assertEqual(made_dict.type, Routing.Source_Route)
@@ -477,7 +483,10 @@ def test_ipv6_route_read_make_registry_and_property_edges(self) -> None:
             type=Routing.Type_2_Routing_Header,
             data=route_schema.Type2(ip='2001:db8::4'),
         )
-        self.assertEqual(made_schema.length, 3)
+        # 4 reserved + 16-octet address = 20 octets of type-specific data ->
+        # Hdr Ext Len = (20 - 4) / 8 = 2 (:rfc:`8200#section-4.4`). See #487
+        # -- this used to assert ``3``, the pre-fix (wrong-sign) value.
+        self.assertEqual(made_schema.length, 2)
         with self.assertRaises(ProtocolError):
             proto.make(data=object())
 
@@ -2054,22 +2063,27 @@ def test_ipv6_route_source_route_make_accepts_tuple_and_list_addresses(self) ->
         Boundaries: zero addresses, one, and two, so a fix that special-cases
         "empty" or stops one item short of the general case is still caught.
 
-        The construct-then-parse check below goes through
+        #487 update: this test previously round-tripped through
         ``Schema_SourceRoute.unpack`` directly rather than a full
-        ``IPv6_Route`` read (i.e. ``_read_data_type_src``): that method's own
-        ``(header.length - 8) % 16`` check rejects every ``length``
-        ``IPv6_Route.make`` itself computes for this routing type -- for any
-        address count, tuple or list alike -- which looks like a pre-existing
-        defect independent of #480 and is reported separately rather than
-        papered over here.
+        ``IPv6_Route`` read, because ``_read_data_type_src``'s own
+        ``(header.length - 8) % 16`` guard rejected every ``length``
+        ``IPv6_Route.make`` computed for this routing type, for any address
+        count -- and separately, the ``length`` ``make`` computed was itself
+        wrong (raw octets, not the 8-octet units :rfc:`8200#section-4.4`
+        specifies for ``Hdr Ext Len``). Both halves are fixed now (see #487),
+        so this goes through the full ``IPv6_Route`` construct-then-parse
+        path -- which is a strictly stronger check than unpacking the nested
+        schema alone -- and the expected on-wire ``Hdr Ext Len`` byte below
+        changed from the old (buggy) octet counts ``0x04``/``0x14``/``0x24``
+        to the RFC-correct 8-octet-unit values ``0x00``/``0x02``/``0x04``.
+        The tuple-versus-list assertion this test exists for is unchanged.
 
         """
         from ipaddress import ip_address
 
         from pcapkit.const.ipv6.routing import Routing
         from pcapkit.const.reg.transtype import TransType
         from pcapkit.protocols.internet.ipv6_route import IPv6_Route
-        from pcapkit.protocols.schema.internet import ipv6_route as route_schema
 
         addr1 = ip_address('2001:db8::1')
         addr2 = ip_address('2001:db8::2')
@@ -2088,11 +2102,11 @@ def make_and_pack(ip):
         # (case name, tuple form, list form, expected on-wire bytes)
         cases = [
             ('empty', (), [],
-             bytes([0x11, 0x04, 0x00, 0x00]) + b'\x00' * 4),
+             bytes([0x11, 0x00, 0x00, 0x00]) + b'\x00' * 4),
             ('single', (addr1,), [addr1],
-             bytes([0x11, 0x14, 0x00, 0x01]) + b'\x00' * 4 + addr1.packed),
+             bytes([0x11, 0x02, 0x00, 0x01]) + b'\x00' * 4 + addr1.packed),
             ('double', (addr1, addr2), [addr1, addr2],
-             bytes([0x11, 0x24, 0x00, 0x02]) + b'\x00' * 4 + addr1.packed + addr2.packed),
+             bytes([0x11, 0x04, 0x00, 0x02]) + b'\x00' * 4 + addr1.packed + addr2.packed),
         ]
 
         for name, as_tuple, as_list, expected in cases:
@@ -2107,12 +2121,72 @@ def make_and_pack(ip):
                 # 4 octets fixed header + 4 reserved + 16 per address
                 self.assertEqual(len(packed_tuple), 8 + 16 * len(as_list))
 
-                # construct-then-parse: the addresses come back, in the same
-                # order, through the schema's own pack/unpack pair
-                data_bytes = packed_tuple[4:]
-                parsed = route_schema.SourceRoute.unpack(
-                    data_bytes, len(data_bytes), {'__length__': len(data_bytes)})
-                self.assertEqual(list(parsed.ip), as_list)
+                # construct-then-parse, through the full public API: the
+                # addresses come back, in the same order, from a real
+                # IPv6_Route.read() of the bytes IPv6_Route.make() produced
+                info = IPv6_Route(io.BytesIO(packed_tuple), len(packed_tuple)).info
+                self.assertEqual(list(info.ip), as_list)
+
+    def test_ipv6_route_source_route_construct_then_parse_round_trip(self) -> None:
+        """#487: ``IPv6_Route.make()`` for a Source Route (Type 0) header emitted
+        bytes its own parser rejected, for *every* address count -- the two
+        branches of ``make`` disagreed with each other, and both disagreed with
+        :rfc:`8200#section-4.4`, on the unit of ``Hdr Ext Len``. This is the
+        check the issue asked for explicitly: construct through the public API,
+        parse the result straight back, for each of 0, 1, 2 and 3 addresses --
+        the boundaries, rather than one comfortable middle case.
+
+        """
+        from ipaddress import IPv6Address
+
+        from pcapkit.const.ipv6.routing import Routing
+        from pcapkit.protocols.internet.ipv6_route import IPv6_Route
+
+        # (n addresses, expected total octets, expected Hdr Ext Len)
+        cases = [
+            (0, 8, 0),
+            (1, 24, 2),
+            (2, 40, 4),
+            (3, 56, 6),
+        ]
+
+        for n, expected_octets, expected_hdr_ext_len in cases:
+            with self.subTest(addresses=n):
+                addrs = tuple(IPv6Address(f'2001:db8::{i + 1}') for i in range(n))
+
+                proto = object.__new__(IPv6_Route)
+                raw = proto.make(type=Routing.Source_Route, data={'ip': addrs},
+                                  next=0, seg_left=n).pack()
+
+                self.assertEqual(len(raw), expected_octets)
+                self.assertEqual(raw[1], expected_hdr_ext_len)
+
+                info = IPv6_Route(io.BytesIO(raw), len(raw)).info
+                self.assertEqual(tuple(info.ip), addrs)
+
+    def test_ipv6_route_source_route_parses_hand_built_wire_form(self) -> None:
+        """#487's more important half: a hand-built, spec-correct Source Route
+        header -- one this library never constructed -- must parse, independent
+        of whatever ``IPv6_Route.make()`` does. A round-trip test alone can pass
+        on two mistakes that cancel out (a constructor and a parser that agree
+        with each other but not with the wire format); this does not go through
+        ``make()`` at all, so it catches exactly that failure mode. Bytes match
+        the issue's own hand-built example: ``next=0``, ``Hdr Ext Len=2``,
+        ``type=0`` (Source Route), ``seg_left=1``, 4 reserved octets, one
+        16-octet address, 24 octets total.
+
+        """
+        from ipaddress import IPv6Address
+
+        from pcapkit.protocols.internet.ipv6_route import IPv6_Route
+
+        addr = IPv6Address('2001:db8::1')
+        hand_built = bytes([0x00, 0x02, 0x00, 0x01]) + b'\x00' * 4 + addr.packed
+        self.assertEqual(len(hand_built), 24)
+
+        info = IPv6_Route(io.BytesIO(hand_built), len(hand_built)).info
+        self.assertEqual(tuple(info.ip), (addr,))
+        self.assertEqual(info.length, 24)
 
 
 if __name__ == '__main__':