From bedff42186f5942ef9e6f80485da34d313219536 Mon Sep 17 00:00:00 2001
From: Damian Mooyman <damian@silverstripe.com>
Date: Tue, 28 Feb 2017 10:41:45 +1300
Subject: [PATCH] ENHANCEMENT Protect against infinity loops

---
 src/Collections/CachedConfigCollection.php    | 20 ++++++++-
 .../CachedConfigCollectionTest.php            | 43 ++++++++++++++++---
 2 files changed, 56 insertions(+), 7 deletions(-)

diff --git a/src/Collections/CachedConfigCollection.php b/src/Collections/CachedConfigCollection.php
index f4f2ac7..cecad1d 100644
--- a/src/Collections/CachedConfigCollection.php
+++ b/src/Collections/CachedConfigCollection.php
@@ -37,6 +37,14 @@ class CachedConfigCollection implements ConfigCollectionInterface
      */
     protected $flush = false;
 
+    /**
+     * Set to true while building config.
+     * Used to protect against infinite loops.
+     *
+     * @var bool
+     */
+    protected $building = false;
+
     /**
      * @return static
      */
@@ -91,8 +99,18 @@ public function getCollection()
             return $this->collection;
         }
 
+        // Protect against infinity loop
+        if ($this->building) {
+            throw new BadMethodCallException("Infinite loop detected. Config could not be bootstrapped.");
+        }
+        $this->building = true;
+
         // Cache missed
-        $this->collection = call_user_func($this->collectionCreator);
+        try {
+            $this->collection = call_user_func($this->collectionCreator);
+        } finally {
+            $this->building = false;
+        }
 
         // Note: Config may be yet modified prior to deferred save, but after Core.php
         // however no formal api for this yet
diff --git a/tests/Collections/CachedConfigCollectionTest.php b/tests/Collections/CachedConfigCollectionTest.php
index d2a907f..430d2dc 100644
--- a/tests/Collections/CachedConfigCollectionTest.php
+++ b/tests/Collections/CachedConfigCollectionTest.php
@@ -2,6 +2,7 @@
 
 namespace SilverStripe\Config\Tests\Collections;
 
+use BadMethodCallException;
 use Prophecy\Argument;
 use Prophecy\Prophecy\ObjectProphecy;
 use SilverStripe\Config\Collections\CachedConfigCollection;
@@ -10,7 +11,6 @@
 use Psr\Cache\CacheItemPoolInterface;
 use Psr\Cache\CacheItemInterface;
 use SilverStripe\Config\Collections\ConfigCollectionInterface;
-use SilverStripe\Config\Collections\MemoryConfigCollection;
 
 class CachedConfigCollectionTest extends TestCase
 {
@@ -82,11 +82,6 @@ public function testCacheMiss()
         /** @var CacheItemPoolInterface|ObjectProphecy $mockCache */
         $mockCache = $this->prophet->prophesize(CacheItemPoolInterface::class);
 
-        /** @var ConfigCollectionInterface|ObjectProphecy $mockCollection */
-        $mockCollection = $this->prophet->prophesize(ConfigCollectionInterface::class);
-        $mockCollection->get('test', 'name', 0)->willReturn('value');
-        $mockCollection->exists('test', 'name', 0)->willReturn(true);
-
         // Mock item for collection key
         /** @var CacheItemInterface|ObjectProphecy $mockCacheItem */
         $mockCacheItem = $this->prophet->prophesize(CacheItemInterface::class);
@@ -96,6 +91,11 @@ public function testCacheMiss()
             ->willReturn(false)
             ->shouldBeCalled();
 
+        /** @var ConfigCollectionInterface|ObjectProphecy $mockCollection */
+        $mockCollection = $this->prophet->prophesize(ConfigCollectionInterface::class);
+        $mockCollection->get('test', 'name', 0)->willReturn('value');
+        $mockCollection->exists('test', 'name', 0)->willReturn(true);
+
         // Save immediately, save again on __destruct
         $mockCacheItem->set($mockCollection->reveal())->shouldBeCalledTimes(2);
 
@@ -120,4 +120,35 @@ public function testCacheMiss()
         // Write back changes to cache
         $collection->__destruct();
     }
+
+    public function testInifiniteLoop()
+    {
+        $this->setExpectedException(
+            BadMethodCallException::class,
+            "Infinite loop detected. Config could not be bootstrapped."
+        );
+
+        // Mock cache
+        /** @var CacheItemPoolInterface|ObjectProphecy $mockCache */
+        $mockCache = $this->prophet->prophesize(CacheItemPoolInterface::class);
+        /** @var CacheItemInterface|ObjectProphecy $mockCacheItem */
+        $mockCacheItem = $this->prophet->prophesize(CacheItemInterface::class);
+        $mockCache
+            ->getItem(CachedConfigCollection::CACHE_KEY)
+            ->willReturn($mockCacheItem->reveal())
+            ->shouldBeCalled();
+        $mockCacheItem
+            ->isHit()
+            ->willReturn(false)
+            ->shouldBeCalled();
+
+        // Build new config
+        $collection = new CachedConfigCollection();
+        $collection->setPool($mockCache->reveal());
+        $collection->setCollectionCreator(function() use ($collection) {
+            $collection->getCollection();
+        });
+        $collection->getCollection();
+        $this->fail("Expected exception");
+    }
 }