RRULE BYDAY=xMO with x >=10 raises ValueError with to_ical()

[semiprime]

According to RFC-5545, section 3.8.5.3, the BYDAY component of a yearly RRULE can take values like:

RRULE:FREQ=YEARLY;BYDAY=xMO

to mean xth Monday of the year. They explicitly give an example with x=20.

However, in icalendar calling to_ical() on such an event with x>=10 raises a ValueError.

To Reproduce

import icalendar
from datetime import date

event1 = icalendar.Event()
event1.add('SUMMARY', 'Ev 1')
event1.add('DTSTART', date(2023,2,27))
event1.add('RRULE', {'FREQ':['YEARLY'], 'BYDAY':['9MO']}) # 9th Monday of year
print(event1.to_ical()) # works

event2 = icalendar.Event()
event2.add('SUMMARY', 'Ev 2')
event2.add('DTSTART', date(2023,3,6))
event2.add('RRULE', {'FREQ':['YEARLY'], 'BYDAY':['10MO']}) # 10th Monday of year
print(event2.to_ical()) # Raises ValueError with icalendar 5.0.5 & 4.1.0

This gives:

...
  File ".../python3.9/site-packages/icalendar/prop.py", line 581, in __new__
    raise ValueError(f'Expected weekday abbrevation, got: {self}')
ValueError: Expected weekday abbrevation, got: 10MO

Environment

• OS: Slackware 15.0
• Python version: 3.9.16
• icalendar version: 5.0.5 or 4.1.0 (installed from PyPI)

Proposed fix

The problem seems to be the regular expression WEEKDAY_RULE in file prop.py, which only accepts 0/1 characters for the relative component. A simple fix would therefore be to allow {0,2} characters:

diff --git a/src/icalendar/prop.py b/src/icalendar/prop.py
index ac87d0b..e00e805 100644
--- a/src/icalendar/prop.py
+++ b/src/icalendar/prop.py
@@ -69,7 +69,7 @@ DATETIME_PART = f'(?:{DATE_PART})?(?:{TIME_PART})?'
 WEEKS_PART = r'(\d+)W'
 DURATION_REGEX = re.compile(r'([-+]?)P(?:%s|%s)$'
                             % (WEEKS_PART, DATETIME_PART))
-WEEKDAY_RULE = re.compile(r'(?P<signal>[+-]?)(?P<relative>[\d]?)'
+WEEKDAY_RULE = re.compile(r'(?P<signal>[+-]?)(?P<relative>[\d]{0,2})'
                           r'(?P<weekday>[\w]{2})$')
 

With this change, my text code above works, and running the icalendar 5.0.5 test code python3 setup.py test, all the tests pass.

(There may be other issues with the regular expression: it accepts, for example, BYDAY=+MO which as far as I can see is invalid. However, that is perhaps best treated as a different issue, and is not affected by my proposed fix.)

[niccokunzmann]

Wow, thanks for that! If you like, you can create a pull request. It is nice to test this, too so people do not accidentally revert the change.

[semiprime]

I was thinking of backporting the fix to the 4.x branch. It's a crashing bug, and AFAICS the fix is safe, so it seems a good candidate.

The bugfix itself should be identical. I'll need to tweak the test a little, but it shouldn't be a problem.

[niccokunzmann]

You are welcome to do that!

[semiprime]

I submitted pull request #524 to the 4.x branch, fixing this issue.

Unfortunately, several automated tests failed. However, looking at the log files for the failures, it seems that, for example test py35 failed because it couldn't access Python3.5.10 - so I don't think the failures are related to my patch.

I've run the tests locally with Python versions 2.7.18 & 3.9.16 (x86_64) and 3.5.3 (Arm64) which I had easily available. The tests passed for me.

[niccokunzmann]

This fix is included in version 5.0.7 and should be available as 4.1.1 to older Python versions, too.

Thanks @semiprime and @jacadzaca for your cooperation!