Framework: Validation does not fail when "integer" is checked, on a string

Created on 24 Apr 2017  路  10Comments  路  Source: laravel/framework

  • Laravel Version: 5.4.18
  • PHP Version: PHP 7.0.6-1+donate.sury.org~trusty+1 (cli) ( NTS )
  • Database Driver & Version: n/a

Description:

When I am trying to validate an string that has only numbers, the validation will pass, when i use the rule "integer".
I expect the validation to fail, because the value is a string, and not integer.
'5' => should fail validation
5 => should not fail validation

Steps To Reproduce:

$rules            = ['id' => 'integer'];
$validatorFactory = app(\Illuminate\Contracts\Validation\Factory::class);
$validator        = $validatorFactory->make(['id' => '5'], $rules);
var_dump($validator->fails());

The result should be true, and not false, because id is string '5'.

Proposed solution

Use

return is_int($value);

instead of current validator:

return filter_var($value, FILTER_VALIDATE_INT) !== false;

or, to avoid breaking backwards compatibility, we need to have the ability to force the validator to be strict.

Most helpful comment

Reopening this because defending weak typing / casting strings to ints in 2019 as a "feature" is just not on.

@themsaid wrote:

An HTML input field would return an integer in string form, thus why the validator can't fail on this case.

No - it returns a numeric value in string form.

An integer is a data type, not the content of a string. Please don't conflate a string containing numeric/integer-like values with an integer data type. We aren't working with PHP 4/5 in 2005 - 2015.

For a modern framework I don't think it's acceptable to say "we're ok with weak typing even though the community is moving away from that". PHP has made a lot of positive steps towards much stricter typing to weed out problems weak typing causes, and in turn weeding out a lot of criticism of PHP. Calling this a "feature" and having people defend it because "someone needs to ensure there aren't decimal places on form input" is a little silly.

Your argument that a HTML input field would return a string is invalid, because:

  1. A JSON response would return an integer. So should we support them too or just people doing soft checks on strings?
  2. A developer can use numeric. I doubt someone who thinks casting a form input string to an int is a good idea, really cares whether the data type is truly an "integer".
  3. The validation is called "integer", you would expect it would check the data type is an integer, not that it loosely matches an integer on some but not all aspects.

If you really are keen to support form requests, validate them for what they are with numeric - and a regex to look for decimal places if you truly want an int.

As it stands, people looking to do things "the right way" have to write custom validators which is not right.

All 10 comments

@cosminardeleanu I think this is intended (maybe for forms)

@m1guelpf I agree, however I am trying to validate an application/json post.

Imho, when I use an validator, I expect to work for any possible cases, not just for form-data.
If this validator is made to validate form-data posts, then it should be renamed into something more appropriate, like FormDataValidator, or something similar.

@cosminardeleanu Maybe you could add a custom validator for your use case, or open a PR and wait for Taylor's decision...
Personally, I've only used validators to validate form requests.
Also, could you share more data about why you can't process a string/cast it to an integer?

@m1guelpf

I already made a custom validator for my needs.
I'm thinking that the validation rule is misleading. Maybe it should be "form_integer" instead of "integer".

As for your question: I work with strict data. For example, think to a post request that creates an configuration from a json. That configuration is saved in database. When data is retrieved from database, I expect to have data with correct scalars.

Example:

$config = [
    'label' => 'Buy now !',
    'product_id' => 961
    ...
];
...
// Validate $config.
if ($validator->fails()) {
    // Exception. The request will not be accepted.
}
...
$attributes = [
    ...
    'config' => $config,
    ...
];

$model = new Model($attributes);
$model->save();

... some other class ...

public function getLatestProduct() {
    $model = Model::find(...); // to get the item i just created.
    return $this->buildJsonInfo($model->config['label'], $model->config['product_id']);
}

protected function buildJsonInfo(string $label, int $id) {
    // do stuff
}

When i will call the function getLatestProduct then the type hinting will cause my code to crash.
I don't want to do additional validations on what i got from database, because is supposed to already be correct and validated before accepting it.

@cosminardeleanu You can cast elements when saving retrieving Eloquent models with Mutators.
Maybe that would fix it?

@m1guelpf casts works only for main attributes. In my case config is an array so is already casted.
I am aware that there are a lot of work arounds, i can even make an model class out of the config attribute, and use the casts.
But this is not my point: you should not be required to do work arounds, when a simple validator can fix a lot of headaches.

@cosminardeleanu Maybe ping Taylor and get his opinion?

It does make sense to enable a strict mode for validator for your usecase, maybe a flag like strict would change the behavior of the validations...
Personally I'd refactor my code to work with strings too but I get your point.

Ping @themsaid do you think that the validator should cover this usecase?

An HTML input field would return an integer in string form, thus why the validator can't fail on this case.

I'm not sure if a strict flag would be useful, you can always sanitise your input before passing to the validator.

Anyways this is more of a feature request, so we better discuss it on
https://github.com/laravel/internals repo.

Reopening this because defending weak typing / casting strings to ints in 2019 as a "feature" is just not on.

@themsaid wrote:

An HTML input field would return an integer in string form, thus why the validator can't fail on this case.

No - it returns a numeric value in string form.

An integer is a data type, not the content of a string. Please don't conflate a string containing numeric/integer-like values with an integer data type. We aren't working with PHP 4/5 in 2005 - 2015.

For a modern framework I don't think it's acceptable to say "we're ok with weak typing even though the community is moving away from that". PHP has made a lot of positive steps towards much stricter typing to weed out problems weak typing causes, and in turn weeding out a lot of criticism of PHP. Calling this a "feature" and having people defend it because "someone needs to ensure there aren't decimal places on form input" is a little silly.

Your argument that a HTML input field would return a string is invalid, because:

  1. A JSON response would return an integer. So should we support them too or just people doing soft checks on strings?
  2. A developer can use numeric. I doubt someone who thinks casting a form input string to an int is a good idea, really cares whether the data type is truly an "integer".
  3. The validation is called "integer", you would expect it would check the data type is an integer, not that it loosely matches an integer on some but not all aspects.

If you really are keen to support form requests, validate them for what they are with numeric - and a regex to look for decimal places if you truly want an int.

As it stands, people looking to do things "the right way" have to write custom validators which is not right.

Was this page helpful?
0 / 5 - 0 ratings