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

Avoid infinite loop with invalid YEARLY recurrence rule #621

Merged
merged 4 commits into from
Apr 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 44 additions & 6 deletions lib/ical/recur_iterator.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,19 @@ class RecurIterator {
this.initialized = options.initialized || false;

if (!this.initialized) {
this.init();
try {
this.init();
} catch (e) {
if (e instanceof InvalidRecurrenceRuleError) {
// 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;
} else {
// Propagate other errors to consumers.
throw e;
}
}
}
}

Expand Down Expand Up @@ -251,14 +263,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 InvalidRecurrenceRuleError();
}

this._nextByYearDay();
}

Expand Down Expand Up @@ -346,11 +372,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;
}

Expand All @@ -360,7 +385,6 @@ class RecurIterator {
return this.last;
}


let valid;
do {
valid = 1;
Expand Down Expand Up @@ -1360,4 +1384,18 @@ class RecurIterator {
return result;
}
}

/**
* An error indicating that a recurrence rule is invalid and produces no
* occurrences.
*
* @extends {Error}
* @class
*/
class InvalidRecurrenceRuleError extends Error {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add some jsdoc here?

constructor() {
super("Recurrence rule has no valid occurrences");
}
}

export default RecurIterator;
15 changes: 15 additions & 0 deletions test/recur_iterator_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,14 +143,22 @@ suite('recur_iterator', function() {

let start = ICAL.Time.fromString(options.dtStart);
let recur = ICAL.Recur.fromString(ruleString);

if (options.throws) {
assert.throws(function() {
recur.iterator(start);
});
return;
}

let iterator = recur.iterator(start);

if (options.noInstance) {
assert.equal(iterator.next(), null);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could also assert that completed is set, while you are at it.

assert.ok(iterator.completed);
return;
}

let inc = 0;
let dates = [];
let next, max;
Expand Down Expand Up @@ -992,6 +1000,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,' +
Expand Down