You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Description
If an XML attribute contains a positive float value which starts with 0, e.g. 0.123, XmlEncoder will decode it as a string.
If a float value starts with some other number, or is negative, then it is decoded as a float.
How to reproduce
<?phprequire_once__DIR__.'/vendor/autoload.php';
useSymfony\Component\Serializer\Encoder\XmlEncoder;
$encoder = newXmlEncoder();
$xml = '<document value="0.123">...</document>';
$result = $encoder->decode($xml, 'xml');
var_dump($result);
// array(2) {// ["@value"]=>// string(5) "0.123" <-- a float was expected, but it's a string// ["#"]=>// string(3) "..."// }
Possible Solution
It looks like this behaviour was introduced in #32438 (as suggested here).
The naive approach would be to just extend the added check with the "but zero is not followed by a dot" condition, like in #38669.
Seems to work, but it feels like a nasty workaround, not a proper solution. And probably someone will find another case where it fails to produce a reasonable result.
Also, this section of code, i.e. trying to decode floats, has already been changed and fixed several times before (#22478, 8f6e67d and #32438), so it's not that easy to get it right and consider all edge cases. Maybe it would be better to try a different approach, something like #36482 or similar?
Btw, I assume we don't want to support comma as decimal separator.
Additional context
Normally I would say it's a limitation of the XmlEncoder, but returning a string only for floats which start with zero - it might be super confusing and difficult to debug.
For example (actually, that's how I found it), if you have a method which accepts floats (type hinting it) and receives as arguments values decoded by XmlEncoder, and the XML data looks like this:
…th 0 (Marcin Kruk)
This PR was squashed before being merged into the 3.4 branch.
Discussion
----------
[Serializer] fix decoding float XML attributes starting with 0
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#38666
| License | MIT
| Doc PR | n/a
This is a naive approach to fix#38666, assuming it is something worth fixing.
I checked different cases and it seems to be fixing all of them, but I bet there will be some other edge cases which still won't be covered properly.
Commits
-------
97b4306 [Serializer] fix decoding float XML attributes starting with 0
Symfony version(s) affected: 3.4
Description
If an XML attribute contains a positive float value which starts with
0
, e.g.0.123
, XmlEncoder will decode it as a string.If a float value starts with some other number, or is negative, then it is decoded as a float.
How to reproduce
Note that:
Possible Solution
It looks like this behaviour was introduced in #32438 (as suggested here).
The naive approach would be to just extend the added check with the "but zero is not followed by a dot" condition, like in #38669.
Seems to work, but it feels like a nasty workaround, not a proper solution. And probably someone will find another case where it fails to produce a reasonable result.
Also, this section of code, i.e. trying to decode floats, has already been changed and fixed several times before (#22478, 8f6e67d and #32438), so it's not that easy to get it right and consider all edge cases. Maybe it would be better to try a different approach, something like #36482 or similar?
Btw, I assume we don't want to support comma as decimal separator.
Additional context
Normally I would say it's a limitation of the XmlEncoder, but returning a string only for floats which start with zero - it might be super confusing and difficult to debug.
For example (actually, that's how I found it), if you have a method which accepts floats (type hinting it) and receives as arguments values decoded by XmlEncoder, and the XML data looks like this:
The process will be sometimes failing, but it will look very randomly. Looking at the data, everything seems to be valid.
The text was updated successfully, but these errors were encountered: