Skip to content

Commit 43f7e2b

Browse files
committed
fix(reassembly): make Deferred picklable under protocol 0 and 1 (#2423)
Deferred declares __slots__ with no __getstate__/__setstate__, so protocol 0 and 1 (which copyreg refuses outright once __getstate__ is still the inherited default) raised TypeError for any Deferred instance, regardless of its analyze/proto/payload values. Independent of #2417, a different concern on the same class. A cross-review before this merged raised two more defects on the same fix (#2425), both folded in here: - __getstate__ now mirrors the (dict_or_None, slots) pair Python's own default slotted-class reduction already produced for every released build, instead of a bespoke flat dict. A Deferred pickled by any released version still unpickles, and __setstate__ additionally still accepts the first revision's own flat shape. - both methods walk type(self).__mro__ rather than reading self.__slots__ directly, which resolves through the MRO like any other attribute and so silently dropped the base class's slots under a subclass declaring its own. Covered by tests/foundation/reassembly/test_deferred_pickle_2423_unit.py, pinning protocol 0/1, the native state shape, backward compatibility with both pickle shapes, and MRO-aware subclassing (unset slots, a __weakref__ slot, a subclass with a bare __dict__). Confirmed every new/changed test fails against each prior revision by temporarily stashing just the relevant code change and re-running. Build: brazil-build not applicable (GitHub project); ran coverage run -m pytest, mypy and pylint with this project's own CI flags, and isort -- all clean on the touched files. Closes #2423 Closes #2425
1 parent 867616b commit 43f7e2b

4 files changed

Lines changed: 414 additions & 1 deletion

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,7 @@ Preceded by `1.5.0a1` (2026-09-15), `1.5.0b1` to `1.5.0b3` (all 2026-09-19) and
178178
- the number registrars `register_tcp`, `register_udp`, `register_sctp` and `register_apptype` refuse a `bool` code with `RegistryError`, where `register_tcp(True, ...)` registered port 1, since `bool` is an `int` subclass ([#2269](https://github.com/JarryShaw/PyPCAPKit/issues/2269), [#2344](https://github.com/JarryShaw/PyPCAPKit/pull/2344)).
179179
- 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)).
180180
- 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)).
181+
- `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; both methods walk `type(self).__mro__` rather than reading `self.__slots__` directly, which would otherwise shadow a base class's slots under a subclass declaring its own ([#2423](https://github.com/JarryShaw/PyPCAPKit/issues/2423), [#2425](https://github.com/JarryShaw/PyPCAPKit/issues/2425), [#2424](https://github.com/JarryShaw/PyPCAPKit/pull/2424)).
181182

182183
### pcapkit.interface
183184

‎docs/source/changelog/1.5.0.rst‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1262,6 +1262,16 @@ Fixed
12621262
naming the engine, that they ignore a parsing context supplied through
12631263
:class:`.Extractor`'s ``context=``, as the dpkt, scapy and pyshark engines already
12641264
did, where they dropped it silently (:issue:`2313`, :pr:`2332`).
1265+
* :class:`~pcapkit.foundation.reassembly.data.data.Deferred` could not be pickled
1266+
under protocol 0 or 1 -- a slotted class with no ``__getstate__`` raises
1267+
:exc:`TypeError` there, independent of the ``analyze``/``proto`` regression
1268+
:issue:`2417` fixed. It now defines ``__getstate__`` and ``__setstate__``,
1269+
mirroring the ``(dict_or_None, slots)`` pair Python's own default slotted-class
1270+
reduction already used for every released build, so a ``Deferred`` pickled by
1271+
one of those still unpickles; both methods walk ``type(self).__mro__`` rather
1272+
than reading ``self.__slots__`` directly, which would otherwise shadow a base
1273+
class's slots under a subclass declaring its own (:issue:`2423`, :issue:`2425`,
1274+
:pr:`2424`).
12651275

12661276
pcapkit.interface
12671277
-----------------

‎pcapkit/foundation/reassembly/data/data.py‎

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
__all__ = ['ReassemblyData', 'Completion', 'Deferred']
1111

1212
if TYPE_CHECKING:
13-
from typing import Callable, Optional
13+
from typing import Any, Callable, Optional
1414

1515
from pcapkit.const.reg.transtype import TransType
1616
from pcapkit.foundation.reassembly.data.ip import Datagram as IP_Datagram
@@ -206,6 +206,70 @@ def __call__(self) -> 'ProtocolBase':
206206
"""
207207
return self.analyze(self.proto, self.payload)
208208

209+
def __getstate__(self) -> 'tuple[Optional[dict[str, Any]], dict[str, Any]]':
210+
"""Pickling support.
211+
212+
A slotted class with no ``__dict__`` has nothing for the default
213+
:mod:`pickle` reduction to read. :mod:`copyreg` refuses outright with
214+
*"cannot be pickled"* under protocol 0 or 1 whenever
215+
``type(self).__getstate__`` is still the inherited default -- which is
216+
why defining *any* ``__getstate__`` here, even one that only calls the
217+
default, clears protocols 0 and 1 (:issue:`2423`). Protocol 2 and above
218+
never needed one: their own ``__reduce_ex__`` already walks
219+
``__slots__`` directly.
220+
221+
The shape returned -- an instance-``__dict__`` state (or :data:`None`)
222+
paired with a slots-state :obj:`dict` -- is deliberately the same one
223+
Python's own default state construction produces for a slotted class,
224+
rather than a bespoke flat :obj:`dict`. A flat shape would be a *new*
225+
pickle format for this class: a ``Deferred`` already pickled by a
226+
released version under protocol 2 or above -- every one of them built
227+
this same ``(dict_or_None, slots)`` pair -- would fail to unpickle
228+
against a flat-shaped :meth:`__setstate__`, and the reverse. Walking
229+
``type(self).__mro__`` rather than reading ``self.__slots__`` directly
230+
is what keeps that promise for a subclass that declares its own
231+
slots: ``self.__slots__`` resolves through the MRO like any other
232+
attribute, so it would silently return only the *subclass's* tuple,
233+
dropping every slot ``Deferred`` itself declares (:issue:`2425`).
234+
235+
"""
236+
slots: 'dict[str, Any]' = {}
237+
for cls in type(self).__mro__:
238+
for slot in getattr(cls, '__slots__', ()):
239+
# ``__dict__`` and ``__weakref__`` are bookkeeping slots a class
240+
# may declare to opt back into one; neither is a real attribute
241+
# to carry here -- the instance ``__dict__`` state below covers
242+
# the first, and a weak reference cannot survive a pickle round
243+
# trip regardless.
244+
if slot in ('__dict__', '__weakref__') or slot in slots:
245+
continue
246+
if hasattr(self, slot):
247+
slots[slot] = getattr(self, slot)
248+
dict_state = dict(self.__dict__) if getattr(self, '__dict__', None) else None
249+
return (dict_state, slots)
250+
251+
def __setstate__(
252+
self,
253+
state: 'tuple[Optional[dict[str, Any]], dict[str, Any]] | dict[str, Any]',
254+
) -> 'None':
255+
"""Unpickling support; the counterpart to :meth:`__getstate__`.
256+
257+
Accepts the ``(dict_or_None, slots)`` pair :meth:`__getstate__` builds
258+
above -- the shape every released, pre-:issue:`2423` pickle already
259+
used under protocol 2 and above -- and, for a pickle made by this
260+
class's own first, unreleased fix for :issue:`2423`, a bare :obj:`dict`
261+
of ``{slot: value}`` (:issue:`2425`).
262+
263+
"""
264+
if isinstance(state, dict):
265+
dict_state, slots = None, state
266+
else:
267+
dict_state, slots = state
268+
if dict_state:
269+
self.__dict__.update(dict_state)
270+
for slot, value in slots.items():
271+
setattr(self, slot, value)
272+
209273

210274
@info_final
211275
class ReassemblyData(Info):

0 commit comments

Comments
 (0)