todoman icon indicating copy to clipboard operation
todoman copied to clipboard

empty PRIORITY field after syncing causes task not to be read

Open Mome opened this issue 6 years ago • 10 comments

When I create a new task without setting the priority the "PRIORITY" field in the corresponding *.ics file is omitted. After syncing my tasks to a server with vdirsyncer sync a PRIORITY: field is created, however lacking any value. Calling todoman results in an error:

Failed to read entry /home/mome/.calendars/tasks~ygzkBl6WgmyiuufuC7jh0NW/[email protected].
Traceback (most recent call last):
  File "/home/mome/.local/lib/python3.6/site-packages/icalendar/prop.py", line 234, in from_ical
    return cls(ical)
  File "/home/mome/.local/lib/python3.6/site-packages/icalendar/prop.py", line 224, in __new__
    self = super(vInt, cls).__new__(cls, *args, **kwargs)
ValueError: invalid literal for int() with base 10: ''

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/mome/code/todoman/todoman/model.py", line 922, in update_cache
    cal = icalendar.Calendar.from_ical(cal)
  File "/home/mome/.local/lib/python3.6/site-packages/icalendar/cal.py", line 383, in from_ical
    vals = factory(factory.from_ical(vals))
  File "/home/mome/.local/lib/python3.6/site-packages/icalendar/prop.py", line 236, in from_ical
    raise ValueError('Expected int, got: %s' % ical)
ValueError: Expected int, got: 

Setting a default priority or allowing to read the empty value would solve the problem.

For a fast fix I simply filter every line that ends with a colon just before cal = icalendar.Calendar.from_ical(cal) in model.py

def filter_no_value(ical):
        f = lambda line : not line.strip().endswith(b':')
        lines = ical.split(b'\n')
        lines = filter(f, lines)
        return b'\n'.join(lines)

works for me ...

Mome avatar Jul 12 '18 17:07 Mome

An empty priority is invalid, it MUST be numeric, or absent (in which case 0) is assumed. That said, converting empty values to 0 might not be bad practice if we see this a going on a lot (eg: some popular server adding an empty priority).

Do you have any idea if it's vdirsyncer or your server that's adding the empty priority?

WhyNotHugo avatar Jul 12 '18 17:07 WhyNotHugo

I made a clean installation on another (Debian) system. I am syncing to Horde, which uses SabreDAV. So when I create a new task, PRIORITY is absent. If I sync, the task is uploaded and the Horde webclient tells me the priority is 0. Which is not a value you can choose from in horde. When I sync again, without changing anything in between, vdirsyncer updates the local file and PRIORITY has no value. If I set the value to 0 in the file, it stays after syncing or is changed to 5, not really deterministic.

Do you know another client I can test the same thing with?

Mome avatar Jul 12 '18 20:07 Mome

(FWIW, 0 is a valid value, and mean "no priority given").

Hmm, this is really odd. I sounds to me like Horde might be doing something funky. The only other CalDAV client I've used is the stock iOS one, and it doesn't seem to have reproduced that (I'm using Fastmail as a server, which is basically Cyrus, AFAIK).

@untitaker: Have you seen anything like this before?

WhyNotHugo avatar Jul 12 '18 20:07 WhyNotHugo

I haven't encountered this before and I'd say it's perfectly clear from @Mome's description what Horde is doing, but not why. I don't think you'll see anything different with other clients unless they remove the priority value if it is 0.

I would suggest to file a bug against Horde and work around this within todoman.

untitaker avatar Jul 12 '18 20:07 untitaker

It seems that (at least my instance of) Horde allows priority values between 1 and 5. vdirsyncer+horde works perfectly within this range, outside it becomes weird.

For me an additional entry in the todoman configuration with a priority_range would be best.

Mome avatar Jul 17 '18 07:07 Mome

Ok, I tried implementing priority_range. I failed. Instead I introduced a default_priority: #316

Mome avatar Jul 17 '18 13:07 Mome

The filter suggested above will strip out any other lines that end in colons, eg: summaries (which can be valid, albeit odd):

SUMMARY: Do stuff:

WhyNotHugo avatar Jul 17 '18 16:07 WhyNotHugo

I only just realized it's icalendar that blows up with these values, and not us, so I'm still thinking about how to work around it.

WhyNotHugo avatar Jul 17 '18 16:07 WhyNotHugo

I have exactly the same problem with a Horde server. I solved with a "default_priority=9" in the configuration file. However, it is not a universal solution, because any task generated with other client without a priority will produce todoman to crash.

chrpinedo avatar Jan 16 '20 13:01 chrpinedo

@chrpinedo This bug is really on the Horde side. PRIORITY: is invalid; the line is either missing or has a numeric value. An empty string is not a valid choice. You should really report the bug to them so they may address it.

That said, I'm willing to accept PRs that interpret PRIORITY: as PRIORITY:0. Even though it's technically illegal input, I do adhere to the "Be conservative in what you send, be liberal in what you accept" principle.

WhyNotHugo avatar Jan 28 '20 14:01 WhyNotHugo