fix(ipv4): build the TS option from its default timestamp, and print a bad one with !r (#2277)

Problem: _make_opt_ts accepted only a list or a dict for timestamp, but its
default is None, so a TS option made without timestamps always raised
ProtocolError; the error printed the value without !r.

Fix: treat None as an empty list before the type dispatch, so it builds a
Timestamp-only option with `counts` empty slots; any other non-list,
non-dict raises "invalid timestamp value: must be a list or a dict, not
<repr>". The docstring's Raises names ProtocolError for exactly that.

Test: test_ipv4_ts_default_unit pins the default option make -> parse ->
rebuild byte for byte and the message text; test_ipv4_unit's
timestamp=None ProtocolError case now uses 'abc'.

Author: JarryShaw

examples/generators/options.py

@@ -541,7 +541,8 @@ def _ipv4_overrides() -> 'dict[Any, dict[str, Any]]':
         OptionNumber.LSR: {'counts': 9},
         OptionNumber.RR: {'counts': 9},
         OptionNumber.SSR: {'counts': 9},
-        # ``timestamp=None``, the default, is rejected outright.
+        # The default builds an option with no timestamp in it, which is
+        # degenerate; one is recorded.
         OptionNumber.TS: {'counts': 1, 'timestamp': [1]},
         OptionNumber.E_SEC: {'info': b'\x00'},
     }

pcapkit/protocols/internet/ipv4.py

@@ -1700,7 +1700,7 @@ def _make_opt_ts(self, kind: 'Enum_OptionNumber', option: 'Optional[Data_TSOptio
             timestamp: list of timestamps, i.e. the slots before the pointer, or
                 a mapping of the addresses to their timestamps; an address may
                 be anything :func:`~pcapkit.corekit.fields.ipaddress.parse_ip_address`
-                accepts for IPv4
+                accepts for IPv4; :data:`None` is an empty list
             remaining: raw 32-bit words for the slots at or beyond the pointer;
                 the slots left over up to ``counts`` are zero
             **kwargs: arbitrary keyword arguments
@@ -1710,8 +1710,7 @@ def _make_opt_ts(self, kind: 'Enum_OptionNumber', option: 'Optional[Data_TSOptio
 
         Raises:
             ProtocolError: If ``option`` is not given and ``timestamp`` is
-                neither a :obj:`list` nor a :obj:`dict`, which includes the
-                default :data:`None`.
+                neither :data:`None`, a :obj:`list`, nor a :obj:`dict`.
             FieldValueError: If an address is a :obj:`bool` or is not a valid
                 IPv4 address.
 
@@ -1748,6 +1747,8 @@ def _make_opt_ts(self, kind: 'Enum_OptionNumber', option: 'Optional[Data_TSOptio
         else:
             ts_list = []
             remainder = b''
+            if timestamp is None:
+                timestamp = []
             if isinstance(timestamp, list):
                 flag = Enum_TSFlag.Timestamp_Only  # type: ignore[assignment]
                 counts = min(9, counts)  # 9 is the maximum number of timestamps
@@ -1785,7 +1786,8 @@ def _make_opt_ts(self, kind: 'Enum_OptionNumber', option: 'Optional[Data_TSOptio
                         flag = Enum_TSFlag.Prespecified_IP_with_Timestamp  # type: ignore[assignment]
                     ts_list.append(ts)
             else:
-                raise ProtocolError(f'{self.alias}: [OptNo {kind}] invalid timestamp value: {timestamp}')
+                raise ProtocolError(f'{self.alias}: [OptNo {kind}] invalid timestamp value: '
+                                    f'must be a list or a dict, not {timestamp!r}')
             pointer = 5 + len(ts_list) * 4
 
             # NOTE: Every slot of the option is written, the empty ones as zero.

tests/protocols/internet/test_ipv4_ts_default_unit.py

@@ -0,0 +1,70 @@
+# -*- coding: utf-8 -*-
+"""The IPv4 Timestamp option builds from its default ``timestamp``.
+
+GitHub issue #2277: ``_make_opt_ts`` dispatched on ``timestamp`` being a
+:obj:`list` or a :obj:`dict`, but its default is :data:`None`, so a ``TS``
+option made without timestamps always raised ``ProtocolError``. :data:`None` is
+an empty list: a Timestamp-only option with ``counts`` empty slots. Any other
+value still raises, with the value printed by :func:`repr`.
+
+Every case builds its own octets in memory and reads no capture. Classes are
+imported inside each test, after :func:`~tests._support.reimport_once_per_class`.
+
+"""
+
+import io
+import unittest
+
+from tests._support import reimport_once_per_class
+
+#: Test tier (#1958): hand-picked round-trip cases, not a sweep.
+__tier__ = 'ALL'
+
+#: The default ``TS`` option: length 24, pointer 5, Timestamp-only, five empty slots.
+DEFAULT_TS = '44180500' + '00000000' * 5
+
+
+class TestIPv4TimestampDefault(unittest.TestCase):
+    """Pin the default TS option through make, parse and rebuild."""
+
+    def setUp(self) -> None:
+        reimport_once_per_class(self)
+
+    def make(self, option: 'dict[str, object]') -> 'bytes':
+        from pcapkit.const.ipv4.option_number import OptionNumber
+        from pcapkit.protocols.internet.ipv4 import IPv4
+
+        return bytes(IPv4(src='10.0.0.1', dst='10.0.0.2', ttl=64, id=0,
+                          options=[(OptionNumber.TS, option)], payload=b''))
+
+    def test_default_timestamp_round_trips_byte_for_byte(self) -> None:
+        from pcapkit.const.ipv4.option_number import OptionNumber
+        from pcapkit.const.ipv4.ts_flag import TSFlag
+        from pcapkit.protocols.internet.ipv4 import IPv4
+
+        for name, option in (('omitted', {}), ('None', {'timestamp': None})):
+            with self.subTest(timestamp=name):
+                data = self.make(option)
+                self.assertEqual(data[20:].hex(), DEFAULT_TS)
+                self.assertEqual(data.hex(), self.make({'timestamp': []}).hex())
+
+                parsed = IPv4(io.BytesIO(data), len(data))
+                ts = parsed.info.options[OptionNumber.TS]  # type: ignore[attr-defined]
+                self.assertEqual((ts.length, ts.pointer, ts.flag, ts.timestamp),
+                                 (24, 5, TSFlag.Timestamp_Only, ()))
+                self.assertEqual(bytes(IPv4.from_data(parsed.info)).hex(), data.hex())  # type: ignore[attr-defined]
+
+    def test_invalid_timestamp_message_uses_repr(self) -> None:
+        from pcapkit.const.ipv4.option_number import OptionNumber
+        from pcapkit.protocols.internet.ipv4 import IPv4
+        from pcapkit.utilities.exceptions import ProtocolError
+
+        proto = IPv4.__new__(IPv4)
+        with self.assertRaises(ProtocolError) as ctx:
+            proto._make_opt_ts(OptionNumber.TS, timestamp='abc')  # type: ignore[arg-type]
+        self.assertEqual(str(ctx.exception),
+                         "IPv4: [OptNo 68] invalid timestamp value: must be a list or a dict, not 'abc'")
+
+
+if __name__ == '__main__':
+    unittest.main()

tests/protocols/internet/test_ipv4_unit.py

@@ -1385,7 +1385,7 @@ def test_ipv4_option_constructors_cover_common_and_error_branches(self) -> None:
         self.assertEqual(padded[-1].type, OptionNumber.EOOL)
 
         with self.assertRaises(ProtocolError):
-            proto._make_opt_ts(OptionNumber.TS, timestamp=None)
+            proto._make_opt_ts(OptionNumber.TS, timestamp='abc')  # type: ignore[arg-type]
         with self.assertRaises(ProtocolError):
             proto._make_opt_qs(OptionNumber.QS, func=99)