search

python-caldav / caldav

Apache License 2.0
319 stars 95 forks source link

Task editing issue / uuid / Nextcloud #153

Closed l0ga closed 2 years ago

l0ga commented 2 years ago

It looks like there's an issue with the way caldav 0.8.0 handles task editing (or with the way I do it 😄 )

When I try to edit any task (located on Nextcloud 21.0.3 / calendar ver 2.3.3 / tasks ver 0.14.2) with uids that look like, well uuids (22bf21be-9d8f-4cae-98a8-161e370a997e), I constantly get an error. When uids look like a hex strings (d34e3a88a5ab4de08e389b6383610586), I can update tasks without any code corrections.

The code I tried I basically took from examples:

client = caldav.DAVClient(url=caldav_url, username=username, password=password)
my_principal = client.principal()
calendars = my_principal.calendars()
if calendars:
  for c in calendars:    
    todos = c.todos()    
    for todo in todos:
      ...
      c.save_todo(todo.data)

The error itself:

Traceback (most recent call last):
  File "C:\Users\usernamehere\AppData\Local\Programs\Python\Python37\lib\site-packages\caldav\objects.py", line 1464, in save
    self._create(self.data, self.id, path)
  File "C:\Users\usernamehere\AppData\Local\Programs\Python\Python37\lib\site-packages\caldav\objects.py", line 1374, in _create
    raise error.PutError(errmsg(r))
caldav.lib.error.PutError: 400 Bad Request

b'<?xml version="1.0" encoding="utf-8"?>\n<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">\n  <s:exception>Sabre\\DAV\\Exception\\BadRequest</s:exception>\n  <s:message>Calendar object with uid already exists in this calendar collection.</s:message>\n</d:error>\n'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "M:\Downloads\force_update_task.py", line 34, in <module>
    c.save_todo(todo.data)
  File "C:\Users\usernamehere\AppData\Local\Programs\Python\Python37\lib\site-packages\caldav\objects.py", line 598, in save_todo
    return Todo(self.client, data=ical, parent=self).save(no_overwrite=no_overwrite, no_create=no_create, obj_type='todo')
  File "C:\Users\usernamehere\AppData\Local\Programs\Python\Python37\lib\site-packages\caldav\objects.py", line 1466, in save
    self._create(self.vobject_instance.serialize(), self.id, path)
  File "C:\Users\usernamehere\AppData\Local\Programs\Python\Python37\lib\site-packages\caldav\objects.py", line 1374, in _create
    raise error.PutError(errmsg(r))
caldav.lib.error.PutError: 400 Bad Request

b'<?xml version="1.0" encoding="utf-8"?>\n<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">\n  <s:exception>Sabre\\DAV\\Exception\\BadRequest</s:exception>\n  <s:message>Calendar object with uid already exists in this calendar collection.</s:message>\n</d:error>\n'

The error can be easily replicated with Nextcloud Tasks (by default it creates tasks with uuids). Maybe it's related to dashes?

I think it's an issue with the library because other caldav clients (2do, nextcloud itself) update tasks with such uids just fine.

tobixen commented 2 years ago

For me it looks like a server error. :-) There is no difference on the library side on weather the uuid contains dashes or not, and the server should not complain that the task already exists when one is trying to do a PUT towards it.

It would be very nice to have a debug log of all the communication with the server, when using caldav library vs when using another caldav client.

l0ga commented 2 years ago

Did some more research. Nextcloud-created tasks' URL/URI and UID are not the same. So a task located at /apps/tasks/#/calendars/testcal/tasks/15340142-5713-4847-907D-7539A05E0EA0.ics can have UID inside .ics b6de9389-98c0-483c-a842-8c2908a239ef. Can it be related?

tobixen commented 2 years ago

That's right. The file name and UID may not always match, but it's important to PUT things to the correct path. If the caldav library is actually doing a GET from /apps/tasks/#/calendars/testcal/tasks/15340142-5713-4847-907D-7539A05E0EA0.ics and later a PUT to /apps/tasks/#/calendars/testcal/tasks/b6de9389-98c0-483c-a842-8c2908a239ef.ics, then that would be a big bug. Are you able to check that up as well?

raimund-schluessler commented 2 years ago

That's right. The file name and UID may not always match, but it's important to PUT things to the correct path. If the caldav library is actually doing a GET from /apps/tasks/#/calendars/testcal/tasks/15340142-5713-4847-907D-7539A05E0EA0.ics and later a PUT to /apps/tasks/#/calendars/testcal/tasks/b6de9389-98c0-483c-a842-8c2908a239ef.ics, then that would be a big bug. Are you able to check that up as well?

My guess is that exactly this happens and it only gets problematic because the UID and URI are different (which they are allowed to 😉).

Without knowing your code to well, I guess that this fallback: https://github.com/python-caldav/caldav/blob/8ccf97d13bb6a2b92ffcc7709e249085e387ffc9/caldav/objects.py#L1380-L1384

might be triggered, because when saving a Todo, the url variable is not given and ends up being None: https://github.com/python-caldav/caldav/blob/8ccf97d13bb6a2b92ffcc7709e249085e387ffc9/caldav/objects.py#L601-L608 https://github.com/python-caldav/caldav/blob/8ccf97d13bb6a2b92ffcc7709e249085e387ffc9/caldav/objects.py#L1192

Could that lead to path being None as well and triggering the fallback, maybe?

tobixen commented 2 years ago

Line 1384 was modified some few days ago in the master branch, but the general logic (default the URL to the uuid) is the same (only difference is that the uuid should be safe-quoted, with slashes being doubly quoted to let it pass more easily through proxies). In any case, line 1384 should only apply when creating new calendar objects, not when editing old ones.

When editing a task, you should basically first load it, edit it and save it - and when loading the URL should be saved in self.url or self.path.

Please do some debugging if you have the chance ... otherwise I'll try to find some time to dig deeper into it.

l0ga commented 2 years ago

So, can I help somehow? This behavior can break (at least) all Nextcloud-based setups. Right now the code works only if such tasks are explicitly ignored.

tobixen commented 2 years ago

If I can have an URL and some credentials for a Nextcloud caldav server, I will run the functional tests towards it. If they break, then I will have something to look into. If not, then we should try to write up a new test to catch this issue and then look into it.

Sigmun commented 2 years ago

you can create an account on https://e.foundation/e-email-invite/ which gives you free access to a nextcloud caldav and space with 1Gb

tobixen commented 2 years ago

Cool. Maybe I should try their android operating system as well. Managed to set up an account and run the test suite. 2 errors and 4 failures. I've seen it worse :-) I'll do a little bit of digging before bedtime.

tobixen commented 2 years ago

Found three issues ... and only one of them is remotely associated with the reported issue

Next I will try to make a test that explicitly creates an task with a UID and a different file name, fetches it, saves it and fetches it again ...

tobixen commented 2 years ago

Trying to make a test to replicate the problem ... it would be nice to first know exactly what the problem is. And then I read this ...

if calendars:
  for c in calendars:    
    todos = c.todos()    
    for todo in todos:
      ...
      c.save_todo(todo.data)

Of course this will break! Perhaps the problem is neither on the server side nor on the client side, but in the example code!

The code above reads a list of todos, and then attempts to save the icalendar data back to the calendar. The todo object contains some information, like the URL of the object itself (including the file name). todo.data only contains the icalendar data. When the caldav library is tasked to saving some ical data to the calendar, it will make up a URL, with "${uuid}.ics" as the file name. If the task already exists with another filename but the same uuid, the server is in it's full right to politely reject it.

(Why on earth should a "calendar resource object" have a unique file name independent from it's uuid? I don't know. There is a saying that "If all you have is a hammer, everything looks like a nail" - probably the guy inventing or championing the CalDAV protocol had the DAV protocol as his only hammer).

Try replacing it with something like this (untested code ahead - may or may not work):

if calendars:
  for c in calendars:
    todos = c.todos()
    for todo in todos:
      ...
      todo.save()
tobixen commented 2 years ago

There seems to be no references to save_todo(todo.data) in the examples/*.py in the HEAD of the master branch.

Can I close this issue? :-)

l0ga commented 2 years ago

Let me run some tests

l0ga commented 2 years ago

Hard to say why I'd decided to use c.save_todo(todo.data). Maybe there were not enough different examples/documentation of proper VTODO handling for me :( Still, save_todo(ical_text_here) IS the way to create a new task, correct?

Right now everything works fine (nextcloud-created tasks included). Thanks! The library is really helpful 👍

tobixen commented 2 years ago

Maybe there were not enough different examples/documentation of proper VTODO handling for me

I should probably look into that

Still, save_todo(ical_text_here) IS the way to create a new task, correct?

Yes, indeed

Shahin-rmz commented 2 years ago

Hello, Thank for providing such an awesome library. I also happen to make a todo on my selfhosted Nextcloud. Thanks to the nice examples and docs, I was able to add events and todos to calendars app. However it is not possible to add todo to my tasks app. here is how my function looks like so far:

my_task = workingcal.save_todo("""BEGIN:VCALENDAR
  VERSION:2.0
  PRODID:-//Example Corp.//CalDAV Client//EN
  BEGIN:VEVENT
  UID:20220516T06i3000Z-123401@example.com
  DTSTAMP:20220716T060000Z
  DTSTART:20220717T060000Z
  DTEND:20220717T230000Z
  RRULE:FREQ=YEARLY
  SUMMARY:Python4 test
  END:VEVENT
  END:VCALENDAR
  """)

Any idea how should I save the app to be shown on tasks? Thanks alot.

tobixen commented 2 years ago

Hmm ... maybe this:

my_task = workingcal.save_todo("""BEGIN:VCALENDAR
  VERSION:2.0
  PRODID:-//Example Corp.//CalDAV Client//EN
  BEGIN:VTODO
  UID:20220516T06i3000Z-123401@example.com
  DTSTAMP:20220716T060000Z
  SUMMARY:Python4 test
  END:VTODO
  END:VCALENDAR
  """)
Shahin-rmz commented 2 years ago

@tobixen Thanks so much. works perfectly. Any idea where can I read about cardav format? I want to set due date for the task and just play with it. Thanks

tobixen commented 2 years ago

https://icalendar.org/RFC-Specifications/iCalendar-RFC-5545/ perhaps.

I'm intending to fix the caldav library so that one can add simple tasks and events without knowing the ics format

Shahin-rmz commented 2 years ago

I'm intending to fix the caldav library so that one can add simple tasks and events without knowing the ics format

Thanks so much. I'll be happy to contribute as well.

tobixen commented 2 years ago

Oh, I have already implemented something, though I haven't tested it very well yet ... try if this works for you:

from datetime import datetime
my_task = workingcal.save_todo(summary="python4 test", due=datetime.now())
Shahin-rmz commented 2 years ago

Oh, I have already implemented something, though I haven't tested it very well yet ... try if this works for you:

Yes and it seems awesome. Actually last time I've tried it out but got some problems. Will double check it later on.