Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Converting money to localized string fails in german locale. #30114

Closed
addiks opened this issue Feb 8, 2019 · 7 comments
Closed

Converting money to localized string fails in german locale. #30114

addiks opened this issue Feb 8, 2019 · 7 comments

Comments

@addiks
Copy link
Contributor

addiks commented Feb 8, 2019

Symfony version(s) affected: I'm using 2.8.34
(but the problematic code is also in the master branch, so all versions should be affected)

Description
In PHP, when converting a float value to string, the character used as a decimal-seperator depends on the locale. In an english locale, the full-stop "." is used ("123.45") while in german (f.e.) the comma "," is used ("123,45"). The PHP internal function "is_numeric" does not recognize the comma and would only consider the first representation ("123.45") as a number, but not the second one:

php > setlocale(LC_ALL, "de_DE.UTF-8");
php > var_dump(12 / 34);
float(0,35294117647059)

php > setlocale(LC_ALL, "en_US.UTF-8");
php > var_dump(12 / 34);
float(0.35294117647059)

php > var_dump(is_numeric("0.35294117647059"));
bool(true)
php > var_dump(is_numeric("0,35294117647059"));
bool(false)

php > setlocale(LC_ALL, "de_DE.UTF-8");
php > var_dump(is_numeric((string)(12 / 34)));
bool(false)

php > setlocale(LC_ALL, "en_US.UTF-8");
php > var_dump(is_numeric((string)(12 / 34)));
bool(true)

There is at least one piece of code in symfony where this behaviour leads to code breaking in certain locales and not in others. There are probably more but i have not checked yet. I have just stumbled over one such code in the MoneyToLocalizedStringTransformer on line 61 and NumberToLocalizedStringTransformer on line 113:

    public function transform($value)
    {
        if (null !== $value && 1 !== $this->divisor) {
            ...
            $value = (string) ($value / $this->divisor);
        }

        return parent::transform($value);
    }

In an english locale, $value contains something like "123.45", which passes the is_number check.
In an german locale, $value contains something like "123,45", which fails the is_number check.

The call "parent::transform($value)" from above directly calls the code below:

    public function transform($value)
    {
        ...
        if (!is_numeric($value)) {
            throw new TransformationFailedException('Expected a numeric.');
        }
        ...

This piece of code will now fail in a german locale and pass in an english one.

How to reproduce
Switch to a german locale ("de_DE.UTF-8") and use the Money transformers. I do with the Tbbc\MoneyBundle, but the problematic code is in symfony (see above) and not that bundle.

Possible Solution
I don't know if that is the best solution, but a simple "$value = str_replace(',', '.', $value);" in the MoneyToLocalizedStringTransformer directly after calculating and string-casting the $value should do the trick at least for me; I'm not sure about other locales. Maybe more research is required here for a better solution. This works for me:

    public function transform($value)
    {
        if (null !== $value && 1 !== $this->divisor) {
            ...
            $value = (string) ($value / $this->divisor);
            $value = str_replace(",", ".", $value);
        }

        return parent::transform($value);
    }
@xabbuh
Copy link
Member

xabbuh commented Feb 8, 2019

Does this mean that the transform() method is called with a string as the $value argument in your case?

@addiks
Copy link
Contributor Author

addiks commented Feb 8, 2019

The first transform method is called with (string)"3365" as $value, the second then with (string)"33,65".

@xabbuh
Copy link
Member

xabbuh commented Feb 8, 2019

It looks like you need to investigate then why this is happening. The transform() method needs to be called with an integer or float. It will convert the value into a string. If you need to get the float or integer value from a formatted string, you will need to use the reverseTransform() method.

@addiks
Copy link
Contributor Author

addiks commented Feb 8, 2019

Have you read the description? I already know why this happens and i have explained it in the description. I even provided a possible solution. This is not a question of why it happens but how to solve it in the best way.

The MoneyToLocalizedStringTransformer does something that will always fail if a certain locale is set. It performs a division, casts that to string and then throws an exception if the result of that float-cast is not numeric, which it never will be in a german locale. It does not matter what the transform method is called with (except null), as long as this sequence is in place, it will always fail in a german locale. This will also fail if it is called with a float or an integer.

This is a problem with the symfony component above and needs to be fixed. Trying to play the blame-game when the program here is obviously broken is not helpful.

@xabbuh
Copy link
Member

xabbuh commented Feb 8, 2019

I think we are talking about different things or I may be missing some detail. I was not able to reproduce what you describe so far. You can take a look at a small example I created at https://github.com/xabbuh/issue-30114/blob/master/test.php.

Its output looks like this for me:

float(0.35294117647059)
string(4) "0.35"
float(0,35294117647059)
string(4) "0.35"

I think the best is if you can fork it and make changes that allow to reproduce the failure that you experience.

@addiks
Copy link
Contributor Author

addiks commented Feb 8, 2019

You missed defining a divisor. Without a divisor the problematic devision does not get executed.
Forked test-script: https://github.com/addiks/issue-30114/blob/master/test.php

$transformer = new MoneyToLocalizedStringTransformer(null, null, null, 2);
float(0.35294117647059)
string(4) "0.18"
float(0,35294117647059)
PHP Fatal error:  Uncaught Symfony\Component\Form\Exception\TransformationFailedException: Expected a numeric. in /home/gerrit/Schreibtisch/asd/issue-30114/vendor/symfony/form/Extension/Core/DataTransformer/NumberToLocalizedStringTransformer.php:113
Stack trace:
#0 /home/gerrit/Schreibtisch/asd/issue-30114/vendor/symfony/form/Extension/Core/DataTransformer/MoneyToLocalizedStringTransformer.php(64): Symfony\Component\Form\Extension\Core\DataTransformer\NumberToLocalizedStringTransformer->transform('0,1764705882352...')
#1 /home/gerrit/Schreibtisch/asd/issue-30114/test.php(17): Symfony\Component\Form\Extension\Core\DataTransformer\MoneyToLocalizedStringTransformer->transform('0,1764705882352...')
#2 {main}
  thrown in /home/gerrit/Schreibtisch/asd/issue-30114/vendor/symfony/form/Extension/Core/DataTransformer/NumberToLocalizedStringTransformer.php on line 113

@xabbuh
Copy link
Member

xabbuh commented Feb 9, 2019

Thanks 👍 I think this should be fixed by #30126.

@fabpot fabpot closed this as completed Feb 13, 2019
fabpot added a commit that referenced this issue Feb 13, 2019
…bbuh)

This PR was merged into the 3.4 branch.

Discussion
----------

[Form] forward valid numeric values to transform()

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #30114
| License       | MIT
| Doc PR        |

Commits
-------

3be0d35 forward valid numeric values to transform()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants