stefankoegl / python-json-patch

Applying JSON Patches in Python
https://python-json-patch.readthedocs.org/
BSD 3-Clause "New" or "Revised" License
440 stars 96 forks source link

Bug: patch didn't catch changed index causing the apply to fail #124

Open zliang11 opened 3 years ago

zliang11 commented 3 years ago

I found a bug with an example here that failed when applying the patch that was generated.

>>> L1 = ['a', 'b', ['d', 'e'], 'f']
>>> L2 = ['a', 'd', ['e', 'g']]
>>> patch = jsonpatch.make_patch(L1,L2)
>>> list(patch)
[{u'path': u'/1', u'op': u'remove'}, {u'path': u'/1', u'from': **u'/2/0'**, u'op': u'move'}, {u'path': u'/2/1', u'value': 'g', u'op': u'add'}, {u'path': u'/3', u'op': u'remove'}]

>>> newL2 = jsonpatch.apply_patch(L1,patch)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File ...... line 151, in apply_patch
    return patch.apply(doc, in_place)
  File ...... line 669, in apply
    obj = operation.apply(obj)
  File ...... line 386, in apply
    }, pointer_cls=self.pointer_cls).apply(obj)
  File ...... line 238, in apply
    del subobj[part]
TypeError: 'str' object doesn't support item deletion

By looking at the patches, when 'b' is removed, the index for 'd' should be u'/1/0' but the patch still has u'2/0', which causes the error because u'/2' is now 'f', which is not a list, so it can't find it to do the "delete at /2/0 then add it back at /1" - the Move operation. I tested using this alternative jsondiff make function and it worked fine. https://github.com/nxsofsys/jsondiff

>>> make(L1,L2)
[{'path': '/1', 'op': 'remove'}, {'path': '/1', 'from': '/1/0', 'op': 'move'}, {'path': '/2/1', 'value': 'g', 'op': 'add'}, {'path': '/3', 'op': 'remove'}]
>>> jsonpatch.apply_patch(L1, make(L1,L2))
['a', 'd', ['e', 'g']]
zliang11 commented 3 years ago

Hi, @stefankoegl do you perhaps know where it isn't catching the index change?

zliang11 commented 3 years ago

I noticed that the error is probably caused by _undo_remove and _undo_add directly comparing the path and the key. It is very likely for nested lists that the index affected is not the key (last value) of its location. We can probably split the locations by '/', get the index of the key, then compare the value at that corresponding index to do the +,- changes.

rmorshea commented 2 years ago

I think this may be related: https://github.com/stefankoegl/python-json-patch/issues/138#issuecomment-1004469874