search

python-caldav / caldav

Apache License 2.0
314 stars 94 forks source link

Add support for very big event #300

Closed aaujon closed 1 year ago

aaujon commented 1 year ago

Hi,

The current code doesn't work with events with very big content, exception I get is : Traceback (most recent call last): File "/home/arnaud/.local/lib/python3.10/site-packages/caldav/davclient.py", line 72, in __init__ self.tree = etree.XML( File "src/lxml/etree.pyx", line 3231, in lxml.etree.XML File "src/lxml/parser.pxi", line 1915, in lxml.etree._parseMemoryDocument File "src/lxml/parser.pxi", line 1803, in lxml.etree._parseDoc File "src/lxml/parser.pxi", line 1144, in lxml.etree._BaseParser._parseDoc File "src/lxml/parser.pxi", line 618, in lxml.etree._ParserContext._handleParseResultDoc File "src/lxml/parser.pxi", line 728, in lxml.etree._handleParseResult File "src/lxml/parser.pxi", line 657, in lxml.etree._raiseParseError File "<string>", line 202389 lxml.etree.XMLSyntaxError: xmlSAX2Characters: huge text node, line 202389, column 65 This is produced when handling an event with a very big description, I manage to fix this by activating 'huge_tree' in XMLParser in davclient.py. This option is not activated by default du to a security issue : https://lxml.de/api/lxml.etree.XMLParser-class.html I'm not sure exactly what it is about but we may want to enable it by default for caldav or at least add an option for it in DAVClient ?

tobixen commented 1 year ago

Sorry, I'm not going to "disable security restrictions" by default in the python caldav library, I suppose there is a reason why default is to enable those security restrictions in the lxml library. Passing it as an option when initiating the DAVClient object is acceptable.

aaujon commented 1 year ago

I understand that, I rewrite the MR to allow to use an option when initiating DAVClient, I'm not sure about the way it's done, I don't really like having huge_tree variable in both DAVClient and DAVResponse class. Let me know if you have a better idea on how to handle it.

tobixen commented 1 year ago

Perhaps it would be a better idea to have the reference to the DAVClient object stored in the DAVResponse class

tobixen commented 1 year ago

There is an assert in the unit test that a huge string will cause an error to be raised unless huge_tree is passed ... but it seems like the huge XML is parsed without problems in some environments now, ref #320

I will remove that test but leave the left of the logic as it is.

tobixen commented 1 year ago

... after all, the important thing is not that the caldav library fails on huge xml objects, the important thing is that they pass if the right flags are passed to the connection object :-)