diff --git a/pcapkit/foundation/extraction.py b/pcapkit/foundation/extraction.py index d3ea4c8103..a9d6ab8683 100644 --- a/pcapkit/foundation/extraction.py +++ b/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. ````. + + """ 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 ```` 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 ````-style, after Python's own ````. + 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 ```` 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] diff --git a/tests/foundation/test_extraction_unnamed_stream_unit.py b/tests/foundation/test_extraction_unnamed_stream_unit.py new file mode 100644 index 0000000000..52d38abf7b --- /dev/null +++ b/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 ` + 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, ````. +* 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, '') # 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, '') + 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], '') + 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 ```` 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()