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

[BUG] Numericality validator does not work in some locales #13450

Closed
mervick opened this issue Jul 25, 2018 · 4 comments
Closed

[BUG] Numericality validator does not work in some locales #13450

mervick opened this issue Jul 25, 2018 · 4 comments
Labels
bug A bug report status: low Low

Comments

@mervick
Copy link
Contributor

mervick commented Jul 25, 2018

Expected and Actual Behavior

Numericality validator returns validation error 'Field :field does not have a valid numeric format' for valid float number when php uses locales which uses coma , in locality number format, for example ru_RU.utf8 locale

The main problem is that Numericality validator uses regexp:

if !preg_match("/^-?\d+\.?\d*$/", value) {

Code below perfectly demonstrates this bug
(make sure that you installed both en_US.utf8 and ru_RU.utf8 locales in your system)

$regexp = '/^-?\d+\.?\d*$/';
$number = 123.12;

$locale = setlocale(LC_ALL, 'en_US.utf8');
echo $locale; // en_US.utf8
echo $number; // 123.12
var_dump(preg_match($regexp, $number)); // int(1)

$locale = setlocale(LC_ALL, 'ru_RU.utf8');
echo $locale; // ru_RU.utf8
echo $number; // 123,12
var_dump(preg_match($regexp, $number)); // int(0)

Provide minimal script to reproduce the issue

class TestForm extends \Phalcon\Forms\Form {
    public function initialize()
    {
        $el = new \Phalcon\Forms\Element\Hidden('price');
        $el->addValidator(new \Phalcon\Validation\Validator\Numericality());
        $this->add($el);
    }
}

function testNumericalityWithLocale($locale)
{
    $setlocale = setlocale(LC_ALL, $locale);

    if ($setlocale !== $locale) {
        throw new \Exception(sprintf('Looks like you need to install %s locale on your OS', $locale));
    }

    $price = 123.12;

    $form = new TestForm();
    $data = ['price' => $price];

    echo 'Price is ' . $price . PHP_EOL;

    if (!$form->isValid($data)) {
        throw new \Exception($form->getMessagesFor('price')[0]->getMessage());
    }

    echo 'OK', PHP_EOL;
}

testNumericalityWithLocale('en_US.utf8');
testNumericalityWithLocale('ru_RU.utf8');

Output:

php > testNumericalityWithLocale('en_US.utf8');
Price is 123.12
OK

php > testNumericalityWithLocale('ru_RU.utf8');
Price is 123,12
PHP Warning:  Uncaught Exception: Field price does not have a valid numeric format in php shell code:18
Stack trace:
#0 php shell code(1): testNumericalityWithLocale('ru_RU.utf8')
#1 {main}
  thrown in php shell code on line 18

Details

  • Phalcon version:
Web framework delivered as a C-extension for PHP
phalcon => enabled
Author => Phalcon Team and contributors
Version => 3.2.4
Build Date => Dec  1 2017 16:26:26
Powered by Zephir => Version 0.10.4-11e39849b0

Directive => Local Value => Master Value
phalcon.db.escape_identifiers => On => On
phalcon.db.force_casting => Off => Off
phalcon.orm.events => On => On
phalcon.orm.virtual_foreign_keys => On => On
phalcon.orm.column_renaming => On => On
phalcon.orm.not_null_validations => On => On
phalcon.orm.exception_on_failed_save => Off => Off
phalcon.orm.enable_literals => On => On
phalcon.orm.late_state_binding => Off => Off
phalcon.orm.enable_implicit_joins => On => On
phalcon.orm.cast_on_hydrate => Off => Off
phalcon.orm.ignore_unknown_columns => Off => Off
phalcon.orm.update_snapshot_on_save => On => On
phalcon.orm.disable_assign_setters => Off => Off
  • PHP Version:
PHP 7.2.2-3+ubuntu16.04.1+deb.sury.org+1 (cli) (built: Feb  6 2018 16:11:23) ( NTS )
Copyright (c) 1997-2018 The PHP Group
Zend Engine v3.2.0, Copyright (c) 1998-2018 Zend Technologies
    with Zend OPcache v7.2.2-3+ubuntu16.04.1+deb.sury.org+1, Copyright (c) 1999-2018, by Zend Technologies
  • Operating System:
Ubuntu 16.04 - Linux  4.4.116 #1 SMP Thu Feb 22 08:45:34 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
  • Installation type: Compiling from source
  • Server: Nginx
@mervick
Copy link
Contributor Author

mervick commented Jul 25, 2018

Proposal

It can be fixed if this line

if !preg_match("/^-?\d+\.?\d*$/", value) {

change to this

if !preg_match("/^-?\d+[\.,]?\d*$/", value) || !is_numeric(value) {

@JABirchall
Copy link

This would be an issue with other locales as well. I think EU uses comma seperated minor as well.

@sergeyklay
Copy link
Contributor

sergeyklay commented Aug 2, 2018

@JABirchall, @mervick Could you please send a PR to the 3.4.x branch?

@niden
Copy link
Member

niden commented May 17, 2019

#14085

@niden niden added bug A bug report status: low Low and removed Bug - Low labels Dec 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug report status: low Low
Projects
None yet
Development

No branches or pull requests

5 participants