Fix issue with modified backtrace

php allows the backtrace arguments to be modified via the backtrace. By using array_map we make sure that we do not overwrite references.
phpDocumentor · Aug 12, 2020 · 6ff90eb · 6ff90eb
1 parent f0af3b7
commit 6ff90eb
Show file tree

Hide file tree

Showing 3 changed files with 110 additions and 20 deletions.
diff --git a/phpcs.xml.dist b/phpcs.xml.dist
@@ -15,4 +15,8 @@
     <rule ref="SlevomatCodingStandard.Classes.SuperfluousAbstractClassNaming.SuperfluousPrefix">
         <exclude-pattern>*/src/*/Abstract*.php</exclude-pattern>
     </rule>
+
+    <rule ref="SlevomatCodingStandard.Classes.UnusedPrivateElements.UnusedMethod">
+        <exclude-pattern>*/src/DocBlock/Tags/InvalidTag.php</exclude-pattern>
+    </rule>
 </ruleset>
diff --git a/src/DocBlock/Tags/InvalidTag.php b/src/DocBlock/Tags/InvalidTag.php
@@ -8,12 +8,13 @@
 use Exception;
 use phpDocumentor\Reflection\DocBlock\Tag;
 use ReflectionClass;
+use ReflectionException;
 use ReflectionFunction;
 use Throwable;
 use function array_map;
-use function array_walk_recursive;
 use function get_class;
 use function get_resource_type;
+use function is_array;
 use function is_object;
 use function is_resource;
 use function sprintf;
@@ -80,29 +81,12 @@ private function flattenExceptionBacktrace(Throwable $exception) : void
         $traceProperty = (new ReflectionClass(Exception::class))->getProperty('trace');
         $traceProperty->setAccessible(true);
 
-        $flatten =
-            /** @param mixed $value */
-            static function (&$value) : void {
-                if ($value instanceof Closure) {
-                    $closureReflection = new ReflectionFunction($value);
-                    $value             = sprintf(
-                        '(Closure at %s:%s)',
-                        $closureReflection->getFileName(),
-                        $closureReflection->getStartLine()
-                    );
-                } elseif (is_object($value)) {
-                    $value = sprintf('object(%s)', get_class($value));
-                } elseif (is_resource($value)) {
-                    $value = sprintf('resource(%s)', get_resource_type($value));
-                }
-            };
-
         do {
             $trace = $exception->getTrace();
             if (isset($trace[0]['args'])) {
                 $trace = array_map(
-                    static function (array $call) use ($flatten) : array {
-                        array_walk_recursive($call['args'], $flatten);
+                    function (array $call) : array {
+                        $call['args'] = array_map([$this, 'flattenArguments'], $call['args']);
 
                         return $call;
                     },
@@ -117,6 +101,33 @@ static function (array $call) use ($flatten) : array {
         $traceProperty->setAccessible(false);
     }
 
+    /**
+     * @param mixed $value
+     *
+     * @return mixed
+     *
+     * @throws ReflectionException
+     */
+    private function flattenArguments($value)
+    {
+        if ($value instanceof Closure) {
+            $closureReflection = new ReflectionFunction($value);
+            $value             = sprintf(
+                '(Closure at %s:%s)',
+                $closureReflection->getFileName(),
+                $closureReflection->getStartLine()
+            );
+        } elseif (is_object($value)) {
+            $value = sprintf('object(%s)', get_class($value));
+        } elseif (is_resource($value)) {
+            $value = sprintf('resource(%s)', get_resource_type($value));
+        } elseif (is_array($value)) {
+            $value = array_map([$this, 'flattenArguments'], $value);
+        }
+
+        return $value;
+    }
+
     public function render(?Formatter $formatter = null) : string
     {
         if ($formatter === null) {

diff --git a/tests/integration/ModifyBackTraceSafeTest.php b/tests/integration/ModifyBackTraceSafeTest.php
@@ -0,0 +1,75 @@
+<?php
+
+declare(strict_types=1);
+
+namespace phpDocumentor\Reflection;
+
+
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @coversNothing
+ */
+class ModifyBackTraceSafeTest extends TestCase
+{
+    public function testBackTraceModificationDoesNotImpactFunctionArguments()
+    {
+        $traverser = new Traverser();
+        $node1 = new Node();
+        $node1->children[] = new Node();
+        $node1->children[] = new Node();
+
+        $traverser->traverse([new Node(), $node1]);
+    }
+}
+
+
+class Node {
+    public $children = [];
+}
+
+class Traverser
+{
+    public function traverse(array $nodes)
+    {
+        $this->traverseArray($nodes);
+    }
+
+    public function traverseArray(array $nodes): array
+    {
+        $doNodes = [];
+
+        foreach ($nodes as &$node) {
+            $node = $this->callback($node);
+            $node = $this->traverseNode($node);
+
+            $doNodes[] = $node;
+        }
+
+        return $doNodes;
+    }
+
+    public function callback(Node $class) : Node
+    {
+        $docblock = <<<DOCBLOCK
+ /**
+  * @see sql.php
+  */
+DOCBLOCK;
+
+        $factor = DocBlockFactory::createInstance();
+
+        $factor->create($docblock);
+
+        return $class;
+    }
+
+    private function traverseNode(Node $node) : Node
+    {
+        if ($node->children) {
+            $this->traverseArray($node->children);
+        }
+
+        return $node;
+    }
+}