search

Shoobx / xmldiff

A library and command line utility for diffing xml
MIT License
196 stars 50 forks source link

xmlpatch use of csvreader causes too many arguments if a string contains a comma #133

Open glennehrlich opened 1 month ago

glennehrlich commented 1 month ago

Things like elements or comments with commas in them will cause the csv reader used in the patch code to incorrectly parse the patch line, causing a TypeError complaining about more positional arguments than expected. Not sure what the right way to configure csvreader is for this, but it does seem that there are some options that may help with this.

Anyway, test data that's similar to one of the unit tests:

glenn@ubuntu:~/tmp/sv030/test$ python3.11 --version
Python 3.11.6+
glenn@ubuntu:~/tmp/sv030/test$ xmldiff --version
xmldiff 2.7.0
glenn@ubuntu:~/tmp/sv030/test$ xmlpatch --version
xmldiff 2.7.0
glenn@ubuntu:~/tmp/sv030/test$ cat test1.xml
<root><anode/></root>
glenn@ubuntu:~/tmp/sv030/test$ cat test2.xml
<root><anode/>foo,bar</root>
glenn@ubuntu:~/tmp/sv030/test$ xmldiff test1.xml test2.xml > patch
glenn@ubuntu:~/tmp/sv030/test$ xmlpatch patch test1.xml
Traceback (most recent call last):
  File "/home/glenn/.local/bin/xmlpatch", line 8, in <module>
    sys.exit(patch_command())
             ^^^^^^^^^^^^^^^
  File "/home/glenn/.local/lib/python3.11/site-packages/xmldiff/main.py", line 254, in patch_command
    result = patch_file(args.patchfile, args.xmlfile, args.diff_encoding)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/glenn/.local/lib/python3.11/site-packages/xmldiff/main.py", line 223, in patch_file
    tree = patch_tree(actions, tree)
           ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/glenn/.local/lib/python3.11/site-packages/xmldiff/main.py", line 199, in patch_tree
    return patcher.patch(actions, tree)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/glenn/.local/lib/python3.11/site-packages/xmldiff/patch.py", line 25, in patch
    for action in actions:
  File "/home/glenn/.local/lib/python3.11/site-packages/xmldiff/patch.py", line 112, in parse
    yield self.make_action(line)
          ^^^^^^^^^^^^^^^^^^^^^^
  File "/home/glenn/.local/lib/python3.11/site-packages/xmldiff/patch.py", line 127, in make_action
    return method(*params)
           ^^^^^^^^^^^^^^^
TypeError: DiffParser._handle_update_text_after() takes 3 positional arguments but 4 were given
glenn@ubuntu:~/tmp/sv030/test$
glennehrlich commented 1 month ago

Forgot the contents of the patch file:

glenn@ubuntu:~/tmp/sv030/test$ cat patch
[update-text-after, /root/anode[1], "foo,bar"]
glenn@ubuntu:~/tmp/sv030/test$
cdbassett commented 2 days ago

I don't have permission to create a pull request, but here is a patch that fixes it for me (note that this fixes the formatter, so existing "patch" files will have to be recreated before they will work):

[PATCH] fix issue #133 - writing comma separated values in DiffFormatter does not take commas in the values into account. By using the csv module to write and not just read, this is fixed.

xmldiff/formatting.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/xmldiff/formatting.py b/xmldiff/formatting.py index 640ec15..5e42fb8 100644 --- a/xmldiff/formatting.py +++ b/xmldiff/formatting.py @@ -1,8 +1,10 @@ import json import re +import io

from collections import namedtuple from copy import deepcopy +from csv import writer from lxml import etree from xmldiff.diff_match_patch import diff_match_patch from xmldiff import utils @@ -742,7 +744,11 @@ class DiffFormatter(BaseFormatter): def handle_action(self, action): action_type = type(action) method = getattr(self, "handle" + action_type.name)