From 82e4de552ada3131e4748b3a8883b7f63a479f1c Mon Sep 17 00:00:00 2001 From: Michael Seifert Date: Sun, 12 Mar 2017 21:54:25 +0100 Subject: [PATCH 1/4] bpo-29800: Check if the keys of partial.keywords are unicodes in partial.__repr__ --- Lib/test/test_functools.py | 13 +++++++++++++ Misc/ACKS | 1 + Modules/_functoolsmodule.c | 5 +++++ 3 files changed, 19 insertions(+) diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py index 612ca17a60bc8cd..3708de2880a9d23 100644 --- a/Lib/test/test_functools.py +++ b/Lib/test/test_functools.py @@ -403,6 +403,19 @@ def test_attributes_unwritable(self): else: self.fail('partial object allowed __dict__ to be deleted') + def test_manually_adding_non_string_keyword(self): + p = self.partial(capture) + # Adding a non-string/unicode keyword to partial kwargs + p.keywords[1] = 10 + self.assertRaises(TypeError, repr, p) + try: + p() + except TypeError: + pass + else: + self.fail('partial object allowed passing non-string keywords') + + class TestPartialPy(TestPartial, unittest.TestCase): partial = py_functools.partial diff --git a/Misc/ACKS b/Misc/ACKS index b7f1282c69c0ce0..c99aeaa335e9713 100644 --- a/Misc/ACKS +++ b/Misc/ACKS @@ -1374,6 +1374,7 @@ Federico Schwindt Barry Scott Steven Scott Nick Seidenman +Michael Seifert Žiga Seilnacht Yury Selivanov Fred Sells diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c index dcaa99f91fb394f..a457d252950f4fd 100644 --- a/Modules/_functoolsmodule.c +++ b/Modules/_functoolsmodule.c @@ -253,6 +253,11 @@ partial_repr(partialobject *pto) /* Pack keyword arguments */ assert (PyDict_Check(pto->kw)); for (i = 0; PyDict_Next(pto->kw, &i, &key, &value);) { + if (!PyUnicode_Check(key)) { + PyErr_SetString(PyExc_TypeError, "keywords must be strings"); + Py_DECREF(arglist); + goto done; + } Py_SETREF(arglist, PyUnicode_FromFormat("%U, %U=%R", arglist, key, value)); if (arglist == NULL) From cf9a03e9c42d1f41f94b55c167e81e986fd7e9aa Mon Sep 17 00:00:00 2001 From: Michael Seifert Date: Tue, 14 Mar 2017 15:23:12 +0100 Subject: [PATCH 2/4] bpo-29800: Use %S formatting for keys in partial.__repr__ --- Lib/test/test_functools.py | 16 +++++++++++++++- Modules/_functoolsmodule.c | 12 ++++++------ 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py index 3708de2880a9d23..bdf99c26dcbc6bf 100644 --- a/Lib/test/test_functools.py +++ b/Lib/test/test_functools.py @@ -407,7 +407,7 @@ def test_manually_adding_non_string_keyword(self): p = self.partial(capture) # Adding a non-string/unicode keyword to partial kwargs p.keywords[1] = 10 - self.assertRaises(TypeError, repr, p) + self.assertIn('1=10', repr(p)) try: p() except TypeError: @@ -415,6 +415,20 @@ def test_manually_adding_non_string_keyword(self): else: self.fail('partial object allowed passing non-string keywords') + # Deleting the key-value pair during key formatting shouldn't segfault. + class MutatesYourDict(object): + def __init__(self, dct): + self.dct = dct + def __str__(self): + del self.dct[self] + return 'astr' + def __repr__(self): + return 'arepr' + + p = partial(capture) + p.keywords[MutatesYourDict(p.keywords)] = MutatesYourDict(p.keywords) + self.assertIn('astr=arepr', repr(p)) + class TestPartialPy(TestPartial, unittest.TestCase): partial = py_functools.partial diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c index a457d252950f4fd..2619c6f1c9302ce 100644 --- a/Modules/_functoolsmodule.c +++ b/Modules/_functoolsmodule.c @@ -253,13 +253,13 @@ partial_repr(partialobject *pto) /* Pack keyword arguments */ assert (PyDict_Check(pto->kw)); for (i = 0; PyDict_Next(pto->kw, &i, &key, &value);) { - if (!PyUnicode_Check(key)) { - PyErr_SetString(PyExc_TypeError, "keywords must be strings"); - Py_DECREF(arglist); - goto done; - } - Py_SETREF(arglist, PyUnicode_FromFormat("%U, %U=%R", arglist, + /* Prevent key.__str__ from deleting key or value during formatting. */ + Py_INCREF(key); + Py_INCREF(value); + Py_SETREF(arglist, PyUnicode_FromFormat("%U, %S=%R", arglist, key, value)); + Py_DECREF(key); + Py_DECREF(value); if (arglist == NULL) goto done; } From 286faeae73f8df32ac5a9d896411f028875739b9 Mon Sep 17 00:00:00 2001 From: Michael Seifert Date: Tue, 14 Mar 2017 17:50:38 +0100 Subject: [PATCH 3/4] bpo-29800: Simplified and robustified tests and revert incref'ing the key --- Lib/test/test_functools.py | 31 +++++++++++++++---------------- Modules/_functoolsmodule.c | 4 +--- 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py index bdf99c26dcbc6bf..29ea49362262dd0 100644 --- a/Lib/test/test_functools.py +++ b/Lib/test/test_functools.py @@ -406,28 +406,27 @@ def test_attributes_unwritable(self): def test_manually_adding_non_string_keyword(self): p = self.partial(capture) # Adding a non-string/unicode keyword to partial kwargs - p.keywords[1] = 10 - self.assertIn('1=10', repr(p)) - try: + p.keywords[1234] = 'value' + r = repr(p) + self.assertIn('1234', r) + self.assertIn("'value'", r) + with self.assertRaises(TypeError): p() - except TypeError: - pass - else: - self.fail('partial object allowed passing non-string keywords') - # Deleting the key-value pair during key formatting shouldn't segfault. + def test_keystr_replaces_value(self): + p = self.partial(capture) + class MutatesYourDict(object): - def __init__(self, dct): - self.dct = dct def __str__(self): - del self.dct[self] + p.keywords[self] = ['sth2'] return 'astr' - def __repr__(self): - return 'arepr' - p = partial(capture) - p.keywords[MutatesYourDict(p.keywords)] = MutatesYourDict(p.keywords) - self.assertIn('astr=arepr', repr(p)) + # Raplacing the value during key formatting should keep the original + # value alive (at least long enough). + p.keywords[MutatesYourDict()] = ['sth'] + r = repr(p) + self.assertIn('astr', r) + self.assertIn("['sth']", r) class TestPartialPy(TestPartial, unittest.TestCase): diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c index 2619c6f1c9302ce..487470310432efb 100644 --- a/Modules/_functoolsmodule.c +++ b/Modules/_functoolsmodule.c @@ -253,12 +253,10 @@ partial_repr(partialobject *pto) /* Pack keyword arguments */ assert (PyDict_Check(pto->kw)); for (i = 0; PyDict_Next(pto->kw, &i, &key, &value);) { - /* Prevent key.__str__ from deleting key or value during formatting. */ - Py_INCREF(key); + /* Prevent key.__str__ from deleting the value. */ Py_INCREF(value); Py_SETREF(arglist, PyUnicode_FromFormat("%U, %S=%R", arglist, key, value)); - Py_DECREF(key); Py_DECREF(value); if (arglist == NULL) goto done; From 31cecca961fb0f94c6bf108e9b63c23a0d34c469 Mon Sep 17 00:00:00 2001 From: Michael Seifert Date: Tue, 14 Mar 2017 23:56:52 +0100 Subject: [PATCH 4/4] bpo-29800: Add news entry --- Misc/NEWS | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Misc/NEWS b/Misc/NEWS index 72404b3ac2413d6..65fc398a1b2eecf 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -278,6 +278,9 @@ Extension Modules Library ------- +- bpo-29800: Fix crashes in partial.__repr__ if the keys of partial.keywords + are not strings. Patch by Michael Seifert. + - bpo-8256: Fixed possible failing or crashing input() if attributes "encoding" or "errors" of sys.stdin or sys.stdout are not set or are not strings.