From 1b3d89b78248a284ced71dabfa534a5c87707bb4 Mon Sep 17 00:00:00 2001 From: Nick Pope Date: Fri, 13 Jan 2023 16:59:24 +0000 Subject: [PATCH] Fix backward compatibility with pickles before v22.2.0 Unfortunately we're using `django-redis` to cache some `attrs` instances and the change to make `__getstate__` and `__setstate__` use `dict` values instead of `tuple` values prevents reading any values from the cache when updating to `attrs>=22.2.0`. We get an `AttributeError` raised for all attributes. This happens because the tuple of values stored in the original pickle is checked for the name of the attribute and so none of the attributes are set on unpickle. To address this I've made a change so that `__setstate__` will still be able to handle values pickled as `tuple`s allowing existing pickles to be unpickled. The test just contains a pre-pickled value due to the complexities around tying to mock methods on a slotted as documented[^1]. I realise that documentation also mentions that we should "think twice" before using pickle, but sometimes those decisions predate our involvement in a project! In addition, pretty much all of Django's cache implementations are built around using pickle by default. This change will allow an easier path to upgrade by unpickling and repickling all values in the cache. Regression in 89a890a3262273514619d1881c6a8b0fe26c66ac. [^1]: https://www.attrs.org/en/latest/glossary.html#term-slotted-classes --- changelog.d/1085.change.md | 1 + src/attr/_make.py | 12 +++++++++--- tests/test_slots.py | 17 +++++++++++++++++ 3 files changed, 27 insertions(+), 3 deletions(-) create mode 100644 changelog.d/1085.change.md diff --git a/changelog.d/1085.change.md b/changelog.d/1085.change.md new file mode 100644 index 000000000..f81545775 --- /dev/null +++ b/changelog.d/1085.change.md @@ -0,0 +1 @@ +Restored ability to unpickle instances pickled before to v22.2.0. diff --git a/src/attr/_make.py b/src/attr/_make.py index 1bb0f5089..014b7bc23 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -934,9 +934,15 @@ def slots_setstate(self, state): Automatically created by attrs. """ __bound_setattr = _obj_setattr.__get__(self) - for name in state_attr_names: - if name in state: - __bound_setattr(name, state[name]) + if isinstance(state, tuple): + # Backward compatibility with attrs instances pickled with + # attrs versions before v22.2.0 which stored tuples. + for name, value in zip(state_attr_names, state): + __bound_setattr(name, value) + else: + for name in state_attr_names: + if name in state: + __bound_setattr(name, state[name]) # The hash code cache is not included when the object is # serialized, but it still needs to be initialized to None to diff --git a/tests/test_slots.py b/tests/test_slots.py index 7902128e0..18513ce73 100644 --- a/tests/test_slots.py +++ b/tests/test_slots.py @@ -801,3 +801,20 @@ class NEW_A: assert new_a.b == 2 assert new_a.c == 3 assert not hasattr(new_a, "d") + + +def test_slots_unpickle_is_backward_compatible(frozen): + """ + Ensure object pickled before v22.2.0 can still be unpickled. + """ + a = A(1, 2, 3) + + a_pickled = ( + b"\x80\x04\x95&\x00\x00\x00\x00\x00\x00\x00\x8c\x10" + + a.__module__.encode() + + b"\x94\x8c\x01A\x94\x93\x94)\x81\x94K\x01K\x02K\x03\x87\x94b." + ) + + a_unpickled = pickle.loads(a_pickled) + + assert a_unpickled == a