search

teambition / rrule-go

Go library for working with recurrence rules for calendar dates.
MIT License
310 stars 57 forks source link

Validating parameters to RFC 5545 #31

Closed KatrinaHoffert closed 5 years ago

KatrinaHoffert commented 5 years ago

Per the RFC, many fields have limits that were not being checked (except Bysetpos, for whatever reason). This can result in user error not being caught early and also prevents you from using this library to catch said errors.

As well, FREQ is required by the RFC and if it wasn't specified, we would badly assume some default. For these bounds in the standard, see: https://tools.ietf.org/html/rfc5545#section-3.3.10

Note: it turns out we don't handle leap seconds right (they're why seconds go to 60 and not 59). In fact, when I tried to write a test with BYSECOND=60, it was an infinite loop. Not sure if the bounds should be restricted below the RFC as a result (I mean, who would honestly use leap seconds in a RRule, anyway?).

coveralls commented 5 years ago

Coverage Status

Coverage increased (+0.01%) to 99.661% when pulling 3a53a780955c03f215d9b644810973cf61fa54c0 on KatrinaHoffert:master into 02033d412abcbfa867474ee2dcb0cc0f7912d905 on teambition:master.

KatrinaHoffert commented 5 years ago

In hindsight, it felt obvious we should prevent BYSECOND=60, given that it has the ability to cause serious issues. A future issue should try to fix whatever is making it loop indefinitely, but that's beyond the scope of this PR.

zensh commented 5 years ago

@KatrinaHoffert Thank you.