pimutils / pimutils/vdirsyncer

_clean_body mangles UTF-16

Open
#1,001 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

type: bug
Dominant language
Python
Stars
1.9k
Forks
185
Avg merge
11h 49m
Merged PRs (30d)
1

Description

Version: 6d0f9e94e423353f6101360c8e805258eab1c78e
Python: Python 3.10.4 on macOS

I'm using vdirsyncer with a CalDAV server which returns results in UTF-16.

Test case:

import vdirsyncer.storage.dav

a = '<foo><bar>\u1234</bar></foo>'.encode('utf-16')
print('bytes:', a)
print('cleaned:', vdirsyncer.storage.dav._clean_body(a))
print('parsed:', vdirsyncer.storage.dav._parse_xml(a))

Output:

bytes: b'\xff\xfe<\x00f\x00o\x00o\x00>\x00<\x00b\x00a\x00r\x00>\x004\x12<\x00/\x00b\x00a\x00r\x00>\x00<\x00/\x00f\x00o\x00o\x00>\x00'
Your server incorrectly returned ASCII control characters in its XML. Vdirsyncer ignores those, but this is a bug in your server.
cleaned: b'\xff\xfe<foo><bar>4</bar></foo>'
Your server incorrectly returned ASCII control characters in its XML. Vdirsyncer ignores those, but this is a bug in your server.
Traceback (most recent call last):
  File "/Users/comex/src/vdirsyncer/vdirsyncer/storage/dav.py", line 92, in _parse_xml
    return etree.XML(_clean_body(content))
  File "/opt/homebrew/Cellar/python@3.9/3.9.12/Frameworks/Python.framework/Versions/3.9/lib/python3.9/xml/etree/ElementTree.py", line 1349, in XML
    parser.feed(text)
xml.etree.ElementTree.ParseError: not well-formed (invalid token): line 1, column 3

In this case, _clean_body is stripping the 0x00 byte representing the upper half of each ASCII code unit, as well as the 0x12 byte representing the upper half of the non-ASCII code unit 0x1234. This results in something that resembles UTF-8, but where the non-ASCII character is corrupted, and the byte-order mark at the beginning is invalid for UTF-8. The latter causes etree.XML to fail to parse it.

If _parse_xml is changed to bypass _clean_body and pass the UTF-16 string directly to etree.XML, it works fine in this case; the parser is able to guess the encoding.

I suggest changing _parse_xml to first try to parse XML as-is, and only call _clean_body if that fails.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in vdirsyncer/storage/dav.py at _parse_xml, where the traceback shows _clean_body is called before etree.XML. Reproduce the UTF-16 example, then verify that parsing the original content works and cleanup is only used after a parse failure, while preserving the existing malformed-XML behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.