Avoid infinite loop with invalid YEARLY recurrence rule.

lib/ical/recur_iterator.js

@@ -179,7 +179,14 @@ class RecurIterator {
  this.initialized = options.initialized || false;
 
  if (!this.initialized) {
- this.init();
+ try {
+ this.init();
+ } catch (e) {
+ // Init may error if there are no possible recurrence instances from the
+ // rule, but we don't want to bubble this error up. Instead, we create
+ // an empty iterator.
+ this.completed = true;
+ }
  }
  }
 
@@ -251,14 +258,28 @@ class RecurIterator {
  }
 
  if (this.rule.freq == "YEARLY") {
- for (;;) {
+ // Some yearly recurrence rules may be specific enough to not actually
+ // occur on a yearly basis, e.g. the 29th day of February or the fifth
+ // Monday of a given month. The standard isn't clear on the intended
+ // behavior in these cases, but `libical` at least will iterate until it
+ // finds a matching year.
+ // CAREFUL: Some rules may specify an occurrence that can never happen,
+ // e.g. the first Monday of April so long as it falls on the 15th
+ // through the 21st. Detecting these is non-trivial, so ensure that we
+ // stop iterating at some point.
+ const untilYear = this.rule.until ? this.rule.until.year : 20000;
+ while (this.last.year <= untilYear) {
  this.expand_year_days(this.last.year);
  if (this.days.length > 0) {
  break;
  }
  this.increment_year(this.rule.interval);
  }
 
+ if (this.days.length == 0) {
+ throw new Error("No possible occurrences");
+ }
+
  this._nextByYearDay();
  }
 
@@ -331,11 +352,10 @@ class RecurIterator {
 
  if ((this.rule.count && this.occurrence_number >= this.rule.count) ||
  (this.rule.until && this.last.compare(this.rule.until) > 0)) {
-
- //XXX: right now this is just a flag and has no impact
- // we can simplify the above case to check for completed later.
  this.completed = true;
+ }
 
+ if (this.completed) {
  return null;
  }
 
@@ -345,7 +365,6 @@ class RecurIterator {
  return this.last;
  }
 
-
  let valid;
  do {
  valid = 1;

test/recur_iterator_test.js

@@ -145,6 +145,11 @@ suite('recur_iterator', function() {
  let recur = ICAL.Recur.fromString(ruleString);
  let iterator = recur.iterator(start);
 
+ if (options.noInstance) {
+ assert.equal(iterator.next(), null);
+ return;
+ }
+
  let inc = 0;
  let dates = [];
  let next, max;
@@ -915,6 +920,13 @@ suite('recur_iterator', function() {
  ]
  });
 
+ // Invalid recurrence rule. The first Monday can never fall later than the
+ // 7th.
+ testRRULE('FREQ=YEARLY;BYMONTHDAY=15,16,17,18,19,20,21;BYDAY=1MO', {
+ dtStart: '2015-01-01T08:00:00',
+ noInstance: true,
+ });
+
  // Tycho brahe days - yearly, byYearDay with negative offsets
  testRRULE('FREQ=YEARLY;BYYEARDAY=1,2,4,6,11,12,20,42,48,49,-306,-303,' +
  '-293,-292,-266,-259,-258,-239,-228,-209,-168,-164,-134,-133,' +

test/recur_test.js

@@ -26,56 +26,56 @@ suite('recur', function() {
  });
  }
 
- function checkThrow(data, expectedMessage, dtstart, stack) {
+ function checkNoInstance(data, expectedMessage, dtstart, stack) {
  test(expectedMessage, function() {
  let recur = new ICAL.Recur(data);
  if (dtstart) {
  dtstart = ICAL.Time.fromString(dtstart);
  } else {
  dtstart = ICAL.Time.epochTime.clone();
  }
- assert.throws(function() {
-   recur.iterator(dtstart);
- }, expectedMessage);
+
+ const iter = recur.iterator(dtstart);
+ assert.equal(iter.next(), null);
  });
  }
 
- checkThrow({
+ checkNoInstance({
  parts: {
  BYYEARDAY: [3, 4, 5],
  BYMONTH: [2]
  }
  }, 'Invalid BYYEARDAY rule');
 
- checkThrow({
+ checkNoInstance({
  parts: {
  BYWEEKNO: [3],
  BYMONTHDAY: [2]
  }
  }, 'BYWEEKNO does not fit to BYMONTHDAY');
 
- checkThrow({
+ checkNoInstance({
  freq: 'MONTHLY',
  parts: {
  BYWEEKNO: [30]
  }
  }, 'For MONTHLY recurrences neither BYYEARDAY nor BYWEEKNO may appear');
 
- checkThrow({
+ checkNoInstance({
  freq: 'WEEKLY',
  parts: {
  BYMONTHDAY: [20]
  }
  }, 'For WEEKLY recurrences neither BYMONTHDAY nor BYYEARDAY may appear');
 
- checkThrow({
+ checkNoInstance({
  freq: 'DAILY',
  parts: {
  BYYEARDAY: [200]
  }
  }, 'BYYEARDAY may only appear in YEARLY rules');
 
- checkThrow({
+ checkNoInstance({
  freq: 'MONTHLY',
  parts: {
  BYDAY: ['-6TH']