Parsing UNTIL before DTSTART throws format error

[ghost]

php-rrule release: v1.4.0

I'm creating an RRule instance: $rrule = new RRule('DTSTART=20170202T000000Z;FREQ=DAILY;UNTIL=20170205T000000Z')

I get an exception thrown as follows:
Invalid UNTIL property: The value of the UNTIL rule part MUST be a date if DTSTART is a date.

Stack:

in vendor/rlanvin/php-rrule/src/RRule.php at line 723   -
                            switch ( $dtstart_type ) {
                                case 'date':
                                    if ( strpos($value, 'T') !== false) {
                                        throw new \InvalidArgumentException(
                                            'Invalid UNTIL property: The value of the UNTIL rule part MUST be a date if DTSTART is a date.'
                                        );
                                    }
at RRule ::parseRfcString ('DTSTART=20170202T000000Z;FREQ=MONTHLY;UNTIL=20170706T000000Z') 
in vendor/rlanvin/php-rrule/src/RRule.php at line 187   + 
at RRule ->__construct ('DTSTART=20170202T000000Z;FREQ=MONTHLY;UNTIL=20170706T000000Z')

I had a look into the cause and it seems to be because UNTIL is being parsed before DTSTART.

The variable $dtstart_type is initialised to date on line 648, so if it parses UNTIL first then it will always throw this error if UNTIL is a string.

Actually i'm not entirely sure what the desired behaviour is as there is no handling of DTSTART specifically within the RRULE case there. I think my string will always end up in the RRULE case?

If I can understand the thinking then I can maybe help with fixing for my particular use case. Or if my case is invalid then please let me know! I'm no master of the rrule rfc!

[rlanvin]

Thanks for the detailed bug report, I'll try to provided an answer as detailed.

So let's start by stating the obvious: the issue is that the format of your rule is invalid according to the RFC. DTSTART is not actually part of the RRULE string for various reasons (mainly because the RFC is about the iCalendar format, and DTSTART and RRULE are 2 separates things).

It should be:

$rrule = new RRule("DTSTART:20170202T000000Z\nRRULE:FREQ=DAILY;UNTIL=20170205T000000Z");

That being said, the error message returned by the parser is indeed confusing and not that helpful, and denotes a wrong behavior.

1. If DTSTART is missing (which is the case here) the parser assumes it is simply a date, and then apply the RFC rules to check UNTIL. Since your UNTIL is a datetime, it complains. At first glance, I think when DTSTART is missing, it should accept any format of UNTIL.

2. The parser relies on the constructor to performs more compliance checks (and initialize DTSTART if it's missing). However in this case, the constructor will indeed accept DTSTART as a valid argument (because that's how it works when you pass an array). So I think the parser should probably throw on exception for extraneous DTSTART. The lib could also be adapted to accept this kind of string, but since they are not RFC compliant (as far as I know) I'm not sure it's a great idea.

If I can understand the thinking then I can maybe help with fixing for my particular use case.

Where does your string come from? Do you need the lib to accept it "as is" (i.e. not compliant with the RFC), or is it something that you can fix?

[ghost]

Thanks for the detailed answer ;)

That makes sense for why it isn't working. The string actually comes from a javascript client side library: https://github.com/jkbrzt/rrule in which it works as (un)intended.

For now I can fix my server side code to provide the php-rrule library with the correct data and I will look into the client side library to see if it can output the correct string.

I'm guessing it makes sense to close this Issue? If I end up needing to handle it differently I can PR for that and open a discussion in there.

[rlanvin]

I'm guessing it makes sense to close this Issue?

Well actually I'll keep the issue open for now. As I explained I think the error message could be improved and clarified, so I'll work on that and close it once I'm done.

Also since the JS lib is for some reason generating this kind of string, I'll have a deeper look and see if it's necessary to support it. It would be nice to have good compatibility between frontend and backend - I'm sure you are not the only one is this case.

[rlanvin]

For reference, there is a bug report on the JS lib (from January 2015...) about this issue. It is generating a non-compliant string: jkbrzt/rrule#83