vobject
vobject copied to clipboard
Fix infinite loop caused by yearly with bySetPos
Consider the rrule:
FREQ=YEARLY;BYMONTH=5;BYSETPOS=3;BYMONTHDAY=3
It will not generate any occurrence, but the RRuleIterator
will stuck in the infinite loop
Codecov Report
Merging #564 (1c66700) into master (06feff3) will increase coverage by
0.00%
. The diff coverage is100.00%
.
@@ Coverage Diff @@
## master #564 +/- ##
=========================================
Coverage 98.34% 98.34%
- Complexity 1884 1885 +1
=========================================
Files 71 71
Lines 4536 4539 +3
=========================================
+ Hits 4461 4464 +3
Misses 75 75
Impacted Files | Coverage Δ | |
---|---|---|
lib/Recur/RRuleIterator.php | 99.44% <100.00%> (+<0.01%) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 06feff3...1c66700. Read the comment docs.
FREQ=YEARLY;BYMONTH=5;BYSETPOS=3;BYMONTHDAY=3
I think that means that the event happens yearly, on the 5th month and the 3rd day - for example 2022-05-03, just one event in a year. But then BYSETPOS=3
means, of all the events in the year, take only the 3rd event. But there is only ever one event in any year, so the BYSETPOS=3
filter means that no events ever match the criteria and we get an infinite loop that tries to find a "next" event in the future, but there is never an event.
It seems "a good thing" to "forcibly" stop the infinite loop at some point, to catch all these sorts of cases where the rule iterator will never get a match.
But it would also be nice to be able to analyse/parse rules "up front" and deduce that the rule can, mathematically, never generate any match, and so it is pointless wasting the effort of looping from now to year 9999.
Maybe there are a lot more rule cases that result in there never being a match for some yearly/monthly/weekly... rule? And so there might be infinite loops waiting in monthly/weekly etc processing?
You are right! It will be more efficient to be able to detect rrule that never generate any occurrence. The fix was took from the monthly case here: https://github.com/sabre-io/vobject/issues/442
Current CI passes in my test PR #564 so this works OK with the current code base and CI.
Let's take this fix - it fixes the problem in the same way as the existing code added in #443 - so we might as well take the same approach for now. I will merge.