Fix JSON Patch move and copy array targets

CHANGELOG.md

@@ -1,5 +1,11 @@
 # 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/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,22 +241,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 _add(path=self.dest, value=source_obj, data=data)
 
     def asdict(self) -> Dict[str, object]:
         """Return a dictionary representation of this operation."""
@@ -308,22 +268,8 @@ 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)
+        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."""
@@ -836,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

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,11 @@ 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 +168,20 @@ def test_copy_to_immutable_mapping() -> None:
         )
 
 
+def test_move_past_array_end() -> None:
+    patch = JSONPatch().move("/foo/0", "/foo/3")
+
+    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"]})
+
+
 def test_patch_from_file_like() -> None:
     patch_doc = StringIO(
         json.dumps(