Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Crash when importing event with invalid VALARM/TRIGGER #1217

Closed
WhyNotHugo opened this issue Jan 22, 2023 · 3 comments
Closed

Crash when importing event with invalid VALARM/TRIGGER #1217

WhyNotHugo opened this issue Jan 22, 2023 · 3 comments
Labels
type: bug type: 3rd party compatibility

Comments

@WhyNotHugo
Copy link
Member

I tried importing this file, which is generated by NS International.

BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//NS Internationaal B.V.//sales-v2//en
BEGIN:VTIMEZONE
TZID:Europe/Brussels
LAST-MODIFIED:20221105T024525Z
TZURL:http://www.tzurl.org/zoneinfo/Europe/Brussels
X-LIC-LOCATION:Europe/Brussels
X-PROLEPTIC-TZNAME:LMT
BEGIN:STANDARD
TZNAME:BMT
TZOFFSETFROM:+0017
TZOFFSETTO:+0017
DTSTART:18800101T000000
END:STANDARD
BEGIN:STANDARD
TZNAME:WET
TZOFFSETFROM:+0017
TZOFFSETTO:+0000
DTSTART:18920501T001730
END:STANDARD
BEGIN:STANDARD
TZNAME:CET
TZOFFSETFROM:+0000
TZOFFSETTO:+0100
DTSTART:19141108T000000
END:STANDARD
BEGIN:STANDARD
TZNAME:CET
TZOFFSETFROM:+0200
TZOFFSETTO:+0100
DTSTART:19161001T010000
RDATE:19421102T030000
RDATE:19431004T030000
RDATE:19440917T030000
RDATE:19450916T030000
RDATE:19461007T030000
RDATE:19770925T030000
RDATE:19781001T030000
END:STANDARD
BEGIN:STANDARD
TZNAME:CET
TZOFFSETFROM:+0200
TZOFFSETTO:+0100
DTSTART:19170917T030000
RRULE:FREQ=YEARLY;UNTIL=19180916T010000Z;BYDAY=3MO;BYMONTH=9
END:STANDARD
BEGIN:STANDARD
TZNAME:WET
TZOFFSETFROM:+0100
TZOFFSETTO:+0000
DTSTART:19181111T120000
RDATE:19191005T000000
RDATE:19201024T000000
RDATE:19211026T000000
RDATE:19391119T030000
END:STANDARD
BEGIN:STANDARD
TZNAME:WET
TZOFFSETFROM:+0100
TZOFFSETTO:+0000
DTSTART:19221008T000000
RRULE:FREQ=YEARLY;UNTIL=19271001T230000Z;BYDAY=SU;BYMONTHDAY=2,3,4,5,6,7,8;
 BYMONTH=10
END:STANDARD
BEGIN:STANDARD
TZNAME:WET
TZOFFSETFROM:+0100
TZOFFSETTO:+0000
DTSTART:19281007T030000
RRULE:FREQ=YEARLY;UNTIL=19381002T020000Z;BYDAY=SU;BYMONTHDAY=2,3,4,5,6,7,8;
 BYMONTH=10
END:STANDARD
BEGIN:STANDARD
TZNAME:CET
TZOFFSETFROM:+0100
TZOFFSETTO:+0100
DTSTART:19770101T000000
END:STANDARD
BEGIN:STANDARD
TZNAME:CET
TZOFFSETFROM:+0200
TZOFFSETTO:+0100
DTSTART:19790930T030000
RRULE:FREQ=YEARLY;UNTIL=19950924T010000Z;BYDAY=-1SU;BYMONTH=9
END:STANDARD
BEGIN:STANDARD
TZNAME:CET
TZOFFSETFROM:+0200
TZOFFSETTO:+0100
DTSTART:19961027T030000
RRULE:FREQ=YEARLY;BYDAY=-1SU;BYMONTH=10
END:STANDARD
BEGIN:DAYLIGHT
TZNAME:CEST
TZOFFSETFROM:+0100
TZOFFSETTO:+0200
DTSTART:19160501T000000
RDATE:19400520T030000
RDATE:19430329T020000
RDATE:19440403T020000
RDATE:19450402T020000
RDATE:19460519T020000
END:DAYLIGHT
BEGIN:DAYLIGHT
TZNAME:CEST
TZOFFSETFROM:+0100
TZOFFSETTO:+0200
DTSTART:19170416T020000
RRULE:FREQ=YEARLY;UNTIL=19180415T010000Z;BYDAY=3MO;BYMONTH=4
END:DAYLIGHT
BEGIN:DAYLIGHT
TZNAME:WEST
TZOFFSETFROM:+0000
TZOFFSETTO:+0100
DTSTART:19190301T230000
RDATE:19200214T230000
RDATE:19210314T230000
RDATE:19220325T230000
RDATE:19230421T230000
RDATE:19240329T230000
RDATE:19250404T230000
RDATE:19260417T230000
RDATE:19270409T230000
RDATE:19280414T230000
RDATE:19290421T020000
RDATE:19300413T020000
RDATE:19310419T020000
RDATE:19320403T020000
RDATE:19330326T020000
RDATE:19340408T020000
RDATE:19350331T020000
RDATE:19360419T020000
RDATE:19370404T020000
RDATE:19380327T020000
RDATE:19390416T020000
RDATE:19400225T020000
END:DAYLIGHT
BEGIN:DAYLIGHT
TZNAME:CEST
TZOFFSETFROM:+0200
TZOFFSETTO:+0200
DTSTART:19440903T000000
END:DAYLIGHT
BEGIN:DAYLIGHT
TZNAME:CEST
TZOFFSETFROM:+0100
TZOFFSETTO:+0200
DTSTART:19770403T020000
RRULE:FREQ=YEARLY;UNTIL=19800406T010000Z;BYDAY=1SU;BYMONTH=4
END:DAYLIGHT
BEGIN:DAYLIGHT
TZNAME:CEST
TZOFFSETFROM:+0100
TZOFFSETTO:+0200
DTSTART:19810329T020000
RRULE:FREQ=YEARLY;BYDAY=-1SU;BYMONTH=3
END:DAYLIGHT
END:VTIMEZONE
BEGIN:VEVENT
UID:4e3e3186-28c6-42d4-81cf-eb4f56d587f1
DTSTAMP:20230122T142045Z
SUMMARY:Trip from Utrecht to Bruxelles Central/Brussel Centraal
DESCRIPTION:Trip from Utrecht to Bruxelles Central/Brussel Centraal
LOCATION:Utrecht
URL:https://www.nsinternational.com/en/traintickets-v3/
DTSTART;TZID=Europe/Brussels:20230203T151500
DTEND;TZID=Europe/Brussels:20230203T180600
BEGIN:VALARM
ACTION:DISPLAY
TRIGGER;RELATED=START:PT-2H
END:VALARM
END:VEVENT
BEGIN:VEVENT
UID:28d25a55-84cd-4564-910b-088066fd5244
DTSTAMP:20230122T142045Z
SUMMARY:Trip from Bruxelles Central/Brussel Centraal to Utrecht
DESCRIPTION:Trip from Bruxelles Central/Brussel Centraal to Utrecht
LOCATION:Bruxelles Central/Brussel Centraal
URL:https://www.nsinternational.com/en/traintickets-v3/
DTSTART;TZID=Europe/Brussels:20230206T095000
DTEND;TZID=Europe/Brussels:20230206T125000
BEGIN:VALARM
ACTION:DISPLAY
TRIGGER;RELATED=START:PT-2H
END:VALARM
END:VEVENT
END:VCALENDAR

Khal crashed with the following error:

Traceback (most recent call last):
  File "/usr/bin/khal", line 33, in <module>
    sys.exit(load_entry_point('khal==0.10.5', 'console_scripts', 'khal')())
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/click/core.py", line 1130, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/click/core.py", line 1055, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/click/core.py", line 1657, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/click/core.py", line 760, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/click/decorators.py", line 26, in new_func
    return f(get_current_context(), *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/khal/cli.py", line 465, in import_ics
    controllers.import_ics(
  File "/usr/lib/python3.11/site-packages/khal/controllers.py", line 595, in import_ics
    vevents = split_ics(ics, random_uid, conf['locale']['default_timezone'])
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/khal/icalendar.py", line 50, in split_ics
    cal = cal_from_ics(ics)
          ^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/khal/icalendar.py", line 536, in cal_from_ics
    return cal
           ^^^
UnboundLocalError: cannot access local variable 'cal' where it is not associated with a value

The issue seems to be this line in particular: TRIGGER;RELATED=START:PT-2H. I recently wrote a parser for the icalendar duration type, and it also rejects this input. After looking at the spec closely, I found the issue; this should be TRIGGER;RELATED=START:-PT2H. Note that the - goes before the duration itself, and not before each part (otherwise we'd be able to have 2H-3M, which is super quirky.

Anyway, it's clear that this input has a technically invalid alarm, but I think we can do better and khal should be able to:

  • Ignore any error in an ALARM anyway, khal doesn't deal with them for now, and importing it is better then crashing.
  • Show a better error message? It took me a while to figure out that my installation wasn't actually broken.

More into details:

The cal_from_ics function tries to return cal even if it's never defined it. Note that the if might be false, at which point cal is never assigned.

I don't think we need to parse alarms; if an alarm is invalid, we should ignore it and import it as-is. Khal doesn't deal with alarms anyway.
@geier
Copy link
Member

geier commented Jun 3, 2023 •
edited

The issue with trying to return cal will be fixed with #1259

With that applied we get

Invalid iCalendar duration: PT-2H
Traceback (most recent call last):
  File "/Users/cg/workspace/khal/khal/controllers.py", line 612, in import_ics
    vevents = split_ics(ics, random_uid, conf['locale']['default_timezone'])
  File "/Users/cg/workspace/khal/khal/icalendar.py", line 49, in split_ics
    cal = cal_from_ics(ics)
  File "/Users/cg/workspace/khal/khal/icalendar.py", line 539, in cal_from_ics
    cal = icalendar.Calendar.from_ical(ics)
  File "/Users/cg/mambaforge/lib/python3.9/site-packages/icalendar/cal.py", line 380, in from_ical
    vals = factory(factory.from_ical(vals))
  File "/Users/cg/mambaforge/lib/python3.9/site-packages/icalendar/prop.py", line 332, in from_ical
    return vDuration.from_ical(ical)
  File "/Users/cg/mambaforge/lib/python3.9/site-packages/icalendar/prop.py", line 477, in from_ical
    raise ValueError(f'Invalid iCalendar duration: {ical}')
ValueError: Invalid iCalendar duration: PT-2H

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/cg/workspace/khal/khal/cli.py", line 464, in import_ics
    controllers.import_ics(
  File "/Users/cg/workspace/khal/khal/controllers.py", line 614, in import_ics
    raise FatalError(error)
khal.exceptions.FatalError: Invalid iCalendar duration: PT-2H
critical: An error occurred when trying to import the file from ns.ics

@geier
Copy link
Member

geier commented Jun 3, 2023

For the actual issue: I believe we can't do anything about this at khal at the moment, instead we could need to teach icalendar to accept somewhat broken icalendar files.

@geier geier mentioned this issue Jun 3, 2023
Open
@geier
Copy link
Member

geier commented Jun 3, 2023

closed as duplicate of #1264

@geier geier closed this as completed Jun 3, 2023
@magarnicle magarnicle mentioned this issue Jun 6, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
type: bug type: 3rd party compatibility
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@geier @WhyNotHugo