diff --git a/Lib/pickle.py b/Lib/pickle.py index c4fc2c4d1f6..372d5b623ac 100644 --- a/Lib/pickle.py +++ b/Lib/pickle.py @@ -1034,19 +1034,15 @@ class _Unpickler: def _instantiate(self, klass, k): args = tuple(self.stack[k+1:]) del self.stack[k:] - instantiated = False - if (not args and - isinstance(klass, type) and - not hasattr(klass, "__getinitargs__")): - value = _EmptyClass() - value.__class__ = klass - instantiated = True - if not instantiated: + if (args or not isinstance(klass, type) or + hasattr(klass, "__getinitargs__")): try: value = klass(*args) except TypeError as err: raise TypeError("in constructor for %s: %s" % (klass.__name__, str(err)), sys.exc_info()[2]) + else: + value = klass.__new__(klass) self.append(value) def load_inst(self): @@ -1239,14 +1235,7 @@ class _Unpickler: raise _Stop(value) dispatch[STOP[0]] = load_stop -# Helper class for load_inst/load_obj - -class _EmptyClass: - pass - -# Encode/decode longs in linear time. - -import binascii as _binascii +# Encode/decode longs. def encode_long(x): r"""Encode a long to a two's complement little-endian binary string. diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index dd0ed15f2db..70e5ad0fcf7 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -65,9 +65,21 @@ class C: def __eq__(self, other): return self.__dict__ == other.__dict__ +class D(C): + def __init__(self, arg): + pass + +class E(C): + def __getinitargs__(self): + return () + import __main__ __main__.C = C C.__module__ = "__main__" +__main__.D = D +D.__module__ = "__main__" +__main__.E = E +E.__module__ = "__main__" class myint(int): def __init__(self, x): @@ -425,6 +437,63 @@ class AbstractPickleTests(unittest.TestCase): def test_load_from_data2(self): self.assertEqual(self._testdata, self.loads(DATA2)) + def test_load_classic_instance(self): + # See issue5180. Test loading 2.x pickles that + # contain an instance of old style class. + for X, args in [(C, ()), (D, ('x',)), (E, ())]: + xname = X.__name__.encode('ascii') + # Protocol 0 (text mode pickle): + """ + 0: ( MARK + 1: i INST '__main__ X' (MARK at 0) + 15: p PUT 0 + 18: ( MARK + 19: d DICT (MARK at 18) + 20: p PUT 1 + 23: b BUILD + 24: . STOP + """ + pickle0 = (b"(i__main__\n" + b"X\n" + b"p0\n" + b"(dp1\nb.").replace(b'X', xname) + self.assertEqual(X(*args), self.loads(pickle0)) + + # Protocol 1 (binary mode pickle) + """ + 0: ( MARK + 1: c GLOBAL '__main__ X' + 15: q BINPUT 0 + 17: o OBJ (MARK at 0) + 18: q BINPUT 1 + 20: } EMPTY_DICT + 21: q BINPUT 2 + 23: b BUILD + 24: . STOP + """ + pickle1 = (b'(c__main__\n' + b'X\n' + b'q\x00oq\x01}q\x02b.').replace(b'X', xname) + self.assertEqual(X(*args), self.loads(pickle1)) + + # Protocol 2 (pickle2 = b'\x80\x02' + pickle1) + """ + 0: \x80 PROTO 2 + 2: ( MARK + 3: c GLOBAL '__main__ X' + 17: q BINPUT 0 + 19: o OBJ (MARK at 2) + 20: q BINPUT 1 + 22: } EMPTY_DICT + 23: q BINPUT 2 + 25: b BUILD + 26: . STOP + """ + pickle2 = (b'\x80\x02(c__main__\n' + b'X\n' + b'q\x00oq\x01}q\x02b.').replace(b'X', xname) + self.assertEqual(X(*args), self.loads(pickle2)) + # There are gratuitous differences between pickles produced by # pickle and cPickle, largely because cPickle starts PUT indices at # 1 and pickle starts them at 0. See XXX comment in cPickle's put2() -- diff --git a/Misc/NEWS b/Misc/NEWS index 04309a7f23b..afe263ad218 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -1429,6 +1429,9 @@ Library Extension Modules ----------------- +- Issue #5180: Fixed a bug that prevented loading 2.x pickles in 3.x + python when they contain instances of old-style classes. + - Issue #9165: Add new functions math.isfinite and cmath.isfinite, to accompany existing isinf and isnan functions. diff --git a/Modules/_pickle.c b/Modules/_pickle.c index 0e1c2cdc882..96f194ecd4d 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -3466,29 +3466,19 @@ load_dict(UnpicklerObject *self) static PyObject * instantiate(PyObject *cls, PyObject *args) { - PyObject *r = NULL; - - /* XXX: The pickle.py module does not create instances this way when the - args tuple is empty. See Unpickler._instantiate(). */ - if ((r = PyObject_CallObject(cls, args))) - return r; - - /* XXX: Is this still nescessary? */ - { - PyObject *tp, *v, *tb, *tmp_value; - - PyErr_Fetch(&tp, &v, &tb); - tmp_value = v; - /* NULL occurs when there was a KeyboardInterrupt */ - if (tmp_value == NULL) - tmp_value = Py_None; - if ((r = PyTuple_Pack(3, tmp_value, cls, args))) { - Py_XDECREF(v); - v = r; - } - PyErr_Restore(tp, v, tb); + PyObject *result = NULL; + /* Caller must assure args are a tuple. Normally, args come from + Pdata_poptuple which packs objects from the top of the stack + into a newly created tuple. */ + assert(PyTuple_Check(args)); + if (Py_SIZE(args) > 0 || !PyType_Check(cls) || + PyObject_HasAttrString(cls, "__getinitargs__")) { + result = PyObject_CallObject(cls, args); } - return NULL; + else { + result = PyObject_CallMethod(cls, "__new__", "O", cls); + } + return result; } static int @@ -3547,7 +3537,7 @@ load_inst(UnpicklerObject *self) if (len < 2) return bad_readline(); class_name = PyUnicode_DecodeASCII(s, len - 1, "strict"); - if (class_name == NULL) { + if (class_name != NULL) { cls = find_class(self, module_name, class_name); Py_DECREF(class_name); } @@ -4276,7 +4266,7 @@ load_reduce(UnpicklerObject *self) return -1; PDATA_POP(self->stack, callable); if (callable) { - obj = instantiate(callable, argtup); + obj = PyObject_CallObject(callable, argtup); Py_DECREF(callable); } Py_DECREF(argtup);