Author: amaury.forgeotdarc Date: Thu Oct 30 22:40:05 2008 New Revision: 67056 Log: Issue #4176: Pickle would crash the interpreter when a __reduce__ function does not return an iterator for the 4th and 5th items. (sequence-like and mapping-like state) Backport of r67049. Modified: python/branches/release25-maint/Lib/test/pickletester.py python/branches/release25-maint/Misc/NEWS python/branches/release25-maint/Modules/cPickle.c Modified: python/branches/release25-maint/Lib/test/pickletester.py ============================================================================== --- python/branches/release25-maint/Lib/test/pickletester.py (original) +++ python/branches/release25-maint/Lib/test/pickletester.py Thu Oct 30 22:40:05 2008 @@ -849,6 +849,29 @@ y = self.loads(s) self.assertEqual(y._reduce_called, 1) + def test_reduce_bad_iterator(self): + # Issue4176: crash when 4th and 5th items of __reduce__() + # are not iterators + class C(object): + def __reduce__(self): + # 4th item is not an iterator + return list, (), None, [], None + class D(object): + def __reduce__(self): + # 5th item is not an iterator + return dict, (), None, None, [] + + # Protocol 0 is less strict and also accept iterables. + for proto in 0, 1, 2: + try: + self.dumps(C(), proto) + except (AttributeError, pickle.PickleError, cPickle.PickleError): + pass + try: + self.dumps(D(), proto) + except (AttributeError, pickle.PickleError, cPickle.PickleError): + pass + # Test classes for reduce_ex class REX_one(object): Modified: python/branches/release25-maint/Misc/NEWS ============================================================================== --- python/branches/release25-maint/Misc/NEWS (original) +++ python/branches/release25-maint/Misc/NEWS Thu Oct 30 22:40:05 2008 @@ -12,6 +12,9 @@ Core and builtins ----------------- +- Issue #4176: Fixed a crash when pickling an object which ``__reduce__`` + method does not return iterators for the 4th and 5th items. + - Issue #3967: Fixed a crash in the count() and find() methods of string-like objects, when the "start" parameter is a huge value. Modified: python/branches/release25-maint/Modules/cPickle.c ============================================================================== --- python/branches/release25-maint/Modules/cPickle.c (original) +++ python/branches/release25-maint/Modules/cPickle.c Thu Oct 30 22:40:05 2008 @@ -2141,13 +2141,14 @@ * appropriate __reduce__ method for ob. */ static int -save_reduce(Picklerobject *self, PyObject *args, PyObject *ob) +save_reduce(Picklerobject *self, PyObject *args, PyObject *fn, PyObject *ob) { PyObject *callable; PyObject *argtup; - PyObject *state = NULL; - PyObject *listitems = NULL; - PyObject *dictitems = NULL; + PyObject *state = NULL; + PyObject *listitems = Py_None; + PyObject *dictitems = Py_None; + Py_ssize_t size; int use_newobj = self->proto >= 2; @@ -2155,6 +2156,14 @@ static char build = BUILD; static char newobj = NEWOBJ; + size = PyTuple_Size(args); + if (size < 2 || size > 5) { + cPickle_ErrFormat(PicklingError, "tuple returned by " + "%s must contain 2 through 5 elements", + "O", fn); + return -1; + } + if (! PyArg_UnpackTuple(args, "save_reduce", 2, 5, &callable, &argtup, @@ -2164,17 +2173,32 @@ return -1; if (!PyTuple_Check(argtup)) { - PyErr_SetString(PicklingError, - "args from reduce() should be a tuple"); + cPickle_ErrFormat(PicklingError, "Second element of " + "tuple returned by %s must be a tuple", + "O", fn); return -1; } if (state == Py_None) state = NULL; + if (listitems == Py_None) listitems = NULL; + else if (!PyIter_Check(listitems)) { + cPickle_ErrFormat(PicklingError, "Fourth element of " + "tuple returned by %s must be an iterator, not %s", + "Os", fn, listitems->ob_type->tp_name); + return -1; + } + if (dictitems == Py_None) dictitems = NULL; + else if (!PyIter_Check(dictitems)) { + cPickle_ErrFormat(PicklingError, "Fifth element of " + "tuple returned by %s must be an iterator, not %s", + "Os", fn, dictitems->ob_type->tp_name); + return -1; + } /* Protocol 2 special case: if callable's name is __newobj__, use * NEWOBJ. This consumes a lot of code. @@ -2298,9 +2322,8 @@ { PyTypeObject *type; PyObject *py_ob_id = 0, *__reduce__ = 0, *t = 0; - PyObject *arg_tup; int res = -1; - int tmp, size; + int tmp; if (self->nesting++ > Py_GetRecursionLimit()){ PyErr_SetString(PyExc_RuntimeError, @@ -2527,30 +2550,14 @@ goto finally; } - if (! PyTuple_Check(t)) { + if (!PyTuple_Check(t)) { cPickle_ErrFormat(PicklingError, "Value returned by " "%s must be string or tuple", "O", __reduce__); goto finally; } - size = PyTuple_Size(t); - if (size < 2 || size > 5) { - cPickle_ErrFormat(PicklingError, "tuple returned by " - "%s must contain 2 through 5 elements", - "O", __reduce__); - goto finally; - } - - arg_tup = PyTuple_GET_ITEM(t, 1); - if (!(PyTuple_Check(arg_tup) || arg_tup == Py_None)) { - cPickle_ErrFormat(PicklingError, "Second element of " - "tuple returned by %s must be a tuple", - "O", __reduce__); - goto finally; - } - - res = save_reduce(self, t, args); + res = save_reduce(self, t, __reduce__, args); finally: self->nesting--;