Merge pull request #7 from open-sausages/pulls/fix-non-config

BUG Prevent non-config vars attempting to be serialized as config
silverstripe · Mar 2, 2017 · 7fd1459 · 7fd1459
2 parents 91b9672 + bb91d2d
commit 7fd1459
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 9 deletions.
diff --git a/src/Collections/MemoryConfigCollection.php b/src/Collections/MemoryConfigCollection.php
@@ -252,15 +252,15 @@ public function getHistory()
 
     public function serialize()
     {
-        return serialize(
-            [
+        return serialize([
             $this->config,
             $this->history,
             $this->metadata,
             $this->trackMetadata,
             $this->middlewares,
-            ]
-        );
+            $this->callCache
+        ]);
+
     }
 
     public function unserialize($serialized)
@@ -270,7 +270,8 @@ public function unserialize($serialized)
             $this->history,
             $this->metadata,
             $this->trackMetadata,
-            $this->middlewares
+            $this->middlewares,
+            $this->callCache
         ) = unserialize($serialized);
     }
 

diff --git a/src/Transformer/PrivateStaticTransformer.php b/src/Transformer/PrivateStaticTransformer.php
@@ -64,18 +64,23 @@ protected function getClassConfig($class)
     {
         $reflection = new ReflectionClass($class);
 
-        /** @var ReflectionProperty[] **/
+        /** @var ReflectionProperty[] $props **/
         $props = $reflection->getProperties(ReflectionProperty::IS_STATIC);
 
         $classConfig = [];
         foreach ($props as $prop) {
-            if (!$prop->isPrivate()) {
-                // Ignore anything which isn't private
+            // Check if this property is configurable
+            if (!$this->isConfigProperty($prop)) {
                 continue;
             }
 
+            // Note that some non-config private statics may be assigned
+            // un-serializable values. Detect these here
             $prop->setAccessible(true);
-            $classConfig[$prop->getName()] = $prop->getValue();
+            $value = $prop->getValue();
+            if ($this->isConfigValue($value)) {
+                $classConfig[$prop->getName()] = $value;
+            }
         }
 
         // Create the metadata for our new item
@@ -88,6 +93,49 @@ protected function getClassConfig($class)
         return ['value' => $classConfig, 'metadata' => $metadata];
     }
 
+    /**
+     * Is a var config or not?
+     *
+     * @param ReflectionProperty $prop
+     * @return bool
+     */
+    protected function isConfigProperty(ReflectionProperty $prop)
+    {
+        if (!$prop->isPrivate()) {
+            return false;
+        }
+        $annotations = $prop->getDocComment();
+        // Whitelist @config
+        if (strstr($annotations, '@config')) {
+            return true;
+        }
+        // Don't treat @internal as config
+        if (strstr($annotations, '@internal')) {
+            return false;
+        }
+        return true;
+    }
+
+    /**
+     * Detect if a value is a valid config
+     *
+     * @param mixed $input
+     * @return true
+     */
+    protected function isConfigValue($input) {
+        if (is_object($input) || is_resource($input)) {
+            return false;
+        }
+        if (is_array($input)) {
+            foreach ($input as $next) {
+                if (!$this->isConfigValue($next)) {
+                    return false;
+                }
+            }
+        }
+        return true;
+    }
+
     /**
      * @return array
      */