dragonmantank / cron-expression

CRON for PHP: Calculate the next or previous run date and determine if a CRON expression is due
MIT License
4.57k stars 124 forks source link

Fix "," delimiter precedence #122

Closed selfiens closed 2 years ago

selfiens commented 3 years ago

Thanks to everyone who contributed to this amazing library. This is my first pull request to an open source project, and I'm ready to learn, open to any comments :)

I had trouble with this library running cron expressions like */3,1,1-12 * * * * or 1-30/2,31-59/5 * * * *. I found that, according to the assertions in the unit tests, this project regards cron expressions as invalid when multiple ranges and step are mixed. However, those expressions pass many online validators, and also seem valid to me.

I switched the handling order of / and , characters in \Cron\AbstractField::validate() method to meet the semantic order of cron expression, and the library now works as expected.

After the code change, existing unit tests that invalidate such expressions now fail.

If these invalidations are by intention, my fix would be an incorrect one. Thanks again for this great library. Regards

dragonmantank commented 2 years ago

Huh, while I have never seen an expression that way it totally passes the C implementation's tests and a few others. Thanks!