Skip to content

Commit d7773d9

Browse files
bennorthorsenthil
authored andcommitted
bpo-18533: Avoid RecursionError from repr() of recursive dictview (#4823)
dictview_repr(): Use a Py_ReprEnter() / Py_ReprLeave() pair to check for recursion, and produce "..." if so. test_recursive_repr(): Check for the string rather than a RecursionError. (Test cannot be any tighter as contents are implementation-dependent.) test_deeply_nested_repr(): Add new test, replacing the original test_recursive_repr(). It checks that a RecursionError is raised in the case of a non-recursive but deeply nested structure. (Very similar to what test_repr_deep() in test/test_dict.py does for a normal dict.) OrderedDictTests: Add new test case, to test behavior on OrderedDict instances containing their own values() or items().
1 parent e76daeb commit d7773d9

File tree

4 files changed

+44
-4
lines changed

4 files changed

+44
-4
lines changed

Lib/test/test_dictviews.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import collections.abc
22
import copy
33
import pickle
4+
import sys
45
import unittest
56

67
class DictSetTest(unittest.TestCase):
@@ -202,6 +203,20 @@ def test_items_set_operations(self):
202203
def test_recursive_repr(self):
203204
d = {}
204205
d[42] = d.values()
206+
r = repr(d)
207+
# Cannot perform a stronger test, as the contents of the repr
208+
# are implementation-dependent. All we can say is that we
209+
# want a str result, not an exception of any sort.
210+
self.assertIsInstance(r, str)
211+
d[42] = d.items()
212+
r = repr(d)
213+
# Again.
214+
self.assertIsInstance(r, str)
215+
216+
def test_deeply_nested_repr(self):
217+
d = {}
218+
for i in range(sys.getrecursionlimit() + 100):
219+
d = {42: d.values()}
205220
self.assertRaises(RecursionError, repr, d)
206221

207222
def test_copy(self):

Lib/test/test_ordered_dict.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,20 @@ def test_repr_recursive(self):
355355
self.assertEqual(repr(od),
356356
"OrderedDict([('a', None), ('b', None), ('c', None), ('x', ...)])")
357357

358+
def test_repr_recursive_values(self):
359+
OrderedDict = self.OrderedDict
360+
od = OrderedDict()
361+
od[42] = od.values()
362+
r = repr(od)
363+
# Cannot perform a stronger test, as the contents of the repr
364+
# are implementation-dependent. All we can say is that we
365+
# want a str result, not an exception of any sort.
366+
self.assertIsInstance(r, str)
367+
od[42] = od.items()
368+
r = repr(od)
369+
# Again.
370+
self.assertIsInstance(r, str)
371+
358372
def test_setdefault(self):
359373
OrderedDict = self.OrderedDict
360374
pairs = [('c', 1), ('b', 2), ('a', 3), ('d', 4), ('e', 5), ('f', 6)]
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
``repr()`` on a dict containing its own ``values()`` or ``items()`` no
2+
longer raises ``RecursionError``; OrderedDict similarly. Instead, use
3+
``...``, as for other recursive structures. Patch by Ben North.

Objects/dictobject.c

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3853,14 +3853,22 @@ static PyObject *
38533853
dictview_repr(_PyDictViewObject *dv)
38543854
{
38553855
PyObject *seq;
3856-
PyObject *result;
3856+
PyObject *result = NULL;
3857+
Py_ssize_t rc;
38573858

3859+
rc = Py_ReprEnter((PyObject *)dv);
3860+
if (rc != 0) {
3861+
return rc > 0 ? PyUnicode_FromString("...") : NULL;
3862+
}
38583863
seq = PySequence_List((PyObject *)dv);
3859-
if (seq == NULL)
3860-
return NULL;
3861-
3864+
if (seq == NULL) {
3865+
goto Done;
3866+
}
38623867
result = PyUnicode_FromFormat("%s(%R)", Py_TYPE(dv)->tp_name, seq);
38633868
Py_DECREF(seq);
3869+
3870+
Done:
3871+
Py_ReprLeave((PyObject *)dv);
38643872
return result;
38653873
}
38663874

0 commit comments

Comments
 (0)