Merge pull request #13114 from yiisoft/fix-numbervalidator-comma-deci…

…mal-separator Fixes incorrect behavior of `yii\validation\NumberValidator` when used with locales where decimal separator is comma
yiisoft · Jan 15, 2017 · 9ec4799 · 9ec4799
2 parents 3a1cd7f + 3f66f19
commit 9ec4799
Show file tree

Hide file tree

Showing 4 changed files with 94 additions and 2 deletions.
diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md
@@ -9,6 +9,7 @@ Yii Framework 2 Change Log
 - Bug #9305: Fixed MSSQL `Schema::TYPE_TIMESTAMP` to be 'datetime' instead of 'timestamp', which is just an incremental number (nkovacs)
 - Bug #9616: Fixed mysql\Schema::loadColumnSchema to set enumValues attribute correctly if enum definition contains commas (fphammerle)
 - Bug #9796: Initialization of not existing `yii\grid\ActionColumn` default buttons (arogachev)
+- Bug #10488: Fixed incorrect behavior of `yii\validation\NumberValidator` when used with locales where decimal separator is comma (quantum13, samdark, rob006)
 - Bug #11122: Fixed can not use `orderBy` with aggregate functions like `count` (Ni-san)
 - Bug #11771: Fixed semantics of `yii\di\ServiceLocator::__isset()` to match the behavior of `__get()` which fixes inconsistent behavior on newer PHP versions (cebe)
 - Bug #12213: Fixed `yii\db\ActiveRecord::unlinkAll()` to respect `onCondition()` of the relational query (silverfire)

diff --git a/framework/helpers/BaseStringHelper.php b/framework/helpers/BaseStringHelper.php
@@ -284,4 +284,25 @@ public static function countWords($string)
     {
         return count(preg_split('/\s+/u', $string, null, PREG_SPLIT_NO_EMPTY));
     }
+
+    /**
+     * Returns string represenation of number value with replaced commas to dots, if decimal point
+     * of current locale is comma
+     * @param int|float|string $value
+     * @return string
+     * @since 2.0.11
+     */
+    public static function normalizeNumber($value)
+    {
+        $value = "$value";
+
+        $localeInfo = localeconv();
+        $decimalSeparator = isset($localeInfo['decimal_point']) ? $localeInfo['decimal_point'] : null;
+
+        if ($decimalSeparator !== null && $decimalSeparator !== '.') {
+            $value = str_replace($decimalSeparator, '.', $value);
+        }
+
+        return $value;
+    }
 }
diff --git a/framework/validators/NumberValidator.php b/framework/validators/NumberValidator.php
@@ -8,6 +8,7 @@
 namespace yii\validators;
 
 use Yii;
+use yii\helpers\StringHelper;
 use yii\web\JsExpression;
 use yii\helpers\Json;
 
@@ -85,7 +86,8 @@ public function validateAttribute($model, $attribute)
             return;
         }
         $pattern = $this->integerOnly ? $this->integerPattern : $this->numberPattern;
-        if (!preg_match($pattern, "$value")) {
+
+        if (!preg_match($pattern, StringHelper::normalizeNumber($value))) {
             $this->addError($model, $attribute, $this->message);
         }
         if ($this->min !== null && $value < $this->min) {
@@ -105,7 +107,7 @@ protected function validateValue($value)
             return [Yii::t('yii', '{attribute} is invalid.'), []];
         }
         $pattern = $this->integerOnly ? $this->integerPattern : $this->numberPattern;
-        if (!preg_match($pattern, "$value")) {
+        if (!preg_match($pattern, StringHelper::normalizeNumber($value))) {
             return [$this->message, []];
         } elseif ($this->min !== null && $value < $this->min) {
             return [$this->tooSmall, ['min' => $this->min]];

diff --git a/tests/framework/validators/NumberValidatorTest.php b/tests/framework/validators/NumberValidatorTest.php
@@ -12,10 +12,42 @@
  */
 class NumberValidatorTest extends TestCase
 {
+    private $commaDecimalLocales = ['fr_FR.UTF-8', 'fr_FR.UTF8', 'fr_FR.utf-8', 'fr_FR.utf8', 'French_France.1252'];
+    private $pointDecimalLocales = ['en_US.UTF-8', 'en_US.UTF8', 'en_US.utf-8', 'en_US.utf8', 'English_United States.1252'];
+    private $oldLocale;
+
+    private function setCommaDecimalLocale()
+    {
+        if ($this->oldLocale === false) {
+            $this->markTestSkipped('Your platform does not support locales.');
+        }
+
+        if (setlocale(LC_NUMERIC, $this->commaDecimalLocales) === false) {
+            $this->markTestSkipped('Could not set any of required locales: ' . implode(', ', $this->commaDecimalLocales));
+        }
+    }
+
+    private function setPointDecimalLocale()
+    {
+        if ($this->oldLocale === false) {
+            $this->markTestSkipped('Your platform does not support locales.');
+        }
+
+        if (setlocale(LC_NUMERIC, $this->pointDecimalLocales) === false) {
+            $this->markTestSkipped('Could not set any of required locales: ' . implode(', ', $this->pointDecimalLocales));
+        }
+    }
+
+    private function restoreLocale()
+    {
+        setlocale(LC_NUMERIC, $this->oldLocale);
+    }
+
     protected function setUp()
     {
         parent::setUp();
         $this->mockApplication();
+        $this->oldLocale = setlocale(LC_NUMERIC, 0);
     }
 
     public function testEnsureMessageOnInit()
@@ -37,7 +69,13 @@ public function testValidateValueSimple()
         $this->assertTrue($val->validate(-20));
         $this->assertTrue($val->validate('20'));
         $this->assertTrue($val->validate(25.45));
+
+        $this->setPointDecimalLocale();
         $this->assertFalse($val->validate('25,45'));
+        $this->setCommaDecimalLocale();
+        $this->assertTrue($val->validate('25,45'));
+        $this->restoreLocale();
+
         $this->assertFalse($val->validate('12:45'));
         $val = new NumberValidator(['integerOnly' => true]);
         $this->assertTrue($val->validate(20));
@@ -70,6 +108,19 @@ public function testValidateValueAdvanced()
         $this->assertFalse($val->validate('12.23^4'));
     }
 
+    public function testValidateValueWithLocaleWhereDecimalPointIsComma()
+    {
+        $val = new NumberValidator();
+
+        $this->setPointDecimalLocale();
+        $this->assertTrue($val->validate(.5));
+
+        $this->setCommaDecimalLocale();
+        $this->assertTrue($val->validate(.5));
+
+        $this->restoreLocale();
+    }
+
     public function testValidateValueMin()
     {
         $val = new NumberValidator(['min' => 1]);
@@ -159,6 +210,23 @@ public function testValidateAttribute()
 
     }
 
+    public function testValidateAttributeWithLocaleWhereDecimalPointIsComma()
+    {
+        $val = new NumberValidator();
+        $model = new FakedValidationModel();
+        $model->attr_number = 0.5;
+
+        $this->setPointDecimalLocale();
+        $val->validateAttribute($model, 'attr_number');
+        $this->assertFalse($model->hasErrors('attr_number'));
+
+        $this->setCommaDecimalLocale();
+        $val->validateAttribute($model, 'attr_number');
+        $this->assertFalse($model->hasErrors('attr_number'));
+
+        $this->restoreLocale();
+    }
+
     public function testEnsureCustomMessageIsSetOnValidateAttribute()
     {
         $val = new NumberValidator([