-
-
Notifications
You must be signed in to change notification settings - Fork 167
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
RRULE BYDAY=xMO with x >=10 raises ValueError with to_ical() #518
Comments
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.
|
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. |
You are welcome to do that!
|
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. |
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! |
According to RFC-5545, section 3.8.5.3, the BYDAY component of a yearly RRULE can take values like:
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
This gives:
Environment
Proposed fix
The problem seems to be the regular expression
WEEKDAY_RULE
in fileprop.py
, which only accepts 0/1 characters for therelative
component. A simple fix would therefore be to allow {0,2} characters: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.)The text was updated successfully, but these errors were encountered: