From 7e99716dc54850ea5ea8dad8654aa8d49918d51a Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 11 Nov 2026 10:31:24 +0000 Subject: [PATCH] Reject diffs that change a member whose name is not a string The diff turned object keys that are not strings into strings for the paths of its operations. A path can only refer to members whose names are strings, though: '/1' is the member '1', not the member 1. So make_patch({1: 'a'}, {1: 'b'}) returned a patch that failed on its own source, make_patch({1: 'a'}, {1: 'a', 2: 'b'}) one that added a member '2' instead of 2, and make_patch({1: 'a'}, {'1': 'a'}) an empty one. Creating a patch now raises a TypeError if one of its operations would have to refer to such a member. Documents with such members can still be compared as long as these members are equal, or are part of a value that is added, removed or moved as a whole, as those patches are correct. Co-Authored-By: Claude Fable 5.2 Claude-Session: https://claude.ai/code/session_01PewzpjVeke4LTQ7Q88nyu6 --- jsonpatch.py | 63 +++++++++++++++++++++++++++++++++++++++++------ tests.py | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 125 insertions(+), 7 deletions(-) diff --git a/jsonpatch.py b/jsonpatch.py index 9ff14d5..0e3ee0c 100644 --- a/jsonpatch.py +++ b/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 """ diff --git a/tests.py b/tests.py index f0d7581..5668dc1 100644 --- a/tests.py +++ b/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')}