From 0f6dce8aae8fd4e1ab4931b35224deec1cc7da71 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Fri, 9 Oct 2026 17:45:50 -0400 Subject: [PATCH] refactor(corekit): move the stream proxy classes into corekit.io (#1516) - `_PeekableStream` (foundation/extraction.py) and `_NamedStream` (foundation/engines/pypcapfile.py) move to `pcapkit.corekit.io` as the public `PeekableStream` and `NamedStream`, beside `SeekableReader`, and join its `__all__`. Bodies are unchanged; both importers now take them from there. - Document both on the corekit io page. - tests/corekit/test_io_stream_proxies_1516_unit.py pins peek semantics, attribute forwarding, `.name` and read forwarding from the new module; the `_NamedStream` test in test_pypcapfile_engine.py moves there. --- docs/source/pcapkit/corekit/io.rst | 20 +- pcapkit/corekit/io.py | 83 +++++++- pcapkit/foundation/engines/pypcapfile.py | 37 +--- pcapkit/foundation/extraction.py | 44 +---- .../test_io_stream_proxies_1516_unit.py | 181 ++++++++++++++++++ .../engines/test_pypcapfile_engine.py | 13 -- 6 files changed, 282 insertions(+), 96 deletions(-) create mode 100644 tests/corekit/test_io_stream_proxies_1516_unit.py diff --git a/docs/source/pcapkit/corekit/io.rst b/docs/source/pcapkit/corekit/io.rst index 4e350382a5..7f8c0e26e7 100644 --- a/docs/source/pcapkit/corekit/io.rst +++ b/docs/source/pcapkit/corekit/io.rst @@ -1,11 +1,14 @@ -Seekable I/O Object -=================== +I/O Objects +=========== .. module:: pcapkit.corekit.io :mod:`pcapkit.corekit.io` contains :class:`~pcapkit.corekit.io.SeekableReader`, a seekable customisation of -:class:`io.BufferedReader`. +:class:`io.BufferedReader`, and two stream proxies: +:class:`~pcapkit.corekit.io.PeekableStream`, which adds ``peek`` to a seekable +stream, and :class:`~pcapkit.corekit.io.NamedStream`, which gives a stream a +``name``. .. autoclass:: pcapkit.corekit.io.SeekableReader :no-members: @@ -41,3 +44,14 @@ Seekable I/O Object .. automethod:: fileno .. automethod:: isatty + +.. autoclass:: pcapkit.corekit.io.PeekableStream + :no-members: + + .. automethod:: peek + +.. autoclass:: pcapkit.corekit.io.NamedStream + :no-members: + + .. autoattribute:: name + .. automethod:: read diff --git a/pcapkit/corekit/io.py b/pcapkit/corekit/io.py index 2812b2e4dd..14656e4a01 100644 --- a/pcapkit/corekit/io.py +++ b/pcapkit/corekit/io.py @@ -1,12 +1,15 @@ # -*- coding: utf-8 -*- -"""Seekable I/O Object -========================= +"""I/O Objects +=============== .. module:: pcapkit.corekit.io :mod:`pcapkit.corekit.io` contains seekable I/O object :class:`~pcapkit.corekit.io.SeekableReader`, which is a customised -implementation to :class:`io.BufferedReader`. +implementation to :class:`io.BufferedReader`, and the stream proxies +:class:`~pcapkit.corekit.io.PeekableStream`, which adds ``peek`` to a +seekable stream, and :class:`~pcapkit.corekit.io.NamedStream`, which gives +a stream a ``name``. """ import io @@ -19,11 +22,11 @@ if TYPE_CHECKING: from io import BytesIO, RawIOBase - from typing import IO, Iterable, Optional + from typing import IO, Any, BinaryIO, Iterable, Optional from typing_extensions import Buffer -__all__ = ['SeekableReader'] +__all__ = ['SeekableReader', 'PeekableStream', 'NamedStream'] class SeekableReader(io.BufferedReader): @@ -677,3 +680,73 @@ def peek(self, size: 'int' = 0) -> 'bytes': self._write_buffer(buf_tmp) buf += buf_tmp return buf + + +class PeekableStream: + """Give a seekable binary stream without ``peek`` one. + + :class:`~pcapkit.foundation.extraction.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 NamedStream: + """Read-only proxy that gives a stream the ``name`` attribute. + + :func:`pcapfile.savefile.load_savefile` dereferences ``input_file.name`` + unconditionally, on the way into its trace helper. That is fine for the + :class:`~io.BufferedReader` :class:`~pcapkit.foundation.extraction.Extractor` + normally holds, but not for the :class:`~pcapkit.corekit.io.SeekableReader` + it substitutes when the caller supplied a non-seekable stream -- that class + exposes no ``name``, so the load would fail with :exc:`AttributeError` before + a single byte was read. + + Args: + stream: Underlying binary stream. + name: Name to report as :attr:`name`. + + """ + + def __init__(self, stream: 'BinaryIO', name: 'str') -> 'None': + self._stream = stream + #: Name of the underlying stream. + self.name = name + + def read(self, size: 'int' = -1) -> 'bytes': + """Read from the underlying stream. + + Args: + size: Number of bytes to read; all remaining bytes if negative. + + """ + return self._stream.read(size) diff --git a/pcapkit/foundation/engines/pypcapfile.py b/pcapkit/foundation/engines/pypcapfile.py index f13c6cb704..0826ad98a0 100644 --- a/pcapkit/foundation/engines/pypcapfile.py +++ b/pcapkit/foundation/engines/pypcapfile.py @@ -16,6 +16,7 @@ from typing import TYPE_CHECKING, cast from pcapkit.const.reg.linktype import LinkType as Enum_LinkType +from pcapkit.corekit.io import NamedStream from pcapkit.foundation.engines.engine import EngineBase from pcapkit.foundation.reassembly import ReassemblyManager from pcapkit.utilities.exceptions import FormatError, stacklevel @@ -24,45 +25,13 @@ __all__ = ['PyPCAPFile'] if TYPE_CHECKING: - from typing import Any, BinaryIO, Callable, Iterator, Optional + from typing import Any, Callable, Iterator, Optional from pcapfile.structs import pcap_packet as PCAPFilePacket from pcapkit.foundation.extraction import Extractor -class _NamedStream: - """Read-only proxy that gives a stream the ``name`` attribute. - - :func:`pcapfile.savefile.load_savefile` dereferences ``input_file.name`` - unconditionally, on the way into its trace helper. That is fine for the - :class:`~io.BufferedReader` :class:`~pcapkit.foundation.extraction.Extractor` - normally holds, but not for the :class:`~pcapkit.corekit.io.SeekableReader` - it substitutes when the caller supplied a non-seekable stream -- that class - exposes no ``name``, so the load would fail with :exc:`AttributeError` before - a single byte was read. - - Args: - stream: Underlying binary stream. - name: Name to report as :attr:`name`. - - """ - - def __init__(self, stream: 'BinaryIO', name: 'str') -> 'None': - self._stream = stream - #: Name of the underlying stream. - self.name = name - - def read(self, size: 'int' = -1) -> 'bytes': - """Read from the underlying stream. - - Args: - size: Number of bytes to read; all remaining bytes if negative. - - """ - return self._stream.read(size) - - class PyPCAPFile(EngineBase['PCAPFilePacket']): """PyPCAPFile engine support. @@ -233,7 +202,7 @@ def run(self) -> 'None': "so 'ipv6=True' will be ignored", AttributeWarning, stacklevel=stacklevel()) sfile = pcapfile.savefile.load_savefile( - _NamedStream(ext._ifile, ext._ifnm), layers=0, lazy=True, + NamedStream(ext._ifile, ext._ifnm), layers=0, lazy=True, ) self._dlink = Enum_LinkType.get(sfile.header.ll_type) self._declf = self._get_decoder(sfile.header.ll_type) diff --git a/pcapkit/foundation/extraction.py b/pcapkit/foundation/extraction.py index a9d6ab8683..f7b4e44d08 100644 --- a/pcapkit/foundation/extraction.py +++ b/pcapkit/foundation/extraction.py @@ -25,7 +25,7 @@ from dictdumper.dumper import Dumper from pcapkit.corekit.context import ContextRegistry -from pcapkit.corekit.io import SeekableReader +from pcapkit.corekit.io import PeekableStream, SeekableReader from pcapkit.corekit.module import ModuleDescriptor from pcapkit.dumpkit.common import make_dumper from pcapkit.foundation.engines.engine import Engine, EngineBase @@ -187,44 +187,6 @@ def _restore_shipped_dumper(registry: 'dict[str, Any]', format: 'str', # pylint _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. @@ -1487,8 +1449,8 @@ def __init__(self, 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)) + 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/corekit/test_io_stream_proxies_1516_unit.py b/tests/corekit/test_io_stream_proxies_1516_unit.py new file mode 100644 index 0000000000..73e622d941 --- /dev/null +++ b/tests/corekit/test_io_stream_proxies_1516_unit.py @@ -0,0 +1,181 @@ +# -*- coding: utf-8 -*- +"""The stream proxies live in :mod:`pcapkit.corekit.io` (#1516). + +:class:`~pcapkit.corekit.io.PeekableStream` gives a seekable stream without +``peek`` one, for :class:`~pcapkit.foundation.extraction.Extractor` (#1506, +#1509). :class:`~pcapkit.corekit.io.NamedStream` gives a stream the ``name`` +:func:`pcapfile.savefile.load_savefile` reads, for the PyPCAPFile engine. Both +were file-local, as ``_PeekableStream`` in :mod:`pcapkit.foundation.extraction` +and ``_NamedStream`` in :mod:`pcapkit.foundation.engines.pypcapfile`, and moved +beside :class:`~pcapkit.corekit.io.SeekableReader` unchanged. + +Every class is imported from its new module inside each test, after +:func:`~tests._support.reimport_once_per_class`. + +""" + +import gc +import importlib.util +import io +import unittest +from unittest import mock + +from tests._support import reimport_once_per_class, sample_path + +RUNTIME_DEPS = ('tbtrim', 'aenum', 'chardet', 'dictdumper') +HAS_RUNTIME = all(importlib.util.find_spec(name) is not None for name in RUNTIME_DEPS) + + +class TestPeekableStream(unittest.TestCase): + """:class:`~pcapkit.corekit.io.PeekableStream` peeks and forwards the rest.""" + + def setUp(self) -> None: + reimport_once_per_class(self) + + def test_exported(self) -> None: + import pcapkit.corekit.io as corekit_io # pylint: disable=import-outside-toplevel + + self.assertIn('PeekableStream', corekit_io.__all__) + + def test_peek_does_not_consume(self) -> None: + from pcapkit.corekit.io import PeekableStream # pylint: disable=import-outside-toplevel + + stream = PeekableStream(io.BytesIO(b'abcdef')) + self.assertEqual(stream.peek(3), b'abc') + self.assertEqual(stream.tell(), 0) + self.assertEqual(stream.read(2), b'ab') + self.assertEqual(stream.peek(10), b'cdef') + self.assertEqual(stream.tell(), 2) + + def test_peek_of_nothing_returns_one_octet(self) -> None: + """``peek(0)``, the default, still returns an octet, as :meth:`io.BufferedReader.peek` does.""" + from pcapkit.corekit.io import PeekableStream # pylint: disable=import-outside-toplevel + + stream = PeekableStream(io.BytesIO(b'abc')) + self.assertEqual(stream.peek(), b'a') + self.assertEqual(stream.peek(0), b'a') + self.assertEqual(stream.tell(), 0) + + def test_peek_at_eof_is_empty(self) -> None: + from pcapkit.corekit.io import PeekableStream # pylint: disable=import-outside-toplevel + + stream = PeekableStream(io.BytesIO(b'ab')) + stream.read() + self.assertEqual(stream.peek(4), b'') + self.assertEqual(stream.tell(), 2) + + def test_peek_restores_the_position_when_the_read_fails(self) -> None: + from pcapkit.corekit.io import PeekableStream # pylint: disable=import-outside-toplevel + + raw = mock.Mock(spec=['tell', 'read', 'seek']) + raw.tell.return_value = 3 + raw.read.side_effect = OSError('boom') + with self.assertRaises(OSError): + PeekableStream(raw).peek(2) + raw.read.assert_called_once_with(2) + raw.seek.assert_called_once_with(3, io.SEEK_SET) + + def test_other_attributes_are_the_streams_own(self) -> None: + from pcapkit.corekit.io import PeekableStream # pylint: disable=import-outside-toplevel + + raw = io.BytesIO(b'abcdef') + raw.name = 'given.pcap' # type: ignore[attr-defined] + stream = PeekableStream(raw) + self.assertEqual(stream.name, 'given.pcap') + self.assertTrue(stream.seekable()) + self.assertEqual(stream.seek(4), 4) + self.assertEqual(raw.tell(), 4) + self.assertEqual(stream.getvalue(), b'abcdef') + self.assertEqual(stream.read1(), b'ef') + with self.assertRaises(AttributeError): + stream.no_such_attribute # pylint: disable=pointless-statement + + def test_dropping_the_proxy_leaves_the_stream_open(self) -> None: + """Not an :class:`io.IOBase`, so no finaliser closes the caller's stream (#610).""" + from pcapkit.corekit.io import PeekableStream # pylint: disable=import-outside-toplevel + + raw = io.BytesIO(b'abc') + stream = PeekableStream(raw) + self.assertNotIsInstance(stream, io.IOBase) + del stream + gc.collect() + self.assertFalse(raw.closed) + + @unittest.skipUnless(HAS_RUNTIME, 'pcapkit runtime dependencies are not installed') + def test_extractor_wraps_a_peekless_stream_in_it(self) -> None: + from pcapkit.corekit.io import PeekableStream # pylint: disable=import-outside-toplevel + from pcapkit.foundation.extraction import Extractor # pylint: disable=import-outside-toplevel + + with open(sample_path('in.pcap'), 'rb') as file: + raw = io.BytesIO(file.read()) + extractor = Extractor(fin=raw, nofile=True, auto=False) + self.assertIs(type(extractor._ifile), PeekableStream) # pylint: disable=protected-access + self.assertIs(extractor._ifile._stream, raw) # type: ignore[attr-defined] # pylint: disable=protected-access + + +class TestNamedStream(unittest.TestCase): + """:class:`~pcapkit.corekit.io.NamedStream` carries a name and forwards reads.""" + + def setUp(self) -> None: + reimport_once_per_class(self) + + def test_exported(self) -> None: + import pcapkit.corekit.io as corekit_io # pylint: disable=import-outside-toplevel + + self.assertIn('NamedStream', corekit_io.__all__) + + def test_name_is_the_given_one(self) -> None: + from pcapkit.corekit.io import NamedStream # pylint: disable=import-outside-toplevel + + raw = io.BytesIO(b'') + raw.name = 'own.pcap' # type: ignore[attr-defined] + self.assertEqual(NamedStream(raw, 'given.pcap').name, 'given.pcap') + + def test_reads_forward_to_the_stream(self) -> None: + from pcapkit.corekit.io import NamedStream # pylint: disable=import-outside-toplevel + + stream = NamedStream(io.BytesIO(b'abcdef'), 'given.pcap') + self.assertEqual(stream.read(2), b'ab') + self.assertEqual(stream.read(), b'cdef') + self.assertEqual(stream.read(), b'') + + def test_read_passes_the_size_through(self) -> None: + from pcapkit.corekit.io import NamedStream # pylint: disable=import-outside-toplevel + + raw = mock.Mock(spec=['read']) + raw.read.return_value = b'x' + stream = NamedStream(raw, 'given.pcap') + self.assertEqual(stream.read(), b'x') + self.assertEqual(stream.read(5), b'x') + self.assertEqual(raw.read.call_args_list, [mock.call(-1), mock.call(5)]) + + def test_nothing_but_read_is_forwarded(self) -> None: + """A read-only proxy: no ``__getattr__``, so the stream's other methods are not reachable.""" + from pcapkit.corekit.io import NamedStream # pylint: disable=import-outside-toplevel + + stream = NamedStream(io.BytesIO(b'abc'), 'given.pcap') + for name in ('seek', 'tell', 'peek', 'close', 'closed'): + with self.subTest(name=name): + self.assertFalse(hasattr(stream, name)) + + def test_names_a_seekable_reader(self) -> None: + """The case it exists for: :class:`~pcapkit.corekit.io.SeekableReader` has no ``name``.""" + from pcapkit.corekit.io import NamedStream, SeekableReader # pylint: disable=import-outside-toplevel + + reader = SeekableReader(io.BytesIO(b'abcdef'), buffer_size=4) + self.assertFalse(hasattr(reader, 'name')) + stream = NamedStream(reader, 'given.pcap') + self.assertEqual(stream.name, 'given.pcap') + self.assertEqual(stream.read(4), b'abcd') + self.assertEqual(reader.tell(), 4) + reader.close() + + def test_pypcapfile_engine_uses_it(self) -> None: + from pcapkit.corekit.io import NamedStream # pylint: disable=import-outside-toplevel + from pcapkit.foundation.engines import pypcapfile # pylint: disable=import-outside-toplevel + + self.assertIs(pypcapfile.NamedStream, NamedStream) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/foundation/engines/test_pypcapfile_engine.py b/tests/foundation/engines/test_pypcapfile_engine.py index 2713eaa2b5..82d998fd28 100644 --- a/tests/foundation/engines/test_pypcapfile_engine.py +++ b/tests/foundation/engines/test_pypcapfile_engine.py @@ -426,19 +426,6 @@ def test_read_frame_ignores_protocols_that_were_not_requested(self) -> None: extractor._reasm.tcp.assert_not_called() extractor._trace.tcp.assert_not_called() - ########################################################################## - # _NamedStream - ########################################################################## - - def test_named_stream_proxies_reads_and_carries_a_name(self) -> None: - from pcapkit.foundation.engines.pypcapfile import _NamedStream - - stream = _NamedStream(io.BytesIO(b'abcdef'), 'given.pcap') - self.assertEqual(stream.name, 'given.pcap') - self.assertEqual(stream.read(2), b'ab') - self.assertEqual(stream.read(), b'cdef') - self.assertEqual(stream.read(), b'') - class PyPCAPFilePythonCeilingTests(unittest.TestCase): """The engine rules itself out above its dependency's Python ceiling.