Cakephp: Marshal misinterprets decimal values when locale parser is enabled

[micdobro]

This is a (multiple allowed):

• [x] bug
• [ ] enhancement

• [ ] feature-discussion (RFC)

• CakePHP Version: 3.5.0

• Platform and Target: NGINX, MariaDB, PHP 7.0.15

What you did

Please check issue #10873 - it's the same problem, this time detected on decimals.

intl.default_locale is set to da_DK.

In bootstrap.php I have the following:

Cake\I18n\Number::config('da_DK', \NumberFormatter::DECIMAL);

Type::build('decimal')
    ->useLocaleParser();

This actually works OK, although it fails when:

• I have a decimal field for a price: value of 47500 is entered
• that value is rendered by the FormHelper as 47.500 which is correct according to Danish locale (, as decimal separator, . as thousands separator)
• when saving it fails, and sets the field to 47,50, as it inteprets the first dot wrong

What happened

In Database/Type/DecimalType.php the original code is:

public function marshal($value)
    {
        if ($value === null || $value === '') {
            return null;
        }
        if (is_numeric($value)) {
            return (float)$value;
        }
        if (is_string($value) && $this->_useLocaleParser) {
            return $this->_parseValue($value);
        }
        if (is_array($value)) {
            return 1;
        }

        return $value;
    }

The first check with is_numeric() should be in my opinion exchanged and the $this->_useLocaleParser test should be first - so that Cake checks my locale-aware choices first (as I told Cake to do so by calling useLocaleParser() in bootstrap.php.

When I changed the order of tests - everything worked as it should: value of 47500 was displayed then as 47.500 and saved again correctly.

What you expected to happen

Please check the same paragraph in issue #10873. I would in general expect, that when I tell Cake to work with locale-aware values that it will respect my choice.

[ADmad]

Checking for localized string before doing is_numeric() check when locale parser has been enabled does make sense.

[ADmad]

@micdobro Would you like to make a PR with suggested change?

[markstory]

Pull request open now.