From 1a70a61a01735c13687542edf50b8a1abe8251ee Mon Sep 17 00:00:00 2001 From: Berend Klein Haneveld Date: Tue, 26 May 2026 10:58:58 +0200 Subject: [PATCH 1/2] Fix iapply silent failure on invalid patch paths Pointer.evaluate previously swallowed KeyError/TypeError mid-traversal and returned a partially-walked (parent, key) pair, causing iapply to silently write to the wrong parent or surface confusing downstream AttributeErrors. Make the parent walk strict (any missing intermediate token raises KeyError/IndexError/TypeError from the underlying container), while keeping the leaf lookup lenient so add ops and the list "-" append token continue to work. Co-Authored-By: Claude Opus 4.7 --- patchdiff/pointer.py | 29 ++++++++++++++++++++++------- tests/test_apply.py | 21 +++++++++++++++++++++ tests/test_pointer.py | 17 +++++++++++++++++ 3 files changed, 60 insertions(+), 7 deletions(-) diff --git a/patchdiff/pointer.py b/patchdiff/pointer.py index cf4dbb1..35c6c95 100644 --- a/patchdiff/pointer.py +++ b/patchdiff/pointer.py @@ -50,14 +50,29 @@ def evaluate(self, obj: Diffable) -> tuple[Diffable, Hashable, Any]: key = "" parent = None cursor = obj - if tokens := self.tokens: + tokens = self.tokens + if tokens: + # Walk to the parent strictly: any failure here is a path that + # doesn't exist in the target, and silently landing on a partial + # parent would let iapply write to the wrong place. + for key in tokens[:-1]: + parent = cursor + cursor = parent[key] + # The leaf may legitimately not exist (add ops on dicts, list + # "-" append) so we tolerate lookup failures there — but only + # when the parent is itself a container we can write into. + parent = cursor + key = tokens[-1] try: - for key in tokens: - parent = cursor - cursor = parent[key] - except (KeyError, TypeError): - # KeyError for dicts, TypeError for sets and lists - pass + cursor = parent[key] + except (KeyError, IndexError, TypeError): + if not ( + hasattr(parent, "keys") + or hasattr(parent, "append") + or hasattr(parent, "add") + ): + raise + cursor = None return parent, key, cursor def append(self, token: Hashable) -> "Pointer": diff --git a/tests/test_apply.py b/tests/test_apply.py index a5df072..14648dc 100644 --- a/tests/test_apply.py +++ b/tests/test_apply.py @@ -1,4 +1,7 @@ +import pytest + from patchdiff import apply, diff +from patchdiff.pointer import Pointer def test_apply(): @@ -132,3 +135,21 @@ def test_add_remove_list_extended_inverse_leaving_end(): d = apply(b, rops) assert a == d + + +def test_apply_raises_on_missing_dict_key(): + ops = [{"op": "replace", "path": Pointer(["missing", "key"]), "value": 99}] + with pytest.raises(KeyError): + apply({"present": 1}, ops) + + +def test_apply_raises_on_out_of_range_list_index(): + ops = [{"op": "replace", "path": Pointer([10, "x"]), "value": 99}] + with pytest.raises(IndexError): + apply([1, 2, 3], ops) + + +def test_apply_raises_when_traversing_into_primitive(): + ops = [{"op": "replace", "path": Pointer(["a", "b"]), "value": 99}] + with pytest.raises(TypeError): + apply({"a": 5}, ops) diff --git a/tests/test_pointer.py b/tests/test_pointer.py index 045d7a9..2c7e2ef 100644 --- a/tests/test_pointer.py +++ b/tests/test_pointer.py @@ -1,3 +1,5 @@ +import pytest + from patchdiff.pointer import Pointer @@ -44,3 +46,18 @@ def test_pointer_eq(): def test_pointer_append(): assert Pointer([1]).append("foo") == Pointer([1, "foo"]) + + +def test_pointer_evaluate_raises_on_missing_dict_key(): + with pytest.raises(KeyError): + Pointer(["missing", "key"]).evaluate({"present": 1}) + + +def test_pointer_evaluate_raises_on_out_of_range_list_index(): + with pytest.raises(IndexError): + Pointer([10, "x"]).evaluate([1, 2, 3]) + + +def test_pointer_evaluate_raises_when_traversing_into_primitive(): + with pytest.raises(TypeError): + Pointer(["a", "b"]).evaluate({"a": 5}) From 03e7740deb71e373fd9c768e3cbff79149f99ed1 Mon Sep 17 00:00:00 2001 From: Berend Klein Haneveld Date: Tue, 26 May 2026 13:04:46 +0200 Subject: [PATCH 2/2] Revert unneeded change --- patchdiff/pointer.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/patchdiff/pointer.py b/patchdiff/pointer.py index 35c6c95..d088a2a 100644 --- a/patchdiff/pointer.py +++ b/patchdiff/pointer.py @@ -50,8 +50,7 @@ def evaluate(self, obj: Diffable) -> tuple[Diffable, Hashable, Any]: key = "" parent = None cursor = obj - tokens = self.tokens - if tokens: + if tokens := self.tokens: # Walk to the parent strictly: any failure here is a path that # doesn't exist in the target, and silently landing on a partial # parent would let iapply write to the wrong place.