fix(extraction): accept a file object without a name as fin (#1506) (#1509)

Author: JarryShaw

pcapkit/foundation/extraction.py

@@ -182,6 +182,49 @@ def _restore_shipped_dumper(registry: 'dict[str, Any]', format: 'str',  # pylint
     registry[format] = shipped if ext == shipped[1] else (shipped[0], ext)
 
 
+#: Engines whose library opens the input by *path*, ``Extractor._ifnm``, rather
+#: than reading ``Extractor._ifile``, so they cannot take an unnamed stream.
+_PATH_ENGINES = frozenset({'scapy', 'pyshark', 'pypcap', 'pcap_ct'})
+
+
+class _PeekableStream:
+    """Give a seekable binary stream without ``peek`` one.
+
+    :class:`Extractor` and the PCAP-NG engine read ahead with ``peek``, which
+    :class:`io.BufferedReader` has and a plain :class:`typing.IO` such as
+    :class:`io.BytesIO` does not (#1506). The read-ahead is a read and a seek
+    back, and every other attribute is the stream's own. It is not an
+    :class:`io.IOBase`, so it has no finaliser that could close the caller's
+    stream behind their back (#610).
+
+    Arguments:
+        stream: seekable binary stream
+
+    """
+
+    def __init__(self, stream: 'IO[bytes]') -> 'None':
+        self._stream = stream
+
+    def peek(self, size: 'int' = 0) -> 'bytes':
+        """Return up to ``size`` octets, at least one if any remain, without consuming them.
+
+        Arguments:
+            size: number of octets wanted
+
+        Returns:
+            The octets at the current position.
+
+        """
+        pos = self._stream.tell()
+        try:
+            return self._stream.read(max(size, 1))
+        finally:
+            self._stream.seek(pos, io.SEEK_SET)
+
+    def __getattr__(self, name: 'str') -> 'Any':
+        return getattr(self._stream, name)
+
+
 class Extractor(Generic[_P]):
     """Extractor for PCAP files.
 
@@ -388,7 +431,12 @@ def format(self) -> 'Formats':
 
     @property
     def input(self) -> 'str':
-        """Name of input PCAP file."""
+        """Name of input PCAP file.
+
+        For a binary IO object without a :obj:`str` ``name``, a placeholder
+        naming its type, e.g. ``<BytesIO>``.
+
+        """
         return self._ifnm
 
     @property
@@ -931,7 +979,8 @@ def make_name(cls, fin: 'str | IO[bytes]' = 'in.pcap', fout: 'str' = 'out',
 
         1. sanitise ``fin`` as the input PCAP filename; ``in.pcap`` as default value and
            append ``.pcap`` extension if needed and ``extension`` is :data:`True`; as well
-           as test if the file exists; for a binary IO object, use its ``name``;
+           as test if the file exists; for a binary IO object, use its ``name``,
+           or a placeholder such as ``<BytesIO>`` if it has no :obj:`str` one;
         2. if ``nofile`` is :data:`True`, skips following processing;
         3. look up the output file extension registered for ``fmt``; should none be
            registered, then it raises :exc:`~pcapkit.utilities.exceptions.FormatError`;
@@ -977,7 +1026,13 @@ def make_name(cls, fin: 'str | IO[bytes]' = 'in.pcap', fout: 'str' = 'out',
             if not os.path.isfile(ifnm):
                 raise FileNotFound(2, 'No such file or directory', ifnm)
         else:
-            ifnm = fin.name
+            # NOTE: a binary IO object need not have a ``name`` -- an
+            # :class:`io.BytesIO` has none, and one wrapping a descriptor has an
+            # :obj:`int` (#1506). The name is only a label here: the stream is
+            # read as given, and the output name comes from ``fout`` alone. So
+            # fall back to ``<BytesIO>``-style, after Python's own ``<stdin>``.
+            name = getattr(fin, 'name', None)
+            ifnm = name if isinstance(name, str) else f'<{type(fin).__name__}>'
 
         if nofile:
             ofnm = None
@@ -1234,6 +1289,11 @@ def __init__(self,
             pcapkit.utilities.warnings.AttributeWarning: If ``trace_analyse`` is
                 requested while the extraction engine is PyShark.
 
+        Raises:
+            UnsupportedCall: If ``fin`` is a stream without a :obj:`str` ``name``
+                and the engine that will run -- Scapy, PyShark, PyPCAP or
+                pcap-ct -- opens its input by path.
+
         """
         if fin is None:
             fin = 'in.pcap'
@@ -1259,6 +1319,19 @@ def __init__(self,
                  "using 'format=\"json\"' instead", FormatWarning, stacklevel=stacklevel())
             format = 'json'
 
+        # NOTE: these engines hand ``_ifnm`` to their library as a *path*, so a
+        # stream without a ``str`` name -- labelled ``<BytesIO>`` by
+        # :meth:`make_name` -- would be opened as a file of that name (#1506).
+        # Refuse before anything runs, rather than relying on no such file
+        # existing. ``dpkt``, ``pypcapfile`` and the default engine read
+        # ``_ifile`` and take a stream as it is.
+        if (self._exnam in _PATH_ENGINES and not isinstance(fin, str)
+                and not isinstance(getattr(fin, 'name', None), str)):
+            raise UnsupportedCall(f"'Extractor(engine={self._exnam})' requires a file on disk, "
+                                  f'but the {type(fin).__name__} given as fin has no name; '
+                                  'the engine opens savefiles by name and cannot read an '
+                                  'in-memory stream')
+
         ifnm, ofnm, fmt, oext, files = self.make_name(fin, fout, format, extension, files=files, nofile=nofile)
 
         self._ifnm = ifnm  # input file name
@@ -1413,6 +1486,9 @@ def __init__(self,
             # same one ``_cleanup`` uses (#610).
             self._ifile = SeekableReader(self._ifile, buffer_size, buffer_save, buffer_path,
                                          stream_closing=self._flag_s)
+        elif not hasattr(self._ifile, 'peek'):
+            logger.debug('input stream has no peek(), wrapping it in _PeekableStream')
+            self._ifile = cast('BufferedReader', _PeekableStream(self._ifile))
 
         if not self._flag_q:
             output, ext = self.__output__[fmt]

tests/foundation/test_extraction_unnamed_stream_unit.py

@@ -0,0 +1,161 @@
+# -*- coding: utf-8 -*-
+"""A binary stream with no :obj:`str` ``name`` is a valid ``fin`` (#1506).
+
+``fin`` is documented and typed as ``str | IO[bytes]`` -- "file name to be read
+or a binary IO object" -- on both :func:`pcapkit.extract` and
+:class:`~pcapkit.foundation.extraction.Extractor`. Two things on the way in
+assumed more than that and raised :exc:`AttributeError` on an
+:class:`io.BytesIO` before a single frame was read:
+
+* :meth:`Extractor.make_name <pcapkit.foundation.extraction.Extractor.make_name>`
+  read ``fin.name``, which a :class:`io.BytesIO` does not have. The name only
+  labels the input -- the output name comes from ``fout`` alone -- so a stream
+  without one is now labelled after its type, ``<BytesIO>``.
+* The magic number, and the PCAP-NG engine's block type, are read with
+  ``peek``, which :class:`io.BufferedReader` has and :class:`typing.IO` does not.
+
+The engines whose library opens the input by path -- Scapy, PyShark, PyPCAP and
+pcap-ct -- cannot read such a stream at all, so they refuse it with
+:exc:`~pcapkit.utilities.exceptions.UnsupportedCall` instead.
+
+"""
+
+from __future__ import annotations
+
+import importlib.util
+import io
+import os
+import tempfile
+import unittest
+from unittest import mock
+
+from tests._support import sample_path
+
+RUNTIME_DEPS = ('tbtrim', 'aenum', 'chardet', 'dictdumper')
+HAS_RUNTIME = all(importlib.util.find_spec(name) is not None for name in RUNTIME_DEPS)
+
+
+@unittest.skipUnless(HAS_RUNTIME, 'pcapkit runtime dependencies are not installed')
+class TestUnnamedStream(unittest.TestCase):
+    """``fin`` given as a stream that has no :obj:`str` ``name``."""
+
+    def _frames(self, capture: str) -> tuple[int, 'object', io.BytesIO]:
+        """Extract ``capture`` from a :class:`io.BytesIO`, and count it by path too."""
+        import pcapkit  # pylint: disable=import-outside-toplevel
+
+        path = sample_path(capture)
+        expected = len(pcapkit.extract(fin=path, nofile=True).frame)
+        with open(path, 'rb') as file:
+            stream = io.BytesIO(file.read())
+        return expected, pcapkit.extract(fin=stream, nofile=True), stream
+
+    def test_pcap_from_bytesio(self) -> None:
+        expected, extractor, stream = self._frames('in.pcap')
+        self.assertGreater(expected, 0)
+        self.assertEqual(len(extractor.frame), expected)  # type: ignore[attr-defined]
+        self.assertEqual(extractor.input, '<BytesIO>')  # type: ignore[attr-defined]
+        self.assertFalse(stream.closed, 'the caller owns the stream (#610)')
+
+    def test_pcapng_from_bytesio(self) -> None:
+        expected, extractor, stream = self._frames('dhcp.pcapng')
+        self.assertGreater(expected, 0)
+        self.assertEqual(len(extractor.frame), expected)  # type: ignore[attr-defined]
+        self.assertFalse(stream.closed, 'the caller owns the stream (#610)')
+
+    def test_output_name_comes_from_fout(self) -> None:
+        import pcapkit  # pylint: disable=import-outside-toplevel
+
+        with open(sample_path('in.pcap'), 'rb') as file:
+            data = file.read()
+        with tempfile.TemporaryDirectory() as tmp:
+            fout = os.path.join(tmp, 'out')
+            extractor = pcapkit.extract(fin=io.BytesIO(data), fout=fout, format='json')
+            self.assertEqual(extractor.output, f'{fout}.json')
+            self.assertTrue(os.path.isfile(f'{fout}.json'))
+
+    def test_non_str_name_is_a_placeholder(self) -> None:
+        """A stream opened on a descriptor has an :obj:`int` ``name``."""
+        import pcapkit  # pylint: disable=import-outside-toplevel
+
+        path = sample_path('in.pcap')
+        with open(os.open(path, os.O_RDONLY), 'rb') as stream:
+            self.assertIsInstance(stream.name, int)
+            extractor = pcapkit.extract(fin=stream, nofile=True)
+            self.assertEqual(extractor.input, '<BufferedReader>')
+            self.assertEqual(len(extractor.frame), len(pcapkit.extract(fin=path, nofile=True).frame))
+
+    def test_make_name_labels_by_name_or_type(self) -> None:
+        from pcapkit.foundation.extraction import Extractor  # pylint: disable=import-outside-toplevel
+
+        stream = io.BytesIO()
+        self.assertEqual(Extractor.make_name(stream, nofile=True)[0], '<BytesIO>')
+        stream.name = 'capture.pcap'  # type: ignore[attr-defined]
+        self.assertEqual(Extractor.make_name(stream, nofile=True)[0], 'capture.pcap')
+
+
+@unittest.skipUnless(HAS_RUNTIME, 'pcapkit runtime dependencies are not installed')
+class TestPathEngines(unittest.TestCase):
+    """Engines that open the input by path refuse an unnamed stream up front.
+
+    Scapy and PyShark hand ``Extractor._ifnm`` to their library as a file name,
+    so before the refusal the ``<BytesIO>`` label was opened as a path --
+    ``FileNotFoundError`` from inside :mod:`scapy`. The refusal is raised by
+    :class:`~pcapkit.foundation.extraction.Extractor` itself, before the engine
+    is constructed, so it does not depend on the engine being able to run.
+
+    """
+
+    def _data(self) -> bytes:
+        with open(sample_path('in.pcap'), 'rb') as file:
+            return file.read()
+
+    def _assert_refused(self, engine: str) -> None:
+        import pcapkit  # pylint: disable=import-outside-toplevel
+        from pcapkit.utilities.exceptions import UnsupportedCall  # pylint: disable=import-outside-toplevel
+
+        stream = io.BytesIO(self._data())
+        with self.assertRaises(UnsupportedCall) as caught:
+            pcapkit.extract(fin=stream, nofile=True, engine=engine)  # type: ignore[arg-type]
+        self.assertIn(f"'Extractor(engine={engine})' requires a file on disk", str(caught.exception))
+        self.assertIn('BytesIO', str(caught.exception))
+        self.assertFalse(stream.closed, 'the caller owns the stream (#610)')
+
+    @unittest.skipUnless(importlib.util.find_spec('scapy') is not None, 'scapy is not installed')
+    def test_scapy_refuses_unnamed_stream(self) -> None:
+        self._assert_refused('scapy')
+
+    @unittest.skipUnless(importlib.util.find_spec('pyshark') is not None, 'pyshark is not installed')
+    def test_pyshark_refuses_unnamed_stream(self) -> None:
+        # NOTE: PyShark rules itself out on some interpreters (CPython 3.14), and
+        # the extraction would then fall back to the default engine, which reads
+        # the stream perfectly well. Clear that reason so the refusal is what is
+        # under test; it fires before any PyShark code runs.
+        from pcapkit.foundation.engines.pyshark import PyShark  # pylint: disable=import-outside-toplevel
+
+        with mock.patch.object(PyShark, 'unsupported_reason', return_value=None):
+            self._assert_refused('pyshark')
+
+    def test_named_stream_is_not_refused(self) -> None:
+        """A stream with a real ``name`` gives the path engine a path to open."""
+        from pcapkit.foundation.extraction import Extractor  # pylint: disable=import-outside-toplevel
+
+        path = sample_path('in.pcap')
+        with mock.patch.object(Extractor, 'run'), open(path, 'rb') as stream:
+            extractor = Extractor(fin=stream, nofile=True, engine='scapy')
+            self.assertEqual(extractor.input, path)
+
+    @unittest.skipUnless(importlib.util.find_spec('dpkt') is not None, 'dpkt is not installed')
+    def test_dpkt_reads_unnamed_stream(self) -> None:
+        import pcapkit  # pylint: disable=import-outside-toplevel
+
+        expected = len(pcapkit.extract(fin=sample_path('in.pcap'), nofile=True, engine='dpkt').frame)
+        stream = io.BytesIO(self._data())
+        extractor = pcapkit.extract(fin=stream, nofile=True, engine='dpkt')
+        self.assertEqual(extractor._exnam, 'dpkt')  # pylint: disable=protected-access
+        self.assertGreater(expected, 0)
+        self.assertEqual(len(extractor.frame), expected)
+        self.assertFalse(stream.closed, 'the caller owns the stream (#610)')
+
+
+if __name__ == '__main__':
+    unittest.main()