Merge pull request #12653 from totten/5.4-spaces

(dev/core#316) Fix crash on Memcache systems when session key involves whitespace
civicrm · Aug 14, 2018 · e9e5510 · e9e5510
2 parents 5160dfd + 1c73869
commit e9e5510
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 9 deletions.
diff --git a/CRM/Core/BAO/Cache.php b/CRM/Core/BAO/Cache.php
@@ -73,15 +73,16 @@ public static function &getItem($group, $path, $componentID = NULL) {
     $argString = "CRM_CT_{$group}_{$path}_{$componentID}";
     if (!array_key_exists($argString, self::$_cache)) {
       $cache = CRM_Utils_Cache::singleton();
-      self::$_cache[$argString] = $cache->get(self::cleanKey($argString));
+      $cleanKey = self::cleanKey($argString);
+      self::$_cache[$argString] = $cache->get($cleanKey);
       if (!self::$_cache[$argString]) {
         $table = self::getTableName();
         $where = self::whereCache($group, $path, $componentID);
         $rawData = CRM_Core_DAO::singleValueQuery("SELECT data FROM $table WHERE $where");
         $data = $rawData ? self::decode($rawData) : NULL;
 
         self::$_cache[$argString] = $data;
-        $cache->set(self::cleanKey($argString), self::$_cache[$argString]);
+        $cache->set($cleanKey, self::$_cache[$argString]);
       }
     }
     return self::$_cache[$argString];
@@ -106,7 +107,8 @@ public static function &getItems($group, $componentID = NULL) {
     $argString = "CRM_CT_CI_{$group}_{$componentID}";
     if (!array_key_exists($argString, self::$_cache)) {
       $cache = CRM_Utils_Cache::singleton();
-      self::$_cache[$argString] = $cache->get(self::cleanKey($argString));
+      $cleanKey = self::cleanKey($argString);
+      self::$_cache[$argString] = $cache->get($cleanKey);
       if (!self::$_cache[$argString]) {
         $table = self::getTableName();
         $where = self::whereCache($group, NULL, $componentID);
@@ -119,7 +121,7 @@ public static function &getItems($group, $componentID = NULL) {
         $dao->free();
 
         self::$_cache[$argString] = $result;
-        $cache->set(self::cleanKey($argString), self::$_cache[$argString]);
+        $cache->set($cleanKey, self::$_cache[$argString]);
       }
     }
 
@@ -448,7 +450,7 @@ public static function cleanKey($key) {
       return $escape . md5($key);
     }
 
-    $r = preg_replace_callback(';[^A-Za-z0-9_\. ];', function($m) use ($escape) {
+    $r = preg_replace_callback(';[^A-Za-z0-9_\.];', function($m) use ($escape) {
       return $escape . dechex(ord($m[0]));
     }, $key);
 

diff --git a/CRM/Utils/Cache.php b/CRM/Utils/Cache.php
@@ -175,6 +175,9 @@ public static function getCacheSettings($cachePlugin) {
    * @param array $params
    *   Array with keys:
    *   - name: string, unique symbolic name.
+   *     For a naming convention, use `snake_case` or `CamelCase` to maximize
+   *     portability/cleanliness. Any other punctuation or whitespace
+   *     should function correctly, but it can be harder to inspect/debug.
    *   - type: array|string, list of acceptable cache types, in order of preference.
    *   - prefetch: bool, whether to prefetch all data in cache (if possible).
    * @return CRM_Utils_Cache_Interface
@@ -184,6 +187,10 @@ public static function getCacheSettings($cachePlugin) {
   public static function create($params = array()) {
     $types = (array) $params['type'];
 
+    if (!empty($params['name'])) {
+      $params['name'] = CRM_Core_BAO_Cache::cleanKey($params['name']);
+    }
+
     foreach ($types as $type) {
       switch ($type) {
         case '*memory*':

diff --git a/tests/phpunit/CRM/Core/BAO/CacheTest.php b/tests/phpunit/CRM/Core/BAO/CacheTest.php
@@ -84,11 +84,14 @@ public function testSetGetItem($originalValue) {
 
   public function getCleanKeyExamples() {
     $es = [];
-    $es[] = ['hello_world and other.planets', 'hello_world and other.planets']; // allowed chars
+    $es[] = ['hello_world and/other.planets', 'hello_world-20and-2fother.planets']; // allowed chars
     $es[] = ['hello/world+-#@{}', 'hello-2fworld-2b-2d-23-40-7b-7d']; // escaped chars
-    $es[] = ['123456789 123456789 123456789 123456789 123456789 123456789 123', '123456789 123456789 123456789 123456789 123456789 123456789 123']; // long but allowed
-    $es[] = ['123456789 123456789 123456789 123456789 123456789 123456789 1234', '-2a008e182a4dcd1a78f405f30119e5f2']; // too long, md5 fallback
-    $es[] = ['123456789 /23456789 +23456789 -23456789 123456789 123456789', '-1b6baab5961431ed443ab321f5dfa0fb']; // too long, md5 fallback
+    $es[] = ["LF-\nTAB-\tCR-\remojiskull💀", 'LF-2d-aTAB-2d-9CR-2d-demojiskull-f0-9f-92-80']; // short with emoji
+    $es[] = ["LF-\nTAB-\tCR-\remojibomb💣emojiskull💀", '-5d9324e052f6e10240dce5029c5e8525']; // long with emoji
+    $es[] = ['123456789 123456789 123456789 123456789 123456789 123', '123456789-20123456789-20123456789-20123456789-20123456789-20123']; // spaces are escaped
+    $es[] = ['123456789_123456789_123456789_123456789_123456789_123456789_123', '123456789_123456789_123456789_123456789_123456789_123456789_123']; // long but allowed
+    $es[] = ['123456789_123456789_123456789_123456789_123456789_123456789_1234', '-e02b981aff954fdcc9a81c25f5ec9681']; // too long, md5 fallback
+    $es[] = ['123456789-/23456789-+23456789--23456789_123456789_123456789', '-43b6dec1026187ae6f6a8fe4d56ab22e']; // too long, md5 fallback
     return $es;
   }