From a8ee2c885d37e33e49cab61332fc8f1f97fd1a08 Mon Sep 17 00:00:00 2001 From: Russell Smith Date: Tue, 23 Feb 2016 11:48:05 +1100 Subject: [PATCH] MDL-53206 caching: Ensure get_many and set_many dereference objects. --- cache/classes/dummystore.php | 4 ++-- cache/classes/loaders.php | 13 +++++++++++++ cache/tests/cache_test.php | 28 +++++++++++++++++++++++++++- 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/cache/classes/dummystore.php b/cache/classes/dummystore.php index 14eecd8e8cf..40508387382 100644 --- a/cache/classes/dummystore.php +++ b/cache/classes/dummystore.php @@ -190,9 +190,9 @@ class cachestore_dummy extends cache_store { foreach ($keyvaluearray as $pair) { $this->store[$pair['key']] = $pair['value']; } - return count($keyvaluearray); + } - return 0; + return count($keyvaluearray); } /** diff --git a/cache/classes/loaders.php b/cache/classes/loaders.php index e0a7723e079..fcc22ba31d2 100644 --- a/cache/classes/loaders.php +++ b/cache/classes/loaders.php @@ -450,6 +450,13 @@ class cache implements cache_loader { // Create an array with the original keys and the found values. This will be what we return. $fullresult = array(); foreach ($result as $key => $value) { + 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); + } $fullresult[$parsedkeys[$key]] = $value; } unset($result); @@ -1922,6 +1929,12 @@ class cache_session extends cache { if ($value instanceof cache_cached_object) { /* @var cache_cached_object $value */ $value = $value->restore_object(); + } 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); } $return[$key] = $value; if ($value === false) { diff --git a/cache/tests/cache_test.php b/cache/tests/cache_test.php index 6c9f14ccae1..26ab1454f63 100644 --- a/cache/tests/cache_test.php +++ b/cache/tests/cache_test.php @@ -407,7 +407,7 @@ class core_cache_testcase extends advanced_testcase { $this->assertEquals('pork', $var->subobj->subobj->key); $this->assertTrue($cache->delete('obj')); - // Death reference test... basicaly we don't want this to die. + // Death reference test... basically we don't want this to die. $obj = new stdClass; $obj->key = 'value'; $obj->self =& $obj; @@ -433,6 +433,32 @@ class core_cache_testcase extends advanced_testcase { $this->assertTrue($cache->delete('obj')); + // Death reference test on get_many... basically we don't want this to die. + $obj = new stdClass; + $obj->key = 'value'; + $obj->self =& $obj; + $this->assertEquals(1, $cache->set_many(array('obj' => $obj))); + $var = $cache->get_many(array('obj')); + $this->assertInstanceOf('stdClass', $var['obj']); + $this->assertEquals('value', $var['obj']->key); + + // Reference test after retrieve. + $obj = new stdClass; + $obj->key = 'value'; + $this->assertEquals(1, $cache->set_many(array('obj' => $obj))); + + $var1 = $cache->get_many(array('obj')); + $this->assertInstanceOf('stdClass', $var1['obj']); + $this->assertEquals('value', $var1['obj']->key); + $var1['obj']->key = 'eulav'; + $this->assertEquals('eulav', $var1['obj']->key); + + $var2 = $cache->get_many(array('obj')); + $this->assertInstanceOf('stdClass', $var2['obj']); + $this->assertEquals('value', $var2['obj']->key); + + $this->assertTrue($cache->delete('obj')); + // Test strictness exceptions. try { $cache->get('exception', MUST_EXIST);