Fix some recurrence evaluation issues (#645)

* RecurrencePatternSerializer: Ignore extra semicolons when parsing RRULEs.

* Recurrence evaluation: Fix `BYDAY` expansion in case of `FREQ=WEEKLY` or `BYWEEKNO`, which didn't properly consider the start of the week.

* Test: Add test for illegal rule parts.

Ical.Net.Tests/Calendars/Recurrence/RecurrenceTestCases.txt

@@ -2,3 +2,7 @@
 RRULE:FREQ=WEEKLY;BYDAY=MO,TH;COUNT=3
 DTSTART:20241024
 INSTANCES:20241024,20241028,20241031
+
+# Illegal rule part with multiple '='
+RRULE:FREQ=WEEKLY;BYDAY=MO=;COUNT=3
+EXCEPTION:System.ArgumentException

Ical.Net.Tests/RecurrenceTests.cs

@@ -3631,6 +3631,8 @@ public class RecurrenceTestCase
 
         public IReadOnlyList<CalDateTime> Instances { get; set; }
 
+        public string Exception { get; set; }
+
         public override string ToString()
             => $"Line {LineNumber}: {DtStart}, {RRule}";
     }
@@ -3686,6 +3688,10 @@ private static IEnumerable<RecurrenceTestCase> ParseTestCaseFile(string fileCont
                 case "INSTANCES":
                     current.Instances = val.Split(',').Select(dt => new CalDateTime(dt) { TzId = "UTC" }).ToList();
                     break;
+
+                case "EXCEPTION":
+                    current.Exception = val;
+                    break;
             }
         }
 
@@ -3718,6 +3724,14 @@ public void ExecuteRecurrenceTestCase(RecurrenceTestCase testCase)
 
         // Start at midnight, UTC time
         evt.Start = testCase.DtStart;
+
+        if (testCase.Exception != null)
+        {
+            var exceptionType = Type.GetType(testCase.Exception);
+            Assert.Throws(exceptionType, () => new RecurrencePattern(testCase.RRule));
+            return;
+        }
+
         evt.RecurrenceRules.Add(new RecurrencePattern(testCase.RRule));
 
         var occurrences = evt.GetOccurrences(testCase.StartAt?.Value ?? DateTime.MinValue, DateTime.MaxValue)
@@ -3728,4 +3742,4 @@ public void ExecuteRecurrenceTestCase(RecurrenceTestCase testCase)
 
         Assert.That(startDates, Is.EqualTo(testCase.Instances));
     }
-}
+}

Ical.Net.Tests/contrib/libical/icalrecur_test.out

@@ -189,11 +189,10 @@ DTSTART:19970805T090000
 INSTANCES:19970805T090000,19970817T090000,19970819T090000,19970831T090000
 PREV-INSTANCES:19970819T090000,19970817T090000,19970805T090000
 
-# TODO: FIX (see https://github.com/ical-org/ical.net/issues/618)
-# RRULE:FREQ=WEEKLY;INTERVAL=1;COUNT=2;BYDAY=MO;
-# DTSTART:20141006T090000
-# INSTANCES:20141006T090000,20141013T090000
-# PREV-INSTANCES:20141006T090000
+RRULE:FREQ=WEEKLY;INTERVAL=1;COUNT=2;BYDAY=MO;
+DTSTART:20141006T090000
+INSTANCES:20141006T090000,20141013T090000
+PREV-INSTANCES:20141006T090000
 
 RRULE:FREQ=DAILY;COUNT=10
 DTSTART:19970902T090000
@@ -307,19 +306,17 @@ START-AT:20170915T090000
 INSTANCES:20171006T090000,20171103T090000,20171201T090000
 PREV-INSTANCES:20170901T090000
 
-# TODO: FIX (see https://github.com/ical-org/ical.net/issues/618)
-# RRULE:FREQ=WEEKLY;UNTIL=20170127T000000Z;WKST=MO;BYDAY=SU,TU,TH;INTERVAL=2
-# DTSTART:20161229T090000
-# START-AT:20161231T090000
-# INSTANCES:20170101T090000,20170110T090000,20170112T090000,20170115T090000,20170124T090000,20170126T090000
-# PREV-INSTANCES:20161229T090000
-
-# TODO: FIX (see https://github.com/ical-org/ical.net/issues/618)
-# RRULE:FREQ=WEEKLY;UNTIL=20170127T000000Z;WKST=MO;BYDAY=SU,TU,TH;INTERVAL=2
-# DTSTART:20161229T090000
-# START-AT:20170102T090000
-# INSTANCES:20170110T090000,20170112T090000,20170115T090000,20170124T090000,20170126T090000
-# PREV-INSTANCES:20170101T090000,20161229T090000
+RRULE:FREQ=WEEKLY;UNTIL=20170127T000000Z;WKST=MO;BYDAY=SU,TU,TH;INTERVAL=2
+DTSTART:20161229T090000
+START-AT:20161231T090000
+INSTANCES:20170101T090000,20170110T090000,20170112T090000,20170115T090000,20170124T090000,20170126T090000
+PREV-INSTANCES:20161229T090000
+
+RRULE:FREQ=WEEKLY;UNTIL=20170127T000000Z;WKST=MO;BYDAY=SU,TU,TH;INTERVAL=2
+DTSTART:20161229T090000
+START-AT:20170102T090000
+INSTANCES:20170110T090000,20170112T090000,20170115T090000,20170124T090000,20170126T090000
+PREV-INSTANCES:20170101T090000,20161229T090000
 
 RRULE:FREQ=DAILY;UNTIL=20170131T140000Z;BYMONTH=1;INTERVAL=3
 DTSTART:20170101T090000

Ical.Net/Evaluation/RecurrencePatternEvaluator.cs

@@ -658,6 +658,9 @@ private List<DateTime> GetAbsWeekDays(DateTime date, WeekDay weekDay, Recurrence
         {
             var weekNo = Calendar.GetIso8601WeekOfYear(date, CalendarWeekRule.FirstFourDayWeek, pattern.FirstDayOfWeek);
 
+            // Go to the first day of the week
+            date = date.AddDays(-GetWeekDayOffset(date, pattern.FirstDayOfWeek));
+
             // construct a list of possible week days..
             while (date.DayOfWeek != dayOfWeek)
             {
@@ -725,6 +728,12 @@ private List<DateTime> GetAbsWeekDays(DateTime date, WeekDay weekDay, Recurrence
         return GetOffsetDates(days, weekDay.Offset);
     }
 
+    /// <summary>
+    /// Returns the days since the start of the week, 0 if the date is on the first day of the week.
+    /// </summary>
+    private static int GetWeekDayOffset(DateTime date, DayOfWeek startOfWeek)
+        => date.DayOfWeek + ((date.DayOfWeek < startOfWeek) ? 7 : 0) - startOfWeek;
+
     /// <summary>
     /// Returns a single-element sublist containing the element of <paramref name="dates"/> at <paramref name="offset"/>. 
     /// Valid offsets are from 1 to the size of the list. If an invalid offset is supplied, all elements from <paramref name="dates"/>

Ical.Net/Serialization/DataTypes/RecurrencePatternSerializer.cs

@@ -244,7 +244,20 @@ public override object Deserialize(TextReader tr)
                 var keywordPairs = match.Groups[2].Value.Split(';');
                 foreach (var keywordPair in keywordPairs)
                 {
+                    if (keywordPair.Length == 0)
+                    {
+                        // This is illegal but ignore for now.
+                        continue;
+                    }
+
                     var keyValues = keywordPair.Split('=');
+                    if (keyValues.Length != 2)
+                    {
+                        // ArgumentExceptions seem to be the Exception of choise for this class. Should
+                        // probably be changed to a more specific exception type.
+                        throw new ArgumentException($"The recurrence rule part '{keywordPair}' is invalid.");
+                    }
+
                     var keyword = keyValues[0];
                     var keyValue = keyValues[1];
 
@@ -493,4 +506,4 @@ public override object Deserialize(TextReader tr)
 
         return r;
     }
-}
+}