Skip to content
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

Invalid data validated as valid #6

Closed
codercms opened this issue Dec 25, 2017 · 2 comments
Closed

Invalid data validated as valid #6

codercms opened this issue Dec 25, 2017 · 2 comments

Comments

@codercms
Copy link

For example:

$monthField = new MonthField();
var_dump($monthField->validate('*/123'));

Month field: */123 is invalid, but $monthField->validate return true.
The same situation with other fields. Actually you can get an error only when using CronExpression's getNextRunDate method - "Step cannot be greater than total range" .

And why this code cause an exception?

$cron = CronExpression::factory('* * * */12 *');
$cron->getNextRunDate();

// Throws an ErrorException: "range(): step exceeds the specified range"
@dragonmantank
Copy link
Owner

dragonmantank commented Jan 8, 2018

Thanks @codercms

Can you verify which version of the library you are using for the first test? */123 should return false on a validation test. I was wrong on that, it failed with a patch for the second part of this. Will fix.

The second error is a bit more involved. "Month" is a range for 1-12, with 12 elements. When you pass * as the range, it gets expanded as 1-12, and then the step is applied. So starting at the first element (1), add the step (12). That results in 13, which is outside of the range of 1-12.

What you probably expect to happen is that the value "wraps" around the end and goes back to 1. I will see what I can do about this. Thanks!

@dragonmantank
Copy link
Owner

Resolved this with 2ff19fb

I verified that cronie will actually accept a step larger than the range of a field, so that was good.

Steps that generate numbers that are larger than the actual range of a field should now wrap around back to the beginning of the range with the above commit. So */123 will now validate as well as generate a correct date for getting next runs, previous runs, or checking if a cron is due.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants