fix(dpkt): extract(engine='dpkt') keeps a frame dpkt cannot parse instead of leaking AttributeError (#1351)

pcapkit/foundation/engines/dpkt.py

@@ -161,7 +161,22 @@ def read_frame(self) -> 'DPKTPacket':
         # fetch DPKT packet
         timestamp, pkt = cast('tuple[float, bytes]', next(reader))
         protocol = self._get_protocol(linktype)
-        packet = protocol(pkt)  # type: DPKTPacket
+        try:
+            packet = protocol(pkt)  # type: DPKTPacket
+        except Exception as exc:  # pylint: disable=broad-except
+            # NOTE: Caught broadly, as :meth:`ProtocolBase.analyze
+            # <pcapkit.protocols.protocol.ProtocolBase.analyze>` does: dpkt raises
+            # more than its own ``UnpackError`` while parsing -- dpkt 1.9.8 reads
+            # ``frag_off`` from whatever header follows an IPv6 Fragment header,
+            # so a Fragment header followed by ESP or Destination Options raises
+            # :exc:`AttributeError` (:issue:`1351`). The frame is kept as a raw
+            # packet, so one undecodable frame does not end the extraction and
+            # the frame numbering still matches the other engines, which fall
+            # back to a raw payload in the same way.
+            warn(f'Frame {ext._frnum + 1}: dpkt cannot parse the frame '
+                 f'({type(exc).__name__}: {exc}); kept as raw data',
+                 DPKTWarning, stacklevel=stacklevel())
+            packet = self._get_raw_protocol()(pkt)
 
         # DPKT hands the record's timestamp back beside its octets and only the
         # octets become a packet, so the frame would otherwise not know when it was
@@ -244,18 +259,23 @@ def _get_protocol(self, linktype: 'Optional[Enum_LinkType]' = None) -> 'Type[DPK
         else:
             warn('unrecognised link layer protocol; all analysis functions ignored',
                  DPKTWarning, stacklevel=stacklevel())
+            pkg = self._get_raw_protocol()
+        return pkg
 
-            class RawPacket(dpkt.dpkt.Packet):  # type: ignore[name-defined]
-                """Raw packet."""
+    def _get_raw_protocol(self) -> 'Type[DPKTPacket]':
+        """Returns a protocol that keeps the packet as raw data."""
+        dpkt = self._expkg
 
-                def __len__(ext) -> 'int':
-                    return len(ext.data)
+        class RawPacket(dpkt.dpkt.Packet):  # type: ignore[name-defined]
+            """Raw packet."""
 
-                def __bytes__(ext) -> 'bytes':
-                    return ext.data
+            def __len__(ext) -> 'int':
+                return len(ext.data)
 
-                def unpack(ext, buf: 'bytes') -> 'None':
-                    ext.data = buf
+            def __bytes__(ext) -> 'bytes':
+                return ext.data
 
-            pkg = RawPacket
-        return pkg
+            def unpack(ext, buf: 'bytes') -> 'None':
+                ext.data = buf
+
+        return RawPacket

tests/foundation/engines/test_dpkt_parse_error_unit.py

@@ -0,0 +1,72 @@
+# -*- coding: utf-8 -*-
+"""The DPKT engine keeps a frame dpkt cannot parse, rather than aborting.
+
+GitHub issue #1351: dpkt 1.9.8 reads ``frag_off`` from whatever header follows
+an IPv6 Fragment header, so a Fragment header followed by ESP or Destination
+Options raises :exc:`AttributeError` from ``dpkt/ip6.py``. The engine did not
+catch it, so the third-party exception ended the whole extraction.
+
+Every case writes its own one-frame capture to a temporary directory. Everything
+from :mod:`pcapkit` is imported inside each test, after
+:func:`~tests._support.reimport_once_per_class`, not at module load.
+
+"""
+
+import importlib.util
+import os
+import struct
+import tempfile
+import unittest
+import warnings
+
+from tests._support import reimport_once_per_class
+
+HAS_DPKT = importlib.util.find_spec('dpkt') is not None
+
+#: IPv6 header with Next Header 44 (Fragment) and a 16-octet payload.
+IPV6 = (bytes.fromhex('60000000') + (16).to_bytes(2, 'big') + bytes([44, 64])
+        + bytes(15) + b'\x01' + bytes(15) + b'\x02')
+#: Fragment header (offset 0, M=0) followed by eight octets, per next header.
+PAYLOADS = {
+    50: bytes([50, 0, 0, 0, 0, 0, 0, 1]) + bytes(8),
+    60: bytes([60, 0, 0, 0, 0, 0, 0, 1]) + bytes.fromhex('3b00010400000000'),
+}
+
+
+def _write_pcap(path: str, frame: bytes) -> None:
+    """Write a little-endian Ethernet PCAP holding one frame."""
+    with open(path, 'wb') as file:
+        file.write(struct.pack('<IHHiIII', 0xa1b2c3d4, 2, 4, 0, 0, 65535, 1))
+        file.write(struct.pack('<IIII', 1, 0, len(frame), len(frame)) + frame)
+
+
+@unittest.skipUnless(HAS_DPKT, 'dpkt is not installed')
+class TestDPKTParseError(unittest.TestCase):
+    """Pin how the DPKT engine handles a frame dpkt fails to parse."""
+
+    def setUp(self) -> None:
+        reimport_once_per_class(self)
+
+    def test_unparsable_frame_is_kept_as_raw_data(self) -> None:
+        import pcapkit
+        from pcapkit.utilities.warnings import DPKTWarning
+
+        for nxt, payload in PAYLOADS.items():
+            frame = bytes(12) + b'\x86\xdd' + IPV6 + payload
+            with self.subTest(next_header=nxt), tempfile.TemporaryDirectory() as tmp:
+                path = os.path.join(tmp, f'frag-{nxt}.pcap')
+                _write_pcap(path, frame)
+
+                with warnings.catch_warnings(record=True) as caught:
+                    warnings.simplefilter('always')
+                    ext = pcapkit.extract(fin=path, engine='dpkt', nofile=True,
+                                          store=True, ip=True, ipv6=True)
+
+                self.assertEqual(ext.length, 1)
+                self.assertEqual(bytes(ext.frame[0]), frame)
+                self.assertTrue(any(issubclass(item.category, DPKTWarning)
+                                    and 'cannot parse' in str(item.message) for item in caught))
+
+
+if __name__ == '__main__':
+    unittest.main()