From d7773d92bd11640a8c950d6c36a9cef1cee36f96 Mon Sep 17 00:00:00 2001 From: bennorth Date: Fri, 26 Jan 2018 15:46:01 +0000 Subject: 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(). --- Lib/test/test_dictviews.py | 15 +++++++++++++++ Lib/test/test_ordered_dict.py | 14 ++++++++++++++ .../2017-12-13-16-46-23.bpo-18533.Dlk8d7.rst | 3 +++ Objects/dictobject.c | 16 ++++++++++++---- 4 files changed, 44 insertions(+), 4 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2017-12-13-16-46-23.bpo-18533.Dlk8d7.rst diff --git a/Lib/test/test_dictviews.py b/Lib/test/test_dictviews.py index 51ad9b3..2763cbf 100644 --- a/Lib/test/test_dictviews.py +++ b/Lib/test/test_dictviews.py @@ -1,6 +1,7 @@ import collections.abc import copy import pickle +import sys import unittest class DictSetTest(unittest.TestCase): @@ -202,6 +203,20 @@ class DictSetTest(unittest.TestCase): def test_recursive_repr(self): d = {} d[42] = d.values() + r = repr(d) + # Cannot perform a stronger test, as the contents of the repr + # are implementation-dependent. All we can say is that we + # want a str result, not an exception of any sort. + self.assertIsInstance(r, str) + d[42] = d.items() + r = repr(d) + # Again. + self.assertIsInstance(r, str) + + def test_deeply_nested_repr(self): + d = {} + for i in range(sys.getrecursionlimit() + 100): + d = {42: d.values()} self.assertRaises(RecursionError, repr, d) def test_copy(self): diff --git a/Lib/test/test_ordered_dict.py b/Lib/test/test_ordered_dict.py index 6bafb8b..20efe37 100644 --- a/Lib/test/test_ordered_dict.py +++ b/Lib/test/test_ordered_dict.py @@ -355,6 +355,20 @@ class OrderedDictTests: self.assertEqual(repr(od), "OrderedDict([('a', None), ('b', None), ('c', None), ('x', ...)])") + def test_repr_recursive_values(self): + OrderedDict = self.OrderedDict + od = OrderedDict() + od[42] = od.values() + r = repr(od) + # Cannot perform a stronger test, as the contents of the repr + # are implementation-dependent. All we can say is that we + # want a str result, not an exception of any sort. + self.assertIsInstance(r, str) + od[42] = od.items() + r = repr(od) + # Again. + self.assertIsInstance(r, str) + def test_setdefault(self): OrderedDict = self.OrderedDict pairs = [('c', 1), ('b', 2), ('a', 3), ('d', 4), ('e', 5), ('f', 6)] diff --git a/Misc/NEWS.d/next/Core and Builtins/2017-12-13-16-46-23.bpo-18533.Dlk8d7.rst b/Misc/NEWS.d/next/Core and Builtins/2017-12-13-16-46-23.bpo-18533.Dlk8d7.rst new file mode 100644 index 0000000..a33eff5 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2017-12-13-16-46-23.bpo-18533.Dlk8d7.rst @@ -0,0 +1,3 @@ +``repr()`` on a dict containing its own ``values()`` or ``items()`` no +longer raises ``RecursionError``; OrderedDict similarly. Instead, use +``...``, as for other recursive structures. Patch by Ben North. diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 0d25739..bb4ea1f 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -3853,14 +3853,22 @@ static PyObject * dictview_repr(_PyDictViewObject *dv) { PyObject *seq; - PyObject *result; + PyObject *result = NULL; + Py_ssize_t rc; + rc = Py_ReprEnter((PyObject *)dv); + if (rc != 0) { + return rc > 0 ? PyUnicode_FromString("...") : NULL; + } seq = PySequence_List((PyObject *)dv); - if (seq == NULL) - return NULL; - + if (seq == NULL) { + goto Done; + } result = PyUnicode_FromFormat("%s(%R)", Py_TYPE(dv)->tp_name, seq); Py_DECREF(seq); + +Done: + Py_ReprLeave((PyObject *)dv); return result; } -- cgit v0.12