From 74009e34006b7bbf19f030efcf4d556be5516cb3 Mon Sep 17 00:00:00 2001 From: Vincent Gao Date: Fri, 17 Jul 2026 01:17:08 +0200 Subject: [PATCH 1/2] Fix JSON Patch move and copy array targets --- CHANGELOG.md | 1 + jsonpath/patch.py | 34 ++-------------------------------- tests/test_json_patch.py | 30 ++++++++++++++++++++++++++++++ 3 files changed, 33 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e97881..8e499ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ **Fixes** - JSONPath parser recursion limit failures now raise `JSONPathRecursionError`, preserving the original `RecursionError` as the cause. `JSONPathRecursionError` now also subclasses `RecursionError`, allowing existing except `RecursionError` handlers to continue working. +- JSON Patch `move` and `copy` operations now use `add` semantics for their target locations. This allows appending to arrays with `-` and rejects indexes greater than the array's length, as required by RFC 6902. ## Version 2.2.0 diff --git a/jsonpath/patch.py b/jsonpath/patch.py index 4b5956b..94831f9 100644 --- a/jsonpath/patch.py +++ b/jsonpath/patch.py @@ -266,22 +266,7 @@ def apply( if isinstance(source_parent, MutableMapping): del source_parent[str(self.source.parts[-1])] - dest_parent, _ = self.dest.resolve_parent(data) - - if dest_parent is None: - # Move source to root - return source_obj # type: ignore - - if isinstance(dest_parent, MutableSequence): - dest_parent.insert(int(self.dest.parts[-1]), source_obj) - elif isinstance(dest_parent, MutableMapping): - dest_parent[str(self.dest.parts[-1])] = source_obj - else: - raise JSONPatchError( - f"unexpected operation on {dest_parent.__class__.__name__!r}" - ) - - return data + return OpAdd(self.dest, source_obj).apply(data) def asdict(self) -> Dict[str, object]: """Return a dictionary representation of this operation.""" @@ -308,22 +293,7 @@ def apply( if source_obj is UNDEFINED: raise JSONPatchError("source object does not exist") - dest_parent, _ = self.dest.resolve_parent(data) - - if dest_parent is None: - # Copy source to root - return copy.deepcopy(source_obj) # type: ignore - - if isinstance(dest_parent, MutableSequence): - dest_parent.insert(int(self.dest.parts[-1]), copy.deepcopy(source_obj)) - elif isinstance(dest_parent, MutableMapping): - dest_parent[str(self.dest.parts[-1])] = copy.deepcopy(source_obj) - else: - raise JSONPatchError( - f"unexpected operation on {dest_parent.__class__.__name__!r}" - ) - - return data + return OpAdd(self.dest, copy.deepcopy(source_obj)).apply(data) def asdict(self) -> Dict[str, object]: """Return a dictionary representation of this operation.""" diff --git a/tests/test_json_patch.py b/tests/test_json_patch.py index 16659c5..ebc968e 100644 --- a/tests/test_json_patch.py +++ b/tests/test_json_patch.py @@ -127,6 +127,11 @@ def test_move_to_root() -> None: assert patch.apply({"foo": {"bar": "baz"}}) == {"bar": "baz"} +def test_move_appends_to_array() -> None: + patch = JSONPatch().move(from_="/foo/0", path="/foo/-") + assert patch.apply({"foo": ["bar", "baz"]}) == {"foo": ["baz", "bar"]} + + def test_move_to_immutable_mapping() -> None: patch = JSONPatch().move(from_="/foo/bar", path="/baz/bar") with pytest.raises( @@ -148,6 +153,13 @@ def test_copy_to_root() -> None: assert patch.apply({"foo": {"bar": [1, 2, 3]}}) == [1, 2, 3] +def test_copy_appends_to_array() -> None: + patch = JSONPatch().copy(from_="/foo/0", path="/foo/-") + assert patch.apply({"foo": ["bar", "baz"]}) == { + "foo": ["bar", "baz", "bar"] + } + + def test_copy_to_immutable_mapping() -> None: with pytest.raises( JSONPatchError, @@ -158,6 +170,24 @@ def test_copy_to_immutable_mapping() -> None: ) +@pytest.mark.parametrize( + ("operation", "path"), + [ + pytest.param("move", "/foo/3", id="move"), + pytest.param("copy", "/foo/4", id="copy"), + ], +) +def test_move_and_copy_reject_array_index_beyond_end( + operation: str, path: str +) -> None: + patch = getattr(JSONPatch(), operation)(from_="/foo/0", path=path) + + with pytest.raises( + JSONPatchError, match=re.escape(f"index out of range ({operation}:0)") + ): + patch.apply({"foo": ["bar", "baz", "qux"]}) + + def test_patch_from_file_like() -> None: patch_doc = StringIO( json.dumps( From 03dce17ec68c20444f8981e5a3e319b7fd0e43fe Mon Sep 17 00:00:00 2001 From: James Prior Date: Sat, 18 Jul 2026 08:39:25 +0100 Subject: [PATCH 2/2] Refactor to avoid extra `OpAdd` instantiation --- CHANGELOG.md | 7 ++++- jsonpath/patch.py | 68 +++++++++++++++++++++++----------------- tests/test_json_patch.py | 28 +++++++---------- 3 files changed, 57 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e499ad..6eebf65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,11 +1,16 @@ # Python JSONPath Change Log +# Version 2.2.2 (unreleased) + +**Fixes** + +- JSON Patch `move` and `copy` operations now use `add` semantics for their target locations. Previously it was not possible to move or copy to the end of an array with the special `-` index, and we would silently append to arrays given an index greater than the array's length. We now raise a `JSONPatchError` if the target index is greater than the number of elements in the array. + # Version 2.2.1 **Fixes** - JSONPath parser recursion limit failures now raise `JSONPathRecursionError`, preserving the original `RecursionError` as the cause. `JSONPathRecursionError` now also subclasses `RecursionError`, allowing existing except `RecursionError` handlers to continue working. -- JSON Patch `move` and `copy` operations now use `add` semantics for their target locations. This allows appending to arrays with `-` and rejects indexes greater than the array's length, as required by RFC 6902. ## Version 2.2.0 diff --git a/jsonpath/patch.py b/jsonpath/patch.py index 94831f9..863a8fa 100644 --- a/jsonpath/patch.py +++ b/jsonpath/patch.py @@ -59,32 +59,7 @@ def apply( self, data: Union[MutableSequence[object], MutableMapping[str, object]] ) -> Union[MutableSequence[object], MutableMapping[str, object]]: """Apply this patch operation to _data_.""" - parent, obj = self.path.resolve_parent(data) - if parent is None: - # Replace the root object. - # The following op, if any, will raise a JSONPatchError if needed. - return self.value # type: ignore - - target = self.path.parts[-1] - if isinstance(parent, MutableSequence): - if obj is UNDEFINED: - if target == "-": - parent.append(self.value) - else: - index = self.path._index(target) # type: ignore # noqa: SLF001 - if index == len(parent): - parent.append(self.value) - else: - raise JSONPatchError("index out of range") - else: - parent.insert(int(target), self.value) - elif isinstance(parent, MutableMapping): - parent[str(target)] = self.value - else: - raise JSONPatchError( - f"unexpected operation on {parent.__class__.__name__!r}" - ) - return data + return _add(data=data, path=self.path, value=self.value) def asdict(self) -> Dict[str, object]: """Return a dictionary representation of this operation.""" @@ -266,7 +241,7 @@ def apply( if isinstance(source_parent, MutableMapping): del source_parent[str(self.source.parts[-1])] - return OpAdd(self.dest, source_obj).apply(data) + return _add(path=self.dest, value=source_obj, data=data) def asdict(self) -> Dict[str, object]: """Return a dictionary representation of this operation.""" @@ -293,7 +268,8 @@ def apply( if source_obj is UNDEFINED: raise JSONPatchError("source object does not exist") - return OpAdd(self.dest, copy.deepcopy(source_obj)).apply(data) + # return OpAdd(self.dest, copy.deepcopy(source_obj)).apply(data) + return _add(path=self.dest, value=copy.deepcopy(source_obj), data=data) def asdict(self) -> Dict[str, object]: """Return a dictionary representation of this operation.""" @@ -806,3 +782,39 @@ def patched( unicode_escape=unicode_escape, uri_decode=uri_decode, ).patched(data) + + +_DataT = Union[MutableSequence[object], MutableMapping[str, object]] + + +def _add(*, data: _DataT, path: JSONPointer, value: object) -> _DataT: + """Add _value_ to _data_ at _path_. + + This is semantically the `add` operation, used by `OpAdd`, `OpMove` and + `OpCopy`. + """ + parent, obj = path.resolve_parent(data) + if parent is None: + # Replace the root object. + # The following op, if any, will raise a JSONPatchError if needed. + return value # type: ignore + + target = path.parts[-1] + + if isinstance(parent, MutableSequence): + if obj is UNDEFINED: + if target == "-": + parent.append(value) + else: + index = path._index(target) # type: ignore # noqa: SLF001 + if index == len(parent): + parent.append(value) + else: + raise JSONPatchError("index out of range") + else: + parent.insert(int(target), value) + elif isinstance(parent, MutableMapping): + parent[str(target)] = value + else: + raise JSONPatchError(f"unexpected operation on {parent.__class__.__name__!r}") + return data diff --git a/tests/test_json_patch.py b/tests/test_json_patch.py index ebc968e..377d54b 100644 --- a/tests/test_json_patch.py +++ b/tests/test_json_patch.py @@ -155,9 +155,7 @@ def test_copy_to_root() -> None: def test_copy_appends_to_array() -> None: patch = JSONPatch().copy(from_="/foo/0", path="/foo/-") - assert patch.apply({"foo": ["bar", "baz"]}) == { - "foo": ["bar", "baz", "bar"] - } + assert patch.apply({"foo": ["bar", "baz"]}) == {"foo": ["bar", "baz", "bar"]} def test_copy_to_immutable_mapping() -> None: @@ -170,21 +168,17 @@ def test_copy_to_immutable_mapping() -> None: ) -@pytest.mark.parametrize( - ("operation", "path"), - [ - pytest.param("move", "/foo/3", id="move"), - pytest.param("copy", "/foo/4", id="copy"), - ], -) -def test_move_and_copy_reject_array_index_beyond_end( - operation: str, path: str -) -> None: - patch = getattr(JSONPatch(), operation)(from_="/foo/0", path=path) +def test_move_past_array_end() -> None: + patch = JSONPatch().move("/foo/0", "/foo/3") - with pytest.raises( - JSONPatchError, match=re.escape(f"index out of range ({operation}:0)") - ): + with pytest.raises(JSONPatchError, match=re.escape("index out of range (move:0)")): + patch.apply({"foo": ["bar", "baz", "qux"]}) + + +def test_copy_past_array_end() -> None: + patch = JSONPatch().copy("/foo/0", "/foo/4") + + with pytest.raises(JSONPatchError, match=re.escape("index out of range (copy:0)")): patch.apply({"foo": ["bar", "baz", "qux"]})