From f942c3217c76e559e2e43e1c688d1094e4ceb2fa Mon Sep 17 00:00:00 2001 From: "Donald A. Lobo" Date: Fri, 27 Sep 2013 14:01:13 -0700 Subject: [PATCH] CRM-13460 - Add is_numeric checks and test coverage ---------------------------------------- * CRM-13460: Make the numeric rule checks stricter http://issues.civicrm.org/jira/browse/CRM-13460 --- CRM/Utils/Rule.php | 33 ++++++++++--- tests/phpunit/CRM/Utils/RuleTest.php | 74 ++++++++++++++++++++++++++++ tests/phpunit/CRM/Utils/TypeTest.php | 62 +++++++++++++++++++++++ tools/scripts/git/commit-msg | 2 +- 4 files changed, 164 insertions(+), 7 deletions(-) create mode 100644 tests/phpunit/CRM/Utils/RuleTest.php create mode 100644 tests/phpunit/CRM/Utils/TypeTest.php diff --git a/CRM/Utils/Rule.php b/CRM/Utils/Rule.php index b6b6e8d4fb..cadc93dc78 100644 --- a/CRM/Utils/Rule.php +++ b/CRM/Utils/Rule.php @@ -34,6 +34,7 @@ */ require_once 'HTML/QuickForm/Rule/Email.php'; + class CRM_Utils_Rule { static function title($str, $maxLength = 127) { @@ -272,17 +273,26 @@ class CRM_Utils_Rule { return TRUE; } - if (($value < 0)) { + // CRM-13460 + // ensure number passed is always a string numeral + if (!is_numeric($value)) { + return FALSE; + } + + // note that is_int matches only integer type + // and not strings which are only integers + // hence we do this here + if (preg_match('/^\d+$/', $value)) { + return TRUE; + } + + if ($value < 0) { $negValue = -1 * $value; if (is_int($negValue)) { return TRUE; } } - if (is_numeric($value) && preg_match('/^\d+$/', $value)) { - return TRUE; - } - return FALSE; } @@ -291,7 +301,13 @@ class CRM_Utils_Rule { return ($value < 0) ? FALSE : TRUE; } - if (is_numeric($value) && preg_match('/^\d+$/', $value)) { + // CRM-13460 + // ensure number passed is always a string numeral + if (!is_numeric($value)) { + return FALSE; + } + + if (preg_match('/^\d+$/', $value)) { return TRUE; } @@ -299,6 +315,11 @@ class CRM_Utils_Rule { } static function numeric($value) { + // lets use a php gatekeeper to ensure this is numeric + if (!is_numeric($value)) { + return FALSE; + } + return preg_match('/(^-?\d\d*\.\d*$)|(^-?\d\d*$)|(^-?\.\d\d*$)/', $value) ? TRUE : FALSE; } diff --git a/tests/phpunit/CRM/Utils/RuleTest.php b/tests/phpunit/CRM/Utils/RuleTest.php new file mode 100644 index 0000000000..22071172cb --- /dev/null +++ b/tests/phpunit/CRM/Utils/RuleTest.php @@ -0,0 +1,74 @@ + 'Rule Test', + 'description' => 'Test the validation rules', + 'group' => 'CiviCRM BAO Tests', + ); + } + + function setUp() { + parent::setUp(); + } + + /** + * @dataProvider integerDataProvider + */ + function testInteger($inputData, $expectedResult) { + $this->assertEquals($expectedResult, CRM_Utils_Rule::integer($inputData)); + } + + function integerDataProvider() { + return array( + array(10, true), + array('145E+3', false), + array('10', true), + array(-10, true), + array('-10', true), + array('-10foo', false), + ); + } + + /** + * @dataProvider positiveDataProvider + */ + function testPositive($inputData, $expectedResult) { + $this->assertEquals($expectedResult, CRM_Utils_Rule::positiveInteger($inputData, $inputType)); + } + + function positiveDataProvider() { + return array( + array(10, true), + array('145.0E+3', false), + array('10', true), + array(-10, false), + array('-10', false), + array('-10foo', false), + ); + } + + /** + * @dataProvider numericDataProvider + */ + function testNumeric($inputData, $expectedResult) { + $this->assertEquals($expectedResult, CRM_Utils_Rule::numeric($inputData, $inputType)); + } + + function numericDataProvider() { + return array( + array(10, true), + array('145.0E+3', false), + array('10', true), + array(-10, true), + array('-10', true), + array('-10foo', false), + ); + } + + +} diff --git a/tests/phpunit/CRM/Utils/TypeTest.php b/tests/phpunit/CRM/Utils/TypeTest.php new file mode 100644 index 0000000000..a034e40ff2 --- /dev/null +++ b/tests/phpunit/CRM/Utils/TypeTest.php @@ -0,0 +1,62 @@ + 'Type Test', + 'description' => 'Test the validate function', + 'group' => 'CiviCRM BAO Tests', + ); + } + + function setUp() { + parent::setUp(); + } + + /** + * @dataProvider validateDataProvider + */ + function testValidate($inputData, $inputType, $expectedResult) { + $this->assertEquals($expectedResult, CRM_Utils_Type::validate($inputData, $inputType, FALSE)); + } + + function validateDataProvider() { + return array( + array(10, 'Int', 10), + array('145E+3', 'Int', NULL), + array('10', 'Integer', 10), + array(-10, 'Int', -10), + array('-10', 'Integer', -10), + array('-10foo', 'Int', NULL), + array(10, 'Positive', 10), + array('145.0E+3', 'Positive', NULL), + array('10', 'Positive', 10), + array(-10, 'Positive', NULL), + array('-10', 'Positive', NULL), + array('-10foo', 'Positive', NULL), + ); + } + + /** + * @dataProvider numericDataProvider + */ + function testNumeric($inputData, $expectedResult) { + $this->assertEquals($expectedResult, CRM_Utils_Rule::numeric($inputData, $inputType)); + } + + function numericDataProvider() { + return array( + array(10, true), + array('145.0E+3', false), + array('10', true), + array(-10, true), + array('-10', true), + array('-10foo', false), + ); + } + + +} diff --git a/tools/scripts/git/commit-msg b/tools/scripts/git/commit-msg index 93d0df6934..1d364daca2 100755 --- a/tools/scripts/git/commit-msg +++ b/tools/scripts/git/commit-msg @@ -15,5 +15,5 @@ if [ -f "$PACKAGES/git-footnote/bin/git-footnote" ]; then TMPFILE="$1.jira" cp -p "$FILE" "$TMPFILE" $PACKAGES/git-footnote/bin/git-footnote CRM http://issues.civicrm.org/jira < "$TMPFILE" > "$FILE" - rm -f "$TMPFILE" + # rm -f "$TMPFILE" fi -- 2.25.1