From 65f3ce3e546008aa23a863e3bb227141f22afc2e Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Tue, 20 Oct 2026 11:32:27 -0400 Subject: [PATCH 1/3] fix(protocols): handle declared lengths above sys.maxsize as overruns (#1975) Where Py_ssize_t is 32 bits wide (GraalPy, 32-bit CPython) a read or seek past sys.maxsize raises OverflowError instead of coming back short, so a PCAP incl_len or PCAP-NG length reaching that far crashed the reader before the overrun handling could keep the record as captured. - Schema.unpack bounds its payload and field reads at sys.maxsize. - Frame.read bounds the record read, and a next-record offset past sys.maxsize becomes the end of the octets read. - PCAPNG.read and _read_block_cut bound the block read. - SeekableReader._fill bounds a READ_CHUNK of -1 at sys.maxsize. - Lift #1974's sys.maxsize skips from the #1541 tests, and add a test that runs the narrow build's limit on any interpreter. Claims that fit read exactly as before. Closes #1975 --- pcapkit/corekit/io.py | 8 +- pcapkit/protocols/misc/pcap/frame.py | 13 +- pcapkit/protocols/misc/pcapng.py | 9 +- pcapkit/protocols/schema/schema.py | 11 +- ...t_pcap_trace_truncated_record_1541_unit.py | 4 - .../test_frame_incl_len_overrun_1541_unit.py | 4 - ..._declared_length_past_maxsize_1975_unit.py | 199 ++++++++++++++++++ 7 files changed, 233 insertions(+), 15 deletions(-) create mode 100644 tests/protocols/misc/test_declared_length_past_maxsize_1975_unit.py diff --git a/pcapkit/corekit/io.py b/pcapkit/corekit/io.py index 05df937ff8..00d274befe 100644 --- a/pcapkit/corekit/io.py +++ b/pcapkit/corekit/io.py @@ -13,6 +13,7 @@ """ import io +import sys import tempfile from typing import TYPE_CHECKING, cast @@ -202,11 +203,14 @@ def _fill(self, size: 'int', minimum: 'Optional[int]' = None, /, *, minimum = size if minimum is None else minimum read = getattr(self._stream, 'read1', self._stream.read) - limit = config.READ_CHUNK + # NOTE: ``-1`` sets no limit of its own, but one read still goes no further + # than :data:`sys.maxsize`, past which it raises :exc:`OverflowError` + # rather than coming back short (:issue:`1975`). + limit = sys.maxsize if config.READ_CHUNK == -1 else config.READ_CHUNK chunks = [] # type: list[bytes] count = 0 while True: - chunk = read(size - count if limit == -1 else min(size - count, limit)) + chunk = read(min(size - count, limit)) if not chunk: break chunks.append(chunk) diff --git a/pcapkit/protocols/misc/pcap/frame.py b/pcapkit/protocols/misc/pcap/frame.py index 472f4f9980..35d473e6bf 100644 --- a/pcapkit/protocols/misc/pcap/frame.py +++ b/pcapkit/protocols/misc/pcap/frame.py @@ -342,7 +342,12 @@ def read(self, length: 'Optional[int]' = None, *, _read: 'bool' = True, self._file.seek(_seek_set, io.SEEK_SET) #: bytes: Raw frame data. - self._data = self._read_fileng(self.length + _ilen) + # + # NOTE: Asked for no further than one read can go, :data:`sys.maxsize`. + # Past it the read raises :exc:`OverflowError` instead of coming back + # short, so a record claiming more than that would never reach the + # overrun handling below (:issue:`1975`). + self._data = self._read_fileng(min(self.length + _ilen, sys.maxsize)) # NOTE: A record whose ``incl_len`` runs past the end of the data is # kept as captured, as the PCAP-NG reader keeps a captured length @@ -357,6 +362,12 @@ def read(self, length: 'Optional[int]' = None, *, _read: 'bool' = True, f'which holds {held} octet(s); kept as captured', ProtocolWarning, stacklevel=stacklevel()) + # NOTE: Nor can a seek go past :data:`sys.maxsize`. A record ending + # beyond it was cut short by the end of the data, so the next frame + # starts where the octets read end (:issue:`1975`). + if seek_cur > sys.maxsize: + seek_cur = _seek_set + len(self._data) + # move forward to the beginning of the next frame self._file.seek(seek_cur, io.SEEK_SET) diff --git a/pcapkit/protocols/misc/pcapng.py b/pcapkit/protocols/misc/pcapng.py index 3cd4e5144f..44a178ff03 100644 --- a/pcapkit/protocols/misc/pcapng.py +++ b/pcapkit/protocols/misc/pcapng.py @@ -1350,7 +1350,11 @@ def read(self, length: 'Optional[int]' = None, *, _read: 'bool' = True, self._file.seek(_seek_set, io.SEEK_SET) #: bytes: Raw block data. - self._data = self._read_fileng(schema.block.length) + # + # NOTE: Asked for no further than one read can go, :data:`sys.maxsize`, + # past which the read raises :exc:`OverflowError` instead of coming + # back short as the truncation handling below expects (:issue:`1975`). + self._data = self._read_fileng(min(schema.block.length, sys.maxsize)) # NOTE: Block Total Length is cross-checked against its own trailing # copy and never against the file, so on a capture cut short it runs @@ -1636,7 +1640,8 @@ def _read_block_cut(self, header: 'Schema_PCAPNG', offset: 'int', current = self._file.tell() self._file.seek(offset + 8, io.SEEK_SET) - body = self._read_fileng(header.block.length - GENERIC_BLOCK_MINIMUM) + # NOTE: bounded as :meth:`read` bounds the block's read (:issue:`1975`). + body = self._read_fileng(min(header.block.length - GENERIC_BLOCK_MINIMUM, sys.maxsize)) self._file.seek(current, io.SEEK_SET) return Data_UnknownBlock( diff --git a/pcapkit/protocols/schema/schema.py b/pcapkit/protocols/schema/schema.py index cd275271f0..657c7451c2 100644 --- a/pcapkit/protocols/schema/schema.py +++ b/pcapkit/protocols/schema/schema.py @@ -6,6 +6,7 @@ import collections.abc import io import itertools +import sys from typing import TYPE_CHECKING, Any, Generic, TypeVar, cast from pcapkit.corekit.decorators import prepare @@ -1070,7 +1071,12 @@ def unpack(cls, data: 'bytes | IO[bytes]', if length < 0: length = max(cast('int', packet['__length__']), 0) - payload = data.read(length) + # NOTE: A declared length is asked for no further than one read + # can go, :data:`sys.maxsize`; past it the read raises + # :exc:`OverflowError` rather than coming back short, which is + # how every other overrun is reported. No stream holds more than + # that, so the bounded read is the same short read (:issue:`1975`). + payload = data.read(min(length, sys.maxsize)) self.__buffer__[field.name] = payload packet['__length__'] -= length @@ -1102,7 +1108,8 @@ def unpack(cls, data: 'bytes | IO[bytes]', length = field.length - byte = data.read(length) + # NOTE: bounded as the payload read above is (:issue:`1975`). + byte = data.read(min(length, sys.maxsize)) self.__buffer__[field.name] = byte if not isinstance(field, ForwardMatchField): self._note_short_read(field.name, length, byte) diff --git a/tests/dumpkit/test_pcap_trace_truncated_record_1541_unit.py b/tests/dumpkit/test_pcap_trace_truncated_record_1541_unit.py index 8caf566405..7ee7930e38 100644 --- a/tests/dumpkit/test_pcap_trace_truncated_record_1541_unit.py +++ b/tests/dumpkit/test_pcap_trace_truncated_record_1541_unit.py @@ -21,7 +21,6 @@ import io import os import struct -import sys import tempfile import unittest import warnings @@ -111,9 +110,6 @@ def test_a_pcap_record_cut_at_eof_is_traced_as_the_octets_it_has(self) -> None: """#1541: a record claiming more than the file holds writes what it holds.""" for claim in CLAIMS: with self.subTest(claim=claim): - if 16 + claim > sys.maxsize: - self.skipTest('the frame reader asks the file for the 16-octet header and the claim at once, ' - 'past sys.maxsize on GraalPy (#1963)') self.check(self.trace(_cut_pcap(claim)), _frames()[-1]) def test_a_pcapng_captured_length_past_its_block_is_traced_as_the_octets_it_has(self) -> None: diff --git a/tests/protocols/misc/pcap/test_frame_incl_len_overrun_1541_unit.py b/tests/protocols/misc/pcap/test_frame_incl_len_overrun_1541_unit.py index 5e575a6a80..4248096bf0 100644 --- a/tests/protocols/misc/pcap/test_frame_incl_len_overrun_1541_unit.py +++ b/tests/protocols/misc/pcap/test_frame_incl_len_overrun_1541_unit.py @@ -15,7 +15,6 @@ import io import struct -import sys import unittest import warnings @@ -55,9 +54,6 @@ def test_overrun_warns_and_is_kept_as_captured(self) -> None: header = Header(GLOBAL).info for claim in (61, 1000, 0x7FFF_FFFF): with self.subTest(claim=claim): - if 16 + claim > sys.maxsize: - self.skipTest('the frame reader asks the file for the 16-octet header and the claim at once, ' - 'past sys.maxsize on GraalPy (#1963)') octets = _record(claim) frame, messages = self.parse(octets) self.assertIn(f'PCAP: [Frame 4] captured length {claim} runs past the data, ' diff --git a/tests/protocols/misc/test_declared_length_past_maxsize_1975_unit.py b/tests/protocols/misc/test_declared_length_past_maxsize_1975_unit.py new file mode 100644 index 0000000000..887d5e785d --- /dev/null +++ b/tests/protocols/misc/test_declared_length_past_maxsize_1975_unit.py @@ -0,0 +1,199 @@ +# -*- coding: utf-8 -*- +"""A declared length past :data:`sys.maxsize` is an overrun like any other. C.f. #1975. + +On a build whose ``Py_ssize_t`` is 32 bits wide -- GraalPy, 32-bit CPython -- +:data:`sys.maxsize` is ``2 ** 31 - 1``, and a read or seek asked to go past it +raises :exc:`OverflowError` instead of coming back short. A PCAP record whose +``incl_len`` reaches that far, or a PCAP-NG block whose lengths do, then crashed +the reader before the short-read handling could keep it as captured. + +Each case runs such a build's limit on whatever build the suite runs on: the +in-memory streams raise :exc:`OverflowError` past ``2 ** 31 - 1``, and +:data:`sys.maxsize` reads as that value, so the case parses as it would there and +must match the result of the same octets parsed without the limit. + +Every case builds its own octets and reads no capture. Classes are imported +inside each test, after :func:`~tests._support.reimport_once_per_class`. + +""" + +import contextlib +import io +import struct +import sys +import unittest +import unittest.mock +import warnings + +from tests._support import close_extractor, reimport_once_per_class + +#: Test tier (#1958): pins how the readers meet an interpreter's read and seek +#: limit, which differs between interpreters and builds. +__tier__ = 'ALL' + +#: ``sys.maxsize`` where ``Py_ssize_t`` is 32 bits wide. +NARROW_MAXSIZE = 2 ** 31 - 1 + +#: Little-endian global header: v2.4, Ethernet. +GLOBAL = bytes.fromhex('d4c3b2a1' '0200' '0400' '00000000' '00000000' 'ffff0000' '01000000') +#: Sixty octets of packet data, none of them zero. +PACKET = bytes(range(1, 61)) +#: Captured lengths at and past the narrow limit: the first one the record header +#: and the claim together overrun, the limit itself, one past it, and the largest. +CLAIMS = (NARROW_MAXSIZE - 15, NARROW_MAXSIZE, NARROW_MAXSIZE + 1, 0xFFFF_FFFF) + + +class _NarrowBytesIO(io.BytesIO): + """:class:`io.BytesIO` whose reads and seeks refuse what a 32-bit ``Py_ssize_t`` cannot hold.""" + + @staticmethod + def _check(value: 'int | None') -> 'None': + if value is not None and value > NARROW_MAXSIZE: + raise OverflowError('Python int too large to convert to C ssize_t') + + def read(self, size: 'int | None' = -1, /) -> 'bytes': + self._check(size) + return super().read(size) + + def read1(self, size: 'int | None' = -1, /) -> 'bytes': + self._check(size) + return super().read1(size) + + def seek(self, pos: 'int', whence: 'int' = io.SEEK_SET, /) -> 'int': + self._check(pos) + return super().seek(pos, whence) + + +class _NarrowPipe(io.RawIOBase): + """A non-seekable stream with the same limit, read through a ``SeekableReader``.""" + + def __init__(self, data: 'bytes') -> 'None': + super().__init__() + self._data = data + self._pos = 0 + + def readable(self) -> 'bool': + return True + + def seekable(self) -> 'bool': + return False + + def read(self, size: 'int | None' = -1, /) -> 'bytes': + _NarrowBytesIO._check(size) # pylint: disable=protected-access + if size is None or size < 0: + size = len(self._data) - self._pos + chunk = self._data[self._pos:self._pos + size] + self._pos += len(chunk) + return chunk + + read1 = read + + +@contextlib.contextmanager +def narrow_build(): + """Run the block as a build whose ``Py_ssize_t`` is 32 bits wide would.""" + with unittest.mock.patch.object(sys, 'maxsize', NARROW_MAXSIZE), \ + unittest.mock.patch.object(io, 'BytesIO', _NarrowBytesIO): + yield + + +def _record(claim: 'int', packet: 'bytes' = PACKET) -> 'bytes': + """A record whose header claims ``claim`` octets, followed by ``packet``.""" + return struct.pack(' 'bytes': + """A little-endian PCAP-NG block holding ``body``, padded to 32 bits.""" + body += bytes(-len(body) % 4) + total = 12 + len(body) + return struct.pack(' 'bytes': + """An EPB over :data:`PACKET` whose captured length reads ``captured``.""" + return _block(6, struct.pack(' 'bytes': + """A whole EPB over :data:`PACKET` whose leading Block Total Length reads ``total``.""" + block = _epb(len(PACKET)) + return block[:4] + struct.pack(' None: + reimport_once_per_class(self) + + def test_the_simulated_build_refuses_what_a_narrow_build_does(self) -> None: + """The streams this module reads overflow where a 32-bit build's do.""" + for stream in (_NarrowBytesIO(PACKET), _NarrowPipe(PACKET)): + with self.subTest(stream=type(stream).__name__): + self.assertEqual(stream.read(NARROW_MAXSIZE), PACKET) + with self.assertRaises(OverflowError): + stream.read(NARROW_MAXSIZE + 1) + + def test_a_frame_claiming_past_maxsize_is_kept_as_captured(self) -> None: + from pcapkit.protocols.misc.pcap.frame import Frame + from pcapkit.protocols.misc.pcap.header import Header + + header = Header(GLOBAL).info + for claim in CLAIMS: + with self.subTest(claim=claim): + octets = _record(claim) + with narrow_build(), warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + frame = Frame(io.BytesIO(octets), num=4, header=header) + self.assertIn(f'PCAP: [Frame 4] captured length {claim} runs past the data, ' + f'which holds {len(PACKET)} octet(s); kept as captured', + [str(item.message) for item in caught]) + + info = frame.info # type: ignore[attr-defined] + self.assertEqual((info.frame_info.incl_len, info.cap_len), (claim, claim)) + self.assertEqual(bytes(info.packet), PACKET) + self.assertEqual(frame.data, octets) # type: ignore[attr-defined] + + def extract(self, fin: 'object', **kwargs: 'object') -> 'list[bytes]': + """The packets the default engine extracts from ``fin``, on the narrow build.""" + import pcapkit + + with narrow_build(), warnings.catch_warnings(): + warnings.simplefilter('ignore') + extractor = pcapkit.extract(fin=fin, nofile=True, store=True, engine='default', **kwargs) + close_extractor(extractor) + return [bytes(frame.info.packet) for frame in extractor.frame] + + def test_extraction_keeps_a_pcap_record_claiming_past_maxsize(self) -> None: + for claim in CLAIMS: + with self.subTest(claim=claim): + octets = GLOBAL + _record(len(PACKET)) + _record(claim) + self.assertEqual(self.extract(_NarrowBytesIO(octets)), [PACKET, PACKET]) + + def test_extraction_keeps_a_pcapng_block_declaring_past_maxsize(self) -> None: + cases = { + 'captured length': _epb(0xFFFF_FFFF), + 'block total length': _epb_total(0xFFFF_FFFC), + } + for name, block in cases.items(): + with self.subTest(length=name): + octets = SECTION + _epb(len(PACKET)) + block + self.assertEqual(self.extract(_NarrowBytesIO(octets)), [PACKET, PACKET]) + + def test_a_non_seekable_input_with_no_read_chunk_limit(self) -> None: + """The ``SeekableReader`` a pipe is wrapped in bounds its reads as well.""" + from pcapkit.utilities import config + + octets = GLOBAL + _record(len(PACKET)) + _record(0xFFFF_FFFF) + for chunk in (config.READ_CHUNK, -1): + with self.subTest(read_chunk=chunk), unittest.mock.patch.object(config, 'READ_CHUNK', chunk): + self.assertEqual(self.extract(_NarrowPipe(octets)), [PACKET, PACKET]) + + +if __name__ == '__main__': + unittest.main() From 0fd43fb67de5c6d9764532db2a7ed145a78f30b9 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Tue, 20 Oct 2026 11:41:01 -0400 Subject: [PATCH 2/3] fix(protocols): cite #1975 as #NNN in plain comments (#1975) --- pcapkit/corekit/io.py | 2 +- pcapkit/protocols/misc/pcap/frame.py | 4 ++-- pcapkit/protocols/misc/pcapng.py | 4 ++-- pcapkit/protocols/schema/schema.py | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pcapkit/corekit/io.py b/pcapkit/corekit/io.py index 00d274befe..e45a9adb15 100644 --- a/pcapkit/corekit/io.py +++ b/pcapkit/corekit/io.py @@ -205,7 +205,7 @@ def _fill(self, size: 'int', minimum: 'Optional[int]' = None, /, *, # NOTE: ``-1`` sets no limit of its own, but one read still goes no further # than :data:`sys.maxsize`, past which it raises :exc:`OverflowError` - # rather than coming back short (:issue:`1975`). + # rather than coming back short (#1975). limit = sys.maxsize if config.READ_CHUNK == -1 else config.READ_CHUNK chunks = [] # type: list[bytes] count = 0 diff --git a/pcapkit/protocols/misc/pcap/frame.py b/pcapkit/protocols/misc/pcap/frame.py index 35d473e6bf..9d8ce0468d 100644 --- a/pcapkit/protocols/misc/pcap/frame.py +++ b/pcapkit/protocols/misc/pcap/frame.py @@ -346,7 +346,7 @@ def read(self, length: 'Optional[int]' = None, *, _read: 'bool' = True, # NOTE: Asked for no further than one read can go, :data:`sys.maxsize`. # Past it the read raises :exc:`OverflowError` instead of coming back # short, so a record claiming more than that would never reach the - # overrun handling below (:issue:`1975`). + # overrun handling below (#1975). self._data = self._read_fileng(min(self.length + _ilen, sys.maxsize)) # NOTE: A record whose ``incl_len`` runs past the end of the data is @@ -364,7 +364,7 @@ def read(self, length: 'Optional[int]' = None, *, _read: 'bool' = True, # NOTE: Nor can a seek go past :data:`sys.maxsize`. A record ending # beyond it was cut short by the end of the data, so the next frame - # starts where the octets read end (:issue:`1975`). + # starts where the octets read end (#1975). if seek_cur > sys.maxsize: seek_cur = _seek_set + len(self._data) diff --git a/pcapkit/protocols/misc/pcapng.py b/pcapkit/protocols/misc/pcapng.py index 44a178ff03..94b7c0ea3a 100644 --- a/pcapkit/protocols/misc/pcapng.py +++ b/pcapkit/protocols/misc/pcapng.py @@ -1353,7 +1353,7 @@ def read(self, length: 'Optional[int]' = None, *, _read: 'bool' = True, # # NOTE: Asked for no further than one read can go, :data:`sys.maxsize`, # past which the read raises :exc:`OverflowError` instead of coming - # back short as the truncation handling below expects (:issue:`1975`). + # back short as the truncation handling below expects (#1975). self._data = self._read_fileng(min(schema.block.length, sys.maxsize)) # NOTE: Block Total Length is cross-checked against its own trailing @@ -1640,7 +1640,7 @@ def _read_block_cut(self, header: 'Schema_PCAPNG', offset: 'int', current = self._file.tell() self._file.seek(offset + 8, io.SEEK_SET) - # NOTE: bounded as :meth:`read` bounds the block's read (:issue:`1975`). + # NOTE: bounded as :meth:`read` bounds the block's read (#1975). body = self._read_fileng(min(header.block.length - GENERIC_BLOCK_MINIMUM, sys.maxsize)) self._file.seek(current, io.SEEK_SET) diff --git a/pcapkit/protocols/schema/schema.py b/pcapkit/protocols/schema/schema.py index 657c7451c2..41b7d6a4cc 100644 --- a/pcapkit/protocols/schema/schema.py +++ b/pcapkit/protocols/schema/schema.py @@ -1075,7 +1075,7 @@ def unpack(cls, data: 'bytes | IO[bytes]', # can go, :data:`sys.maxsize`; past it the read raises # :exc:`OverflowError` rather than coming back short, which is # how every other overrun is reported. No stream holds more than - # that, so the bounded read is the same short read (:issue:`1975`). + # that, so the bounded read is the same short read (#1975). payload = data.read(min(length, sys.maxsize)) self.__buffer__[field.name] = payload @@ -1108,7 +1108,7 @@ def unpack(cls, data: 'bytes | IO[bytes]', length = field.length - # NOTE: bounded as the payload read above is (:issue:`1975`). + # NOTE: bounded as the payload read above is (#1975). byte = data.read(min(length, sys.maxsize)) self.__buffer__[field.name] = byte if not isinstance(field, ForwardMatchField): From 9e0c3719d31bea5b82aff7b6a5ee65f73183cb8c Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Tue, 20 Oct 2026 11:57:56 -0400 Subject: [PATCH 3/3] fix(protocols): advance past a record longer than sys.maxsize instead of truncating (#1975) A record or block the file really holds past sys.maxsize was read only to the limit, and the next one was then parsed from inside it. - corekit.io.advance moves a stream forward any distance: relative seeks of at most sys.maxsize, or chunked read-and-discard on a SeekableReader or a non-seekable stream. - Frame.read and PCAPNG.read call it when the bounded read came back whole, so the next record starts at the declared end; a short read is unchanged. - The PCAP and PCAP-NG engines cap the read-ahead at sys.maxsize, and the PCAP frame schema reads no more than the header and payload can hold in one read, so a SeekableReader keeps the record start in its window. - SeekableReader._fill bounds any READ_CHUNK at sys.maxsize. - Tests cover a sparse file holding a record past a lowered limit, seekable and through a pipe, for PCAP and PCAP-NG. Closes #1975 --- docs/source/pcapkit/corekit/io.rst | 5 +- pcapkit/corekit/io.py | 51 +++++- pcapkit/foundation/engines/pcap.py | 6 +- pcapkit/foundation/engines/pcapng.py | 6 +- pcapkit/protocols/misc/pcap/frame.py | 24 ++- pcapkit/protocols/misc/pcapng.py | 15 +- pcapkit/protocols/schema/misc/pcap/frame.py | 7 +- ..._declared_length_past_maxsize_1975_unit.py | 163 ++++++++++++++---- 8 files changed, 222 insertions(+), 55 deletions(-) diff --git a/docs/source/pcapkit/corekit/io.rst b/docs/source/pcapkit/corekit/io.rst index cfe48722b5..0571ebc3cb 100644 --- a/docs/source/pcapkit/corekit/io.rst +++ b/docs/source/pcapkit/corekit/io.rst @@ -8,7 +8,8 @@ I/O Objects :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``. +``name``; and :func:`~pcapkit.corekit.io.advance`, which moves a stream past +more octets than one read or seek can take. .. autoclass:: pcapkit.corekit.io.SeekableReader :no-members: @@ -59,3 +60,5 @@ stream, and :class:`~pcapkit.corekit.io.NamedStream`, which gives a stream a .. autoattribute:: name .. automethod:: read + +.. autofunction:: pcapkit.corekit.io.advance diff --git a/pcapkit/corekit/io.py b/pcapkit/corekit/io.py index e45a9adb15..866005fd31 100644 --- a/pcapkit/corekit/io.py +++ b/pcapkit/corekit/io.py @@ -9,7 +9,8 @@ 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``. +a stream a ``name``, as well as :func:`~pcapkit.corekit.io.advance`, which moves +a stream past more octets than one read or seek can take. """ import io @@ -28,7 +29,7 @@ from typing_extensions import Buffer -__all__ = ['SeekableReader', 'PeekableStream', 'NamedStream'] +__all__ = ['SeekableReader', 'PeekableStream', 'NamedStream', 'advance'] class SeekableReader(io.BufferedReader): @@ -204,9 +205,9 @@ def _fill(self, size: 'int', minimum: 'Optional[int]' = None, /, *, read = getattr(self._stream, 'read1', self._stream.read) # NOTE: ``-1`` sets no limit of its own, but one read still goes no further - # than :data:`sys.maxsize`, past which it raises :exc:`OverflowError` - # rather than coming back short (#1975). - limit = sys.maxsize if config.READ_CHUNK == -1 else config.READ_CHUNK + # than :data:`sys.maxsize`, whatever the setting, past which it raises + # :exc:`OverflowError` rather than coming back short (#1975). + limit = sys.maxsize if config.READ_CHUNK == -1 else min(config.READ_CHUNK, sys.maxsize) chunks = [] # type: list[bytes] count = 0 while True: @@ -830,6 +831,46 @@ def peek(self, size: 'int' = 0) -> 'bytes': return buf +#: Octets :func:`advance` reads and discards at a time when +#: :data:`~pcapkit.utilities.config.READ_CHUNK` sets no limit. +_DISCARD_CHUNK = 1 << 20 + + +def advance(file: 'IO[bytes]', count: 'int', /) -> 'None': + """Move ``file`` forward by ``count`` octets, however many one read or seek can take. + + Args: + file: Stream to move. + count: Octets to move past, counted from the position. + + A frame reader asks for a record no further than one read can go, + :data:`sys.maxsize`, so a record the file really holds past that is read only + 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 + 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(): + chunk = _DISCARD_CHUNK if config.READ_CHUNK == -1 else config.READ_CHUNK + chunk = min(chunk, sys.maxsize) + while count > 0: + buf = file.read(min(count, chunk)) + if not buf: + break + count -= len(buf) + return + + while count > 0: + step = min(count, sys.maxsize) + file.seek(step, io.SEEK_CUR) + count -= step + + class PeekableStream: """Give a seekable binary stream without ``peek`` one. diff --git a/pcapkit/foundation/engines/pcap.py b/pcapkit/foundation/engines/pcap.py index 53054a94f6..1f4b14641a 100644 --- a/pcapkit/foundation/engines/pcap.py +++ b/pcapkit/foundation/engines/pcap.py @@ -9,6 +9,7 @@ """ import io +import sys from typing import TYPE_CHECKING from pcapkit.corekit.io import SeekableReader @@ -240,4 +241,7 @@ def _bound_read_ahead(self) -> 'None': size = RECORD_HEADER_LEN if len(head) == RECORD_HEADER_LEN: size += int.from_bytes(head[8:12], self._gbhdr.byteorder) - ifile._read_ahead_size = size + # NOTE: No buffer holds more than :data:`sys.maxsize` octets, so a record + # longer than that is read ahead only so far; the reader moves past the + # rest itself (#1975). + ifile._read_ahead_size = min(size, sys.maxsize) diff --git a/pcapkit/foundation/engines/pcapng.py b/pcapkit/foundation/engines/pcapng.py index 9d7a9225ba..f0369498f1 100644 --- a/pcapkit/foundation/engines/pcapng.py +++ b/pcapkit/foundation/engines/pcapng.py @@ -10,6 +10,7 @@ """ import collections import os +import sys from typing import TYPE_CHECKING, cast from pcapkit.const.pcapng.block_type import BlockType as Enum_BlockType @@ -453,7 +454,10 @@ def _bound_read_ahead(self) -> 'None': if block_type == Enum_BlockType.Decryption_Secrets_Block: secrets = int.from_bytes(self._peek(16)[12:16], byteorder) size = max(size, BLOCK_MINIMUM[block_type] + secrets + -secrets % 4) - ifile._read_ahead_size = size + # NOTE: No buffer holds more than :data:`sys.maxsize` octets, so a block + # longer than that is read ahead only so far; the reader moves past the + # rest itself (#1975). + ifile._read_ahead_size = min(size, sys.maxsize) def _check_packet_block_context(self) -> 'None': """Reject a packet block in a section that describes no interface. diff --git a/pcapkit/protocols/misc/pcap/frame.py b/pcapkit/protocols/misc/pcap/frame.py index 9d8ce0468d..cce6ab8176 100644 --- a/pcapkit/protocols/misc/pcap/frame.py +++ b/pcapkit/protocols/misc/pcap/frame.py @@ -39,6 +39,7 @@ from typing import TYPE_CHECKING, cast, overload from pcapkit.const.reg.linktype import LinkType as Enum_LinkType +from pcapkit.corekit.io import advance from pcapkit.corekit.module import ModuleDescriptor from pcapkit.protocols.data.misc.pcap.frame import Frame as Data_Frame from pcapkit.protocols.data.misc.pcap.frame import FrameInfo as Data_FrameInfo @@ -347,7 +348,8 @@ def read(self, length: 'Optional[int]' = None, *, _read: 'bool' = True, # Past it the read raises :exc:`OverflowError` instead of coming back # short, so a record claiming more than that would never reach the # overrun handling below (#1975). - self._data = self._read_fileng(min(self.length + _ilen, sys.maxsize)) + bound = min(self.length + _ilen, sys.maxsize) + self._data = self._read_fileng(bound) # NOTE: A record whose ``incl_len`` runs past the end of the data is # kept as captured, as the PCAP-NG reader keeps a captured length @@ -362,14 +364,20 @@ def read(self, length: 'Optional[int]' = None, *, _read: 'bool' = True, f'which holds {held} octet(s); kept as captured', ProtocolWarning, stacklevel=stacklevel()) - # NOTE: Nor can a seek go past :data:`sys.maxsize`. A record ending - # beyond it was cut short by the end of the data, so the next frame - # starts where the octets read end (#1975). - if seek_cur > sys.maxsize: - seek_cur = _seek_set + len(self._data) + # NOTE: A read that came back whole at the bound stopped there, not + # at the end of the data: the record runs on past it, and the next + # one starts where this one declares it ends, so the stream is + # moved over the rest (#1975). A shorter read met the end of the + # data, where a seek past :data:`sys.maxsize` could not go either, + # so the next frame then starts where the octets read end. + if len(self._data) == bound < self.length + _ilen: + advance(self._file, self.length + _ilen - bound) + else: + if seek_cur > sys.maxsize: + seek_cur = _seek_set + len(self._data) - # move forward to the beginning of the next frame - self._file.seek(seek_cur, io.SEEK_SET) + # move forward to the beginning of the next frame + self._file.seek(seek_cur, io.SEEK_SET) #: io.BytesIO: Source data stream. self._file = io.BytesIO(self._data) diff --git a/pcapkit/protocols/misc/pcapng.py b/pcapkit/protocols/misc/pcapng.py index 94b7c0ea3a..0593624b21 100644 --- a/pcapkit/protocols/misc/pcapng.py +++ b/pcapkit/protocols/misc/pcapng.py @@ -67,6 +67,7 @@ from pcapkit.corekit.enum import EnumLookup from pcapkit.corekit.fields.strings import DecodedString from pcapkit.corekit.infoclass import OrderedMultiInfo +from pcapkit.corekit.io import advance from pcapkit.corekit.module import ModuleDescriptor from pcapkit.corekit.multidict import OrderedMultiDict from pcapkit.corekit.version import VersionInfo @@ -1354,7 +1355,8 @@ def read(self, length: 'Optional[int]' = None, *, _read: 'bool' = True, # NOTE: Asked for no further than one read can go, :data:`sys.maxsize`, # past which the read raises :exc:`OverflowError` instead of coming # back short as the truncation handling below expects (#1975). - self._data = self._read_fileng(min(schema.block.length, sys.maxsize)) + bound = min(schema.block.length, sys.maxsize) + self._data = self._read_fileng(bound) # NOTE: Block Total Length is cross-checked against its own trailing # copy and never against the file, so on a capture cut short it runs @@ -1387,8 +1389,15 @@ def read(self, length: 'Optional[int]' = None, *, _read: 'bool' = True, # again in place of building the block (:issue:`1470`). block.__update__([('__truncated_raw__', self._data)]) - # move backward to the beginning of next block - self._file.seek(seek_cur, io.SEEK_SET) + # NOTE: A read that came back whole at the bound stopped there, not + # at the end of the file: the block runs on past it, and the next one + # starts where its Block Total Length says, so the stream is moved + # over the rest (#1975). + if read == bound < schema.block.length: + advance(self._file, schema.block.length - bound) + else: + # move backward to the beginning of next block + self._file.seek(seek_cur, io.SEEK_SET) #: io.BytesIO: Source packet stream. self._file = io.BytesIO(self._data) diff --git a/pcapkit/protocols/schema/misc/pcap/frame.py b/pcapkit/protocols/schema/misc/pcap/frame.py index 1779538552..a1e8de39e5 100644 --- a/pcapkit/protocols/schema/misc/pcap/frame.py +++ b/pcapkit/protocols/schema/misc/pcap/frame.py @@ -2,6 +2,7 @@ # mypy: disable-error-code=assignment """header schema for frame header of PCAP file format""" +import sys from typing import TYPE_CHECKING from pcapkit.corekit.fields.byteorder import byteorder_callback @@ -36,8 +37,10 @@ class Frame(Schema): incl_len: 'int' = UInt32Field(callback=byteorder_callback) #: Actual length of packet. orig_len: 'int' = UInt32Field(callback=byteorder_callback) - #: Payload. - packet: 'bytes' = PayloadField(length=lambda pkt: pkt['incl_len']) + #: Payload, read no further than the record header and it together can go + #: in one read, :data:`sys.maxsize`; the frame reader moves past the rest + #: (:issue:`1975`). + packet: 'bytes' = PayloadField(length=lambda pkt: min(pkt['incl_len'], sys.maxsize - RECORD_HEADER_LEN)) if TYPE_CHECKING: def __init__(self, ts_sec: 'int', ts_usec: 'int', incl_len: 'int', diff --git a/tests/protocols/misc/test_declared_length_past_maxsize_1975_unit.py b/tests/protocols/misc/test_declared_length_past_maxsize_1975_unit.py index 887d5e785d..ccddb362fc 100644 --- a/tests/protocols/misc/test_declared_length_past_maxsize_1975_unit.py +++ b/tests/protocols/misc/test_declared_length_past_maxsize_1975_unit.py @@ -8,9 +8,14 @@ the reader before the short-read handling could keep it as captured. Each case runs such a build's limit on whatever build the suite runs on: the -in-memory streams raise :exc:`OverflowError` past ``2 ** 31 - 1``, and -:data:`sys.maxsize` reads as that value, so the case parses as it would there and -must match the result of the same octets parsed without the limit. +streams raise :exc:`OverflowError` past :data:`sys.maxsize`, which reads as +``2 ** 31 - 1``, so the case parses as it would there and must match the result +of the same octets parsed without the limit. + +A record the file really holds past the limit is read only as far as the limit, +but the next record still starts where it declares it ends. Those cases lower the +limit to :data:`LONG_LIMIT`, so the record need not be gigabytes long, and serve +it from :class:`_SparseStream`, which holds only the octets that are not zero. Every case builds its own octets and reads no capture. Classes are imported inside each test, after :func:`~tests._support.reimport_once_per_class`. @@ -33,6 +38,12 @@ #: ``sys.maxsize`` where ``Py_ssize_t`` is 32 bits wide. NARROW_MAXSIZE = 2 ** 31 - 1 +#: A narrow limit for the records a file really holds past it, cheap to exceed and +#: still past a ``SeekableReader`` buffer, as the real limit is. +LONG_LIMIT = 2 ** 20 - 1 +#: How far those records run past :data:`LONG_LIMIT`: more than one discarding +#: read or relative seek's worth, and not a whole number of either. +LONG_EXCESS = 2 * LONG_LIMIT + 4097 #: Little-endian global header: v2.4, Ethernet. GLOBAL = bytes.fromhex('d4c3b2a1' '0200' '0400' '00000000' '00000000' 'ffff0000' '01000000') @@ -43,56 +54,88 @@ CLAIMS = (NARROW_MAXSIZE - 15, NARROW_MAXSIZE, NARROW_MAXSIZE + 1, 0xFFFF_FFFF) -class _NarrowBytesIO(io.BytesIO): - """:class:`io.BytesIO` whose reads and seeks refuse what a 32-bit ``Py_ssize_t`` cannot hold.""" +def _check(value: 'int | None') -> 'None': + """Refuse a read or seek size that ``Py_ssize_t`` cannot hold on the simulated build.""" + if value is not None and value > sys.maxsize: + raise OverflowError('Python int too large to convert to C ssize_t') - @staticmethod - def _check(value: 'int | None') -> 'None': - if value is not None and value > NARROW_MAXSIZE: - raise OverflowError('Python int too large to convert to C ssize_t') + +class _NarrowBytesIO(io.BytesIO): + """:class:`io.BytesIO` whose reads and seeks refuse what ``Py_ssize_t`` cannot hold.""" def read(self, size: 'int | None' = -1, /) -> 'bytes': - self._check(size) + _check(size) return super().read(size) def read1(self, size: 'int | None' = -1, /) -> 'bytes': - self._check(size) + _check(size) return super().read1(size) def seek(self, pos: 'int', whence: 'int' = io.SEEK_SET, /) -> 'int': - self._check(pos) + _check(pos) return super().seek(pos, whence) -class _NarrowPipe(io.RawIOBase): - """A non-seekable stream with the same limit, read through a ``SeekableReader``.""" +class _SparseStream(io.RawIOBase): + """A file of ``size`` octets, zero but for ``pieces``, that allocates only what is read. + + Its reads refuse what ``Py_ssize_t`` cannot hold. Its seeks do not, as a file's + do not on GraalPy; built with ``seekable=False`` it cannot seek at all, and an + extractor reads it through a ``SeekableReader``. - def __init__(self, data: 'bytes') -> 'None': + """ + + def __init__(self, pieces: 'dict[int, bytes]', size: 'int', *, seekable: 'bool' = True) -> 'None': super().__init__() - self._data = data + self._pieces = pieces + self._size = size + self._seekable = seekable self._pos = 0 def readable(self) -> 'bool': return True def seekable(self) -> 'bool': - return False + return self._seekable + + def tell(self) -> 'int': + if not self._seekable: + raise io.UnsupportedOperation('tell') + return self._pos + + def seek(self, pos: 'int', whence: 'int' = io.SEEK_SET, /) -> 'int': + if not self._seekable: + raise io.UnsupportedOperation('seek') + base = {io.SEEK_SET: 0, io.SEEK_CUR: self._pos, io.SEEK_END: self._size}[whence] + self._pos = max(base + pos, 0) + return self._pos def read(self, size: 'int | None' = -1, /) -> 'bytes': - _NarrowBytesIO._check(size) # pylint: disable=protected-access - if size is None or size < 0: - size = len(self._data) - self._pos - chunk = self._data[self._pos:self._pos + size] - self._pos += len(chunk) - return chunk + _check(size) + start = self._pos + stop = self._size if size is None or size < 0 else min(self._size, start + size) + if stop <= start: + return b'' + out = bytearray(stop - start) + for offset, piece in self._pieces.items(): + low, high = max(offset, start), min(offset + len(piece), stop) + if low < high: + out[low - start:high - start] = piece[low - offset:high - offset] + self._pos = stop + return bytes(out) read1 = read +def _pipe(data: 'bytes') -> '_SparseStream': + """A non-seekable stream holding exactly ``data``.""" + return _SparseStream({0: data}, len(data), seekable=False) + + @contextlib.contextmanager -def narrow_build(): - """Run the block as a build whose ``Py_ssize_t`` is 32 bits wide would.""" - with unittest.mock.patch.object(sys, 'maxsize', NARROW_MAXSIZE), \ +def narrow_build(limit: 'int' = NARROW_MAXSIZE): + """Run the block as a build whose one read or seek goes no further than ``limit``.""" + with unittest.mock.patch.object(sys, 'maxsize', limit), \ unittest.mock.patch.object(io, 'BytesIO', _NarrowBytesIO): yield @@ -133,12 +176,19 @@ def setUp(self) -> None: def test_the_simulated_build_refuses_what_a_narrow_build_does(self) -> None: """The streams this module reads overflow where a 32-bit build's do.""" - for stream in (_NarrowBytesIO(PACKET), _NarrowPipe(PACKET)): - with self.subTest(stream=type(stream).__name__): + for stream in (_NarrowBytesIO(PACKET), _pipe(PACKET)): + with self.subTest(stream=type(stream).__name__), narrow_build(): self.assertEqual(stream.read(NARROW_MAXSIZE), PACKET) with self.assertRaises(OverflowError): stream.read(NARROW_MAXSIZE + 1) + def test_the_sparse_stream_serves_its_pieces_among_zeros(self) -> None: + stream = _SparseStream({2: b'ab', 6: b'cd'}, 9) + self.assertEqual(stream.read(4), b'\x00\x00ab') + self.assertEqual(stream.seek(1, io.SEEK_CUR), 5) + self.assertEqual(stream.read(), b'\x00cd\x00') + self.assertEqual(stream.read(1), b'') + def test_a_frame_claiming_past_maxsize_is_kept_as_captured(self) -> None: from pcapkit.protocols.misc.pcap.frame import Frame from pcapkit.protocols.misc.pcap.header import Header @@ -159,15 +209,21 @@ def test_a_frame_claiming_past_maxsize_is_kept_as_captured(self) -> None: self.assertEqual(bytes(info.packet), PACKET) self.assertEqual(frame.data, octets) # type: ignore[attr-defined] - def extract(self, fin: 'object', **kwargs: 'object') -> 'list[bytes]': - """The packets the default engine extracts from ``fin``, on the narrow build.""" + def extract_frames(self, fin: 'object', limit: 'int' = NARROW_MAXSIZE, + **kwargs: 'object') -> 'tuple[list[object], list[str]]': + """The frames the default engine extracts from ``fin``, and the warnings raised.""" import pcapkit - with narrow_build(), warnings.catch_warnings(): - warnings.simplefilter('ignore') + with narrow_build(limit), warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') extractor = pcapkit.extract(fin=fin, nofile=True, store=True, engine='default', **kwargs) close_extractor(extractor) - return [bytes(frame.info.packet) for frame in extractor.frame] + return list(extractor.frame), [str(item.message) for item in caught] + + def extract(self, fin: 'object', limit: 'int' = NARROW_MAXSIZE, **kwargs: 'object') -> 'list[bytes]': + """The packets the default engine extracts from ``fin``, on the narrow build.""" + frames, _ = self.extract_frames(fin, limit, **kwargs) + return [bytes(frame.info.packet) for frame in frames] # type: ignore[attr-defined] def test_extraction_keeps_a_pcap_record_claiming_past_maxsize(self) -> None: for claim in CLAIMS: @@ -192,7 +248,46 @@ def test_a_non_seekable_input_with_no_read_chunk_limit(self) -> None: octets = GLOBAL + _record(len(PACKET)) + _record(0xFFFF_FFFF) for chunk in (config.READ_CHUNK, -1): with self.subTest(read_chunk=chunk), unittest.mock.patch.object(config, 'READ_CHUNK', chunk): - self.assertEqual(self.extract(_NarrowPipe(octets)), [PACKET, PACKET]) + self.assertEqual(self.extract(_pipe(octets)), [PACKET, PACKET]) + + def check_moved_past(self, frames: 'list[object]', messages: 'list[str]', warning: 'str') -> 'None': + """The long record is bounded and warned of, and the next one parses where it is.""" + packets = [bytes(frame.info.packet) for frame in frames] # type: ignore[attr-defined] + self.assertEqual(len(packets), 3, [len(packet) for packet in packets]) + self.assertEqual((packets[0], packets[2]), (PACKET, PACKET)) + self.assertLessEqual(len(packets[1]), LONG_LIMIT) + self.assertTrue([text for text in messages if text.startswith(warning)], messages) + + def test_a_pcap_record_really_past_maxsize_is_moved_past_whole(self) -> None: + """The record after it parses where it is, past a seek or a read over the rest.""" + from pcapkit.utilities import config + + claim = LONG_LIMIT + LONG_EXCESS + head = GLOBAL + _record(len(PACKET)) + _record(claim, b'') + pieces = {0: head, len(head) + claim: _record(len(PACKET))} + size = len(head) + claim + len(_record(len(PACKET))) + cases = (('seekable', True, config.READ_CHUNK), ('pipe', False, config.READ_CHUNK), + ('pipe, no read chunk limit', False, -1)) + for name, seekable, chunk in cases: + with self.subTest(stream=name), unittest.mock.patch.object(config, 'READ_CHUNK', chunk): + frames, messages = self.extract_frames(_SparseStream(pieces, size, seekable=seekable), + LONG_LIMIT) + self.check_moved_past(frames, messages, f'PCAP: [Frame 2] captured length {claim} runs past') + self.assertEqual(frames[1].info.frame_info.incl_len, claim) # type: ignore[attr-defined] + + def test_a_pcapng_block_really_past_maxsize_is_moved_past_whole(self) -> None: + """So does the block after a PCAP-NG block holding more than the limit.""" + total = (LONG_LIMIT + LONG_EXCESS) // 4 * 4 + head = SECTION + _epb(len(PACKET)) + start = struct.pack('