[Form] 'NaN' string is mapped to a decimal field as a float(NaN)

[zerkms]

I followed the tutorial at http://symfony.com/doc/current/book/doctrine.html and after that generated CRUD.

The price member looks like:

/**
 * @var decimal $price
 *
 * @ORM\Column(name="price", type="decimal", nullable=false)
 */
private $price;

If we specify NaN as a price form value - form is treated as valid and we get a warning from logger

Warning: [json] (php_json_encode) double NAN does not conform to the JSON spec, encoded as 0 in /var/www/sf/vendor/symfony/src/Symfony/Bridge/Doctrine/Logger/DbalLogger.php line 44

I think that string NaN should be treated as invalid decimal value thus form should be treated as invalid

PS: this is a dump of an object after validation and right before persisting (and logging)

object(Acme\StoreBundle\Entity\Product)#654 (4) { ["id":"Acme\StoreBundle\Entity\Product":private]=> int(1) ["name":"Acme\StoreBundle\Entity\Product":private]=> string(3) "Foo" ["price":"Acme\StoreBundle\Entity\Product":private]=> float(NAN) ["description":"Acme\StoreBundle\Entity\Product":private]=> string(1) "a" }

[alexandresalome]

You shoud use validation for making sure it's a number: http://symfony.com/doc/current/book/validation.html

This issue can be closed.

[stof]

@zerkms if you don't specify any validation rules, the validator will not complain about values. And your code does not have such constraints

[zerkms]

@stof: it is not about validation. Why does string maps to float(NaN)?

It should be mapped at least to float(0), but float(NaN) is definitely not an expected result

[zerkms]

@alexandresalome:

to what value will be mapped string 'NaN' with integer ORM model field? Right, to 0. So should be here!

[zerkms]

@stof: so if code doesn't have constraints - then it is good, that DbalLogger class throws warnings? And instead of logging that something unexpected happened - it is being broken itself?

[stof]

the issue is that the logger tries to json_encode the params to put the them in the log message, and it seems like it fails to do so.

But anyway, why do you put NAN as a price ?

[zerkms]

@stof: so logger fails - it is wrong. Logger shouldn't rely on data, should it?

Uhm, I just tried to put it. It doesn't matter. What does matter, that float(NaN) is unexpected value, but float(0) is expected

[stof]

@zerkms how would you convert the params to a string in a way that keep things useful ?

[zerkms]

@stof: To keep things useful - logger shouldn't fail under any circumstance. Otherwise we wouldn't see the issue with model or user input, but only the issue with logger

[zerkms]

@stof: so? SF dev team doesn't care of logger that throws warnings on valid data?

[alexandresalome]

@zerkms : It's a Doctrine issue, address the issue to the Doctrine team.

[zerkms]

@alexandresalome: it is SF issue as well: logger shouldn't fail on any data, because it is logger and its work is to log anything passed to it

[zerkms]

@alexandresalome: and another question: you mentioned

You shoud use validation for making sure it's a number

how would I use validation rules if there is no built-in constraint for decimal?

[stof]

@zerkms there is built-in constraints. But you are not applying them on your class.

[zerkms]

@stof: there is no built-in constraint for decimal. Float constraint wouldn't help as well, because float(NaN) is perfectly valid value for that Type float constraint

and what about:

it is SF issue as well: logger shouldn't fail on any data, because it is logger and its work is to log anything passed to it

[webmozart]

You should use the "number" field type, which helps you in avoiding such problems. Also, there's the Type constraint for asserting that a variable has a specific type ("numeric" in your case).

[zerkms]

@bschussek: There is no number type supported by Doctrine: http://www.doctrine-project.org/docs/orm/2.1/en/reference/basic-mapping.html#doctrine-mapping-types

[stof]

@zerkms you are confusing the ORM mapping and the validator mapping. They are totally separate things

[zerkms]

@stof: Do I?

@Assert\Type(type="numeric", message="oh really?")

doesn't help as well, because float(NaN) is valid numeric value

And you still didn't mention if it is ok that logger cannot log valid values ;-)

[webmozart]

@zerkms: I was speaking about form field types, not Doctrine fields. Sure float(NaN) is a valid numeric value, but it should usually not be generated by the form. In your case, probably the string "NaN" was generated and converted to a float by Doctrine. You can avoid that though by (a) using the "number" form field type or (b) putting constraints on the property.

[zerkms]

@bschussek: I've specified

    $builder
        ->add('name')
        ->add('price', 'number')
        ->add('description')

number as a field type. Still experience the same issue :-S

Form is considered valid with NaN string. This happens because form validation is performed after $editForm->bindRequest($request); which binds that terrible value to the entity :-(

[zerkms]

@stof:

Actually I've debug the code and found out that Doctrine has nothing to do with this transformation:

Here is a tiny way to replicate the issue:

    $formatter = new \NumberFormatter(\Locale::getDefault(), \NumberFormatter::DECIMAL);
    $formatter->setAttribute(\NumberFormatter::GROUPING_USED, false);
    var_dump($formatter->parse('NaN'));

And the code itself is used by NumberToLocalizedStringTransformer::reverseTransform() lines 106 and 107:

    $formatter = $this->getNumberFormatter();
    $value = $formatter->parse($value);

So it is not doctrine issue. Symfony itself transforms any 'NaN' string to the float(NaN) !

[zerkms]

@stof: guys, what do you think about the code above?

[zerkms]

@stof: just want to notice that the issue affects integers as well

SQLSTATE[22P02]: Invalid text representation: 7 ERROR: invalid input syntax for integer: "-9.2233720368548E+18"

this is what I see if pass NaN string for integer field

[reecefowell]

@stoff it is the form validation that seems to be the issue.

The form field 'number' validates non numbers to be type 'NaN' (Not a Number), which if tested in php's is_float() or is_numeric() method will pass instead of being invalidated. The field number type appears to have a bug i suspect, because if user input so happens to be the string (NaN) (and we cannot trust user input on the web) it could be passed and form validated successfully.

Examples of nan failing in some numeric tests:

$nan = acos(-8);
var_dump($nan);
var_dump($nan > 0);
var_dump($nan < 0);

Example 2:

$nan = acos(-8);
var_dump($nan);
var_dump(is_numeric($nan));
var_dump(is_float($nan));

Run the above examples to see them fail in plain old PHP.

A solution to ensuring the 'type' NaN (Not A Number) could be solved by in SF2's built in assert and form validators with a simple regular expression.

[zerkms]

Personally I'd go with implementation that doesn't differ the behaviour with string "omg" and string "nan".

Obviously "omg" is not casted to number and left as is thus we get validation error. So "nan" should behave in absolutely the same way

[zerkms]

@bschussek: Guys, could anyone just give an answer: will some one ever open this bug?

What else should I do to prove it is a bug?! Isn't that obvious for you that for now SF2 works with numbers in a wrong way?

[reecefowell]

@zerkms i think the issue is people are mis-understanding the issue.

For those who don't get it, write an entity, make one of the fields a float, add assert etc. build a form, and then in the numeric field, if you type anything that is not a number, the form invalidates, that is good, now type 'NaN' in the field, and it passes, though it should not! What it should do is invaidate that too. But it seems PHP treats the string NaN as a special case (Not A Number).

You can never trust user input so even in these special cases this should not be allowed, and using a regular expression should help solve this issue to ensure the input is a valid float. This would just be a minor change to SF2s built in validators i think. This issue needs to be reopened.

[zerkms]

@reecefowell,

But it seems PHP treats the string NaN as a special case (Not A Number).

Not php, but SF2 does that. PHP passes the string as expected, but SF2 converts it to some terrible and unexpected float value (and I even referred to the lines that do that messy work)

[reecefowell]

@zerkms do you have a proposed fix? If you correct the code in SF2 that is causing the issue perhaps one of the devs can merge your fix in if it provides enough clarity.

[zerkms]

@reecefowell, I haven't yet, because I just don't want to do the fix that will not be accepted in the future. It will be just the wasting of time if dev-team doesn't treat this bug as a bug

[Tobion]

Well, I tried it with an integer field. My observations:

• when posting '4ab' -> converted to int(4)
• when posting '' -> NULL
• when posting 'abc' -> converted to int(0)
• when posting 'NaN' -> value is not set on the entity at all (seems to be ignored somehow)

So maybe int(NaN) is not possible, but only float(NaN). But simply ignoring NaN is also a bug because the form still uses the posted NaN for display but validation uses the entity default value. So entity state is not in synch with the form client data.

So we have both a bug for float(number) and integer fields.
@stof please reopen.

@zerkms I'm sure it your PR would accepted if you also provide the unit tests that fail with the current code.

[zerkms]

Added test case in pull request: #3196

[mvrhov]

:edited

The problem is in the way PHP does casting. We are casting too soon.

[mvrhov]

@fabpot, @bschussek: PR #3204 fixes just the NaN case, but as per @Tobion comment there are more problems than that with integer field. So either this ticket should be reopened or new one created for integer field.

[webmozart]

when posting '4ab' -> converted to int(4)

I currently don't know how to fix this. Do you think this is a big issue?

when posting '' -> NULL

Normal behaviour.

when posting 'abc' -> converted to int(0)

Can't reproduce. I get an "invalid" error.

when posting 'NaN' -> value is not set on the entity at all (seems to be ignored somehow)

Solved.

[zerkms]

when posting '4ab' -> converted to int(4)

I currently don't know how to fix this. Do you think this is a big issue?

Personally I think it is not a problem, but in some cases it could lead to unexpected application behaviour from user's point of view

[zerkms]

Well, seems like there is the similar issue if you pass single infinity UTF-8 character ∞

:-S