From 71b71763c5ae3fca539c3faff69ebdeaeaa84a80 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Tue, 2 Feb 2016 18:45:59 +0200 Subject: [PATCH] Issue #25945: Fixed bugs in functools.partial. Fixed a crash when unpickle the functools.partial object with wrong state. Fixed a leak in failed functools.partial constructor. "args" and "keywords" attributes of functools.partial have now always types tuple and dict correspondingly. --- Lib/test/test_functools.py | 101 +++++++++++++++++++++++++++++++++++-- Misc/NEWS | 5 ++ Modules/_functoolsmodule.c | 66 +++++++++++++++--------- 3 files changed, 142 insertions(+), 30 deletions(-) diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py index 7b3cb96497..69176f434e 100644 --- a/Lib/test/test_functools.py +++ b/Lib/test/test_functools.py @@ -1,3 +1,4 @@ +import copy import functools import sys import unittest @@ -25,6 +26,16 @@ def signature(part): """ return the signature of a partial object """ return (part.func, part.args, part.keywords, part.__dict__) +class MyTuple(tuple): + pass + +class BadTuple(tuple): + def __add__(self, other): + return list(self) + list(other) + +class MyDict(dict): + pass + class TestPartial(unittest.TestCase): thetype = functools.partial @@ -146,11 +157,84 @@ class TestPartial(unittest.TestCase): self.assertEqual(join(data), '0123456789') def test_pickle(self): - f = self.thetype(signature, 'asdf', bar=True) - f.add_something_to__dict__ = True + f = self.thetype(signature, ['asdf'], bar=[True]) + f.attr = [] for proto in range(pickle.HIGHEST_PROTOCOL + 1): f_copy = pickle.loads(pickle.dumps(f, proto)) - self.assertEqual(signature(f), signature(f_copy)) + self.assertEqual(signature(f_copy), signature(f)) + + def test_copy(self): + f = self.thetype(signature, ['asdf'], bar=[True]) + f.attr = [] + f_copy = copy.copy(f) + self.assertEqual(signature(f_copy), signature(f)) + self.assertIs(f_copy.attr, f.attr) + self.assertIs(f_copy.args, f.args) + self.assertIs(f_copy.keywords, f.keywords) + + def test_deepcopy(self): + f = self.thetype(signature, ['asdf'], bar=[True]) + f.attr = [] + f_copy = copy.deepcopy(f) + self.assertEqual(signature(f_copy), signature(f)) + self.assertIsNot(f_copy.attr, f.attr) + self.assertIsNot(f_copy.args, f.args) + self.assertIsNot(f_copy.args[0], f.args[0]) + self.assertIsNot(f_copy.keywords, f.keywords) + self.assertIsNot(f_copy.keywords['bar'], f.keywords['bar']) + + def test_setstate(self): + f = self.thetype(signature) + f.__setstate__((capture, (1,), dict(a=10), dict(attr=[]))) + self.assertEqual(signature(f), + (capture, (1,), dict(a=10), dict(attr=[]))) + self.assertEqual(f(2, b=20), ((1, 2), {'a': 10, 'b': 20})) + + f.__setstate__((capture, (1,), dict(a=10), None)) + self.assertEqual(signature(f), (capture, (1,), dict(a=10), {})) + self.assertEqual(f(2, b=20), ((1, 2), {'a': 10, 'b': 20})) + + f.__setstate__((capture, (1,), None, None)) + #self.assertEqual(signature(f), (capture, (1,), {}, {})) + self.assertEqual(f(2, b=20), ((1, 2), {'b': 20})) + self.assertEqual(f(2), ((1, 2), {})) + self.assertEqual(f(), ((1,), {})) + + f.__setstate__((capture, (), {}, None)) + self.assertEqual(signature(f), (capture, (), {}, {})) + self.assertEqual(f(2, b=20), ((2,), {'b': 20})) + self.assertEqual(f(2), ((2,), {})) + self.assertEqual(f(), ((), {})) + + def test_setstate_errors(self): + f = self.thetype(signature) + self.assertRaises(TypeError, f.__setstate__, (capture, (), {})) + self.assertRaises(TypeError, f.__setstate__, (capture, (), {}, {}, None)) + self.assertRaises(TypeError, f.__setstate__, [capture, (), {}, None]) + self.assertRaises(TypeError, f.__setstate__, (None, (), {}, None)) + self.assertRaises(TypeError, f.__setstate__, (capture, None, {}, None)) + self.assertRaises(TypeError, f.__setstate__, (capture, [], {}, None)) + self.assertRaises(TypeError, f.__setstate__, (capture, (), [], None)) + + def test_setstate_subclasses(self): + f = self.thetype(signature) + f.__setstate__((capture, MyTuple((1,)), MyDict(a=10), None)) + s = signature(f) + self.assertEqual(s, (capture, (1,), dict(a=10), {})) + self.assertIs(type(s[1]), tuple) + self.assertIs(type(s[2]), dict) + r = f() + self.assertEqual(r, ((1,), {'a': 10})) + self.assertIs(type(r[0]), tuple) + self.assertIs(type(r[1]), dict) + + f.__setstate__((capture, BadTuple((1,)), {}, None)) + s = signature(f) + self.assertEqual(s, (capture, (1,), {}, {})) + self.assertIs(type(s[1]), tuple) + r = f(2) + self.assertEqual(r, ((1, 2), {})) + self.assertIs(type(r[0]), tuple) # Issue 6083: Reference counting bug def test_setstate_refcount(self): @@ -167,7 +251,7 @@ class TestPartial(unittest.TestCase): raise IndexError f = self.thetype(object) - self.assertRaises(SystemError, f.__setstate__, BadSequence()) + self.assertRaises(TypeError, f.__setstate__, BadSequence()) class PartialSubclass(functools.partial): pass @@ -181,7 +265,14 @@ class TestPythonPartial(TestPartial): thetype = PythonPartial # the python version isn't picklable - test_pickle = test_setstate_refcount = None + test_pickle = None + test_setstate = None + test_setstate_errors = None + test_setstate_subclasses = None + test_setstate_refcount = None + + # the python version isn't deepcopyable + test_deepcopy = None # the python version isn't a type test_attributes = None diff --git a/Misc/NEWS b/Misc/NEWS index c87237f6c3..92f6c206eb 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -48,6 +48,11 @@ Core and Builtins Library ------- +- Issue #25945: Fixed a crash when unpickle the functools.partial object with + wrong state. Fixed a leak in failed functools.partial constructor. + "args" and "keywords" attributes of functools.partial have now always types + tuple and dict correspondingly. + - Issue #19883: Fixed possible integer overflows in zipimport. - Issue #26147: xmlrpclib now works with unicode not encodable with used diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c index 2ba92bae9d..2ad556d93a 100644 --- a/Modules/_functoolsmodule.c +++ b/Modules/_functoolsmodule.c @@ -128,7 +128,6 @@ partial_new(PyTypeObject *type, PyObject *args, PyObject *kw) Py_INCREF(func); pto->args = PyTuple_GetSlice(args, 1, PY_SSIZE_T_MAX); if (pto->args == NULL) { - pto->kw = NULL; Py_DECREF(pto); return NULL; } @@ -138,10 +137,6 @@ partial_new(PyTypeObject *type, PyObject *args, PyObject *kw) return NULL; } - - pto->weakreflist = NULL; - pto->dict = NULL; - return (PyObject *)pto; } @@ -162,11 +157,11 @@ static PyObject * partial_call(partialobject *pto, PyObject *args, PyObject *kw) { PyObject *ret; - PyObject *argappl = NULL, *kwappl = NULL; + PyObject *argappl, *kwappl; assert (PyCallable_Check(pto->fn)); assert (PyTuple_Check(pto->args)); - assert (pto->kw == Py_None || PyDict_Check(pto->kw)); + assert (PyDict_Check(pto->kw)); if (PyTuple_GET_SIZE(pto->args) == 0) { argappl = args; @@ -178,11 +173,12 @@ partial_call(partialobject *pto, PyObject *args, PyObject *kw) argappl = PySequence_Concat(pto->args, args); if (argappl == NULL) return NULL; + assert(PyTuple_Check(argappl)); } - if (pto->kw == Py_None) { + if (PyDict_Size(pto->kw) == 0) { kwappl = kw; - Py_XINCREF(kw); + Py_XINCREF(kwappl); } else { kwappl = PyDict_Copy(pto->kw); if (kwappl == NULL) { @@ -289,25 +285,45 @@ PyObject * partial_setstate(partialobject *pto, PyObject *state) { PyObject *fn, *fnargs, *kw, *dict; - if (!PyArg_ParseTuple(state, "OOOO", - &fn, &fnargs, &kw, &dict)) + + if (!PyTuple_Check(state) || + !PyArg_ParseTuple(state, "OOOO", &fn, &fnargs, &kw, &dict) || + !PyCallable_Check(fn) || + !PyTuple_Check(fnargs) || + (kw != Py_None && !PyDict_Check(kw))) + { + PyErr_SetString(PyExc_TypeError, "invalid partial state"); return NULL; - Py_XDECREF(pto->fn); - Py_XDECREF(pto->args); - Py_XDECREF(pto->kw); - Py_XDECREF(pto->dict); - pto->fn = fn; - pto->args = fnargs; - pto->kw = kw; - if (dict != Py_None) { - pto->dict = dict; - Py_INCREF(dict); - } else { - pto->dict = NULL; } + + if(!PyTuple_CheckExact(fnargs)) + fnargs = PySequence_Tuple(fnargs); + else + Py_INCREF(fnargs); + if (fnargs == NULL) + return NULL; + + if (kw == Py_None) + kw = PyDict_New(); + else if(!PyDict_CheckExact(kw)) + kw = PyDict_Copy(kw); + else + Py_INCREF(kw); + if (kw == NULL) { + Py_DECREF(fnargs); + return NULL; + } + Py_INCREF(fn); - Py_INCREF(fnargs); - Py_INCREF(kw); + if (dict == Py_None) + dict = NULL; + else + Py_INCREF(dict); + + Py_SETREF(pto->fn, fn); + Py_SETREF(pto->args, fnargs); + Py_SETREF(pto->kw, kw); + Py_SETREF(pto->dict, dict); Py_RETURN_NONE; } -- 2.50.1