Skip to content

Commit 5ea9390

Browse files
author
Funan Zhou
committed
fix: prevent ErrorTree index access from mutating iteration
Issue #1328: Accessing an index in ErrorTree that has no error incorrectly adds that index to the tree's iteration. This violates the documented behavior that __iter__ iterates over 'indices in the instance with errors'. The root cause was that _contents is a defaultdict, and __getitem__ used defaultdict.__getitem__ which auto-creates entries for missing keys. Fix by: 1. In __init__, explicitly create child trees and add to _contents instead of relying on __getitem__ mutation 2. In __getitem__, check if index exists in _contents first, and return an empty ErrorTree without mutation if not Added tests to verify the fix.
1 parent b747e59 commit 5ea9390

2 files changed

Lines changed: 88 additions & 3 deletions

File tree

jsonschema/exceptions.py

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,13 @@ def __init__(self, errors: Iterable[ValidationError] = ()):
326326
for error in errors:
327327
container = self
328328
for element in error.path:
329-
container = container[element]
329+
if element in container._contents:
330+
container = container._contents[element]
331+
else:
332+
# Create a new child tree and add it to _contents
333+
child = self.__class__()
334+
container._contents[element] = child
335+
container = child
330336
container.errors[error.validator] = error
331337

332338
container._instance = error.instance
@@ -346,9 +352,13 @@ def __getitem__(self, index):
346352
by ``instance.__getitem__`` will be propagated (usually this is
347353
some subclass of `LookupError`.
348354
"""
349-
if self._instance is not _unset and index not in self:
355+
if index in self._contents:
356+
return self._contents[index]
357+
if self._instance is not _unset:
358+
# Validate the index exists in the instance
350359
self._instance[index]
351-
return self._contents[index]
360+
# Return an empty tree without mutating _contents
361+
return self.__class__()
352362

353363
def __setitem__(self, index: str | int, value: ErrorTree):
354364
"""

jsonschema/tests/test_exceptions.py

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -506,6 +506,81 @@ def test_repr_empty(self):
506506
tree = exceptions.ErrorTree([])
507507
self.assertEqual(repr(tree), "<ErrorTree (0 total errors)>")
508508

509+
def test_index_access_does_not_mutate_tree(self):
510+
"""
511+
Accessing an index that exists in the instance but has no error
512+
should not add that index to the tree's iteration.
513+
514+
This is a regression test for issue #1328.
515+
"""
516+
error = exceptions.ValidationError(
517+
"a message",
518+
validator="foo",
519+
instance={"foo": "bar", "baz": "qux"},
520+
path=["foo"],
521+
)
522+
tree = exceptions.ErrorTree([error])
523+
524+
# Before access, only "foo" should be in the tree
525+
self.assertEqual(set(tree), {"foo"})
526+
self.assertIn("foo", tree)
527+
self.assertNotIn("baz", tree)
528+
529+
# Access "baz" which exists in instance but has no error
530+
child = tree["baz"]
531+
self.assertIsInstance(child, exceptions.ErrorTree)
532+
533+
# After access, iteration should still only show "foo"
534+
self.assertEqual(set(tree), {"foo"})
535+
self.assertNotIn("baz", tree)
536+
537+
# Multiple accesses should also not mutate
538+
tree["baz"]
539+
tree["baz"]
540+
self.assertEqual(set(tree), {"foo"})
541+
self.assertNotIn("baz", tree)
542+
543+
def test_nested_index_access_does_not_mutate_tree(self):
544+
"""
545+
Accessing nested indices that have no error should not mutate
546+
any level of the tree.
547+
"""
548+
e1 = exceptions.ValidationError(
549+
"err1", validator="a", path=["bar", 0], instance={"bar": [1, 2, 3]}
550+
)
551+
e2 = exceptions.ValidationError(
552+
"err2", validator="b", path=["bar", 1], instance={"bar": [1, 2, 3]}
553+
)
554+
tree = exceptions.ErrorTree([e1, e2])
555+
556+
# Before access
557+
self.assertEqual(set(tree), {"bar"})
558+
self.assertEqual(set(tree["bar"]), {0, 1})
559+
560+
# Access nested index that has no error
561+
child = tree["bar"][2]
562+
self.assertIsInstance(child, exceptions.ErrorTree)
563+
564+
# After access, neither level should be mutated
565+
self.assertEqual(set(tree), {"bar"})
566+
self.assertEqual(set(tree["bar"]), {0, 1})
567+
self.assertNotIn(2, tree["bar"])
568+
569+
def test_index_access_on_empty_tree_returns_empty_tree(self):
570+
"""
571+
Accessing any index on an empty tree should return an empty tree
572+
without mutating the original tree.
573+
"""
574+
tree = exceptions.ErrorTree([])
575+
576+
# Access an index (tree has no _instance, so no validation)
577+
child = tree["anything"]
578+
self.assertIsInstance(child, exceptions.ErrorTree)
579+
self.assertEqual(len(child), 0)
580+
581+
# Tree should still be empty
582+
self.assertEqual(set(tree), set())
583+
509584

510585
class TestErrorInitReprStr(TestCase):
511586
def make_error(self, **kwargs):

0 commit comments

Comments
 (0)