fix(engines): write real PCAP traces for dpkt and scapy (#1507)

pcapkit/foundation/engines/dpkt.py

@@ -134,7 +134,11 @@ def _read_packet(self, buf: 'bytes', block_type: 'int',
             little: Whether the enclosing section is little-endian.
 
         Returns:
-            Timestamp in seconds since the UNIX epoch, and the packet data.
+            Timestamp in seconds since the UNIX epoch, and the packet data. The
+            timestamp is a :class:`~pcapkit.toolkit.dpkt.Timestamp`: the
+            :obj:`float` this reader always yielded, which also carries the exact
+            value, the interface's resolution and the packet's original length,
+            for the PCAP frame record of the packet (:issue:`1507`).
 
         Raises:
             FormatError: If the section describes no interface of the ID the
@@ -143,6 +147,8 @@ def _read_packet(self, buf: 'bytes', block_type: 'int',
         """
         import dpkt  # isort:skip
 
+        from pcapkit.toolkit.dpkt import Timestamp  # isort:skip
+
         if block_type == Enum_BlockType.Enhanced_Packet_Block:
             tag = 'EPB'
             pkt = (dpkt.pcapng.EnhancedPacketBlockLE(buf) if little
@@ -168,7 +174,81 @@ def _read_packet(self, buf: 'bytes', block_type: 'int',
             timestamp = decimal.Decimal(ticks) / resolution + offset
 
         self._linktype = interface.linktype
-        return float(timestamp), pkt.pkt_data
+        return (Timestamp(float(timestamp), timestamp, resolution, pkt.pkt_len),  # pylint: disable=no-member
+                pkt.pkt_data)
+
+
+def _pcap_reader() -> 'Type[PCAPReader]':
+    """A :class:`dpkt.pcap.Reader` that keeps each record's original length.
+
+    :class:`dpkt.pcap.Reader` parses every record header and then yields only
+    ``(timestamp, octets)``, so the record's original length and the file's
+    resolution are lost by the time a PCAP frame record of the packet is built
+    (:issue:`1507`). This subclass yields the same timestamp, as a
+    :class:`~pcapkit.toolkit.dpkt.Timestamp` or, for a nanosecond file, a
+    :class:`~pcapkit.toolkit.dpkt.DecimalTimestamp`, which carries the two.
+
+    Built when called, so that importing this module does not import `DPKT`_,
+    and so that it always subclasses the :class:`dpkt.pcap.Reader` of the moment.
+
+    Warns:
+        DPKTWarning: If this `DPKT`_'s reader lacks the attributes the loop reads
+            -- they are private, and ``dpkt`` is not pinned. The reader then runs
+            unchanged, and a traced PCAP record's original length is its captured
+            length.
+
+    Returns:
+        The reader type.
+
+    .. _DPKT: https://dpkt.readthedocs.io
+
+    """
+    import dpkt  # isort:skip
+
+    from pcapkit.toolkit.dpkt import DecimalTimestamp, Timestamp  # isort:skip
+
+    #: Attributes of :class:`dpkt.pcap.Reader` the loop below reads, as of dpkt 1.9.8.
+    private = ('_Reader__f', '_Reader__ph', '_divisor')
+
+    class _Reader(dpkt.pcap.Reader):  # type: ignore[name-defined,misc]
+        def __init__(self, fileobj: 'IO[bytes]') -> 'None':
+            super().__init__(fileobj)
+            missing = [name for name in private if not hasattr(self, name)]
+            #: Whether to read with :meth:`dpkt.pcap.Reader.__iter__` unchanged.
+            self._upstream = bool(missing)
+            if missing:
+                warn(f"dpkt {getattr(dpkt, '__version__', '(unknown version)')}: "
+                     f"'dpkt.pcap.Reader' has no {', '.join(repr(name) for name in missing)}; "
+                     "reading PCAP records without their original length, so a traced "
+                     'PCAP record gives its captured length instead', DPKTWarning,
+                     stacklevel=stacklevel())
+
+        # NOTE: the loop is :meth:`dpkt.pcap.Reader.__iter__`'s, over the stream
+        # and the record header type that method reads them through. A generator,
+        # so ``dpkt.pcap.Reader.__init__`` calling it runs none of it before
+        # :attr:`_upstream` is set.
+        def __iter__(self) -> 'Iterator[tuple[float, bytes]]':
+            if self._upstream:
+                yield from super().__iter__()
+                return
+
+            file, header = self._Reader__f, self._Reader__ph  # pylint: disable=no-member
+            nanosecond = isinstance(self._divisor, decimal.Decimal)
+            resolution = 1_000_000_000 if nanosecond else 1_000_000
+            while True:
+                buf = file.read(header.__hdr_len__)
+                if not buf:
+                    break
+                hdr = header(buf)
+                buf = file.read(hdr.caplen)
+                value = hdr.tv_sec + (hdr.tv_usec / self._divisor)
+                if nanosecond:
+                    yield DecimalTimestamp(value, resolution, hdr.len), buf
+                else:
+                    exact = decimal.Decimal(hdr.tv_sec) + decimal.Decimal(hdr.tv_usec) / resolution
+                    yield Timestamp(value, exact, resolution, hdr.len), buf
+
+    return _Reader
 
 
 class DPKT(EngineBase['DPKTPacket']):
@@ -276,7 +356,7 @@ def run(self) -> 'None':
 
         if ext.magic_number in PCAP.MAGIC_NUMBER:
             logger.debug('dpkt: reading %s as PCAP', ext._ifnm)
-            reader = dpkt.pcap.Reader(ext._ifile)
+            reader = _pcap_reader()(ext._ifile)
         elif ext.magic_number in PCAPNG.MAGIC_NUMBER:
             logger.debug('dpkt: reading %s as PCAP-NG', ext._ifnm)
             reader = PCAPNGReader(ext._ifile)

pcapkit/foundation/engines/scapy.py

@@ -47,16 +47,79 @@
 __all__ = ['Scapy']
 
 if TYPE_CHECKING:
-    from typing import Iterator
+    from typing import Any, Iterator, Type
 
     from scapy.packet import Packet as ScapyPacket
+    from scapy.utils import PcapReader
 
     from pcapkit.foundation.extraction import Extractor
 
 #: logging.Logger: Module-level logger, a child of the package-wide
 #: :data:`pcapkit.utilities.logging.logger`.
 logger = get_logger(__name__)
 
+def _reader_type() -> 'Type[PcapReader]':
+    """A :class:`scapy.utils.PcapReader` that stamps each packet with its resolution.
+
+    `Scapy`_'s readers convert a record's timestamp to a :class:`~decimal.Decimal`
+    and drop the resolution it was counted in, and an exact count of microseconds
+    comes out with the same digits whether the capture counted microseconds or
+    nanoseconds. The PCAP frame record of a packet needs that resolution, so these
+    subclasses attach it with :func:`pcapkit.toolkit.scapy.attach_resolution`:
+    from the global header of a PCAP, and from the interface of each PCAP-NG
+    packet block, which :meth:`~scapy.utils.RawPcapNgReader._read_block` is the
+    last place to see.
+
+    Built when called, so that importing this module does not import `Scapy`_,
+    and so that it always subclasses the :class:`scapy.utils.PcapReader` of the
+    moment.
+
+    Returns:
+        A reader type, which opens a PCAP-NG file as its PCAP-NG alternative.
+
+    .. _Scapy: https://scapy.net
+
+    """
+    from scapy.utils import PcapNgReader, PcapReader  # isort:skip
+
+    from pcapkit.toolkit.scapy import attach_resolution  # isort:skip
+
+    class _Closing:
+        # ``sniff`` closes the reader at the end of the file, and the engine closes
+        # it again in case ``sniff`` raised first; only the first close is real,
+        # since closing a gzip reader twice raises
+        _closed = False
+
+        def close(self) -> 'None':
+            if not self._closed:
+                self._closed = True
+                super().close()  # type: ignore[misc]
+
+    class _PcapReader(_Closing, PcapReader):
+        def read_packet(self, *args: 'Any', **kwargs: 'Any') -> 'ScapyPacket':
+            packet = super().read_packet(*args, **kwargs)
+            attach_resolution(packet, 1_000_000_000 if self.nano else 1_000_000)
+            return packet
+
+    class _PcapNgReader(_Closing, PcapNgReader):
+        # the metaclass makes this ``_PcapReader``'s alternative in turn
+        alternative = _PcapReader
+
+        _resolution = 1_000_000
+
+        def _read_block(self, *args: 'Any', **kwargs: 'Any') -> 'Any':
+            block = super()._read_block(*args, **kwargs)
+            if block is not None:
+                self._resolution = block[1].tsresol
+            return block
+
+        def read_packet(self, *args: 'Any', **kwargs: 'Any') -> 'ScapyPacket':
+            packet = super().read_packet(*args, **kwargs)
+            attach_resolution(packet, self._resolution)
+            return packet
+
+    return _PcapReader
+
 
 class Scapy(EngineBase['ScapyPacket']):
     """Scapy engine support.
@@ -179,7 +242,18 @@ def run(self) -> 'None':
 
         # extract & analyse file
         logger.debug('scapy: sniffing %s', ext._ifnm)
-        self._extmp = iter(self._expkg.sniff(offline=ext._ifnm))
+        # NOTE: the reader is opened here rather than by ``sniff(offline=...)`` so
+        # that every packet carries its capture's timestamp resolution, which
+        # Scapy's own readers discard (#1507). Labelled with the file name, as
+        # ``offline=`` labels it, so ``Packet.sniffed_on`` is unchanged. ``sniff``
+        # closes it at the end of the file, as it closes the reader ``offline=``
+        # opens; it is closed here as well in case ``sniff`` raised first, which
+        # the reader takes as a no-op once closed.
+        reader = _reader_type()(ext._ifnm)
+        try:
+            self._extmp = iter(self._expkg.sniff(opened_socket={reader: ext._ifnm}))
+        finally:
+            reader.close()
 
     def read_frame(self) -> 'ScapyPacket':
         """Read frames with Scapy engine.

pcapkit/foundation/extraction.py

@@ -1431,13 +1431,12 @@ def __init__(self,
             # <pcapkit.foundation.traceflow.traceflow.TraceFlowBase.__init__>`
             # itself substitutes ``'pcap'`` for ``None``.
             #
-            # DPKT and Scapy belong on this list too: their adapters build the frame
-            # with ``packet2dict`` exactly as the other two do, so both crash the
-            # same way. The guard is written from what the adapters produce, not
-            # from which engines are seen to crash, because an engine that
-            # dissects nothing never feeds the tracer and so hides the defect
-            # rather than avoiding it (#406).
-            if (self._exnam in ('dpkt', 'scapy', 'pyshark', 'pypcapfile')
+            # The guard is written from what the adapters produce, not from which
+            # engines are seen to crash, because an engine that dissects nothing
+            # never feeds the tracer and so hides the defect rather than avoiding
+            # it (#406). DPKT and Scapy are off the list because their adapters
+            # build a PCAP frame record from the captured octets (#1507).
+            if (self._exnam in ('pyshark', 'pypcapfile')
                     and trace_format in ('pcap', 'cap', None)):
                 warn(f"'Extractor(engine={self._exnam})' does not support 'trace_format={trace_format}'; "
                      "using 'trace_format=\"json\"' instead", FormatWarning, stacklevel=stacklevel())

pcapkit/foundation/traceflow/data/data.py

@@ -5,14 +5,16 @@
 
 from pcapkit.corekit.infoclass import Info, info_final
 
-__all__ = ['TraceFlowData', 'Deferred', 'DeferredPacket']
+__all__ = ['TraceFlowData', 'Deferred', 'DeferredPacket', 'FrameRecord']
 
 if TYPE_CHECKING:
+    from decimal import Decimal
     from typing import Any, Optional
 
     from pcapkit.foundation.reassembly.data.tcp import Datagram as TCP_Datagram
     from pcapkit.foundation.reassembly.tcp import TCP as TCP_Reassembly
     from pcapkit.foundation.traceflow.data.tcp import Index as TCP_Index
+    from pcapkit.protocols.data.misc.pcap.frame import FrameInfo as Data_FrameInfo
 
 
 class Deferred:
@@ -141,6 +143,68 @@ def to_dict(self) -> 'dict[str, Any]':
         return super().to_dict()  # type: ignore[misc] # pylint: disable=no-member
 
 
+class FrameRecord(dict):
+    """A frame as a third-party engine's flow tracing adapter reports it.
+
+    The mapping is the engine's own dissection of the frame, exactly as before, so
+    every trace format that takes a mapping writes it unchanged. Beside it, as
+    attributes the mapping does not show, it carries what the PCAP trace dumper
+    reads of a frame -- :meth:`PCAPIO._append_value
+    <pcapkit.dumpkit.pcap.PCAPIO._append_value>` takes ``packet``,
+    ``frame_info`` and ``time_epoch`` -- so ``trace_format='pcap'`` writes the
+    frame's record too (:issue:`1507`).
+
+    Args:
+        mapping: The engine's dissection of the frame.
+        packet: The frame's captured octets.
+        frame_info: The frame's PCAP record header.
+        time_epoch: The frame's exact UNIX timestamp.
+
+    """
+
+    __slots__ = ('packet', 'frame_info', 'time_epoch')
+
+    #: The frame's captured octets.
+    packet: 'bytes'
+    #: The frame's PCAP record header, with the fraction in the capture's own
+    #: resolution.
+    frame_info: 'Data_FrameInfo'
+    #: The frame's exact UNIX timestamp, from which
+    #: :meth:`PCAPIO._make_timestamp <pcapkit.dumpkit.pcap.PCAPIO._make_timestamp>`
+    #: tells which resolution ``frame_info.ts_usec`` is in. A
+    #: :class:`~decimal.Decimal`, since only that settles it exactly.
+    time_epoch: 'Decimal'
+
+    def __init__(self, mapping: 'dict[str, Any]', *, packet: 'bytes',
+                 frame_info: 'Data_FrameInfo', time_epoch: 'Decimal') -> 'None':
+        super().__init__(mapping)
+        self.packet = packet
+        self.frame_info = frame_info
+        self.time_epoch = time_epoch
+
+    def __reduce__(self) -> 'tuple[Any, ...]':
+        # NOTE: spelled out because a ``dict`` subclass with ``__slots__`` cannot
+        # be pickled with protocol 0 or 1 otherwise
+        return _frame_record, (dict(self), self.packet, self.frame_info, self.time_epoch)
+
+
+def _frame_record(mapping: 'dict[str, Any]', packet: 'bytes',
+                  frame_info: 'Data_FrameInfo', time_epoch: 'Decimal') -> 'FrameRecord':
+    """Rebuild a pickled :class:`FrameRecord`.
+
+    Args:
+        mapping: The engine's dissection of the frame.
+        packet: The frame's captured octets.
+        frame_info: The frame's PCAP record header.
+        time_epoch: The frame's exact UNIX timestamp.
+
+    Returns:
+        The frame record.
+
+    """
+    return FrameRecord(mapping, packet=packet, frame_info=frame_info, time_epoch=time_epoch)
+
+
 @info_final
 class TraceFlowData(Info):
     """Data storage for flow tracing."""

pcapkit/interface/misc.py

@@ -21,7 +21,7 @@
 from pcapkit.foundation.extraction import Extractor
 from pcapkit.foundation.reassembly.tcp import TCP as TCP_Reassembly
 from pcapkit.utilities.exceptions import stacklevel
-from pcapkit.utilities.warnings import EngineWarning, FormatWarning, warn
+from pcapkit.utilities.warnings import EngineWarning, warn
 
 if TYPE_CHECKING:
     from typing import Any, Callable, Optional
@@ -115,30 +115,6 @@ def follow_tcp_stream(fin: 'Optional[str]' = None, verbose: 'bool' = False,
              EngineWarning, stacklevel=stacklevel())
         engine = None
 
-    # NOTE: the DPKT and Scapy engines hand their frames to the flow tracer as plain
-    # :obj:`dict`\\ s, which the PCAP trace dumper cannot re-serialise -- it reaches
-    # for ``frame.packet`` and dies with ``AttributeError: 'dict' object has no
-    # attribute 'packet'`` (#399). The tracer defaults an unset ``format`` to
-    # ``'pcap'``, so following a stream through either engine would crash *during
-    # extraction*, before the reassembly below ever runs.
-    #
-    # :class:`Extractor <pcapkit.foundation.extraction.Extractor>` guards both
-    # engines itself, so this is not what keeps the extraction alive -- it is
-    # what keeps it *quiet*. The two guards choose the same replacement format and so
-    # produce byte-identical traces; they differ only in when they complain. The
-    # Extractor warns for every substitution it makes, including the one nobody asked
-    # for, whereas here an unset ``format`` is not a request and is upgraded silently,
-    # and only an explicit but unusable one draws a warning -- with a message naming
-    # the engine's limitation rather than the ``trace_format=`` argument this function
-    # does not expose. Removing this therefore would not change any trace file, but it
-    # would make ``follow_tcp_stream(engine='dpkt')`` warn about a default the caller
-    # never chose.
-    if engine is not None and engine.lower() in ('dpkt', 'scapy') and format in ('pcap', 'cap', None):
-        if format is not None:
-            warn(f"extraction engine {engine} cannot write '{format}' trace files; "
-                 "using 'json' instead", FormatWarning, stacklevel=stacklevel())
-        format = 'json'
-
     extraction = Extractor(fin=fin, fout=None, format=None, auto=True, extension=extension,
                            store=True, files=False, nofile=True, verbose=verbose, engine=engine,
                            layer=None, protocol=None, ip=False, ipv4=False, ipv6=False, tcp=True,