Skip to content

Commit 971abd2

Browse files
gpsheadgpshead
committed
Avoid an explosion of recursion depth length notes.
1 parent 9962c39 commit 971abd2

File tree

4 files changed

+166
-13
lines changed

4 files changed

+166
-13
lines changed

‎Include/internal/pycore_pyerrors.h

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,6 @@ PyAPI_FUNC(PyObject*) _PyErr_FormatFromCause(
2929
...
3030
);
3131

32-
extern int _PyException_AddNote(
33-
PyObject *exc,
34-
PyObject *note);
35-
3632
extern int _PyErr_CheckSignals(void);
3733

3834
/* Support for adding program text to SyntaxErrors */
@@ -188,6 +184,9 @@ PyAPI_FUNC(Py_ssize_t) _Py_UTF8_Edit_Cost(PyObject *str_a, PyObject *str_b,
188184

189185
// Export for '_json' shared extension
190186
PyAPI_FUNC(void) _PyErr_FormatNote(const char *format, ...);
187+
PyAPI_FUNC(int) _PyException_AddNote(
188+
PyObject *exc,
189+
PyObject *note);
191190

192191
/* Context manipulation (PEP 3134) */
193192

‎Lib/json/encoder.py

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
del i
3434

3535
INFINITY = float('inf')
36+
_RECURSION_EXCEPTION_NOTES_LIMIT = 5
3637

3738
def py_encode_basestring(s):
3839
"""Return a JSON representation of a Python string
@@ -208,7 +209,20 @@ def encode(self, o):
208209
if self.check_circular:
209210
err = ValueError("Circular reference detected")
210211
if notes := getattr(exc, "__notes__", None):
211-
err.__notes__ = notes
212+
# Filter notes to avoid explosion of redundant messages
213+
unique_notes = []
214+
seen = set()
215+
for note in notes:
216+
if note not in seen and len(unique_notes) < _RECURSION_EXCEPTION_NOTES_LIMIT:
217+
unique_notes.append(note)
218+
seen.add(note)
219+
220+
if unique_notes:
221+
err.__notes__ = unique_notes
222+
# Add summary if we truncated many notes
223+
if len(notes) > len(unique_notes):
224+
truncated_count = len(notes) - len(unique_notes)
225+
err.__notes__.append(f"... (truncated {truncated_count} similar messages)")
212226
raise err
213227
raise
214228
return ''.join(chunks)
@@ -440,6 +454,10 @@ def _iterencode(o, _current_indent_level):
440454
except GeneratorExit:
441455
raise
442456
except BaseException as exc:
443-
exc.add_note(f'when serializing {type(o).__name__} object')
457+
note = f'when serializing {type(o).__name__} object'
458+
existing_notes = getattr(exc, '__notes__', [])
459+
# Only add if it's different from the last note to prevent duplication
460+
if not existing_notes or existing_notes[-1] != note:
461+
exc.add_note(note)
444462
raise
445463
return _iterencode

‎Lib/test/test_json/test_recursion.py

Lines changed: 107 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ def test_listrecursion(self):
1313
try:
1414
self.dumps(x)
1515
except ValueError as exc:
16-
self.assertEqual(exc.__notes__[:1], ["when serializing list item 0"])
16+
self._assert_circular_error_notes(exc, "when serializing list item 0")
1717
else:
1818
self.fail("didn't raise ValueError on list recursion")
1919
x = []
@@ -22,7 +22,7 @@ def test_listrecursion(self):
2222
try:
2323
self.dumps(x)
2424
except ValueError as exc:
25-
self.assertEqual(exc.__notes__[:2], ["when serializing list item 0"]*2)
25+
self._assert_circular_error_notes(exc, "when serializing list item 0")
2626
else:
2727
self.fail("didn't raise ValueError on alternating list recursion")
2828
y = []
@@ -36,7 +36,7 @@ def test_dictrecursion(self):
3636
try:
3737
self.dumps(x)
3838
except ValueError as exc:
39-
self.assertEqual(exc.__notes__[:1], ["when serializing dict item 'test'"])
39+
self._assert_circular_error_notes(exc, "when serializing dict item 'test'")
4040
else:
4141
self.fail("didn't raise ValueError on dict recursion")
4242
x = {}
@@ -62,9 +62,13 @@ def default(self, o):
6262
with support.infinite_recursion(5000):
6363
enc.encode(JSONTestObject)
6464
except ValueError as exc:
65-
self.assertEqual(exc.__notes__[:2],
66-
["when serializing list item 0",
67-
"when serializing type object"])
65+
notes = exc.__notes__
66+
# Should have reasonable number of notes and contain expected context
67+
self.assertLessEqual(len(notes), 10)
68+
self.assertGreater(len(notes), 0)
69+
note_strs = [str(note) for note in notes if not str(note).startswith("... (truncated")]
70+
self.assertTrue(any("when serializing list item 0" in note for note in note_strs))
71+
self.assertTrue(any("when serializing type object" in note for note in note_strs))
6872
else:
6973
self.fail("didn't raise ValueError on default recursion")
7074

@@ -113,6 +117,103 @@ def default(self, o):
113117
with support.infinite_recursion(1000):
114118
EndlessJSONEncoder(check_circular=False).encode(5j)
115119

120+
def test_circular_reference_error_notes(self):
121+
"""Test that circular reference errors have reasonable exception notes."""
122+
# Test simple circular list
123+
x = []
124+
x.append(x)
125+
try:
126+
self.dumps(x, check_circular=True)
127+
except ValueError as exc:
128+
self._assert_circular_error_notes(exc, "when serializing list item 0")
129+
else:
130+
self.fail("didn't raise ValueError on list recursion")
131+
132+
# Test simple circular dict
133+
y = {}
134+
y['self'] = y
135+
try:
136+
self.dumps(y, check_circular=True)
137+
except ValueError as exc:
138+
self._assert_circular_error_notes(exc, "when serializing dict item 'self'")
139+
else:
140+
self.fail("didn't raise ValueError on dict recursion")
141+
142+
def test_nested_circular_reference_notes(self):
143+
"""Test that nested circular reference notes don't contain duplicates."""
144+
# Create a nested circular reference
145+
z = []
146+
nested = {'deep': [z]}
147+
z.append(nested)
148+
149+
try:
150+
self.dumps(z, check_circular=True)
151+
except ValueError as exc:
152+
notes = getattr(exc, '__notes__', [])
153+
# All non-truncation notes should be unique
154+
actual_notes = [note for note in notes if not str(note).startswith("... (truncated")]
155+
unique_notes = list(dict.fromkeys(actual_notes)) # preserves order, removes duplicates
156+
self.assertEqual(len(actual_notes), len(unique_notes),
157+
f"Found duplicate notes: {actual_notes}")
158+
else:
159+
self.fail("didn't raise ValueError on nested circular reference")
160+
161+
def test_recursion_error_when_check_circular_false(self):
162+
"""Test that RecursionError is raised when check_circular=False."""
163+
x = []
164+
x.append(x)
165+
166+
with self.assertRaises(RecursionError):
167+
with support.infinite_recursion(1000):
168+
self.dumps(x, check_circular=False)
169+
170+
def test_deep_recursion_note_handling(self):
171+
"""Test that deep recursion scenarios don't create excessive duplicate notes."""
172+
# Create a scenario that triggers deep recursion through custom default method
173+
class DeepObject:
174+
def __init__(self, value):
175+
self.value = value
176+
177+
class DeepEncoder(self.json.JSONEncoder):
178+
def default(self, o):
179+
if isinstance(o, DeepObject):
180+
return [DeepObject(o.value + 1)] if o.value < 10 else "end"
181+
return super().default(o)
182+
183+
encoder = DeepEncoder(check_circular=True)
184+
185+
try:
186+
encoder.encode(DeepObject(0))
187+
except (ValueError, RecursionError) as exc:
188+
notes = getattr(exc, '__notes__', [])
189+
190+
# Should have reasonable number of notes without excessive duplication
191+
self.assertLessEqual(len(notes), 20)
192+
193+
# Count occurrences of each note to verify no excessive duplication
194+
note_counts = {}
195+
for note in notes:
196+
note_str = str(note)
197+
if not note_str.startswith("... (truncated"):
198+
note_counts[note_str] = note_counts.get(note_str, 0) + 1
199+
200+
# No note should appear excessively
201+
max_count = max(note_counts.values()) if note_counts else 0
202+
self.assertLessEqual(max_count, 5,
203+
f"Found excessive duplicate notes: {note_counts}")
204+
205+
def _assert_circular_error_notes(self, exc, expected_context):
206+
"""Helper method to assert circular reference error notes are reasonable."""
207+
notes = getattr(exc, '__notes__', [])
208+
209+
# Should have reasonable number of notes (not thousands)
210+
self.assertLessEqual(len(notes), 10)
211+
self.assertGreater(len(notes), 0)
212+
213+
# Should contain expected context
214+
self.assertTrue(any(expected_context in str(note) for note in notes),
215+
f"Expected context '{expected_context}' not found in notes: {notes}")
216+
116217

117218
class TestPyRecursion(TestRecursion, PyTest): pass
118219
class TestCRecursion(TestRecursion, CTest): pass

‎Modules/_json.c

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1534,7 +1534,42 @@ encoder_listencode_obj(PyEncoderObject *s, PyUnicodeWriter *writer,
15341534

15351535
Py_DECREF(newobj);
15361536
if (rv) {
1537-
_PyErr_FormatNote("when serializing %T object", obj);
1537+
// Check for exception note duplication to prevent redundant messages
1538+
PyObject *exc = PyErr_GetRaisedException();
1539+
if (exc != NULL) {
1540+
// Create the new note
1541+
PyObject *new_note = PyUnicode_FromFormat("when serializing %T object", obj);
1542+
if (new_note == NULL) {
1543+
PyErr_SetRaisedException(exc);
1544+
return -1;
1545+
}
1546+
1547+
// Get existing notes
1548+
PyObject *notes = NULL;
1549+
if (PyObject_GetOptionalAttr(exc, &_Py_ID(__notes__), &notes) < 0) {
1550+
_PyException_AddNote(exc, new_note);
1551+
Py_DECREF(new_note);
1552+
PyErr_SetRaisedException(exc);
1553+
return -1;
1554+
}
1555+
1556+
// Only add if different from last note or if no notes exist
1557+
int should_add = 1;
1558+
if (notes != NULL && PyList_Check(notes) && PyList_GET_SIZE(notes) > 0) {
1559+
PyObject *last_note = PyList_GET_ITEM(notes, PyList_GET_SIZE(notes) - 1);
1560+
if (PyUnicode_Check(last_note) && PyUnicode_Compare(last_note, new_note) == 0) {
1561+
should_add = 0;
1562+
}
1563+
}
1564+
1565+
if (should_add) {
1566+
_PyException_AddNote(exc, new_note);
1567+
}
1568+
1569+
Py_DECREF(new_note);
1570+
Py_XDECREF(notes);
1571+
PyErr_SetRaisedException(exc);
1572+
}
15381573
return -1;
15391574
}
15401575
return rv;

0 commit comments

Comments
 (0)