From dd94ead358168520424468e03c015709aea9d227 Mon Sep 17 00:00:00 2001 From: Vladimir Khramov Date: Mon, 28 Dec 2015 11:47:54 +1000 Subject: [PATCH 1/8] Initial work on #10488 --- framework/validators/NumberValidator.php | 21 +++++++++- .../validators/NumberValidatorTest.php | 41 +++++++++++++++++++ 2 files changed, 60 insertions(+), 2 deletions(-) diff --git a/framework/validators/NumberValidator.php b/framework/validators/NumberValidator.php index a3f27e8186..a5aaaec356 100644 --- a/framework/validators/NumberValidator.php +++ b/framework/validators/NumberValidator.php @@ -85,7 +85,8 @@ class NumberValidator extends Validator return; } $pattern = $this->integerOnly ? $this->integerPattern : $this->numberPattern; - if (!preg_match($pattern, "$value")) { + + if (!preg_match($pattern, $this->getStringValue($value))) { $this->addError($model, $attribute, $this->message); } if ($this->min !== null && $value < $this->min) { @@ -105,7 +106,7 @@ class NumberValidator extends Validator return [Yii::t('yii', '{attribute} is invalid.'), []]; } $pattern = $this->integerOnly ? $this->integerPattern : $this->numberPattern; - if (!preg_match($pattern, "$value")) { + if (!preg_match($pattern, $this->getStringValue($value))) { return [$this->message, []]; } elseif ($this->min !== null && $value < $this->min) { return [$this->tooSmall, ['min' => $this->min]]; @@ -116,6 +117,22 @@ class NumberValidator extends Validator } } + /** + * Returns string represenation of number value with replaced commas to dots, if decimal point + * of current locale is comma + * @param $value + * @return string + */ + private function getStringValue($value) + { + $localeInfo = localeconv(); + if (isset($localeInfo['decimal_point']) && $localeInfo['decimal_point'] ==',') { + return str_replace(',', '.', "$value"); + } else { + return "$value"; + } + } + /** * @inheritdoc */ diff --git a/tests/framework/validators/NumberValidatorTest.php b/tests/framework/validators/NumberValidatorTest.php index cf04cde309..e7f04436f4 100644 --- a/tests/framework/validators/NumberValidatorTest.php +++ b/tests/framework/validators/NumberValidatorTest.php @@ -37,7 +37,14 @@ class NumberValidatorTest extends TestCase $this->assertTrue($val->validate(-20)); $this->assertTrue($val->validate('20')); $this->assertTrue($val->validate(25.45)); + + $oldLocale = setlocale(LC_ALL, "0"); + setlocale(LC_ALL, "en_US.UTF-8"); $this->assertFalse($val->validate('25,45')); + setlocale(LC_ALL, "en_DK.UTF-8"); //decimal point is comma + $this->assertTrue($val->validate('25,45')); + setlocale(LC_ALL, $oldLocale); + $this->assertFalse($val->validate('12:45')); $val = new NumberValidator(['integerOnly' => true]); $this->assertTrue($val->validate(20)); @@ -70,6 +77,21 @@ class NumberValidatorTest extends TestCase $this->assertFalse($val->validate('12.23^4')); } + public function testValidateValueWithLocaleWhereDecimalPointIsComma() + { + $val = new NumberValidator(); + + $oldLocale = setlocale(LC_ALL, "0"); + + setlocale(LC_ALL, "en_US.UTF-8"); + $this->assertTrue($val->validate(.5)); + + setlocale(LC_ALL, "en_DK.UTF-8"); //decimal point is comma + $this->assertTrue($val->validate(.5)); + + setlocale(LC_ALL, $oldLocale); + } + public function testValidateValueMin() { $val = new NumberValidator(['min' => 1]); @@ -159,6 +181,25 @@ class NumberValidatorTest extends TestCase } + public function testValidateAttributeWithLocaleWhereDecimalPointIsComma() + { + $val = new NumberValidator(); + $model = new FakedValidationModel(); + $model->attr_number = 0.5; + + $oldLocale = setlocale(LC_ALL, "0"); + + setlocale(LC_ALL, "en_US.UTF-8"); + $val->validateAttribute($model, 'attr_number'); + $this->assertFalse($model->hasErrors('attr_number')); + + setlocale(LC_ALL, "en_DK.UTF-8"); //decimal point is comma + $val->validateAttribute($model, 'attr_number'); + $this->assertFalse($model->hasErrors('attr_number')); + + setlocale(LC_ALL, $oldLocale); + } + public function testEnsureCustomMessageIsSetOnValidateAttribute() { $val = new NumberValidator([ From b17dfa03a21d12337a8e72346d50675d26d53b88 Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Fri, 2 Dec 2016 00:04:30 +0300 Subject: [PATCH 2/8] Fixes #10488: Fixed incorrect behavior of `yii\validation\NumberValidator` when used with locales where decimal separator is comma --- framework/CHANGELOG.md | 1 + framework/validators/NumberValidator.php | 14 +++++++++----- tests/framework/validators/NumberValidatorTest.php | 12 ++++++------ 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md index 5f8206d1be..ee40661e7b 100644 --- a/framework/CHANGELOG.md +++ b/framework/CHANGELOG.md @@ -8,6 +8,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) - Bug #12681: Changed `data` column type from `text` to `blob` to handle null-byte (`\0`) in serialized RBAC rule properly (silverfire) - Bug #12714: Fixed `yii\validation\EmailValidator` to prevent false-positives checks when property `checkDns` is set to `true` (silverfire) - Bug #12791: Fixed `yii\behaviors\AttributeTypecastBehavior` unable to automatically detect `attributeTypes`, triggering PHP Fatal Error (klimov-paul) diff --git a/framework/validators/NumberValidator.php b/framework/validators/NumberValidator.php index a5aaaec356..60304005cc 100644 --- a/framework/validators/NumberValidator.php +++ b/framework/validators/NumberValidator.php @@ -120,17 +120,21 @@ class NumberValidator extends Validator /** * Returns string represenation of number value with replaced commas to dots, if decimal point * of current locale is comma - * @param $value + * @param int|float|string $value * @return string */ private function getStringValue($value) { + $value = (string)$value; + $localeInfo = localeconv(); - if (isset($localeInfo['decimal_point']) && $localeInfo['decimal_point'] ==',') { - return str_replace(',', '.', "$value"); - } else { - return "$value"; + $decimalPointSeparator = isset($localeInfo['decimal_point']) ? $localeInfo['decimal_point'] : null; + + if ($decimalPointSeparator !== null && $decimalPointSeparator !== '.') { + $value = str_replace($decimalPointSeparator, '.', $value); } + + return $value; } /** diff --git a/tests/framework/validators/NumberValidatorTest.php b/tests/framework/validators/NumberValidatorTest.php index e7f04436f4..7785004e13 100644 --- a/tests/framework/validators/NumberValidatorTest.php +++ b/tests/framework/validators/NumberValidatorTest.php @@ -39,9 +39,9 @@ class NumberValidatorTest extends TestCase $this->assertTrue($val->validate(25.45)); $oldLocale = setlocale(LC_ALL, "0"); - setlocale(LC_ALL, "en_US.UTF-8"); + setlocale(LC_ALL, 'en_US.UTF-8', 'English_United States.1252'); $this->assertFalse($val->validate('25,45')); - setlocale(LC_ALL, "en_DK.UTF-8"); //decimal point is comma + setlocale(LC_ALL, 'da_DK.UTF-8', 'Danish_Denmark.1252'); //decimal point is comma $this->assertTrue($val->validate('25,45')); setlocale(LC_ALL, $oldLocale); @@ -83,10 +83,10 @@ class NumberValidatorTest extends TestCase $oldLocale = setlocale(LC_ALL, "0"); - setlocale(LC_ALL, "en_US.UTF-8"); + setlocale(LC_ALL, 'en_US.UTF-8', 'English_United States.1252'); $this->assertTrue($val->validate(.5)); - setlocale(LC_ALL, "en_DK.UTF-8"); //decimal point is comma + setlocale(LC_ALL, 'da_DK.UTF-8', 'Danish_Denmark.1252'); //decimal point is comma $this->assertTrue($val->validate(.5)); setlocale(LC_ALL, $oldLocale); @@ -189,11 +189,11 @@ class NumberValidatorTest extends TestCase $oldLocale = setlocale(LC_ALL, "0"); - setlocale(LC_ALL, "en_US.UTF-8"); + setlocale(LC_ALL, 'en_US.UTF-8', 'English_United States.1252'); $val->validateAttribute($model, 'attr_number'); $this->assertFalse($model->hasErrors('attr_number')); - setlocale(LC_ALL, "en_DK.UTF-8"); //decimal point is comma + setlocale(LC_ALL, 'da_DK.UTF-8', 'Danish_Denmark.1252'); //decimal point is comma $val->validateAttribute($model, 'attr_number'); $this->assertFalse($model->hasErrors('attr_number')); From 242d850d6cb16f71cd91cfdeeda23da7bb9e3327 Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Fri, 2 Dec 2016 00:40:10 +0300 Subject: [PATCH 3/8] Shorter variable name, different approach to casting --- framework/validators/NumberValidator.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/framework/validators/NumberValidator.php b/framework/validators/NumberValidator.php index 60304005cc..3c45134b15 100644 --- a/framework/validators/NumberValidator.php +++ b/framework/validators/NumberValidator.php @@ -125,13 +125,13 @@ class NumberValidator extends Validator */ private function getStringValue($value) { - $value = (string)$value; + $value = "$value"; $localeInfo = localeconv(); - $decimalPointSeparator = isset($localeInfo['decimal_point']) ? $localeInfo['decimal_point'] : null; + $decimalSeparator = isset($localeInfo['decimal_point']) ? $localeInfo['decimal_point'] : null; - if ($decimalPointSeparator !== null && $decimalPointSeparator !== '.') { - $value = str_replace($decimalPointSeparator, '.', $value); + if ($decimalSeparator !== null && $decimalSeparator !== '.') { + $value = str_replace($decimalSeparator, '.', $value); } return $value; From a056281fa0b915f2d22434a56b21c61d9e17d348 Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Sun, 4 Dec 2016 00:58:40 +0300 Subject: [PATCH 4/8] Attempt to make tests pass on travis environment --- tests/framework/validators/NumberValidatorTest.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/framework/validators/NumberValidatorTest.php b/tests/framework/validators/NumberValidatorTest.php index 7785004e13..6554718f34 100644 --- a/tests/framework/validators/NumberValidatorTest.php +++ b/tests/framework/validators/NumberValidatorTest.php @@ -39,9 +39,9 @@ class NumberValidatorTest extends TestCase $this->assertTrue($val->validate(25.45)); $oldLocale = setlocale(LC_ALL, "0"); - setlocale(LC_ALL, 'en_US.UTF-8', 'English_United States.1252'); + setlocale(LC_ALL, 'en_US.utf8', 'en_US.UTF-8', 'English_United States.1252'); $this->assertFalse($val->validate('25,45')); - setlocale(LC_ALL, 'da_DK.UTF-8', 'Danish_Denmark.1252'); //decimal point is comma + setlocale(LC_ALL, 'he_IL.utf8', 'da_DK.UTF-8', 'Danish_Denmark.1252'); //decimal point is comma $this->assertTrue($val->validate('25,45')); setlocale(LC_ALL, $oldLocale); @@ -83,10 +83,10 @@ class NumberValidatorTest extends TestCase $oldLocale = setlocale(LC_ALL, "0"); - setlocale(LC_ALL, 'en_US.UTF-8', 'English_United States.1252'); + setlocale(LC_ALL, 'en_US.utf8', 'en_US.UTF-8', 'English_United States.1252'); $this->assertTrue($val->validate(.5)); - setlocale(LC_ALL, 'da_DK.UTF-8', 'Danish_Denmark.1252'); //decimal point is comma + setlocale(LC_ALL, 'he_IL.utf8', 'da_DK.UTF-8', 'Danish_Denmark.1252'); //decimal point is comma $this->assertTrue($val->validate(.5)); setlocale(LC_ALL, $oldLocale); @@ -189,11 +189,11 @@ class NumberValidatorTest extends TestCase $oldLocale = setlocale(LC_ALL, "0"); - setlocale(LC_ALL, 'en_US.UTF-8', 'English_United States.1252'); + setlocale(LC_ALL, 'en_US.utf8', 'en_US.UTF-8', 'English_United States.1252'); $val->validateAttribute($model, 'attr_number'); $this->assertFalse($model->hasErrors('attr_number')); - setlocale(LC_ALL, 'da_DK.UTF-8', 'Danish_Denmark.1252'); //decimal point is comma + setlocale(LC_ALL, 'he_IL.utf8', 'da_DK.UTF-8', 'Danish_Denmark.1252'); //decimal point is comma $val->validateAttribute($model, 'attr_number'); $this->assertFalse($model->hasErrors('attr_number')); From 08054bba6df38244fa5089da8e88f1a62bfcb155 Mon Sep 17 00:00:00 2001 From: Robert Korulczyk Date: Sat, 17 Dec 2016 14:37:31 +0100 Subject: [PATCH 5/8] Fix tests. (#13227) --- .../validators/NumberValidatorTest.php | 55 ++++++++++++++----- 1 file changed, 41 insertions(+), 14 deletions(-) diff --git a/tests/framework/validators/NumberValidatorTest.php b/tests/framework/validators/NumberValidatorTest.php index 6554718f34..ebd4f02162 100644 --- a/tests/framework/validators/NumberValidatorTest.php +++ b/tests/framework/validators/NumberValidatorTest.php @@ -12,10 +12,42 @@ use yiiunit\TestCase; */ 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() @@ -38,12 +70,11 @@ class NumberValidatorTest extends TestCase $this->assertTrue($val->validate('20')); $this->assertTrue($val->validate(25.45)); - $oldLocale = setlocale(LC_ALL, "0"); - setlocale(LC_ALL, 'en_US.utf8', 'en_US.UTF-8', 'English_United States.1252'); + $this->setPointDecimalLocale(); $this->assertFalse($val->validate('25,45')); - setlocale(LC_ALL, 'he_IL.utf8', 'da_DK.UTF-8', 'Danish_Denmark.1252'); //decimal point is comma + $this->setCommaDecimalLocale(); $this->assertTrue($val->validate('25,45')); - setlocale(LC_ALL, $oldLocale); + $this->restoreLocale(); $this->assertFalse($val->validate('12:45')); $val = new NumberValidator(['integerOnly' => true]); @@ -81,15 +112,13 @@ class NumberValidatorTest extends TestCase { $val = new NumberValidator(); - $oldLocale = setlocale(LC_ALL, "0"); - - setlocale(LC_ALL, 'en_US.utf8', 'en_US.UTF-8', 'English_United States.1252'); + $this->setPointDecimalLocale(); $this->assertTrue($val->validate(.5)); - setlocale(LC_ALL, 'he_IL.utf8', 'da_DK.UTF-8', 'Danish_Denmark.1252'); //decimal point is comma + $this->setCommaDecimalLocale(); $this->assertTrue($val->validate(.5)); - setlocale(LC_ALL, $oldLocale); + $this->restoreLocale(); } public function testValidateValueMin() @@ -187,17 +216,15 @@ class NumberValidatorTest extends TestCase $model = new FakedValidationModel(); $model->attr_number = 0.5; - $oldLocale = setlocale(LC_ALL, "0"); - - setlocale(LC_ALL, 'en_US.utf8', 'en_US.UTF-8', 'English_United States.1252'); + $this->setPointDecimalLocale(); $val->validateAttribute($model, 'attr_number'); $this->assertFalse($model->hasErrors('attr_number')); - setlocale(LC_ALL, 'he_IL.utf8', 'da_DK.UTF-8', 'Danish_Denmark.1252'); //decimal point is comma + $this->setCommaDecimalLocale(); $val->validateAttribute($model, 'attr_number'); $this->assertFalse($model->hasErrors('attr_number')); - setlocale(LC_ALL, $oldLocale); + $this->restoreLocale(); } public function testEnsureCustomMessageIsSetOnValidateAttribute() From cad400b6ff1dcca32567ab90dac2a9ec653a573e Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Sat, 17 Dec 2016 16:39:47 +0300 Subject: [PATCH 6/8] Added rob006 to changelog line --- framework/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md index b1a99f27f3..20e63a4c7b 100644 --- a/framework/CHANGELOG.md +++ b/framework/CHANGELOG.md @@ -9,7 +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) +- Bug #10488: Fixed incorrect behavior of `yii\validation\NumberValidator` when used with locales where decimal separator is comma (quantum13, samdark, rob006) - 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) - Bug #12681: Changed `data` column type from `text` to `blob` to handle null-byte (`\0`) in serialized RBAC rule properly (silverfire) From ebe30bd5ee6fa0fbea4f9053662fdd08b755d9e5 Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Sun, 15 Jan 2017 22:46:25 +0300 Subject: [PATCH 7/8] Exposed number normalization as StringHelper::normalizeNumber() --- framework/helpers/BaseStringHelper.php | 21 +++++++++++++++++++++ framework/validators/NumberValidator.php | 5 +++-- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/framework/helpers/BaseStringHelper.php b/framework/helpers/BaseStringHelper.php index 48d5b1d8ac..1b8fd81b39 100644 --- a/framework/helpers/BaseStringHelper.php +++ b/framework/helpers/BaseStringHelper.php @@ -284,4 +284,25 @@ class BaseStringHelper { 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 index 261b192bd1..4f8e017fa5 100644 --- 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; @@ -86,7 +87,7 @@ class NumberValidator extends Validator } $pattern = $this->integerOnly ? $this->integerPattern : $this->numberPattern; - if (!preg_match($pattern, $this->getStringValue($value))) { + if (!preg_match($pattern, StringHelper::normalizeNumber($value))) { $this->addError($model, $attribute, $this->message); } if ($this->min !== null && $value < $this->min) { @@ -106,7 +107,7 @@ class NumberValidator extends Validator return [Yii::t('yii', '{attribute} is invalid.'), []]; } $pattern = $this->integerOnly ? $this->integerPattern : $this->numberPattern; - if (!preg_match($pattern, $this->getStringValue($value))) { + if (!preg_match($pattern, StringHelper::normalizeNumber($value))) { return [$this->message, []]; } elseif ($this->min !== null && $value < $this->min) { return [$this->tooSmall, ['min' => $this->min]]; From 3f66f1907f2f74323a076927f3612fb7804f192b Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Mon, 16 Jan 2017 00:08:28 +0300 Subject: [PATCH 8/8] Removed unused method --- framework/validators/NumberValidator.php | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/framework/validators/NumberValidator.php b/framework/validators/NumberValidator.php index 4f8e017fa5..01bbba3c7f 100644 --- a/framework/validators/NumberValidator.php +++ b/framework/validators/NumberValidator.php @@ -118,26 +118,6 @@ class NumberValidator extends Validator } } - /** - * 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 - */ - private function getStringValue($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; - } - /** * @inheritdoc */