Infinite loop caused by timezone rule

georgehrke commented 7 years ago

Reproducible on latest master: https://github.com/mozilla-comm/ical.js/commit/6702f7b36d6302a740bf06da8e986365a37930fa

Expected behavior

it should create a proper ICAL.RecurExpansion object

Actual behavior

it ends up in an infinite loop

Code to reproduce

const ical = require('./ical.js');

const fs = require('fs');
const ics = fs.readFileSync('./vtimezone.ics', 'utf8');

const jCal = ICAL.parse(ics);
const comp = new ICAL.Component(jCal);

const vtimezones = comp.getAllSubcomponents('vtimezone');
vtimezones.forEach(function(vtimezone) {
    const timezone = new ICAL.Timezone(vtimezone);
    ICAL.TimezoneService.register(timezone.tzid, timezone);
});

const vevents = comp.getAllSubcomponents('vevent');
const vevent = vevents.find((vevent) => !vevent.hasProperty('recurrence-id'));

console.log('foo');
const iterator = new ICAL.RecurExpansion({
    component: vevent,
    dtstart: vevent.getFirstPropertyValue('dtstart')
});

console.log('bar');

If you run this code, it will only output foo and keep running If i comment out the ICAL.TimezoneService.register lines, it also prints bar and terminates.

ical data

BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//The Horde Project//Horde iCalendar Library//EN
BEGIN:VEVENT
DTSTART;TZID=Europe/Berlin:20151009T063000
DTEND;TZID=Europe/Berlin:20151009T073000
DTSTAMP:20170510T173927Z
UID:63864894-920F-4BC6-A025-70225C524E25
CREATED:20161013T142713Z
LAST-MODIFIED:20161023T140234Z
SUMMARY:FOOBAR
CLASS:PUBLIC
STATUS:CONFIRMED
TRANSP:OPAQUE
RRULE:FREQ=WEEKLY;INTERVAL=1;BYDAY=FR;UNTIL=20161014T043000Z
EXDATE;TZID=Europe/Berlin:20151211T053000Z
EXDATE;TZID=Europe/Berlin:20160722T043000Z
EXDATE;TZID=Europe/Berlin:20161021T043000Z
END:VEVENT
BEGIN:VTIMEZONE
TZID:Europe/Berlin
BEGIN:DAYLIGHT
TZOFFSETFROM:+0100
TZOFFSETTO:+0200
DTSTART:19160430T230000
TZNAME:CEST
END:DAYLIGHT
BEGIN:STANDARD
TZOFFSETFROM:+0200
TZOFFSETTO:+0100
DTSTART:19161001T010000
TZNAME:CE-T
END:STANDARD
BEGIN:DAYLIGHT
TZOFFSETFROM:+0100
TZOFFSETTO:+0200
DTSTART:19170416T020000
RRULE:FREQ=YEARLY;BYMONTH=4;BYMONTHDAY=15,16,17,18,19,20,21;BYDAY=1MO;UNTIL
 =19180421T000000Z
TZNAME:CEST
END:DAYLIGHT
BEGIN:STANDARD
TZOFFSETFROM:+0200
TZOFFSETTO:+0100
DTSTART:19170917T020000
RRULE:FREQ=YEARLY;BYMONTH=9;BYMONTHDAY=15,16,17,18,19,20,21;BYDAY=1MO;UNTIL
 =19180915T000000Z
TZNAME:CE-T
END:STANDARD
BEGIN:DAYLIGHT
TZOFFSETFROM:+0100
TZOFFSETTO:+0200
DTSTART:19400401T020000
TZNAME:CEST
END:DAYLIGHT
BEGIN:STANDARD
TZOFFSETFROM:+0200
TZOFFSETTO:+0100
DTSTART:19421102T020000
TZNAME:CE-T
END:STANDARD
BEGIN:DAYLIGHT
TZOFFSETFROM:+0100
TZOFFSETTO:+0200
DTSTART:19430329T020000
TZNAME:CEST
END:DAYLIGHT
BEGIN:STANDARD
TZOFFSETFROM:+0200
TZOFFSETTO:+0100
DTSTART:19431004T020000
TZNAME:CE-T
END:STANDARD
BEGIN:DAYLIGHT
TZOFFSETFROM:+0100
TZOFFSETTO:+0200
DTSTART:19440403T020000
RRULE:FREQ=YEARLY;BYMONTH=4;BYDAY=1MO;UNTIL=19450401T010000Z
TZNAME:CEST
END:DAYLIGHT
BEGIN:STANDARD
TZOFFSETFROM:+0200
TZOFFSETTO:+0100
DTSTART:19441002T020000
TZNAME:CE-T
END:STANDARD
BEGIN:STANDARD
TZOFFSETFROM:+0100
TZOFFSETTO:+0100
DTSTART:19450916T020000
TZNAME:CE-T
END:STANDARD
BEGIN:DAYLIGHT
TZOFFSETFROM:+0100
TZOFFSETTO:+0300
DTSTART:19450524T020000
TZNAME:CEMT
END:DAYLIGHT
BEGIN:DAYLIGHT
TZOFFSETFROM:+0300
TZOFFSETTO:+0200
DTSTART:19450924T030000
TZNAME:CEST
END:DAYLIGHT
BEGIN:STANDARD
TZOFFSETFROM:+0200
TZOFFSETTO:+0100
DTSTART:19451118T020000
TZNAME:CE-T
END:STANDARD
BEGIN:DAYLIGHT
TZOFFSETFROM:+0100
TZOFFSETTO:+0200
DTSTART:19460414T020000
TZNAME:CEST
END:DAYLIGHT
BEGIN:STANDARD
TZOFFSETFROM:+0200
TZOFFSETTO:+0100
DTSTART:19461007T020000
TZNAME:CE-T
END:STANDARD
BEGIN:STANDARD
TZOFFSETFROM:+0100
TZOFFSETTO:+0100
DTSTART:19471005T020000
RRULE:FREQ=YEARLY;BYMONTH=10;BYDAY=1SU;UNTIL=19491002T010000Z
TZNAME:CE-T
END:STANDARD
BEGIN:DAYLIGHT
TZOFFSETFROM:+0100
TZOFFSETTO:+0200
DTSTART:19470406T030000
TZNAME:CEST
END:DAYLIGHT
BEGIN:DAYLIGHT
TZOFFSETFROM:+0200
TZOFFSETTO:+0300
DTSTART:19470511T020000
TZNAME:CEMT
END:DAYLIGHT
BEGIN:DAYLIGHT
TZOFFSETFROM:+0300
TZOFFSETTO:+0200
DTSTART:19470629T030000
TZNAME:CEST
END:DAYLIGHT
BEGIN:DAYLIGHT
TZOFFSETFROM:+0200
TZOFFSETTO:+0200
DTSTART:19480418T020000
TZNAME:CEST
END:DAYLIGHT
BEGIN:DAYLIGHT
TZOFFSETFROM:+0200
TZOFFSETTO:+0200
DTSTART:19490410T020000
TZNAME:CEST
END:DAYLIGHT
BEGIN:DAYLIGHT
TZOFFSETFROM:+0100
TZOFFSETTO:+0200
DTSTART:19800406T010000
RRULE:FREQ=YEARLY;BYMONTH=4;BYDAY=1SU;UNTIL=19800406T000000Z
TZNAME:CEST
END:DAYLIGHT
BEGIN:STANDARD
TZOFFSETFROM:+0200
TZOFFSETTO:+0100
DTSTART:19800928T010000
RRULE:FREQ=YEARLY;BYDAY=-1SU;BYMONTH=9;UNTIL=19950923T230000Z
TZNAME:CE-T
END:STANDARD
BEGIN:DAYLIGHT
TZOFFSETFROM:+0100
TZOFFSETTO:+0200
DTSTART:19810329T010000
RRULE:FREQ=YEARLY;BYDAY=-1SU;BYMONTH=3
TZNAME:CEST
END:DAYLIGHT
BEGIN:STANDARD
TZOFFSETFROM:+0200
TZOFFSETTO:+0100
DTSTART:19961027T010000
RRULE:FREQ=YEARLY;BYDAY=-1SU;BYMONTH=10
TZNAME:CE-T
END:STANDARD
END:VTIMEZONE
END:VCALENDAR

georgehrke commented 6 years ago

@mschroeder @kewisch I'd be happy to send a pull-request to fix this. Any pointers how to debug this?

kewisch commented 6 years ago

Thanks for the offer! First of all I would try to minimize the ics in the testcase to see if it is something about the VEVENT or the VTIMEZONE. Timezone calculation is also done with help of the recurrence parser. From there, you can drill down into RecurExpansion, my guess would be that it is the recurrence iterator code instead.

@Decathlon1578 might also have some ideas here, as he had fixed some recurrence parser issues in the past.

BigMichi1 commented 4 years ago

today i run into the same issue, as a dirty hack i added a check for years > 2100 in recur_iterator.js to stop the yearly loop

      if (this.rule.freq == "YEARLY") {
        for (;;) {
          this.expand_year_days(this.last.year);
          if (this.days.length > 0) {
            break;
          }

if(this.last.year > 2100) {
    break;
}
          this.increment_year(this.rule.interval);
        }

        this._nextByYearDay();
      }

in my case the browser was also stuck and putting breakpoints in this loop showed my after some seconds numbers for this.last.year > 44400 which is a little bit out of range.

georgehrke commented 4 years ago

On top of debugging the actual problem here I see multiple ways to mitigate this and similar issues in the future:

Have a counter and stop after a given amount of instances. This is also something that Sabre/VObject does. Although it's not 100% comparable, since Sabre/VObject is running with limited execution time on the server and ical.js is running in the browser / desktop. If we decide to go for this, i would pick a higher limit than Sabre does.
Do not allow Dates bigger than 31.12.9999

@kewisch Would you be fine with such changes?

kewisch / ical.js