MDL-25290 cache: Fixed issues with references when getting, and added unit tests...

diff --git a/cache/classes/loaders.php b/cache/classes/loaders.php
index c97b50a..8595808 100644 (file)
--- a/cache/classes/loaders.php
+++ b/cache/classes/loaders.php
@@ -277,6 +277,13 @@ class cache implements cache_loader, cache_is_key_aware {
             if ($this->perfdebug) {
                 cache_helper::record_cache_hit('** static persist **', $this->definition->get_id());
             }
+            if (!is_scalar($result)) {
+                // If data is an object it will be a reference.
+                // If data is an array if may contain references.
+                // We want to break references so that the cache cannot be modified outside of itself.
+                // Call the function to unreference it (in the best way possible).
+                $result = $this->unref($result);
+            }
             return $result;
         } else if ($this->perfdebug) {
             cache_helper::record_cache_miss('** static persist **', $this->definition->get_id());
@@ -322,6 +329,15 @@ class cache implements cache_loader, cache_is_key_aware {
         if ($setaftervalidation) {
             $this->set($key, $result);
         }
+        // 7. Make sure we don't pass back anything that could be a reference.
+        //    We don't want people modifying the data in the cache.
+        if (!is_scalar($result)) {
+            // If data is an object it will be a reference.
+            // If data is an array if may contain references.
+            // We want to break references so that the cache cannot be modified outside of itself.
+            // Call the function to unreference it (in the best way possible).
+            $result = $this->unref($result);
+        }
         return $result;
     }
 
@@ -570,6 +586,12 @@ class cache implements cache_loader, cache_is_key_aware {
         foreach ($keyvaluearray as $key => $value) {
             if (is_object($value) && $value instanceof cacheable_object) {
                 $value = new cache_cached_object($value);
+            } else if (!is_scalar($value)) {
+                // If data is an object it will be a reference.
+                // If data is an array if may contain references.
+                // We want to break references so that the cache cannot be modified outside of itself.
+                // Call the function to unreference it (in the best way possible).
+                $value = $this->unref($value);
             }
             if ($simulatettl) {
                 $value = new cache_ttl_wrapper($value, $this->definition->get_ttl());

diff --git a/cache/tests/cache_test.php b/cache/tests/cache_test.php
index 7439d73..831b411 100644 (file)
--- a/cache/tests/cache_test.php
+++ b/cache/tests/cache_test.php
@@ -291,6 +291,23 @@ class cache_phpunit_tests extends advanced_testcase {
         $var = $cache->get('obj');
         $this->assertInstanceOf('stdClass', $var);
         $this->assertEquals('value', $var->key);
+
+        // Reference test after retrieve.
+        $obj = new stdClass;
+        $obj->key = 'value';
+        $this->assertTrue($cache->set('obj', $obj));
+
+        $var1 = $cache->get('obj');
+        $this->assertInstanceOf('stdClass', $var1);
+        $this->assertEquals('value', $var1->key);
+        $var1->key = 'eulav';
+        $this->assertEquals('eulav', $var1->key);
+
+        $var2 = $cache->get('obj');
+        $this->assertInstanceOf('stdClass', $var2);
+        $this->assertEquals('value', $var2->key);
+
+        $this->assertTrue($cache->delete('obj'));
     }
 
     /**