Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Fix for #6300 - I18n float validator rewrite #6373

Closed
wants to merge 8 commits into from
11 changes: 3 additions & 8 deletions library/Zend/I18n/Filter/NumberFormat.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,23 +27,18 @@ public function filter($value)
return $value;
}

if (!is_int($value)
&& !is_float($value)
) {
if (!is_int($value) && !is_float($value)) {
$result = parent::filter($value);
} else {
ErrorHandler::start();

$result = $this->getFormatter()->format(
$value,
$this->getType()
);
$result = $this->getFormatter()->format($value, $this->getType());

ErrorHandler::stop();
}

if (false !== $result) {
return str_replace("\xC2\xA0", ' ', $result);
return $result;
}

return $value;
Expand Down
144 changes: 117 additions & 27 deletions library/Zend/I18n/Validator/Float.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
use Traversable;
use Zend\I18n\Exception as I18nException;
use Zend\Stdlib\ArrayUtils;
use Zend\Stdlib\StringUtils;
use Zend\Stdlib\StringWrapper\StringWrapperInterface;
use Zend\Validator\AbstractValidator;
use Zend\Validator\Exception;

Expand All @@ -37,6 +39,13 @@ class Float extends AbstractValidator
*/
protected $locale;

/**
* UTF-8 compatable wrapper for string functions
*
* @var StringWrapperInterface
*/
protected $wrapper;

/**
* Constructor for the integer validator
*
Expand All @@ -46,12 +55,13 @@ class Float extends AbstractValidator
public function __construct($options = array())
{
if (!extension_loaded('intl')) {
throw new I18nException\ExtensionNotLoadedException(sprintf(
'%s component requires the intl PHP extension',
__NAMESPACE__
));
throw new I18nException\ExtensionNotLoadedException(
sprintf('%s component requires the intl PHP extension', __NAMESPACE__)
);
}

$this->wrapper = StringUtils::getWrapper();

if ($options instanceof Traversable) {
$options = ArrayUtils::iteratorToArray($options);
}
Expand Down Expand Up @@ -88,56 +98,136 @@ public function setLocale($locale)
return $this;
}


/**
* Returns true if and only if $value is a floating-point value
* Returns true if and only if $value is a floating-point value. Uses the formal definition of a float as described
* in the PHP manual: {@link http://www.php.net/float}
*
* @param string $value
* @return bool
* @throws Exception\InvalidArgumentException
*/
public function isValid($value)
{
if (!is_string($value) && !is_int($value) && !is_float($value)) {
if (!is_scalar($value) || is_bool($value)) {
$this->error(self::INVALID);
return false;
}

$this->setValue($value);

if (is_float($value)) {
if (is_float($value) || is_int($value)) {
return true;
}

$locale = $this->getLocale();
$format = new NumberFormatter($locale, NumberFormatter::DECIMAL);
if (intl_is_failure($format->getErrorCode())) {
throw new Exception\InvalidArgumentException("Invalid locale string given");
// Need to check if this is scientific formatted string. If not, switch to decimal.
$formatter = new NumberFormatter($this->getLocale(), NumberFormatter::SCIENTIFIC);

if (intl_is_failure($formatter->getErrorCode())) {
throw new Exception\InvalidArgumentException($formatter->getErrorMessage());
}

$parsedFloat = $format->parse($value, NumberFormatter::TYPE_DOUBLE);
if (intl_is_failure($format->getErrorCode())) {
$this->error(self::NOT_FLOAT);
return false;
if (StringUtils::hasPcreUnicodeSupport()) {
$exponentialSymbols = '[Ee' . $formatter->getSymbol(NumberFormatter::EXPONENTIAL_SYMBOL) . ']+';
$search = '/' . $exponentialSymbols . '/u';
} else {
$exponentialSymbols = '[Ee]';
$search = '/' . $exponentialSymbols . '/';
}

$decimalSep = $format->getSymbol(NumberFormatter::DECIMAL_SEPARATOR_SYMBOL);
$groupingSep = $format->getSymbol(NumberFormatter::GROUPING_SEPARATOR_SYMBOL);
if (!preg_match($search, $value)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to @NickBelhomme, this change is causing trouble, as this error message is not being displayed anymore when upgrading to 2.3.3 from any previous version.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #6647

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ocramius Did you mean to put this message on this PR? I know there are a few others out there talking about NOT_FLOAT. Personally, I never saw the utility of the error message as this is a validator - if the method returns false the input is, by definition, not a float. I kept in the error messages that were caused my bad inputs or instantiations. But if y'all want to include errors like this, others are welcome to include them.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@moderndeveloperllc it can be changed, but only as a BC break in a point release

$formatter = new NumberFormatter($this->getLocale(), NumberFormatter::DECIMAL);
}

$valueFiltered = str_replace($groupingSep, '', $value);
$valueFiltered = str_replace($decimalSep, '.', $valueFiltered);
/**
* @desc There are seperator "look-alikes" for decimal and group seperators that are more commonly used than the
* official unicode chracter. We need to replace those with the real thing - or remove it.
*/
$groupSeparator = $formatter->getSymbol(NumberFormatter::GROUPING_SEPARATOR_SYMBOL);
$decSeparator = $formatter->getSymbol(NumberFormatter::DECIMAL_SEPARATOR_SYMBOL);

//NO-BREAK SPACE and ARABIC THOUSANDS SEPARATOR
if ($groupSeparator == "\xC2\xA0") {
$value = str_replace(' ', $groupSeparator, $value);
} elseif ($groupSeparator == "\xD9\xAC") { //NumberFormatter doesn't have grouping at all for Arabic-Indic
$value = str_replace(array('\'', $groupSeparator), '', $value);
}

while (strpos($valueFiltered, '.') !== false
&& (substr($valueFiltered, -1) == '0' || substr($valueFiltered, -1) == '.')
) {
$valueFiltered = substr($valueFiltered, 0, strlen($valueFiltered) - 1);
//ARABIC DECIMAL SEPARATOR
if ($decSeparator == "\xD9\xAB") {
$value = str_replace(',', $decSeparator, $value);
}

if (strval($parsedFloat) !== $valueFiltered) {
$this->error(self::NOT_FLOAT);
$groupSeparatorPosition = $this->wrapper->strpos($value, $groupSeparator);
$decSeparatorPosition = $this->wrapper->strpos($value, $decSeparator);

//We have seperators, and they are flipped. i.e. 2.000,000 for en-US
if ($groupSeparatorPosition && $decSeparatorPosition && $groupSeparatorPosition > $decSeparatorPosition) {
return false;
}

return true;
//If we have Unicode support, we can use the real graphemes, otherwise, just the ASCII characters
$decimal = '['. preg_quote($decSeparator) . ']';
$prefix = '[+-]';
$exp = $exponentialSymbols;
$numberRange = '0-9';
$useUnicode = '';
$suffix = '';

if (StringUtils::hasPcreUnicodeSupport()) {
$prefix = '[' .
preg_quote(
$formatter->getTextAttribute(NumberFormatter::POSITIVE_PREFIX) .
$formatter->getTextAttribute(NumberFormatter::NEGATIVE_PREFIX) .
$formatter->getSymbol(NumberFormatter::PLUS_SIGN_SYMBOL) .
$formatter->getSymbol(NumberFormatter::MINUS_SIGN_SYMBOL)
) . ']{0,3}';
$suffix = ($formatter->getTextAttribute(NumberFormatter::NEGATIVE_SUFFIX))
? '[' .
preg_quote(
$formatter->getTextAttribute(NumberFormatter::POSITIVE_SUFFIX) .
$formatter->getTextAttribute(NumberFormatter::NEGATIVE_SUFFIX) .
$formatter->getSymbol(NumberFormatter::PLUS_SIGN_SYMBOL) .
$formatter->getSymbol(NumberFormatter::MINUS_SIGN_SYMBOL)
) . ']{0,3}'
: '';
$numberRange = '\p{N}';
$useUnicode = 'u';
}

/**
* @desc Match against the formal definition of a float. The exponential number check is modified for RTL
* non-Latin number systems (Arabic-Indic numbering). I'm also switching out the period for the decimal
* separator. The formal definition leaves out +- from the integer and decimal notations so add that.
* This also checks that a grouping sperator is not in the last GROUPING_SIZE graphemes of the string -
* i.e. 10,6 is not valid for en-US.
* @see http://www.php.net/float
*/

$lnum = '[' . $numberRange . ']+';
$dnum = '(([' . $numberRange . ']*' . $decimal . $lnum . ')|(' . $lnum . $decimal . '[' . $numberRange . ']*))';
$expDnum = '((' . $prefix . '((' . $lnum . '|' . $dnum . ')' . $exp . $prefix . $lnum . ')'.$suffix.')|' .
'(' . $suffix . '(' . $lnum . $prefix . $exp . '(' . $dnum . '|' . $lnum . '))' . $prefix . '))';

//LEFT-TO-RIGHT MARK (U+200E) is messing up everything for the handful of locales that have it
$lnumSearch = str_replace("\xE2\x80\x8E", '', '/^' .$prefix . $lnum . $suffix . '$/'.$useUnicode);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either use ( and ) as regex delimiters instead of /, or add the / as second parameter to all preg_quote() calls.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DASPRiD umm... this is a str_replace, not preg_quote...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@weierophinney @DASPRiD was referring to lines 169, 178, and 186. I added the slash as suggested. Not sure why his comment still shows. Github quirk.

$dnumSearch = str_replace("\xE2\x80\x8E", '', '/^' .$prefix . $dnum . $suffix . '$/'.$useUnicode);
$expDnumSearch = str_replace("\xE2\x80\x8E", '', '/^' . $expDnum . '$/'.$useUnicode);
$value = str_replace("\xE2\x80\x8E", '', $value);
$unGroupedValue = str_replace($groupSeparator, '', $value);

//No strrpos() in wrappers yet. ICU 4.x doesn't have grouping size for everything. ICU 52 has 3 for ALL locales.
$groupSize = ($formatter->getAttribute(NumberFormatter::GROUPING_SIZE))
? $formatter->getAttribute(NumberFormatter::GROUPING_SIZE) : 3;
$lastStringGroup = $this->wrapper->substr($value, -$groupSize);

if ((preg_match($lnumSearch, $unGroupedValue) ||
preg_match($dnumSearch, $unGroupedValue) ||
preg_match($expDnumSearch, $unGroupedValue)) &&
false === $this->wrapper->strpos($lastStringGroup, $groupSeparator)) {

return true;
} else {
return false;
}
}
}
4 changes: 2 additions & 2 deletions tests/ZendTest/I18n/Filter/NumberFormatTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public function numberToFormattedProvider()
NumberFormatter::DEFAULT_STYLE,
NumberFormatter::TYPE_DOUBLE,
1234567.8912346,
'1 234 567,891'
'1 234 567,891'
),
);
}
Expand Down Expand Up @@ -161,6 +161,6 @@ public function testReturnUnfiltered($input)
{
$filter = new NumberFormatFilter('de_AT', NumberFormatter::DEFAULT_STYLE, NumberFormatter::TYPE_DOUBLE);

$this->assertEquals($input, $filter->filter($input));
$this->assertEquals($input, $filter->filter($input));
}
}
Loading