test: pin the truncated-data half of the #431 short-read accommodation

CHANGELOG.md

@@ -12,6 +12,7 @@ The largest release since 1.0, and the first recorded here as it happened rather
 - **Added** -- `pcapkit.utilities.logging` as a real interface: `get_logger()` for per-module children, `configure()` to set level, handler, stream, format or propagation at runtime, `reset()` to return to library-neutral, and `ensure_output()`. Seventeen modules now log under their own `__name__`, so a consumer can silence `pcapkit.foundation.registry` while keeping `pcapkit.foundation.extraction` (#384).
 - **Added** -- `conflict` on the reassembly data models: absolute, inclusive ranges where two fragments claimed the same span with different bytes, which was previously lost silently on both the IP (#482) and TCP (#443, #478) paths.
 - **Added** -- an end-to-end test tier (#376), sample-capture generators so a fresh clone can rebuild every fixture (#340), a Dockerised engine benchmark covering every supported Python version (#410), and registry round-trip coverage that records the entries which cannot close the cycle rather than skipping them (#440, #504).
+- **Added** -- coverage for the untested half of the #431 accommodation: a TCP or IPv4 option whose declared length asks for more data than the capture actually holds, pinning that it still parses, with the short read left-padded rather than rejected. The one test #431 left behind only covers an option area with no data behind it at all; a candidate fix for #554 turned the untested half into an unwrapped `FieldValueError` while the rest of the suite stayed green (#571, #572).
 - **Changed** -- `pcapkit` no longer configures logging at import. It installs a `NullHandler` and sets no level, so verbosity is inherited from the application instead of being seized by whichever library was imported second; the old stderr handler stays as the `PCAPKIT_DEVMODE` opt-in. Three consequences worth knowing: the previous behaviour is `configure(logging.INFO, stream=sys.stderr)`; 38 registry and extractor `info` calls became `debug`, so those messages are invisible even at `INFO`; and the handler is no longer `logger.handlers[0]`. `verbose=` output stays on stdout and is not logging (#384).
 - **Changed** -- each warning is reported once per channel, and `pcapkit` no longer inserts a `simplefilter('ignore', ...)` at the front of the process-global `warnings.filters` (#362--#364, #390). The application's own filter therefore wins now, which is the point of the change and also the sharp edge in it: under `-W error`, or pytest's `filterwarnings = error`, a pcapkit warning that used to be suppressed will raise. Suppress them deliberately with `warnings.filterwarnings('ignore', category=BaseWarning)`. `quiet=True` now means no record at any level and no longer sets `sys.tracebacklimit`, and the `pcapkit.utilities.warnings.DEVMODE` re-export is gone -- its canonical home is `pcapkit.utilities.logging`.
 - **Changed** -- `layer=` and `protocol=` are honoured rather than inert. Both were read under the wrong names, so every value a caller passed was dropped into `**kwargs` and discarded; the CLI's `-L` also now validates its argument instead of accepting anything. The packet context reaches the schema layer for the first time as well, so a field the wire elides can be resolved from its enclosing packet (#404). `follow_tcp_stream` dispatches on the engine type, where both branches of the old test were dead and the native adapter ran against every engine's frames (#402).

docs/source/changelog/1.5.0.rst

@@ -79,6 +79,13 @@ pull requests between #326 and #509.
   covering every supported Python version (#410), and registry round-trip
   coverage that records the entries which cannot close the cycle rather than
   skipping them (#440, #504).
+* **Added** -- coverage for the untested half of the #431 accommodation: a TCP
+  or IPv4 option whose declared length asks for more data than the capture
+  actually holds, pinning that it still parses, with the short read left-padded
+  rather than rejected. The one test #431 left behind only covers an option
+  area with no data behind it at all; a candidate fix for #554 turned the
+  untested half into an unwrapped ``FieldValueError`` while the rest of the
+  suite stayed green (#571, #572).
 * **Changed** -- ``pcapkit`` no longer configures logging at import. It
   installs a ``NullHandler`` and sets no level, so verbosity is inherited from
   the application instead of being seized by whichever library was imported

tests/protocols/internet/test_ipv4_unit.py

@@ -2002,6 +2002,69 @@ def test_an_option_area_longer_than_the_datagram_still_parses(self) -> None:
             [(OptionNumber.EOOL, 1)],
         )
 
+    def test_a_truncated_option_still_parses_its_declared_length(self) -> None:
+        """A capture cut short mid-option is tolerated, not just mid-header. C.f. #431, #572.
+
+        :meth:`test_an_option_area_longer_than_the_datagram_still_parses` above
+        pins the *empty*-tail half of the #431 accommodation: an option area
+        that runs out before it starts. Nothing pinned the other half -- an
+        option that *does* start, declares more data than the capture actually
+        holds, and runs out partway through, the shape of a datagram cut short
+        by the snapshot length rather than one with no options at all. A
+        candidate fix for #554 (PR #571) turns that into an unwrapped
+        ``FieldValueError`` while the rest of the suite stays green, because
+        nothing exercises it. See
+        ``TCPUDPUnitTests.test_a_truncated_option_still_parses_its_declared_length``
+        for the same case on TCP, where the shortfall is simpler to reach.
+
+        The header below sets ``ihl`` to 9 -- 16 declared octets of options --
+        for an unassigned option (code 31) declaring ``length=12``, which asks
+        :class:`~pcapkit.protocols.schema.internet.ipv4.UnassignedOption`'s
+        ``data`` field (``BytesField(length=lambda pkt: pkt['length'] - 2)``,
+        10 octets here) for more than the 6 octets actually behind it, and
+        :meth:`FieldBase.unpack <pcapkit.corekit.fields.field.FieldBase.unpack>`
+        left-pads the short read with zero octets rather than raising -- the
+        same accommodation as the TCP case, reached the same way.
+
+        ``ihl`` has to declare *more* than the 8 octets actually present,
+        though, which the TCP case does not need. Unlike TCP,
+        :meth:`~pcapkit.protocols.internet.ipv4.IPv4._read_ipv4_options` sums
+        each option's self-*declared* ``length`` -- not what it actually
+        consumed -- and raises ``IPv4: invalid format`` if that sum exceeds
+        the declared option area; declaring exactly 8 would make the single
+        option's own ``length=12`` trip that check before the accommodation
+        under test is ever reached. Declaring 16 leaves headroom, at the cost
+        of a second effect: once the option loop's 16-octet budget outlives
+        the 8 octets its one real option consumed, the loop reads one further,
+        fully exhausted phantom option, decodes it as end-of-option-list (the
+        same mechanism the test above pins), and the #431 machinery in
+        :meth:`~pcapkit.corekit.fields.collections.OptionField.unpack`
+        rewinds and hands the same 8 octets to the schema a second time as
+        padding. That is why ``bytes(proto.__header__)`` does not round-trip
+        to ``raw`` here and is not asserted -- immaterial to what this test
+        pins, which is solely the ``data`` field's short-read reconstruction.
+
+        """
+        from pcapkit.const.ipv4.option_number import OptionNumber
+        from pcapkit.protocols.internet.ipv4 import IPv4
+        from tests._support import time_limit
+
+        custom = OptionNumber.get(31)
+        trailing = bytes([0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff])
+        raw = (bytes.fromhex('4900001c00010000400600000a0000010a000002') +
+               bytes([custom, 12]) + trailing)
+        with time_limit(5):
+            proto = IPv4(raw, len(raw))
+
+        self.assertEqual(proto.info.hdr_len, 36)
+        self.assertEqual(
+            [(code, opt.length) for code, opt in proto.info.options.items(multi=True)],
+            [(custom, 12), (OptionNumber.EOOL, 1)],
+        )
+        unassigned = next(opt for code, opt in proto.info.options.items(multi=True)
+                           if code == custom)
+        self.assertEqual(unassigned.data, b'\x00\x00\x00\x00' + trailing)
+
 
 if __name__ == '__main__':
     unittest.main()

tests/protocols/transport/test_tcp_udp_unit.py

@@ -1257,6 +1257,71 @@ def test_an_option_area_longer_than_the_segment_still_parses(self) -> None:
                    if code == Option.Maximum_Segment_Size)
         self.assertEqual(mss.mss, 1460)
 
+    def test_a_truncated_option_still_parses_its_declared_length(self) -> None:
+        """A capture cut short mid-option is tolerated, not just mid-header. C.f. #431, #572.
+
+        :meth:`test_an_option_area_longer_than_the_segment_still_parses` above
+        pins the *empty*-tail half of the #431 accommodation: an option area
+        that runs out before it starts, so the type byte decodes as 0 and the
+        loop reads end-of-option-list. Nothing pinned the other half -- an
+        option that *does* start, declares more data than the capture actually
+        holds, and runs out partway through, the shape of a segment cut short
+        by the snapshot length rather than one with no options at all. A
+        candidate fix for #554 (PR #571) turned that into an unwrapped
+        ``FieldValueError`` while the rest of the suite stayed green, because
+        nothing exercised it.
+
+        The segment below sets a data offset of 7 -- 8 octets of option area
+        -- for an unassigned option kind (``0x4f``) declaring ``length=12``,
+        which asks
+        :class:`~pcapkit.protocols.schema.transport.tcp.UnassignedOption`'s
+        ``data`` field (``BytesField(length=lambda pkt: pkt['length'] - 2)``,
+        10 octets here) for more than the 6 octets actually behind it.
+        :meth:`FieldBase.unpack <pcapkit.corekit.fields.field.FieldBase.unpack>`
+        left-pads the short read with zero octets rather than raising, so the
+        option parses with its declared ``length`` intact and a ``data`` value
+        of four zero octets followed by the six real ones. That is reachable
+        here because :meth:`OptionField.unpack
+        <pcapkit.corekit.fields.collections.OptionField.unpack>` sizes this
+        option's schema by what it actually consumed (8 octets) rather than by
+        its self-reported ``length``, so the separate ``TCP: invalid format``
+        threshold in :meth:`~pcapkit.protocols.transport.tcp.TCP._read_tcp_options`
+        never sees the shortfall -- unlike IPv4's equivalent check, which sums
+        the *declared* lengths instead and does see it (see
+        ``IPv4UnitTests.test_a_truncated_option_still_parses_its_declared_length``).
+        ``length=32`` (30 octets of data wanted, still only 6 available) is
+        checked alongside 12, since the fix under discussion would reject both
+        identically.
+
+        """
+        import struct
+
+        from pcapkit.const.tcp.option import Option
+        from pcapkit.protocols.transport.tcp import TCP
+        from tests._support import time_limit
+
+        def segment(data_offset: 'int', options: 'bytes') -> 'bytes':
+            return struct.pack('!HHIIBBHHH', 1, 2, 0, 0, data_offset << 4,
+                               0x10, 0, 0, 0) + options
+
+        custom = Option.get(0x4f)
+        trailing = bytes([0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff])
+        for declared_length, zeroes in ((12, 4), (32, 24)):
+            with self.subTest(declared_length=declared_length):
+                raw = segment(7, bytes([custom, declared_length]) + trailing)
+                with time_limit(5):
+                    proto = TCP(raw, len(raw))
+
+                self.assertEqual(proto.info.hdr_len, 28)
+                self.assertEqual(
+                    [(code, opt.length) for code, opt in proto.info.options.items(multi=True)],
+                    [(custom, declared_length)],
+                )
+                unassigned = next(opt for code, opt in proto.info.options.items(multi=True)
+                                   if code == custom)
+                self.assertEqual(unassigned.data, b'\x00' * zeroes + trailing)
+                self.assertEqual(bytes(proto.__header__), raw)
+
     def test_unregistered_option_kind_does_not_mutate_the_class_registry(self) -> None:
         """Parsing must not write to the shared ``TCP.__option__``.