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

@@ -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;
   }
 

tests/phpunit/CRM/Utils/RuleTest.php

+<?php
+
+require_once 'CiviTest/CiviUnitTestCase.php';
+
+class CRM_Utils_RuleTest extends CiviUnitTestCase {
+
+  function get_info() {
+    return array(
+      'name'      => '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),
+    );
+  }
+
+
+}

tests/phpunit/CRM/Utils/TypeTest.php

+<?php
+
+require_once 'CiviTest/CiviUnitTestCase.php';
+
+class CRM_Utils_TypeTest extends CiviUnitTestCase {
+
+  function get_info() {
+    return array(
+      'name'      => '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),
+    );
+  }
+
+
+}

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