diff --git a/jsonpatch.py b/jsonpatch.py index 9d36c46..8a759a2 100644 --- a/jsonpatch.py +++ b/jsonpatch.py @@ -36,7 +36,7 @@ import copy import functools import json -from collections.abc import MutableMapping, MutableSequence, Sequence +from collections.abc import Mapping, MutableMapping, MutableSequence, Sequence from difflib import SequenceMatcher from types import MappingProxyType @@ -185,6 +185,23 @@ def __init__(self, operation, pointer_cls=JsonPointer): self.operation = operation + def _from_pointer(self): + """Builds the pointer of the 'from' member like __init__ builds the + one of 'path'.""" + try: + from_ptr = self.operation['from'] + except KeyError: + raise InvalidJsonPatch( + "The operation does not contain a 'from' member") + + if isinstance(from_ptr, self.pointer_cls): + return from_ptr + + try: + return self.pointer_cls(from_ptr) + except TypeError: + raise InvalidJsonPatch("Invalid 'from'") + def apply(self, obj): """Abstract method that applies a patch operation to the specified object.""" raise NotImplementedError('should implement the patch operation.') @@ -334,14 +351,7 @@ class MoveOperation(PatchOperation): """Moves an object property or an array element to a new location.""" def apply(self, obj): - try: - if isinstance(self.operation['from'], self.pointer_cls): - from_ptr = self.operation['from'] - else: - from_ptr = self.pointer_cls(self.operation['from']) - except KeyError: - raise InvalidJsonPatch( - "The operation does not contain a 'from' member") + from_ptr = self._from_pointer() # Checked before anything is resolved, as removing an array element # shifts its siblings, so the target would resolve to a different @@ -349,7 +359,7 @@ def apply(self, obj): if self.pointer != from_ptr and self.pointer.contains(from_ptr): raise JsonPatchConflict('Cannot move values into their own children') - subobj, part = _to_last(from_ptr, obj) + subobj, part = _to_existing_last(from_ptr, obj) # Moving the whole document onto itself is a no-op if part is None: @@ -434,13 +444,9 @@ class CopyOperation(PatchOperation): """ Copies an object property or an array element to a new location """ def apply(self, obj): - try: - from_ptr = self.pointer_cls(self.operation['from']) - except KeyError: - raise InvalidJsonPatch( - "The operation does not contain a 'from' member") + from_ptr = self._from_pointer() - subobj, part = _to_last(from_ptr, obj) + subobj, part = _to_existing_last(from_ptr, obj) try: value = copy.deepcopy(subobj if part is None else subobj[part]) except (KeyError, IndexError) as ex: @@ -649,6 +655,9 @@ def apply(self, obj, in_place=False): return obj def _get_operation(self, operation): + if not isinstance(operation, Mapping): + raise InvalidJsonPatch("Operation must be an object") + if 'op' not in operation: raise InvalidJsonPatch("Operation does not contain 'op' member") @@ -1089,3 +1098,16 @@ def _to_last(pointer, doc): part, type(subobj))) return subobj, part + + +def _to_existing_last(pointer, doc): + """Resolve pointer like _to_last, for a location that has to exist. + + The '-' of an array refers to the nonexistent element after its last one. + """ + subobj, part = _to_last(pointer, doc) + + if part == '-' and isinstance(subobj, Sequence): + raise JsonPointerException("invalid array index '{0}'".format(part)) + + return subobj, part diff --git a/property_tests.py b/property_tests.py index 148322c..67d10fb 100644 --- a/property_tests.py +++ b/property_tests.py @@ -336,8 +336,6 @@ def test_does_not_modify_patch(self, case): @given(docs_with_patches()) # 'move' from the whole document crashed for array roots, #214 @example(case=([], [{'op': 'move', 'from': '', 'path': '/-'}])) - # 'copy' or 'move' from the '-' of an array crashes - @example(case=([0], [{'op': 'copy', 'from': '/-', 'path': '/0'}])) # operations on the whole document crash if it is a scalar @example(case=(None, [{'op': 'remove', 'path': ''}])) def test_raises_only_documented_exceptions(self, case): @@ -347,7 +345,6 @@ def test_raises_only_documented_exceptions(self, case): except (jsonpatch.JsonPatchException, JsonPointerException): pass - @unittest.expectedFailure @given(st.lists(st.one_of( json_docs, st.dictionaries(st.sampled_from(['op', 'path', 'from', 'value']), @@ -357,9 +354,9 @@ def test_raises_only_documented_exceptions(self, case): optional={'path': json_docs, 'from': json_docs, 'value': json_docs}), ))) - # patch elements that are not objects + # patch elements that are not objects raised TypeError, #215 @example(patch=[0]) - # 'from' that is not a string + # 'from' that is not a string raised TypeError, #215 @example(patch=[{'op': 'copy', 'from': 0, 'path': '/a'}]) def test_malformed_patch_raises_documented_exceptions(self, patch): try: diff --git a/tests.py b/tests.py index ba52706..72f5759 100755 --- a/tests.py +++ b/tests.py @@ -89,6 +89,20 @@ def test_remove_invalid_item(self): with self.assertRaises(jsonpointer.JsonPointerException): jsonpatch.apply_patch(obj, [{'op': 'remove', 'path': '/foo/-'}]) + def test_move_copy_from_end_of_list(self): + # '-' refers to the nonexistent element after the last one, #215 + obj = {'foo': ['bar', 'qux', 'baz']} + for op in ('move', 'copy'): + with self.assertRaises(jsonpointer.JsonPointerException): + jsonpatch.apply_patch(obj, [{'op': op, 'from': '/foo/-', 'path': '/foo/0'}]) + + def test_move_copy_from_dash_object_key(self): + # '-' is an ordinary key in objects + obj = {'-': 1} + res = jsonpatch.apply_patch(obj, [{'op': 'copy', 'from': '/-', 'path': '/a'}]) + self.assertEqual(res, {'-': 1, 'a': 1}) + res = jsonpatch.apply_patch(obj, [{'op': 'move', 'from': '/-', 'path': '/a'}]) + self.assertEqual(res, {'a': 1}) def test_replace_object_key(self): obj = {'foo': 'bar', 'baz': 'qux'} @@ -1208,6 +1222,20 @@ def test_invalid_op(self): patch_obj = [ { "op": "invalid", "path": "/child", "value": { "grandchild": { } } } ] self.assertRaises(jsonpatch.JsonPatchException, jsonpatch.apply_patch, src, patch_obj) + def test_operation_not_an_object(self): + # each element of a patch has to be an object, #215 + src = {"foo": "bar"} + for operation in [0, None, True, [], ["op", "add"]]: + self.assertRaises(jsonpatch.InvalidJsonPatch, jsonpatch.apply_patch, src, [operation]) + + def test_invalid_from(self): + # "from" has to be a JSON pointer, like "path", #215 + src = {"foo": "bar"} + for op in ("move", "copy"): + for from_ in [0, None, ["foo"], {"foo": "bar"}]: + patch_obj = [ { "op": op, "from": from_, "path": "/baz" } ] + self.assertRaises(jsonpatch.InvalidJsonPatch, jsonpatch.apply_patch, src, patch_obj) + class ConflictTests(unittest.TestCase): @@ -1339,6 +1367,12 @@ def test_create_with_pointer(self): expected = {'bar': 'bar', 'baz': [42]} self.assertEqual(result, expected) + def test_copy_from_pointer(self): + patch = jsonpatch.JsonPatch([ + {'op': 'copy', 'from': jsonpointer.JsonPointer('/foo'), 'path': '/bar'}, + ]) + self.assertEqual(patch.apply({'foo': 1}), {'foo': 1, 'bar': 1}) + class JsonPatchCreationTest(unittest.TestCase):