fix(ipv4): terminate the SEC bitmap, reject its terminator bit, and narrow SID to 16 bits

examples/generators/options.py

@@ -466,8 +466,12 @@ def _ipv4_overrides() -> 'dict[Any, dict[str, Any]]':
     from pcapkit.const.ipv4.option_number import OptionNumber
     from pcapkit.const.ipv4.protection_authority import ProtectionAuthority
     return {
-        # A single authority whose value is 0 makes ``_make_opt_sec`` compute a
-        # zero-octet bitmap and then index into it; two keeps it non-empty.
+        # Two authorities rather than one because two bits set in the bitmap say
+        # more than one does, not because one is unrepresentable: it used to be,
+        # a single ``GENSER`` (value 0) making ``_make_opt_sec`` size a
+        # zero-octet bitmap and then index into it for a bare ``IndexError``, and
+        # #537 fixed that arithmetic. So this is a coverage choice now and no
+        # longer routes around anything.
         OptionNumber.SEC: {'authorities': [ProtectionAuthority.GENSER,
                                            ProtectionAuthority.NSA]},
         # ``counts=10``, the default, needs 43 option octets, which overflows

pcapkit/protocols/internet/ipv4.py

@@ -1392,6 +1392,22 @@ def _make_opt_sec(self, kind: 'Enum_OptionNumber', option: 'Optional[Data_SECOpt
         Returns:
             Constructured option schema.
 
+        Raises:
+            ProtocolError: If ``authorities`` names a bit position that is not a
+                protection authority -- a negative one, or one that :rfc:`1108`
+                reserves as a field termination indicator.
+
+        Notes:
+            :rfc:`1108` section 2.2 lays each protection authority octet out as
+            seven authority bits followed by a *field termination indicator* in
+            bit 0: ``0`` means another octet follows, ``1`` means this is the
+            last. So the authority numbering skips every position that is a
+            termination bit -- 7, 15, 23 -- which is what
+            :meth:`_read_opt_sec` encodes by looping over ``range(7)`` per
+            octet, and the reason ``Field_Termination_Indicator`` is rejected
+            here rather than written: the enumeration names it as structure, and
+            a value written there would be dropped on the way back in. See #537.
+
         """
         if option is not None:
             level_val = option.level
@@ -1402,12 +1418,39 @@ def _make_opt_sec(self, kind: 'Enum_OptionNumber', option: 'Optional[Data_SECOpt
             authorities = [] if authorities is None else authorities
 
         if authorities:
+            for auth in authorities:
+                if auth < 0:
+                    raise ProtocolError(f'{self.alias}: [OptNo {kind}] invalid protection '
+                                        f'authority: {auth}')
+                if auth % 8 == 7:
+                    # ``.name`` where there is one, rather than
+                    # ``Enum_ProtectionAuthority.get(auth)``: that call runs
+                    # ``_missing_`` for an unnamed index, which extends the
+                    # enumeration as a side effect. An error path is the last
+                    # place that should mutate a registry.
+                    raise ProtocolError(f'{self.alias}: [OptNo {kind}] invalid protection '
+                                        f'authority: {getattr(auth, "name", auth)} is a field '
+                                        f'termination indicator, not an authority')
+
+            # ``max_auth`` is the highest bit *index*, so the octet count comes
+            # from the bit *count* one past it. Sizing from the index itself put
+            # a single ``GENSER`` (index 0) in a zero-octet bitmap and then
+            # indexed into it, raising a bare ``IndexError``; and it under-sized
+            # by an octet at every exact multiple of eight. See #537.
             max_auth = max(authorities)
-            int_len = math.ceil(max_auth / 8)
+            int_len = math.ceil((max_auth + 1) / 8)
 
             data_list = [b'0' for _ in range(int_len * 8)]
             for auth in authorities:
                 data_list[auth] = b'1'
+
+            # Bit 0 of the *last* octet terminates the field. The intermediate
+            # octets keep the ``0`` they were initialised with, which is what
+            # says "another octet follows" -- so this single assignment is the
+            # whole of the indicator, and without it every option this method
+            # wrote was one its own reader warned about.
+            data_list[-1] = b'1'
+
             data = int(b''.join(data_list), base=2).to_bytes(int_len, 'big', signed=False)
         else:
             data = b''

pcapkit/protocols/schema/internet/ipv4.py

@@ -364,8 +364,15 @@ def __init__(self, type: 'Enum_OptionNumber', length: 'int', pointer: 'int', rou
 class SIDOption(Option, code=Enum_OptionNumber.SID):
     """Header schema for IPv4 stream identifier (``SID``) option."""
 
-    #: Stream identifier.
-    sid: 'int' = UInt32Field()
+    #: Stream identifier. Two octets, per :rfc:`791` section 3.1, which gives the
+    #: option as four octets in total: one of type, one of length, and a 16-bit
+    #: stream identifier. This was a :class:`~pcapkit.corekit.fields.numbers.UInt32Field`,
+    #: which over-read a well-formed option by two octets on the way in -- the
+    #: ``packet length < 0: -2`` the library warned about -- and re-emitted it two
+    #: octets too wide on the way out, against the ``length=4`` that
+    #: :meth:`~pcapkit.protocols.internet.ipv4.IPv4._make_opt_sid` had always
+    #: written. See #534.
+    sid: 'int' = UInt16Field()
 
     if TYPE_CHECKING:
         def __init__(self, type: 'Enum_OptionNumber', length: 'int', sid: 'int') -> 'None': ...