From d6a6f3e523d7d923abe3463118f2f1b0421a8e43 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Fri, 23 Oct 2026 17:54:10 -0400 Subject: [PATCH] fix(corekit): skip a record by reading when the stream only claims to seek (#2108) advance() trusted seekable(), so a Windows pipe (seekable() True, seek a no-op) handed to Frame(file) or PCAPNG(file) directly was 'skipped' without moving. advance() uses it; extraction.py imports it from there. --- pcapkit/corekit/io.py | 27 ++++- pcapkit/foundation/extraction.py | 23 +--- .../test_io_advance_lying_pipe_2108_unit.py | 105 ++++++++++++++++++ 3 files changed, 131 insertions(+), 24 deletions(-) create mode 100644 tests/corekit/test_io_advance_lying_pipe_2108_unit.py diff --git a/pcapkit/corekit/io.py b/pcapkit/corekit/io.py index 54792e08f3..611b7d6d52 100644 --- a/pcapkit/corekit/io.py +++ b/pcapkit/corekit/io.py @@ -15,6 +15,7 @@ """ import io import os +import stat import sys import tempfile import weakref @@ -863,6 +864,26 @@ def peek(self, size: 'int' = 0) -> 'bytes': return buf +def _can_seek(stream: 'IO[bytes]') -> 'bool': + """Whether ``stream`` can really seek, rather than only saying it can. + + On Windows before CPython 3.16, a pipe reports :meth:`~io.IOBase.seekable` + and its ``seek`` silently does nothing (:issue:`2097`), so the answer is + trusted only for a regular file, or for a stream with no descriptor to ask. + + Args: + stream: The input stream. + + """ + if not stream.seekable(): + return False + try: + mode = os.fstat(stream.fileno()).st_mode + except (AttributeError, OSError, ValueError): + return True + return stat.S_ISREG(mode) + + #: Octets :func:`advance` reads and discards at a time when #: :data:`~pcapkit.utilities.config.READ_CHUNK` sets no limit. _DISCARD_CHUNK = 1 << 20 @@ -880,14 +901,16 @@ def advance(file: 'IO[bytes]', count: 'int', /) -> 'None': as far as the limit; the next record still starts where this one declares it ends (:issue:`1975`). A seekable stream gets there by relative seeks, none longer than :data:`sys.maxsize`, and may land past its end, as an absolute seek - would. A :class:`SeekableReader`, or a stream that cannot seek, reads the + would. A :class:`SeekableReader`, or a stream that cannot seek -- including + one that only says it can, as a Windows pipe does (:issue:`2108`) -- reads the octets and discards them, no more than :data:`~pcapkit.utilities.config.READ_CHUNK` at a time, and stops at the end of the stream: a :class:`SeekableReader` seek reads at most a buffer's worth past what it holds, so a longer one would stop short of the target. """ - if isinstance(file, SeekableReader) or not file.seekable(): + # NOTE: a frame reader built on a caller's stream sees it unwrapped (#2108). + if isinstance(file, SeekableReader) or not _can_seek(file): chunk = _DISCARD_CHUNK if config.READ_CHUNK == -1 else config.READ_CHUNK chunk = min(chunk, sys.maxsize) while count > 0: diff --git a/pcapkit/foundation/extraction.py b/pcapkit/foundation/extraction.py index 0705360d88..e4e41da39c 100644 --- a/pcapkit/foundation/extraction.py +++ b/pcapkit/foundation/extraction.py @@ -18,7 +18,6 @@ import importlib import io import os -import stat import sys import types import weakref @@ -27,7 +26,7 @@ from dictdumper.dumper import Dumper from pcapkit.corekit.context import ContextRegistry -from pcapkit.corekit.io import PeekableStream, SeekableReader +from pcapkit.corekit.io import PeekableStream, SeekableReader, _can_seek from pcapkit.corekit.module import ModuleDescriptor from pcapkit.dumpkit.common import make_dumper from pcapkit.foundation.engines.engine import Engine, EngineBase @@ -137,26 +136,6 @@ def _release_input(ifile: 'IO[bytes]') -> 'None': pass -def _can_seek(stream: 'IO[bytes]') -> 'bool': - """Whether ``stream`` can really seek, rather than only saying it can. - - On Windows before CPython 3.16, a pipe reports :meth:`~io.IOBase.seekable` - and its ``seek`` silently does nothing (:issue:`2097`), so the answer is - trusted only for a regular file, or for a stream with no descriptor to ask. - - Args: - stream: The input stream. - - """ - if not stream.seekable(): - return False - try: - mode = os.fstat(stream.fileno()).st_mode - except (AttributeError, OSError, ValueError): - return True - return stat.S_ISREG(mode) - - class Extractor(Generic[_P]): """Extractor for PCAP files. diff --git a/tests/corekit/test_io_advance_lying_pipe_2108_unit.py b/tests/corekit/test_io_advance_lying_pipe_2108_unit.py new file mode 100644 index 0000000000..5766d74c0a --- /dev/null +++ b/tests/corekit/test_io_advance_lying_pipe_2108_unit.py @@ -0,0 +1,105 @@ +# -*- coding: utf-8 -*- +""":func:`~pcapkit.corekit.io.advance` reads past a record on a pipe that only says it can seek (#2108). + +On Windows before CPython 3.16 a pipe's ``seekable()`` is :data:`True` and its +``seek`` silently does nothing (CPython gh-86768). A frame reader built directly +on a caller's stream -- ``Frame(file, ...)`` or ``PCAPNG(file, ...)`` -- hands that +stream to :func:`advance` unwrapped, which then "skipped" a record without moving. +:class:`LyingPipe` reproduces such a pipe on any platform, over a real +:func:`os.pipe`. + +""" + +from __future__ import annotations + +import io +import os +import pathlib +import tempfile +import threading +import unittest + +from tests._support import reimport_once_per_class, scale_timeout + +#: Test tier (#1958): exercises stream semantics, which differ by interpreter. +__tier__ = 'ALL' + + +class LyingPipe(io.RawIOBase): + """The read end of a pipe, claiming to seek as a Windows pipe does.""" + + def __init__(self, fd: 'int') -> None: + super().__init__() + self._file = os.fdopen(fd, 'rb', buffering=0) + + def readable(self) -> bool: + return True + + def seekable(self) -> bool: + return True + + def seek(self, offset: 'int', whence: 'int' = io.SEEK_SET) -> int: + return offset if whence == io.SEEK_SET else 0 + + def tell(self) -> int: + return 0 + + def fileno(self) -> int: + return self._file.fileno() + + def readinto(self, buffer: 'bytearray | memoryview') -> int: # type: ignore[override] + return self._file.readinto(buffer) # type: ignore[return-value] + + def close(self) -> None: + self._file.close() + super().close() + + +class TestAdvanceOnLyingPipe(unittest.TestCase): + """:func:`advance` trusts ``seekable()`` only where a seek really moves.""" + + def setUp(self) -> None: + reimport_once_per_class(self) + + def test_a_pipe_claiming_to_seek_is_read_past(self) -> None: + """Raw, and buffered as ``sys.stdin.buffer`` is: the octets after the skip come next.""" + from pcapkit.corekit.io import advance # pylint: disable=import-outside-toplevel + + data = b'A' * 10_000 + b'next record' + for buffered in (False, True): + with self.subTest(buffered=buffered): + read_fd, write_fd = os.pipe() + + def write() -> None: + with os.fdopen(write_fd, 'wb', buffering=0) as stream: + stream.write(data) + + thread = threading.Thread(target=write, daemon=True) + thread.start() + pipe = LyingPipe(read_fd) + stream = io.BufferedReader(pipe) if buffered else pipe + try: + advance(stream, 10_000) # type: ignore[arg-type] + self.assertEqual(stream.read(), b'next record') + finally: + stream.close() + thread.join(scale_timeout(30)) + + def test_a_regular_file_and_a_stream_without_a_descriptor_still_seek(self) -> None: + """Seeking, unlike reading, may land past the end, as an absolute seek would.""" + from pcapkit.corekit.io import advance # pylint: disable=import-outside-toplevel + + data = b'0123456789' + with tempfile.TemporaryDirectory() as tmp: + path = pathlib.Path(tmp) / 'capture.bin' + path.write_bytes(data) + with open(path, 'rb') as file: + advance(file, 25) + self.assertEqual(file.tell(), 25) + stream = io.BytesIO(data) + advance(stream, 25) + self.assertEqual(stream.tell(), 25) + + +if __name__ == '__main__': + unittest.main()