diff --git a/CHANGELOG.md b/CHANGELOG.md index b67c26c35..cf9de1b86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -179,6 +179,7 @@ Preceded by `1.5.0a1` (2026-09-15), `1.5.0b1` to `1.5.0b3` (all 2026-09-19) and - IP reassembly reports a datagram that is not itself fragmented but whose BUFID is already buffered: it submits that buffer as incomplete and then reassembles the datagram in a buffer of its own, per [RFC 791 Section 3.2](https://datatracker.ietf.org/doc/html/rfc791#section-3.2), where the datagram was never reported at all. On IPv6 an atomic fragment is reassembled in a buffer of its own and submitted without flushing the datagram already being reassembled, so that one's final fragment still completes it, per [RFC 6946 Section 4](https://datatracker.ietf.org/doc/html/rfc6946#section-4) ([#2270](https://github.com/JarryShaw/PyPCAPKit/issues/2270), [#2345](https://github.com/JarryShaw/PyPCAPKit/pull/2345)). - the PyPCAP, pcap-ct and pypcapfile engines warn with `AttributeWarning`, naming the engine, that they ignore a parsing context supplied through `Extractor`'s `context=`, as the dpkt, scapy and pyshark engines already did, where they dropped it silently ([#2313](https://github.com/JarryShaw/PyPCAPKit/issues/2313), [#2332](https://github.com/JarryShaw/PyPCAPKit/pull/2332)). - `Deferred` could not be pickled under protocol 0 or 1 -- a slotted class with no `__getstate__` raises `TypeError` there, independent of the `analyze`/`proto` regression [#2417](https://github.com/JarryShaw/PyPCAPKit/issues/2417) fixed. It now defines `__getstate__` and `__setstate__`, mirroring the `(dict_or_None, slots)` pair Python's own default slotted-class reduction already used for every released build, so a `Deferred` pickled by one of those still unpickles. The slot names come from `copyreg._slotnames` rather than a hand-rolled walk of `type(self).__mro__`, which would otherwise shadow a base class's slots under a subclass declaring its own, miscount a bare-string `__slots__ = 'name'`, or read a private, name-mangled slot under its unmangled spelling ([#2423](https://github.com/JarryShaw/PyPCAPKit/issues/2423), [#2425](https://github.com/JarryShaw/PyPCAPKit/issues/2425), [#2424](https://github.com/JarryShaw/PyPCAPKit/pull/2424)). +- that same `Deferred` still failed to pickle under protocol 0 or 1 on GraalPy 25.4, after [#2423](https://github.com/JarryShaw/PyPCAPKit/issues/2423)'s fix: `copyreg._reduce_ex`, which handles protocol 0 and 1 before `__getstate__` is ever consulted, walks `cls.__mro__` for a base to compute a base-object state from, and on GraalPy that walk stops at `Generic` itself rather than at `object`, raising `TypeError: typing.Generic() takes no arguments`. `Deferred.__reduce_ex__` now hard-codes the `base=object, state=None` outcome the walk always reached on CPython for protocol 0 and 1, so the walk -- and `Generic` -- is no longer part of the route pickling takes. The same override also checks for, and defers to, a subclass's own `__reduce__` before building that hard-coded tuple -- without that check a `Deferred` subclass overriding `__reduce__` had it silently ignored at protocol 0 and 1 while protocol 2 and above (via `object.__reduce_ex__` unchanged) kept honouring it ([#2433](https://github.com/JarryShaw/PyPCAPKit/issues/2433), [#2435](https://github.com/JarryShaw/PyPCAPKit/pull/2435)). ### pcapkit.interface diff --git a/docs/source/changelog/1.5.0.rst b/docs/source/changelog/1.5.0.rst index c7864095e..860ee3cb0 100644 --- a/docs/source/changelog/1.5.0.rst +++ b/docs/source/changelog/1.5.0.rst @@ -1273,6 +1273,20 @@ Fixed shadow a base class's slots under a subclass declaring its own, miscount a bare-string ``__slots__ = 'name'``, or read a private, name-mangled slot under its unmangled spelling (:issue:`2423`, :issue:`2425`, :pr:`2424`). +* that same ``Deferred`` still failed to pickle under protocol 0 or 1 on GraalPy + 25.4, after :issue:`2423`'s fix: ``copyreg._reduce_ex``, which handles protocol + 0 and 1 before ``__getstate__`` is ever consulted, walks ``cls.__mro__`` for a + base to compute a base-object state from, and on GraalPy that walk stops at + :class:`~typing.Generic` itself rather than at :class:`object`, raising + ``TypeError: typing.Generic() takes no arguments``. ``Deferred.__reduce_ex__`` + now hard-codes the ``base=object, state=None`` outcome the walk always reached + on CPython for protocol 0 and 1, so the walk -- and ``Generic`` -- is no longer + part of the route pickling takes. The same override also checks for, and + defers to, a subclass's own ``__reduce__`` before building that hard-coded + tuple -- without that check a ``Deferred`` subclass overriding ``__reduce__`` + had it silently ignored at protocol 0 and 1 while protocol 2 and above (via + ``object.__reduce_ex__`` unchanged) kept honouring it (:issue:`2433`, + :pr:`2435`). pcapkit.interface ----------------- diff --git a/pcapkit/foundation/reassembly/data/data.py b/pcapkit/foundation/reassembly/data/data.py index 7ee87da78..08aa65ef7 100644 --- a/pcapkit/foundation/reassembly/data/data.py +++ b/pcapkit/foundation/reassembly/data/data.py @@ -11,7 +11,7 @@ __all__ = ['ReassemblyData', 'Completion', 'Deferred'] if TYPE_CHECKING: - from typing import Any, Callable, Optional + from typing import Any, Callable, Optional, SupportsIndex from pcapkit.const.reg.apptype import TCP as AppType_TCP from pcapkit.const.reg.transtype import TransType @@ -222,6 +222,70 @@ def __call__(self) -> 'ProtocolBase': """ return self.analyze(self.proto, self.payload) + def __reduce_ex__(self, protocol: 'SupportsIndex') -> 'Any': + """Pickling support for protocol 0 and 1. + + :meth:`object.__reduce_ex__` hands protocol 0 and 1 off to + :func:`copyreg._reduce_ex`, which -- *before* it ever consults + :meth:`__getstate__` -- walks ``type(self).__mro__`` looking for the + first base that is either not a heap type or whose own ``__new__`` is + bound to itself, to compute a base-object state via ``base(self)``. On + CPython, :class:`~typing.Generic` is a heap type and its ``__new__`` + is just the inherited ``object.__new__`` (bound to ``object``, not to + ``Generic``), so neither check fires there and the walk continues to + :class:`object` itself, which *is* a non-heap type -- the walk's first + check breaks on it, so ``base`` is ``object`` and the computed state is + :data:`None`. On GraalPy 25.4, ``Generic`` is itself a non-heap + (built-in) type there -- ``Generic.__flags__`` lacks ``_HEAPTYPE`` -- + so the walk's *first* check breaks on ``Generic`` directly, one step + earlier than on CPython, without ever reaching the ``__new__`` check + at all. ``base`` therefore resolves to ``Generic`` itself, and the + walk calls ``Generic(self)``, which raises:: + + TypeError: typing.Generic() takes no arguments + + :meth:`__getstate__`/:meth:`__setstate__` never run in that failure: + they only ever supply the *dict* half of the reduction tuple the walk + above is computing the *base-object* half for, so defining them + (:issue:`2423`, :issue:`2425`) does nothing to protect against a + GraalPy-specific outcome of a walk that happens before either is + consulted. + + This override sidesteps the walk entirely rather than trying to steer + it: for protocol 0 and 1 it hard-codes the one outcome the walk was + ever going to reach for this class -- ``base=object``, + ``state=None`` -- and builds the same ``(callable, args, state)`` + reduction :func:`copyreg._reduce_ex` itself would have, with + :meth:`__getstate__`'s own ``(dict_or_None, slots)`` pair as the + third element, restored through :meth:`__setstate__` exactly as + before. ``Deferred`` has no base below :class:`object` that could + need special handling -- :class:`~typing.Generic` carries no runtime + state of its own -- so hard-coding that outcome changes nothing about + *what* gets pickled, only *how* protocol 0 and 1 get there: the MRO + walk, and ``Generic`` along with it, is no longer part of the route at + all. Protocol 2 and above are untouched, deferring to + :meth:`object.__reduce_ex__` exactly as before this method existed -- + their own ``__reduce_ex__`` never walked the MRO this way to begin + with, building a reduction straight from ``__getstate__`` instead. + + A subclass that overrides ``__reduce__`` is checked for first, and + deferred to, before any of the above: :meth:`object.__reduce_ex__` + itself does this -- for *every* protocol, including 0 and 1, it + calls a non-default ``__reduce__`` in preference to building a + reduction of its own -- so hard-coding the protocol 0/1 tuple here + without the same check would silently drop such a subclass's + override at exactly those two protocols, while protocol 2 and above + (going through ``object.__reduce_ex__`` unchanged) kept honouring + it. + + """ + if type(self).__reduce__ is not object.__reduce__: + return self.__reduce__() + if int(protocol) >= 2: + return object.__reduce_ex__(self, protocol) + return (copyreg._reconstructor, (type(self), object, None), # type: ignore[attr-defined] # pylint: disable=protected-access + self.__getstate__()) + def __getstate__(self) -> 'tuple[Optional[dict[str, Any]], dict[str, Any]]': """Pickling support. diff --git a/tests/foundation/reassembly/test_deferred_pickle_graalpy_2433_unit.py b/tests/foundation/reassembly/test_deferred_pickle_graalpy_2433_unit.py new file mode 100644 index 000000000..f7854298b --- /dev/null +++ b/tests/foundation/reassembly/test_deferred_pickle_graalpy_2433_unit.py @@ -0,0 +1,201 @@ +# -*- coding: utf-8 -*- +""":class:`Deferred` could not be pickled under protocol 0 or 1 on GraalPy 25.4. + +GitHub issue :issue:`2433`, fixed by :pr:`2435`. + +:issue:`2423` and :issue:`2425` (pinned in +:mod:`tests.foundation.reassembly.test_deferred_pickle_2423_unit`) gave +``Deferred`` a ``__getstate__``/``__setstate__`` pair so protocol 0 and 1 no +longer raised *there*. Neither method is where GraalPy 25.4 actually fails, +though: :meth:`object.__reduce_ex__` hands protocol 0 and 1 to +:func:`copyreg._reduce_ex`, which walks ``type(self).__mro__`` for a base to +compute a base-object state from *before* ``__getstate__`` is ever consulted. +On CPython that walk passes :class:`~typing.Generic` (a heap type there, +whose own ``__new__`` is just the inherited, differently-bound +``object.__new__``) and stops at :class:`object`. On GraalPy, ``Generic`` is +itself a non-heap (built-in) type, so the walk's first check breaks on +``Generic`` directly and calls ``Generic(self)``, raising:: + + TypeError: typing.Generic() takes no arguments + +:meth:`Deferred.__reduce_ex__` now hard-codes the ``base=object, state=None`` +outcome the walk always reached on CPython for protocol 0 and 1, which means +``copyreg._reduce_ex`` -- and its MRO walk -- is no longer called for those +two protocols at all. + +A cross-reviewer confirmed this directly against real GraalPy 25.4.4 +(``graalpy3.13-community-25.4.4``, downloaded from the +``oracle/graalpython`` GitHub releases and checksum-verified): this +module's tests, together with +:mod:`tests.foundation.reassembly.test_deferred_pickle_2423_unit`, are red +on ``main`` and green on this fix there, and the full +:file:`tests/foundation/reassembly/` suite passes clean on this fix on +both CPython and real GraalPy 25.4.4 (with the generated sample captures +present -- run :file:`examples/generators/make_samples.py` first if +reproducing). GraalPy is not available in this repository's regular +environment, so :class:`DeferredGraalPyPickleTests` below reproduces the +failure mode by monkeypatching :func:`copyreg._reduce_ex` to raise the exact +:exc:`TypeError` real GraalPy does -- RED against ``main`` (which still +routes protocol 0 and 1 through ``object.__reduce_ex__`` -> +``copyreg._reduce_ex``) and GREEN against this fix (which never calls +``copyreg._reduce_ex`` for those two protocols at all). + +The cross-review on this fix's first revision found a second, independent +regression in the same method, fixed alongside this test: the protocol 0/1 +branch unconditionally built a hard-coded ``(copyreg._reconstructor, ...)`` +tuple, silently dropping a *subclass*'s own ``__reduce__`` override at +exactly those two protocols, while protocol 2 and above -- going through +``object.__reduce_ex__`` unchanged, which always checks for and calls a +non-default ``__reduce__`` -- kept honouring it. +:class:`DeferredSubclassReduceOverrideTests` pins the fixed behaviour. It is +new coverage rather than a regression proof: the override check is new to +this fix, not something an earlier release got right and then lost. + +""" + +import copyreg +import pickle +import unittest +from unittest import mock + +from pcapkit.foundation.reassembly.data.data import Deferred as _Deferred +from tests._support import reimport_once_per_class + +#: Test tier (#1958): cross-interpreter pickling behaviour, same reasoning as +#: ``tests/foundation/reassembly/test_deferred_pickle_2423_unit.py`` gives for +#: itself -- pinned on ``ALL`` because it is exactly the kind of thing that can, +#: and here did, differ between interpreters. +__tier__ = 'ALL' + + +def _analyze(proto: 'object', payload: 'bytes') -> 'tuple[object, bytes]': + """Module-level stand-in analyser; picklable under every protocol.""" + return (proto, payload) + + +def _rebuild_reduce_override_sub(analyze: 'object', proto: 'object', payload: 'bytes', + marker: 'object') -> '_ReduceOverrideSub': + """Module-level rebuild function ``_ReduceOverrideSub.__reduce__`` names. + + Has to be importable by qualified name, like ``_analyze`` above, for the + same reason given in ``test_deferred_pickle_2423_unit.py``'s module + docstring: :mod:`pickle` locates a reduction's callable by name, and a + function local to a test method is not importable. + + """ + return _ReduceOverrideSub(analyze, proto, payload, marker) + + +class _ReduceOverrideSub(_Deferred): + """A ``Deferred`` subclass overriding ``__reduce__``. + + Module-level for the same reason ``_rebuild_reduce_override_sub`` above + is: :mod:`pickle` has to be able to locate this class by its qualified + name to build a real pickle, which a class nested in a test method is + not. + + """ + + #: Set by ``__reduce__`` below; read back by the tests. A plain boolean + #: rather than object identity, since the restored instance after a real + #: pickle round trip is a brand-new object built by + #: ``_rebuild_reduce_override_sub``, not ``self``. + reduce_called = False + + def __init__(self, analyze: 'object', proto: 'object', payload: 'bytes', + marker: 'object') -> 'None': + super().__init__(analyze, proto, payload) + self.marker = marker + + def __reduce__(self) -> 'tuple[object, ...]': + _ReduceOverrideSub.reduce_called = True + return (_rebuild_reduce_override_sub, + (self.analyze, self.proto, self.payload, self.marker)) + + +class DeferredGraalPyPickleTests(unittest.TestCase): + """Protocol 0 and 1 must not route through ``copyreg._reduce_ex`` at all. + + GraalPy 25.4's failure happens inside that function, before + ``__getstate__`` is ever consulted (see the module docstring), so the fix + this file pins is that ``Deferred.__reduce_ex__`` never calls it for + protocol 0 or 1 -- simulated here by making the call raise the same + :exc:`TypeError` real GraalPy does. + + """ + + def setUp(self) -> None: + reimport_once_per_class(self) + + def test_protocol_0_1_survive_a_copyreg_reduce_ex_failure(self) -> None: + """RED on ``main`` (pre-:issue:`2433`), GREEN on this fix. + + A cross-reviewer ran this test against real GraalPy 25.4.4 and + confirmed the result, in addition to the monkeypatched reproduction + here; see the module docstring for how. + + """ + from pcapkit.foundation.reassembly.data.data import Deferred + + deferred = Deferred(_analyze, 42, b'payload') + + def _raise_like_graalpy(self: 'object', proto: 'object') -> 'object': + raise TypeError('typing.Generic() takes no arguments') + + with mock.patch.object(copyreg, '_reduce_ex', side_effect=_raise_like_graalpy): + for protocol in (0, 1): + with self.subTest(protocol=protocol): + restored = pickle.loads(pickle.dumps(deferred, protocol=protocol)) + self.assertEqual(restored.proto, 42) + self.assertEqual(restored.payload, b'payload') + self.assertIs(restored.analyze, _analyze) + + def test_protocol_2_and_above_never_call_copyreg_reduce_ex(self) -> None: + """Protocol >= 2 is untouched by this fix -- it never walked the MRO. + + Patching ``copyreg._reduce_ex`` to blow up therefore has no effect at + protocol 2 and above, confirming the bypass above is specific to + protocol 0 and 1 rather than a blanket detour around the function. + + """ + from pcapkit.foundation.reassembly.data.data import Deferred + + deferred = Deferred(_analyze, 42, b'payload') + with mock.patch.object(copyreg, '_reduce_ex', + side_effect=AssertionError('must not be called')): + restored = pickle.loads(pickle.dumps(deferred, protocol=2)) + self.assertEqual(restored.proto, 42) + self.assertEqual(restored.payload, b'payload') + + +class DeferredSubclassReduceOverrideTests(unittest.TestCase): + """A subclass's own ``__reduce__`` must be honoured at every protocol. + + Cross-review finding on this fix's first revision: the hard-coded + protocol 0/1 tuple ignored such an override, while protocol 2 and above + (going through ``object.__reduce_ex__`` unchanged) did not. New coverage + rather than a regression proof -- see the module docstring. + + """ + + def setUp(self) -> None: + reimport_once_per_class(self) + _ReduceOverrideSub.reduce_called = False + + def test_override_is_called_under_every_protocol(self) -> None: + for protocol in range(pickle.HIGHEST_PROTOCOL + 1): + with self.subTest(protocol=protocol): + _ReduceOverrideSub.reduce_called = False + sub = _ReduceOverrideSub(_analyze, 7, b'x', 'marker-value') + restored = pickle.loads(pickle.dumps(sub, protocol=protocol)) + self.assertTrue(_ReduceOverrideSub.reduce_called, + f'__reduce__ override was bypassed at protocol {protocol}') + self.assertIsInstance(restored, _ReduceOverrideSub) + self.assertEqual(restored.proto, 7) + self.assertEqual(restored.payload, b'x') + self.assertEqual(restored.marker, 'marker-value') + self.assertIs(restored.analyze, _analyze) + + +if __name__ == '__main__': + unittest.main()