Data type validation can fail for float/double

[dev-maniac]

With commit afc2a25c5e4cced6856f15c278b561a87d3a8e41, the validation of float values was changed. But since the comparison in the validation is now type strict, it can fail on very small float values (e.g. 0.000001), since PHP will represent these float value as "1.0E-6" but compare them against the original string value. So here, it is better to use a non-strict comparison ("==" instead of "===").

[iricketson]

@j0k3r @thePanz

This issue exists in 1.3.7, via the commit posted by the OP.

https://github.com/FriendsOfSymfony1/doctrine1/blame/ebcfdbbae67b1789240f3bb103acb77da7a42e99/lib/Doctrine/Validator.php#L174

Assuming a string value of '23.00',

echo (string) '23.00'; // 23.00
echo (string) (float) '23.00'; // 23

Comparison fails.

Thoughts?

[alquerci]

@iricketson You almost got it but the test case is not catch see bellow.

It depends on PHP config precision.

>>> ini_get('precision')
=> "14"

>>> $var = '23.012345678912345' // string type
=> "23.012345678912345"
>>> (string) (float) $var
=> "23.012345678912"
>>> (string) $var == (string) (float) $var
=> false
>>> (string) $var === (string) (float) $var
=> false

>>> $var = 23.012345678912345 // float type
=> 23.012345678912
>>> (string) (float) $var
=> "23.012345678912"
>>> (string) $var == (string) (float) $var
=> true
>>> (string) $var === (string) (float) $var
=> true

[iricketson]

@alquerci

It might just be simpler to check for any numeric value:

return is_numeric($var);

The current implementation is a BC break, so it should definitely be something that allows a looser comparison to pass.

[alquerci]

@iricketson

Ho, yeah I catch it, like you did.

The case is when the string have at least one trailing zero.

>>> $var = '23.01234000'
=> "23.01234000"
>>> (string) (float) $var
=> "23.01234"
>>> (string) $var == (string) (float) $var
=> true
>>> (string) $var === (string) (float) $var
=> false
>>> "23.01234" == "23.01234000" // outch ... learning PHP.
=> true

---

Entry point to add the test case is there https://github.com/FriendsOfSymfony1/doctrine1/blob/be13a7d/tests/ValidatorTestCase.php#L53

---

The current implementation is a BC break, so it should definitely be something that allows a looser comparison to pass.

I think we should just revert the strict comparison.

[alquerci]

@iricketson do you want/can to push a PR? otherwise I will try to find a time-frame this week-end to push the patch.

[iricketson]

@alquerci See #80