From 92f5a27115b7cd569cf4eac6291e968d1a612975 Mon Sep 17 00:00:00 2001 From: Zackery Spytz Date: Tue, 14 Jul 2020 23:55:48 -0600 Subject: [PATCH 1/5] 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/5] 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/5] 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/5] 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 = [ From e11fc8a0cf83e542824b9ecab0b1307dcf42efda Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Thu, 22 Jan 2026 12:24:03 +0200 Subject: [PATCH 5/5] Add support for class methods, unbound methods, static methods and nested classes. --- Doc/whatsnew/3.15.rst | 7 ++++ Include/internal/pycore_symtable.h | 7 +++- Lib/pickle.py | 11 +++++ Lib/test/picklecommon.py | 14 +++++++ Lib/test/pickletester.py | 12 ++++++ ...-07-14-23-54-18.gh-issue-77188.TyI3_Q.rst} | 0 Modules/_pickle.c | 41 +++++++++++++++++++ Objects/classobject.c | 20 ++++----- Python/symtable.c | 21 ++++++++++ 9 files changed, 119 insertions(+), 14 deletions(-) rename Misc/NEWS.d/next/Library/{2020-07-14-23-54-18.bpo-33007.TyI3_Q.rst => 2020-07-14-23-54-18.gh-issue-77188.TyI3_Q.rst} (100%) diff --git a/Doc/whatsnew/3.15.rst b/Doc/whatsnew/3.15.rst index 1d4961d7293631..1fa18e8cc83221 100644 --- a/Doc/whatsnew/3.15.rst +++ b/Doc/whatsnew/3.15.rst @@ -646,6 +646,13 @@ os.path (Contributed by Petr Viktorin for :cve:`2025-4517`.) +pickle +------ + +* Add support for pickling private methods and nested classes. + (Contributed by Zackery Spytz and Serhiy Storchaka in :gh:`77188`.) + + resource -------- diff --git a/Include/internal/pycore_symtable.h b/Include/internal/pycore_symtable.h index 9dbfa913219afa..c0164507ea033e 100644 --- a/Include/internal/pycore_symtable.h +++ b/Include/internal/pycore_symtable.h @@ -151,7 +151,12 @@ extern int _PySymtable_LookupOptional(struct symtable *, void *, PySTEntryObject extern void _PySymtable_Free(struct symtable *); extern PyObject *_Py_MaybeMangle(PyObject *privateobj, PySTEntryObject *ste, PyObject *name); -extern PyObject* _Py_Mangle(PyObject *p, PyObject *name); + +// Export for '_pickle' shared extension +PyAPI_FUNC(PyObject *) +_Py_Mangle(PyObject *, PyObject *); +PyAPI_FUNC(int) +_Py_IsPrivateName(PyObject *); /* Flags for def-use information */ diff --git a/Lib/pickle.py b/Lib/pickle.py index 71c12c50f7f035..3e7cf25cb05337 100644 --- a/Lib/pickle.py +++ b/Lib/pickle.py @@ -1175,6 +1175,17 @@ def save_global(self, obj, name=None): if name is None: name = obj.__name__ + if '.__' in name: + # Mangle names of private attributes. + dotted_path = name.split('.') + for i, subpath in enumerate(dotted_path): + if i and subpath.startswith('__') and not subpath.endswith('__'): + prev = prev.lstrip('_') + if prev: + dotted_path[i] = f"_{prev.lstrip('_')}{subpath}" + prev = subpath + name = '.'.join(dotted_path) + module_name = whichmodule(obj, name) if self.proto >= 2: code = _extension_registry.get((module_name, name), _NoValue) diff --git a/Lib/test/picklecommon.py b/Lib/test/picklecommon.py index f9bd9800928f73..b749ee09f564bf 100644 --- a/Lib/test/picklecommon.py +++ b/Lib/test/picklecommon.py @@ -419,3 +419,17 @@ def __private_staticmethod(): @classmethod def get_staticmethod(cls): return cls.__private_staticmethod + +# For test_private_nested_classes +class PrivateNestedClasses: + @classmethod + def get_nested(cls): + return cls.__Nested + + class __Nested: + @classmethod + def get_nested2(cls): + return cls.__Nested2 + + class __Nested2: + pass diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index ed4139bf1877c2..7b1b117d6d3e32 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -4133,6 +4133,18 @@ def test_private_methods(self): unpickled = self.loads(self.dumps(obj.get_staticmethod(), proto)) self.assertEqual(unpickled(), 44) + def test_private_nested_classes(self): + if self.py_version < (3, 15): + self.skipTest('not supported in Python < 3.15') + cls1 = PrivateNestedClasses.get_nested() + cls2 = cls1.get_nested2() + for proto in protocols: + with self.subTest(proto=proto): + unpickled = self.loads(self.dumps(cls1, proto)) + self.assertIs(unpickled, cls1) + unpickled = self.loads(self.dumps(cls2, proto)) + self.assertIs(unpickled, cls2) + def test_object_with_attrs(self): obj = Object() obj.a = 1 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.gh-issue-77188.TyI3_Q.rst similarity index 100% rename from Misc/NEWS.d/next/Library/2020-07-14-23-54-18.bpo-33007.TyI3_Q.rst rename to Misc/NEWS.d/next/Library/2020-07-14-23-54-18.gh-issue-77188.TyI3_Q.rst diff --git a/Modules/_pickle.c b/Modules/_pickle.c index 063547c9a4d020..a897e45f00fab6 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -19,6 +19,7 @@ #include "pycore_pystate.h" // _PyThreadState_GET() #include "pycore_runtime.h" // _Py_ID() #include "pycore_setobject.h" // _PySet_NextEntry() +#include "pycore_symtable.h" // _Py_Mangle() #include "pycore_sysmodule.h" // _PySys_GetSizeOf() #include "pycore_unicodeobject.h" // _PyUnicode_EqualToASCIIString() @@ -1928,6 +1929,37 @@ get_dotted_path(PyObject *name) return PyUnicode_Split(name, _Py_LATIN1_CHR('.'), -1); } +static PyObject * +join_dotted_path(PyObject *dotted_path) +{ + return PyUnicode_Join(_Py_LATIN1_CHR('.'), dotted_path); +} + +/* Returns -1 (with an exception set) on error, 0 if there were no changes, + * 1 if some names were mangled. */ +static int +mangle_dotted_path(PyObject *dotted_path) +{ + int rc = 0; + Py_ssize_t n = PyList_GET_SIZE(dotted_path); + for (Py_ssize_t i = n-1; i > 0; i--) { + PyObject *subpath = PyList_GET_ITEM(dotted_path, i); + if (_Py_IsPrivateName(subpath)) { + PyObject *parent = PyList_GET_ITEM(dotted_path, i-1); + PyObject *mangled = _Py_Mangle(parent, subpath); + if (mangled == NULL) { + return -1; + } + if (mangled != subpath) { + rc = 1; + } + PyList_SET_ITEM(dotted_path, i, mangled); + Py_DECREF(subpath); + } + } + return rc; +} + static int check_dotted_path(PickleState *st, PyObject *obj, PyObject *dotted_path) { @@ -3809,6 +3841,15 @@ save_global(PickleState *st, PicklerObject *self, PyObject *obj, dotted_path = get_dotted_path(global_name); if (dotted_path == NULL) goto error; + switch (mangle_dotted_path(dotted_path)) { + case -1: + goto error; + case 1: + Py_SETREF(global_name, join_dotted_path(dotted_path)); + if (global_name == NULL) { + goto error; + } + } module_name = whichmodule(st, obj, global_name, dotted_path); if (module_name == NULL) goto error; diff --git a/Objects/classobject.c b/Objects/classobject.c index 166b65fc18680f..4c99c194df53a5 100644 --- a/Objects/classobject.c +++ b/Objects/classobject.c @@ -141,25 +141,19 @@ 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; } - 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) { + if (_Py_IsPrivateName(funcname)) { + PyObject *classname = PyType_Check(funcself) + ? PyType_GetName((PyTypeObject *)funcself) + : PyType_GetName(Py_TYPE(funcself)); + if (classname == NULL) { Py_DECREF(funcname); return NULL; } - Py_SETREF(funcname, _Py_Mangle(name, funcname)); - Py_DECREF(name); + Py_SETREF(funcname, _Py_Mangle(classname, funcname)); + Py_DECREF(classname); if (funcname == NULL) { return NULL; } diff --git a/Python/symtable.c b/Python/symtable.c index 29cf9190a4e95b..29ac8f6880c575 100644 --- a/Python/symtable.c +++ b/Python/symtable.c @@ -3183,6 +3183,27 @@ _Py_MaybeMangle(PyObject *privateobj, PySTEntryObject *ste, PyObject *name) return _Py_Mangle(privateobj, name); } +int +_Py_IsPrivateName(PyObject *ident) +{ + if (!PyUnicode_Check(ident)) { + return 0; + } + Py_ssize_t nlen = PyUnicode_GET_LENGTH(ident); + if (nlen < 3 || + PyUnicode_READ_CHAR(ident, 0) != '_' || + PyUnicode_READ_CHAR(ident, 1) != '_') + { + return 0; + } + if (PyUnicode_READ_CHAR(ident, nlen-1) == '_' && + PyUnicode_READ_CHAR(ident, nlen-2) == '_') + { + return 0; /* Don't mangle __whatever__ */ + } + return 1; +} + PyObject * _Py_Mangle(PyObject *privateobj, PyObject *ident) {