From 5ea9390e43e59d21b7aa7f95aba96ea1a1143ee6 Mon Sep 17 00:00:00 2001 From: Funan Zhou Date: Sun, 29 Mar 2026 05:10:59 +0800 Subject: [PATCH 1/3] 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. --- jsonschema/exceptions.py | 16 ++++-- jsonschema/tests/test_exceptions.py | 75 +++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 3 deletions(-) diff --git a/jsonschema/exceptions.py b/jsonschema/exceptions.py index 2e5d4ca0..3ab604b8 100644 --- a/jsonschema/exceptions.py +++ b/jsonschema/exceptions.py @@ -326,7 +326,13 @@ def __init__(self, errors: Iterable[ValidationError] = ()): for error in errors: container = self for element in error.path: - container = container[element] + if element in container._contents: + container = container._contents[element] + else: + # Create a new child tree and add it to _contents + child = self.__class__() + container._contents[element] = child + container = child container.errors[error.validator] = error container._instance = error.instance @@ -346,9 +352,13 @@ def __getitem__(self, index): by ``instance.__getitem__`` will be propagated (usually this is some subclass of `LookupError`. """ - if self._instance is not _unset and index not in self: + if index in self._contents: + return self._contents[index] + if self._instance is not _unset: + # Validate the index exists in the instance self._instance[index] - return self._contents[index] + # Return an empty tree without mutating _contents + return self.__class__() def __setitem__(self, index: str | int, value: ErrorTree): """ diff --git a/jsonschema/tests/test_exceptions.py b/jsonschema/tests/test_exceptions.py index 358b9242..0a749aff 100644 --- a/jsonschema/tests/test_exceptions.py +++ b/jsonschema/tests/test_exceptions.py @@ -506,6 +506,81 @@ def test_repr_empty(self): tree = exceptions.ErrorTree([]) self.assertEqual(repr(tree), "") + def test_index_access_does_not_mutate_tree(self): + """ + Accessing an index that exists in the instance but has no error + should not add that index to the tree's iteration. + + This is a regression test for issue #1328. + """ + error = exceptions.ValidationError( + "a message", + validator="foo", + instance={"foo": "bar", "baz": "qux"}, + path=["foo"], + ) + tree = exceptions.ErrorTree([error]) + + # Before access, only "foo" should be in the tree + self.assertEqual(set(tree), {"foo"}) + self.assertIn("foo", tree) + self.assertNotIn("baz", tree) + + # Access "baz" which exists in instance but has no error + child = tree["baz"] + self.assertIsInstance(child, exceptions.ErrorTree) + + # After access, iteration should still only show "foo" + self.assertEqual(set(tree), {"foo"}) + self.assertNotIn("baz", tree) + + # Multiple accesses should also not mutate + tree["baz"] + tree["baz"] + self.assertEqual(set(tree), {"foo"}) + self.assertNotIn("baz", tree) + + def test_nested_index_access_does_not_mutate_tree(self): + """ + Accessing nested indices that have no error should not mutate + any level of the tree. + """ + e1 = exceptions.ValidationError( + "err1", validator="a", path=["bar", 0], instance={"bar": [1, 2, 3]} + ) + e2 = exceptions.ValidationError( + "err2", validator="b", path=["bar", 1], instance={"bar": [1, 2, 3]} + ) + tree = exceptions.ErrorTree([e1, e2]) + + # Before access + self.assertEqual(set(tree), {"bar"}) + self.assertEqual(set(tree["bar"]), {0, 1}) + + # Access nested index that has no error + child = tree["bar"][2] + self.assertIsInstance(child, exceptions.ErrorTree) + + # After access, neither level should be mutated + self.assertEqual(set(tree), {"bar"}) + self.assertEqual(set(tree["bar"]), {0, 1}) + self.assertNotIn(2, tree["bar"]) + + def test_index_access_on_empty_tree_returns_empty_tree(self): + """ + Accessing any index on an empty tree should return an empty tree + without mutating the original tree. + """ + tree = exceptions.ErrorTree([]) + + # Access an index (tree has no _instance, so no validation) + child = tree["anything"] + self.assertIsInstance(child, exceptions.ErrorTree) + self.assertEqual(len(child), 0) + + # Tree should still be empty + self.assertEqual(set(tree), set()) + class TestErrorInitReprStr(TestCase): def make_error(self, **kwargs): From 946735d9792a64296794f941b4a839da4fb7fc11 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sat, 28 Mar 2026 21:11:39 +0000 Subject: [PATCH 2/3] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- jsonschema/tests/test_exceptions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jsonschema/tests/test_exceptions.py b/jsonschema/tests/test_exceptions.py index 0a749aff..7c053c55 100644 --- a/jsonschema/tests/test_exceptions.py +++ b/jsonschema/tests/test_exceptions.py @@ -546,10 +546,10 @@ def test_nested_index_access_does_not_mutate_tree(self): any level of the tree. """ e1 = exceptions.ValidationError( - "err1", validator="a", path=["bar", 0], instance={"bar": [1, 2, 3]} + "err1", validator="a", path=["bar", 0], instance={"bar": [1, 2, 3]}, ) e2 = exceptions.ValidationError( - "err2", validator="b", path=["bar", 1], instance={"bar": [1, 2, 3]} + "err2", validator="b", path=["bar", 1], instance={"bar": [1, 2, 3]}, ) tree = exceptions.ErrorTree([e1, e2]) From 2be21ccbe2037d1e3077f283c53fb1b04bd277da Mon Sep 17 00:00:00 2001 From: Funan Zhou Date: Sun, 29 Mar 2026 05:14:02 +0800 Subject: [PATCH 3/3] fix: shorten lines in test file to pass E501 check --- jsonschema/tests/test_exceptions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jsonschema/tests/test_exceptions.py b/jsonschema/tests/test_exceptions.py index 7c053c55..ebdb5655 100644 --- a/jsonschema/tests/test_exceptions.py +++ b/jsonschema/tests/test_exceptions.py @@ -546,10 +546,10 @@ def test_nested_index_access_does_not_mutate_tree(self): any level of the tree. """ e1 = exceptions.ValidationError( - "err1", validator="a", path=["bar", 0], instance={"bar": [1, 2, 3]}, + "err1", validator="a", path=["bar", 0], instance={"bar": []}, ) e2 = exceptions.ValidationError( - "err2", validator="b", path=["bar", 1], instance={"bar": [1, 2, 3]}, + "err2", validator="b", path=["bar", 1], instance={"bar": []}, ) tree = exceptions.ErrorTree([e1, e2])