From b9f98e422d541f6b3b716d1bad6e7c1f0df6f3ad Mon Sep 17 00:00:00 2001
From: Coleman Watts <coleman@civicrm.org>
Date: Sun, 27 Sep 2015 11:24:25 -0400
Subject: [PATCH 1/3] Fixes for CRM_Utils_Type

---
 CRM/Utils/Type.php | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/CRM/Utils/Type.php b/CRM/Utils/Type.php
index 8219909a2482..5527f96dcef4 100644
--- a/CRM/Utils/Type.php
+++ b/CRM/Utils/Type.php
@@ -165,37 +165,33 @@ public static function escape($data, $type, $abort = TRUE) {
         break;
 
       case 'Positive':
-        // CRM-8925 the 3 below are for custom fields of this type
+        if (CRM_Utils_Rule::positiveInteger($data)) {
+          return (int) $data;
+        }
+        break;
+
+      // CRM-8925 for custom fields of this type
       case 'Country':
       case 'StateProvince':
-        // Checked for multi valued state/country value
         if (is_array($data)) {
-          $returnData = TRUE;
-          // @todo Reuse of the $data variable = asking for trouble.
-          // @todo This code will always return the last item in the array. Intended?
-          foreach ($data as $data) {
-            if (CRM_Utils_Rule::positiveInteger($data) || CRM_Core_DAO::escapeString($data)) {
-              $returnData = TRUE;
-            }
-            else {
-              $returnData = FALSE;
+          $valid = TRUE;
+          foreach ($data as $item) {
+            if (!CRM_Utils_Rule::positiveInteger($item)) {
+              $valid = FALSE;
             }
           }
-          if ($returnData) {
+          if ($valid) {
             return $data;
           }
         }
-        elseif (!is_numeric($data) && CRM_Core_DAO::escapeString($data)) {
-          return $data;
-        }
         elseif (CRM_Utils_Rule::positiveInteger($data)) {
-          return $data;
+          return (int) $data;
         }
         break;
 
       case 'File':
         if (CRM_Utils_Rule::positiveInteger($data)) {
-          return $data;
+          return (int) $data;
         }
         break;
 

From 1299f8dc02a7d4355851b9f8f0bda8b349d4dda4 Mon Sep 17 00:00:00 2001
From: Coleman Watts <coleman@civicrm.org>
Date: Sun, 27 Sep 2015 14:28:14 -0400
Subject: [PATCH 2/3] Add unit tests for CRM_Utils_Type

---
 CRM/Utils/Rule.php                   |  5 +---
 CRM/Utils/Type.php                   |  7 ++---
 tests/phpunit/CRM/Utils/TypeTest.php | 40 +++++++++++++++++++++++++++-
 3 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/CRM/Utils/Rule.php b/CRM/Utils/Rule.php
index f5b1df363f9f..d2a111984b8a 100644
--- a/CRM/Utils/Rule.php
+++ b/CRM/Utils/Rule.php
@@ -735,10 +735,7 @@ public static function validContact($value, $actualElementValue = NULL) {
       $value = $actualElementValue;
     }
 
-    if ($value && !is_numeric($value)) {
-      return FALSE;
-    }
-    return TRUE;
+    return CRM_Utils_Rule::positiveInteger($value);
   }
 
   /**
diff --git a/CRM/Utils/Type.php b/CRM/Utils/Type.php
index 5527f96dcef4..50e3b38d312d 100644
--- a/CRM/Utils/Type.php
+++ b/CRM/Utils/Type.php
@@ -175,10 +175,11 @@ public static function escape($data, $type, $abort = TRUE) {
       case 'StateProvince':
         if (is_array($data)) {
           $valid = TRUE;
-          foreach ($data as $item) {
+          foreach ($data as &$item) {
             if (!CRM_Utils_Rule::positiveInteger($item)) {
               $valid = FALSE;
             }
+            $item = (int) $item;
           }
           if ($valid) {
             return $data;
@@ -241,7 +242,7 @@ public static function escape($data, $type, $abort = TRUE) {
         }
 
         if (CRM_Utils_Rule::validContact($data)) {
-          return $data;
+          return (int) $data;
         }
         break;
 
@@ -286,7 +287,7 @@ public static function validate($data, $type, $abort = TRUE, $name = 'One of par
 
       case 'Positive':
         if (CRM_Utils_Rule::positiveInteger($data)) {
-          return $data;
+          return (int) $data;
         }
         break;
 
diff --git a/tests/phpunit/CRM/Utils/TypeTest.php b/tests/phpunit/CRM/Utils/TypeTest.php
index 32ee27a4b6ba..eef34932afb7 100644
--- a/tests/phpunit/CRM/Utils/TypeTest.php
+++ b/tests/phpunit/CRM/Utils/TypeTest.php
@@ -18,7 +18,7 @@ public function setUp() {
    * @param $expectedResult
    */
   public function testValidate($inputData, $inputType, $expectedResult) {
-    $this->assertEquals($expectedResult, CRM_Utils_Type::validate($inputData, $inputType, FALSE));
+    $this->assertTrue($expectedResult === CRM_Utils_Type::validate($inputData, $inputType, FALSE));
   }
 
   /**
@@ -41,4 +41,42 @@ public function validateDataProvider() {
     );
   }
 
+  /**
+   * @dataProvider escapeDataProvider
+   * @param $inputData
+   * @param $inputType
+   * @param $expectedResult
+   */
+  public function testEscape($inputData, $inputType, $expectedResult) {
+    $this->assertTrue($expectedResult === CRM_Utils_Type::escape($inputData, $inputType, FALSE));
+  }
+
+  /**
+   * @return array
+   */
+  public function escapeDataProvider() {
+    return array(
+      array(10, 'Int', 10),
+      array('145E+3', 'Int', NULL),
+      array('10', 'Integer', 10),
+      array(-10, 'Int', -10),
+      array(array(), 'Integer', NULL),
+      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),
+      array(array('10', 20), 'Country', array(10, 20)),
+      array(array('10', '-10foo'), 'Country', NULL),
+      array('', 'Timestamp', ''),
+      array('', 'ContactReference', ''),
+      array('3', 'ContactReference', 3),
+      array('-3', 'ContactReference', NULL),
+      // Escape function is meant for sql, not xss
+      array('<p onclick="alert(\'xss\');">Hello</p>', 'Memo', '<p onclick=\\"alert(\\\'xss\\\');\\">Hello</p>'),
+    );
+  }
+
 }

From 6a03ea43c78cd6ba4873834858896aabbeba2a57 Mon Sep 17 00:00:00 2001
From: Coleman Watts <coleman@civicrm.org>
Date: Mon, 28 Sep 2015 20:22:33 -0400
Subject: [PATCH 3/3] Fix test failures

---
 CRM/Utils/Type.php                   | 6 +++---
 tests/phpunit/CRM/Utils/TypeTest.php | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/CRM/Utils/Type.php b/CRM/Utils/Type.php
index 50e3b38d312d..81efac86b067 100644
--- a/CRM/Utils/Type.php
+++ b/CRM/Utils/Type.php
@@ -173,13 +173,13 @@ public static function escape($data, $type, $abort = TRUE) {
       // CRM-8925 for custom fields of this type
       case 'Country':
       case 'StateProvince':
-        if (is_array($data)) {
+        // Handle multivalued data in delimited or array format
+        if (is_array($data) || (strpos($data, CRM_Core_DAO::VALUE_SEPARATOR) !== FALSE)) {
           $valid = TRUE;
-          foreach ($data as &$item) {
+          foreach (CRM_Utils_Array::explodePadded($data) as $item) {
             if (!CRM_Utils_Rule::positiveInteger($item)) {
               $valid = FALSE;
             }
-            $item = (int) $item;
           }
           if ($valid) {
             return $data;
diff --git a/tests/phpunit/CRM/Utils/TypeTest.php b/tests/phpunit/CRM/Utils/TypeTest.php
index eef34932afb7..e9235565cc67 100644
--- a/tests/phpunit/CRM/Utils/TypeTest.php
+++ b/tests/phpunit/CRM/Utils/TypeTest.php
@@ -68,7 +68,7 @@ public function escapeDataProvider() {
       array(-10, 'Positive', NULL),
       array('-10', 'Positive', NULL),
       array('-10foo', 'Positive', NULL),
-      array(array('10', 20), 'Country', array(10, 20)),
+      array(array('10', 20), 'Country', array('10', 20)),
       array(array('10', '-10foo'), 'Country', NULL),
       array('', 'Timestamp', ''),
       array('', 'ContactReference', ''),