Skip to content

Commit

Permalink
Avoid infinite loop with invalid YEARLY recurrence rule.
Browse files Browse the repository at this point in the history
  • Loading branch information
leftmostcat committed Sep 14, 2023
1 parent 3754b83 commit 36a3f3e
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 22 deletions.
31 changes: 25 additions & 6 deletions lib/ical/recur_iterator.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}

Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -346,11 +367,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 +380,6 @@ class RecurIterator {
return this.last;
}


let valid;
do {
valid = 1;
Expand Down
19 changes: 13 additions & 6 deletions test/recur_iterator_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,13 +143,13 @@ 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);
});

let iterator = recur.iterator(start);

if (options.noInstance) {
assert.equal(iterator.next(), null);
return;
}
let iterator = recur.iterator(start);

let inc = 0;
let dates = [];
Expand Down Expand Up @@ -688,7 +688,7 @@ suite('recur_iterator', function() {
// Invalid rule. There's never a 31st of Feburary, check that this fails.
testRRULE('FREQ=MONTHLY;INTERVAL=12;BYMONTHDAY=31', {
dtStart: '2022-02-01T08:00:00',
throws: true,
noInstance: true,
});

// monthly + by month
Expand Down Expand Up @@ -963,6 +963,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
20 changes: 10 additions & 10 deletions test/recur_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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']
Expand Down

0 comments on commit 36a3f3e

Please sign in to comment.