From 92f5a27115b7cd569cf4eac6291e968d1a612975 Mon Sep 17 00:00:00 2001 From: Zackery Spytz Date: Tue, 14 Jul 2020 23:55:48 -0600 Subject: [PATCH 1/4] bpo-33007: Name-mangled private methods don't roundtrip when pickling --- Lib/test/pickletester.py | 16 ++++++++++++++++ .../2020-07-14-23-54-18.bpo-33007.TyI3_Q.rst | 1 + Objects/classobject.c | 14 +++++++++++++- 3 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Library/2020-07-14-23-54-18.bpo-33007.TyI3_Q.rst diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index fb972a3ba5e9b0..2a3a0c6ab26c7b 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -150,6 +150,15 @@ def __reduce__(self): # Shouldn't support the recursion itself return K, (self.value,) + +class L: + def __init__(self): + self.attr = self.__foo + + def __foo(self): + return 42 + + import __main__ __main__.C = C C.__module__ = "__main__" @@ -2249,6 +2258,13 @@ def test_many_puts_and_gets(self): loaded = self.loads(dumped) self.assert_is_copy(obj, loaded) + def test_mangled_private_methods_roundtrip(self): + obj = L() + for proto in protocols: + with self.subTest(proto=proto): + loaded = self.loads(self.dumps(obj, proto)) + self.assertEqual(loaded.attr(), 42) + def test_attribute_name_interning(self): # Test that attribute names of pickled objects are interned when # unpickling. diff --git a/Misc/NEWS.d/next/Library/2020-07-14-23-54-18.bpo-33007.TyI3_Q.rst b/Misc/NEWS.d/next/Library/2020-07-14-23-54-18.bpo-33007.TyI3_Q.rst new file mode 100644 index 00000000000000..3e956409d52a58 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2020-07-14-23-54-18.bpo-33007.TyI3_Q.rst @@ -0,0 +1 @@ +The :mod:`pickle` module now properly handles name-mangled private methods. diff --git a/Objects/classobject.c b/Objects/classobject.c index fd9f8757f84ab4..f8d2aa9e3a1078 100644 --- a/Objects/classobject.c +++ b/Objects/classobject.c @@ -128,8 +128,20 @@ method_reduce(PyMethodObject *im, PyObject *Py_UNUSED(ignored)) if (funcname == NULL) { return NULL; } + PyObject *name = _PyObject_GetAttrId((PyObject *)Py_TYPE(self), + &PyId___name__); + if (name == NULL) { + Py_DECREF(funcname); + return NULL; + } + PyObject *mangled = _Py_Mangle(name, funcname); + Py_DECREF(name); + Py_DECREF(funcname); + if (mangled == NULL) { + return NULL; + } return Py_BuildValue("N(ON)", _PyEval_GetBuiltinId(&PyId_getattr), - self, funcname); + self, mangled); } static PyMethodDef method_methods[] = { From 90cf9b3f29ae4cd3ae1873eafcef92d29bc6fbcd Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Wed, 29 Nov 2023 17:19:25 +0200 Subject: [PATCH 2/4] Optimize. --- Objects/classobject.c | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/Objects/classobject.c b/Objects/classobject.c index bd2c9f103a256b..5f4e22e0283662 100644 --- a/Objects/classobject.c +++ b/Objects/classobject.c @@ -135,23 +135,31 @@ method___reduce___impl(PyMethodObject *self) PyObject *funcself = PyMethod_GET_SELF(self); PyObject *func = PyMethod_GET_FUNCTION(self); PyObject *funcname = PyObject_GetAttr(func, &_Py_ID(__name__)); + Py_ssize_t len; if (funcname == NULL) { return NULL; } - PyObject *name = PyObject_GetAttr((PyObject *)Py_TYPE(funcself), - &_Py_ID(__name__)); - if (name == NULL) { - Py_DECREF(funcname); - return NULL; - } - PyObject *mangled = _Py_Mangle(name, funcname); - Py_DECREF(name); - Py_DECREF(funcname); - if (mangled == NULL) { - return NULL; + if (PyUnicode_Check(funcname) && + (len = PyUnicode_GET_LENGTH(funcname)) > 2 && + PyUnicode_READ_CHAR(funcname, 0) == '_' && + PyUnicode_READ_CHAR(funcname, 1) == '_' && + !(PyUnicode_READ_CHAR(funcname, len-1) == '_' && + PyUnicode_READ_CHAR(funcname, len-2) == '_')) + { + PyObject *name = PyObject_GetAttr((PyObject *)Py_TYPE(funcself), + &_Py_ID(__name__)); + if (name == NULL) { + Py_DECREF(funcname); + return NULL; + } + Py_SETREF(funcname, _Py_Mangle(name, funcname)); + Py_DECREF(name); + if (funcname == NULL) { + return NULL; + } } return Py_BuildValue( - "N(ON)", _PyEval_GetBuiltin(&_Py_ID(getattr)), funcself, mangled); + "N(ON)", _PyEval_GetBuiltin(&_Py_ID(getattr)), funcself, funcname); } static PyMethodDef method_methods[] = { From 1dbe82ab1892fdb3ca2946b8b1ecb7b78cd86898 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Tue, 6 Jan 2026 18:01:49 +0200 Subject: [PATCH 3/4] Refactor tests. --- Lib/test/pickletester.py | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index e0ed00b642a88c..15b026b07ba30a 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -174,13 +174,15 @@ def __reduce__(self): # Shouldn't support the recursion itself return K, (self.value,) -class L: - def __init__(self): - self.attr = self.__foo +class PrivateMethods: + def __init__(self, value): + self.value = value - def __foo(self): - return 42 + def __private_method(self): + return self.value + def get_method(self): + return self.__private_method class myint(int): def __init__(self, x): @@ -3663,13 +3665,6 @@ def test_many_puts_and_gets(self): loaded = self.loads(dumped) self.assert_is_copy(obj, loaded) - def test_mangled_private_methods_roundtrip(self): - obj = L() - for proto in protocols: - with self.subTest(proto=proto): - loaded = self.loads(self.dumps(obj, proto)) - self.assertEqual(loaded.attr(), 42) - def test_attribute_name_interning(self): # Test that attribute names of pickled objects are interned when # unpickling. @@ -4087,6 +4082,13 @@ class Nested(str): with self.subTest(proto=proto, descr=descr): self.assertRaises(TypeError, self.dumps, descr, proto) + def test_private_methods(self): + obj = PrivateMethods(42) + for proto in protocols: + with self.subTest(proto=proto): + unpickled = self.loads(self.dumps(obj.get_method(), proto)) + self.assertEqual(unpickled(), 42) + def test_compat_pickle(self): tests = [ (range(1, 7), '__builtin__', 'xrange'), From 6317677c24cbeb8fa97e87c2ef673d7db0e1fed3 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Tue, 6 Jan 2026 18:05:07 +0200 Subject: [PATCH 4/4] Add tests for other types of methods. --- Lib/test/pickletester.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index 15b026b07ba30a..ba382cbc69501a 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -184,6 +184,26 @@ def __private_method(self): def get_method(self): return self.__private_method + @classmethod + def get_unbound_method(cls): + return cls.__private_method + + @classmethod + def __private_classmethod(cls): + return 43 + + @classmethod + def get_classmethod(cls): + return cls.__private_classmethod + + @staticmethod + def __private_staticmethod(): + return 44 + + @classmethod + def get_staticmethod(cls): + return cls.__private_staticmethod + class myint(int): def __init__(self, x): self.str = str(x) @@ -4088,6 +4108,12 @@ def test_private_methods(self): with self.subTest(proto=proto): unpickled = self.loads(self.dumps(obj.get_method(), proto)) self.assertEqual(unpickled(), 42) + unpickled = self.loads(self.dumps(obj.get_unbound_method(), proto)) + self.assertEqual(unpickled(obj), 42) + unpickled = self.loads(self.dumps(obj.get_classmethod(), proto)) + self.assertEqual(unpickled(), 43) + unpickled = self.loads(self.dumps(obj.get_staticmethod(), proto)) + self.assertEqual(unpickled(), 44) def test_compat_pickle(self): tests = [