Reject diffs that change a member whose name is not a string

jsonpatch.py

@@ -205,6 +205,9 @@ def make_patch(src, dst, pointer_cls=JsonPointer, generate_test_ops=False):
                               fails on documents that differ from `src` there.
     :type generate_test_ops: bool
 
+    :raises TypeError: if the documents differ in an object member whose
+                       name is not a string, see :meth:`JsonPatch.from_diff`.
+
     >>> src = {'foo': 'bar', 'numbers': [1, 3, 4, 8]}
     >>> dst = {'baz': 'qux', 'numbers': [1, 4, 7]}
     >>> patch = make_patch(src, dst)
@@ -1043,6 +1046,13 @@ def from_diff(
 
         :return: :class:`JsonPatch` instance.
 
+        :raises TypeError: if the documents differ in, or inside of, an
+                           object member whose name is not a string, such
+                           as the ``1`` of ``{1: 'a'}``. The path of an
+                           operation can't refer to such a member, which
+                           does not exist in JSON. Other differences of
+                           documents with such members are no problem.
+
         >>> src = {'foo': 'bar', 'numbers': [1, 3, 4, 8]}
         >>> dst = {'baz': 'qux', 'numbers': [1, 4, 7]}
         >>> patch = JsonPatch.from_diff(src, dst)
@@ -1252,8 +1262,7 @@ def execute(self):
                         op_second['op'] == 'add':
                     yield {
                         'op': 'replace',
-                        'path': path_join(op_second['path'],
-                                          self.pointer_cls),
+                        'path': self._path(op_second['path']),
                         'value': op_second['value'],
                     }
                     i += 2
@@ -1262,11 +1271,20 @@ def execute(self):
             operation = operations[i]
             for member in ('from', 'path'):
                 if member in operation:
-                    operation[member] = path_join(operation[member],
-                                                  self.pointer_cls)
+                    operation[member] = self._path(operation[member])
             yield operation
             i += 1
 
+    def _path(self, parts):
+        """ The path of an operation, in the syntax of pointer_cls """
+        for part in parts:
+            if isinstance(part, _NonStringKey):
+                raise TypeError(
+                    "can't create an operation for the object member "
+                    "{0!r}: its name is not a string, which the path of an "
+                    "operation can't refer to".format(part.key))
+        return path_join(parts, self.pointer_cls)
+
     def _replay(self):
         """ Applies the operations to the arrays of the source document, and
         returns them with the array indices at which they apply """
@@ -1406,13 +1424,13 @@ def _compare_dicts(self, path, src, dst):
         intersection = [key for key in src if key in dst]
 
         for key in removed_keys:
-            self._item_removed(path, str(key), src[key])
+            self._item_removed(path, _member_name(key), src[key])
 
         for key in added_keys:
-            self._item_added(path, str(key), dst[key])
+            self._item_added(path, _member_name(key), dst[key])
 
         for key in intersection:
-            self._compare_values(path, str(key), src[key], dst[key])
+            self._compare_values(path, _member_name(key), src[key], dst[key])
 
     def _differing_runs(self, src, dst):
         """ Returns (i1, i2, j1, j2) for each run of items src[i1:i2] that is
@@ -1548,6 +1566,37 @@ def _item_replaced(self, path, key, item, old_item):
             super()._item_replaced(path, key, item, old_item)
 
 
+class _NonStringKey(str):
+    """ Takes the place of an object key that is not a string in the parts
+    of an operation.
+
+    A path can only refer to members whose names are strings: '/1' is the
+    member '1', not the member 1. Members with other names are compared like
+    all others, as nothing is wrong unless they differ; an operation for
+    such a member, or for something in it, can't be written, though (see
+    DiffBuilder._path). """
+
+    def __new__(cls, key):
+        self = super().__new__(cls, key)
+        self.key = key
+        return self
+
+    # The member 1 is not the member '1', which this is equal to as a str
+    def __eq__(self, other):
+        return isinstance(other, _NonStringKey) and self.key == other.key
+
+    def __ne__(self, other):
+        return not self == other
+
+    def __hash__(self):
+        return hash(self.key)
+
+
+def _member_name(key):
+    """ The part of an operation for the object key """
+    return key if isinstance(key, str) else _NonStringKey(key)
+
+
 def _positional_cost(old, new):
     """ Estimates how many operations change old into new, if the items of
     arrays are compared by position """

tests.py

@@ -1040,6 +1040,75 @@ def test_move_only_values_equal_in_json(self):
                 res = patch.apply(src)
                 self.assertEqual(json.dumps(res), json.dumps(dst))
 
+    def test_changed_member_with_non_string_name_is_rejected(self):
+        """A path can't refer to such a member: '/1' is the member '1', so
+        the patch did not apply to src, or added a member '1' to it"""
+        cases = [
+            ({1: 'a'}, {1: 'b'}),
+            ({1: 'a'}, {}),
+            ({}, {1: 'a'}),
+            ({1: 'a'}, {1: 'a', 2: 'b'}),
+            ({1: 'a'}, {'1': 'a'}),
+            ({None: 1}, {None: 2}),
+            ({True: 1}, {True: 2}),
+            ({1.5: 1}, {1.5: 2}),
+            ({(1, 2): 1}, {(1, 2): 2}),
+            # inside of such a member
+            ({1: {'a': 1}}, {1: {'a': 2}}),
+            ({1: [1, 2]}, {1: [1, 2, 3]}),
+            ({'a': [{1: 'a'}]}, {'a': [{1: 'b'}]}),
+            # moved from or to such a member
+            ({'a': {1: [1, 2]}}, {'a': {}, 'b': [1, 2]}),
+            ({'a': {}, 'b': [1, 2]}, {'a': {1: [1, 2]}}),
+        ]
+        for src, dst in cases:
+            for kwargs in ({}, {'optimization': False},
+                           {'generate_test_ops': True}):
+                with self.subTest(src=src, dst=dst, **kwargs):
+                    with self.assertRaises(TypeError) as caught:
+                        jsonpatch.JsonPatch.from_diff(src, dst, **kwargs)
+                    self.assertIn('not a string', str(caught.exception))
+        with self.assertRaises(TypeError) as caught:
+            jsonpatch.make_patch({1: 'a'}, {1: 'b'})
+        self.assertIn('object member 1:', str(caught.exception))
+        with self.assertRaises(TypeError):
+            jsonpatch.JsonPatchWithAppend.from_diff({1: 'a'}, {1: 'ab'})
+
+    def test_non_string_member_names_differ_from_strings(self):
+        name = jsonpatch._member_name(1)
+        self.assertTrue(name != '1')
+        self.assertFalse(name == '1')
+        self.assertFalse(name != jsonpatch._member_name(1))
+        self.assertEqual(hash(name), hash(jsonpatch._member_name(1)))
+        self.assertEqual(jsonpatch._member_name('a'), 'a')
+
+    def test_unchanged_member_with_non_string_name_is_accepted(self):
+        cases = [
+            ({1: 'a'}, {1: 'a'}),
+            ({1: 'a', 'b': 0}, {1: 'a', 'b': 1}),
+            ({1: {2: [3]}, 'b': 0}, {1: {2: [3]}, 'c': 0}),
+            ([{None: 1}, 2], [2, {None: 1}]),
+            # members of values that are added, removed or moved as a whole
+            ({}, {'a': {1: 'a'}}),
+            ({'a': {1: 'a'}}, {}),
+            ({'a': {1: 'a'}}, {'b': {1: 'a'}}),
+            ([1, 2], [1, {3: 4}, 2]),
+            ({'a': 1}, {'a': {1: 'a'}}),
+            # the member '1' is not the member 1
+            ({1: 'a', '1': 'b'}, {1: 'a', '1': 'c'}),
+            ({1: 'a', '1': 'b'}, {1: 'a'}),
+        ]
+        for src, dst in cases:
+            for optimization in (True, False):
+                with self.subTest(src=src, dst=dst, optimization=optimization):
+                    original = copy.deepcopy(src)
+                    patch = jsonpatch.JsonPatch.from_diff(
+                        src, dst, optimization=optimization)
+                    self.assertEqual(patch.apply(src), dst)
+                    self.assertEqual(src, original)
+                    for operation in patch:
+                        self.assertIs(type(operation['path']), str)
+
     def test_move_only_values_equal_in_given_dumps(self):
         """The given dumps decides which values are the same"""
         src = {'a': decimal.Decimal('1.0')}