October: Validation bug with numbers

[trsh]

Expected behavior

When "numeric" rule applied to model and saving NON numeric value, an error should be displayed.

Actual behavior

Value is saved as '0'. No error displayed.

Reproduce steps

1) Add 'required|numeric' to model field
2) In fields.yaml declare it as an Integer -> type: number
3) Try to save form with value 'aaa'

Tip: I added some debugging vendor\laravel\framework\src\Illuminate\Validation\Validator.php, and seems that the problem is, that the value is first turned to integer and only then pushed to Validation (as '0' integer). Wrong execution order.

October build

Where can I check?

[trsh]

public function beforeValidate(){
var_dump($this->wide);

Value is already 0. So can't also to manually. This an serious problem.

[daftspunk]

It is fair that a string value that can be cast to an integer should be. i.e post('number') will return a string always, we would expect it to validate as a number.

[trsh]

@daftspunk yeah, but this is with every string value. I experimented with -> "aaaxxxx". The "Not numeric" error is never ever displayed, so why then it's there?

Here is another guy not getting it -> https://octobercms.com/forum/post/validating-rules-numeric-or-integer-seemed-to-be-not-working.

[trsh]

@daftspunk let's say someone work's with bills orsmth in backend. Serious money stuff, etc. He/she want's to enter number 16500, and by accident hits "r" instead of "5" (because it's close in the keyboard). So he/she will never get an error, instead 1600 is saved with no exception so ever. Is this really right?

[phegman]

I am having the same issue. Anybody found a fix for this? I tried using regex as well, but again can not seem to get the validation to work correctly for the numeric fields.

[trsh]

@phegman, @daftspunk thinks this is working Right, so we can just do head-bangs (personal opinion before common logic and user-cases). A workaround is to use before/afterValidate and take Number from POST (post('number')) and validate it with your own logic.

[jBOKA]

Isn't it easier to use a regex rule. like 'regex:/^[0-9]+(.[0-9]+)?$/' ?

When and why does the casting happen when you use the type: number field config?

[daftspunk]

@jBOKA You make an interesting point, perhaps we can add this validation to the number field itself.

We can't do anything about the numeric validation type because it is governed by Laravel.

[jBOKA]

@daftspunk The whole idea of course depends on what field types are for and how the lifecycle of postbacks, validation and so on are designed.

So far I thought it was meant to be like this:

Field types are just there for providing different kinds of input methods.
After posting a form, every field is of course converted to a string.

The question is, if a postback should be cast to a certain type and when this should occur.
Laravel's numeric validation rule just uses PHP's is_numeric, which would indeed validate the mentioned typo '16r00' as invalid.

The question I have is: If, why and when in the whole postback/validation lifecycle does the type casting occur right now?

Also, if there were no type casting, you could easily agree upon string validation in every case.

[trsh]

@jBOKA but the problem is in the life-cycle. The inputed form value (with type number) is very first casted to Number and Then fed to validator (so already modified -> a23a becomes 23). So doesn't matter regex or numeric or whatever, it just fails to it's job.

[jBOKA]

Sounds a little like an inconsistency.

Doesn't fetching integer values from the database also result in string values in eloquent?

In the case of dates, the date object is only casted by the model as a carbon instance upon "getting", right?

[trsh]

@jBOKA duno. I just did test with Number and form validation. My initial report say's it all.

[trsh]

I actually ended up validating manually from post() in afterValidate.

[daftspunk]

IIRC - The numbers are cast to (float) to satisfy the database when the column is declared as INTEGER, etc. (citation needed) This is probably why it is flying through the numeric validation. That's why "number" form fields type should probably bring some validation, since it has its finger in the pie already.

[phegman]

Thanks for the info! @trsh would you mind sharing how did you do the validation in afterValidate? I tried this but couldn't get it to work. Still saves the form when entering non numeric values.

public function afterValidate()
{
        if ( !is_numeric($this->rows) ) {
            return false;
        }
}

rows is the name of one of my numeric fields. Thanks for the help in advance!

[trsh]

@phegman some-have like this. Mark, I'm right now doing from my head

public function afterValidate()
{
        if (!is_numeric(post('something')) ) {
            throw new \ValidationException(['something' => \Lang::get('system::validation.numeric', ['attribute' => \Lang::get('xx.xxx::lang.xx.xx')])]);
        }
}

[phegman]

@trsh Thanks! Works perfectly!

[LukeTowers]

Additional note:
Any solution should implement form field validation as a separate layer from the model class so that we can also fix the issue with required form fields not actually being required unless the rule was made in the model class as well. I'll take a look at implementing this stuff soon.

[getrightnl]

The between: min, max rule is also not working because of the data being passed as string through the validate method in the Validation trait. Setting the casts property in the model to integer is still passing through as a string sadly. Where are the field types defined from the fields.yaml, is it possible at that point to match the type of Number and than cast the string to a integer/float

[trsh]

Almost an year o_O

[daftspunk]

Hmm, doesn't look like anyone is willing to fix it.

[daftspunk]

The support demand for October is increasing, while the available time the maintainers have to give remains the same. These issues will be handled in time, however there are ways you can help:

• Submit a pull request that introduces this feature and wait for it to be reviewed
• Make a financial contribution to the project so we can allocate more time towards making improvements like these that benefit everyone