Events with bad start / end order

[fabien-michel]

Describe the bug Some calendars in the wild contain events with end datetime before start datetime. (Seen with Sogo as calendar provider)

It make the library crash because of an assertion in Repetition.init ensuring the good order.

ICS file

BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//SabreDAV//SabreDAV//EN
CALSCALE:GREGORIAN
X-WR-CALNAME:test
X-APPLE-CALENDAR-COLOR:#e78074
BEGIN:VTIMEZONE
TZID:Europe/Berlin
X-LIC-LOCATION:Europe/Berlin
BEGIN:DAYLIGHT
TZOFFSETFROM:+0100
TZOFFSETTO:+0200
TZNAME:CEST
DTSTART:19700329T020000
RRULE:FREQ=YEARLY;BYMONTH=3;BYDAY=-1SU
END:DAYLIGHT
BEGIN:STANDARD
TZOFFSETFROM:+0200
TZOFFSETTO:+0100
TZNAME:CET
DTSTART:19701025T030000
RRULE:FREQ=YEARLY;BYMONTH=10;BYDAY=-1SU
END:STANDARD
END:VTIMEZONE
BEGIN:VEVENT
CREATED:20190303T111937
DTSTAMP:20190303T111937
LAST-MODIFIED:20190303T111937
UID:UYDQSG9TH4DE0WM3QFL2J
SUMMARY:test1
DTEND;TZID=Europe/Berlin:20190304T080000
DTSTART;TZID=Europe/Berlin:20190304T083000
END:VEVENT
END:VCALENDAR

Workaround

Currently, I have to monkey patch Repetiton class to override init method to invert start stop when needed (it is ok for what I need, but may not be suitable for others app use-cases)

def fix_start_end_order_in_repetition_init(original_init):
    def fixed_init(self, source, start, stop, *args, **kwargs):
        if stop < start:
            (start, stop) = (stop, start)
        return original_init(self, source, start, stop, *args, **kwargs)

    return fixed_init

recurring_ical_events.Repetition.__init__ = fix_start_end_order_in_repetition_init(
    recurring_ical_events.Repetition.__init__
)

Version:

 **Suggested implementation** Maybe it should be nice to offer a proper way to customize the library. For example, allow to passe custom classes to replace the default ones.

---

We're using Polar.sh so you can upvote and help fund this issue. We receive the funding once the issue is completed & confirmed by you. Thank you in advance for helping prioritize & fund our work.

[niccokunzmann]

Thanks for reporting this! Would you like to submit a fix?

I would add that it is probably good to check that JOURNAL and the other components also work in such a case.

[niccokunzmann]

Maybe it should be nice to offer a proper way to customize the library. For example, allow to passe custom classes to replace the default ones.

True. This might be another issue and a good suggestion is welcome!

[fabien-michel]

Thanks for reporting this! Would you like to submit a fix?

I'm not sure what would be a fix for that. Inverting start/end may not be the right solution since lib users would not expect it to change the data.

I'm thinking about 3 options:

1. Add a parameter "reorder_start_end" passed to of method which would allow handle this case
2. Add a parameter "fail_safe" passed to of method which could, in many parts of the code, make the library either do minor adaptations like this or would just skip the event if something wrong happen.
3. Add a proper way to customize the classes but I don't know what is common to do. May be passing a class mapping to of method ?

   
   class MyCustomRepetition(Repetition):
   pass

class_override = { Repetition: MyCustomRepetition, }

recurring_ical_events.of(calendar, class_override=class_override)

[niccokunzmann]

Inverting start/end may not be the right solution since lib users would not expect it to change the data.

This is the data:

DTEND;TZID=Europe/Berlin:20190304T080000
DTSTART;TZID=Europe/Berlin:20190304T083000

It looks like someone just accidentally swapped DTSTART and DTEND. (usually, the one attribute is positioned before the other. It might be worth reporting this back to the calendar implementation.) I would assume in this case that the library would just swap them and people do not have to wonder about a use-case that occurs once every few years for a few people only. One could set a value "X-DTSTART-DTEND-SWAPPED" to true.

I see two issues:

1. Fix what happens if DTSTART and DTEND are swapped. - I would discuss this here.
2. Change the architecture towards allowing to replace classes more easily - I would like to discuss this in another issue - this issue is an instance of the use-case (by title).

Splitting allows clear closing criteria.

What are your thoughts?

[fabien-michel]

Sorry, I lied and did not copy-paste true-life iCal data sample 😬. It's just an example which caused the lib to crash. I'm responsible for the strange order of DTSTART and DTEND.

Here's a real-life sample extracted from a Sogo calendar:

BEGIN:VEVENT
SUMMARY:XXX
DTSTART;TZID=Europe/Paris:20231218T234500
DTEND;TZID=Europe/Paris:20231218T233000
DTSTAMP:20231213T104027Z
UID:84A72-65798A00-5-20465200
SEQUENCE:1
ATTENDEE;CN="XXX";PARTSTAT=NEEDS-ACTION;ROLE=REQ-PARTICIPA
 NT;RSVP=TRUE:mailto:xxx@yyy.zzz
CLASS:PUBLIC
CREATED:20231213T104027Z
LAST-MODIFIED:20231213T104027Z
ORGANIZER;CN="XXX":mailto:aaa@yyy.zzz
TRANSP:OPAQUE
END:VEVENT

I agree that it should not happen often, but I've two seen it two times here with Sogo calendars. So I suppose it can occur to others.

I think we shouldn't reorder dates, it make the library doing unintended data manipulation. Make it less reliable in my opinion.

I suggest to add an option something like "strict=False" or "fail_safe=True" which allow to skip events with this kind of failures. This one and others that can occur during parsing.

It this vision I think we can start by replacing all assert calls by exceptions. I'm preparing a PR to do this.

[niccokunzmann]

It this vision I think we can start by replacing all assert calls by exceptions. I'm preparing a PR to do this.

I saw it: #134. Good idea!

Here's a real-life sample extracted from a Sogo calendar:

Often, such calendars have a human viewable form like a month-table/calendar view e.g. where you can enter events. I wonder what that looks like for this specific event. This library would ideally return the expected same result as the frontend would show. The Open Web Calendar uses recurring-ical-events. Thus, if the Open Web Calendar would produce the same view as Sogo does, that would be ideal!

[fabien-michel]

This is how the event appear in Sogo

[niccokunzmann]

Cool, thanks!

Debut means start, I think, and Au means end... It seems like in the calendar, they do not enforce START <= END. That might have reasons.

DTEND The value type of this property MUST be the same as the "DTSTART" property, and its value MUST be later in time than the value of the "DTSTART" property. - https://www.rfc-editor.org/rfc/rfc5545#page-96

So, I would say: On the calendar it looks like the event starts at 23:30, ends at 23:45. Start and end are reversed. This is a case that is forbidden by the RFC. These would be the steps I propose:

• [ ] Swap start and end as soon as possible in the code
• [ ] make sure that recurrence calculation uses the swapped values and returns the swapped values
• [ ] write to the developers of Sogo, telling them of the bug and that they do not produce valid ICS calendars.

This way, users of this library can input a Sogo calendar and not wonder about this special case and enjoy getting the result the calendar shows. What do you think about that?

[fabien-michel]

I didn't make it clear that I didn't figure out how such an event was created with the interface. I don't know how that happened. I can't repeat the bug.

[fabien-michel]

I will do a PR for swap start/end.