Prevent infinite recursion in Category\Order::getCategoryTree()

phpmyfaq/src/phpMyFAQ/Category/Order.php

@@ -118,19 +118,35 @@ public function setCategoryTree(
     /**
      * Returns the category tree.
      *
-     * @param stdClass[] $categories
+     * @param array<array{category_id: string, parent_id: string}> $categories
+     * @param int $parentId
+     * @param array<int, bool> $visited Array to track visited category IDs to prevent infinite recursion
      */
-    public function getCategoryTree(array $categories, int $parentId = 0): array
+    public function getCategoryTree(array $categories, int $parentId = 0, array &$visited = []): array
     {
         $result = [];
 
         foreach ($categories as $category) {
-            if ((int) $category['parent_id'] !== $parentId) {
+            $categoryId = (int) $category['category_id'];
+            $categoryParentId = (int) $category['parent_id'];
+
+            // First check if this category belongs at this level (parent_id matches)
+            if ($categoryParentId !== $parentId) {
                 continue;
             }
 
-            $childCategories = $this->getCategoryTree($categories, (int) $category['category_id']);
-            $result[$category['category_id']] = $childCategories;
+            // Then prevent infinite recursion by checking if we've already processed this category
+            // This check must come after the parent_id check to ensure we only mark categories
+            // as visited when they're actually being added to the tree at the correct level
+            if (isset($visited[$categoryId])) {
+                continue;
+            }
+
+            // Mark this category as visited
+            $visited[$categoryId] = true;
+
+            $childCategories = $this->getCategoryTree($categories, $categoryId, $visited);
+            $result[$categoryId] = $childCategories;
         }
 
         return $result;

tests/phpMyFAQ/Category/OrderTest.php

@@ -193,10 +193,10 @@ public function testGetCategoryTreeWithCategories(): void
 
         $result = $this->order->getCategoryTree($categories, 0);
 
-        $this->assertArrayHasKey('1', $result);
-        $this->assertArrayHasKey('3', $result);
-        $this->assertArrayHasKey('2', $result['1']);
-        $this->assertArrayHasKey('4', $result['1']['2']);
+        $this->assertArrayHasKey(1, $result);
+        $this->assertArrayHasKey(3, $result);
+        $this->assertArrayHasKey(2, $result[1]);
+        $this->assertArrayHasKey(4, $result[1][2]);
     }
 
     public function testGetCategoryTreeWithSpecificParent(): void
@@ -209,9 +209,69 @@ public function testGetCategoryTreeWithSpecificParent(): void
 
         $result = $this->order->getCategoryTree($categories, 1);
 
-        $this->assertArrayHasKey('2', $result);
-        $this->assertArrayHasKey('3', $result);
-        $this->assertArrayNotHasKey('1', $result);
+        $this->assertArrayHasKey(2, $result);
+        $this->assertArrayHasKey(3, $result);
+        $this->assertArrayNotHasKey(1, $result);
+    }
+
+    public function testGetCategoryTreeWithSelfReference(): void
+    {
+        // Test case where a category references itself as parent
+        $categories = [
+            ['category_id' => '1', 'parent_id' => '0'],
+            ['category_id' => '2', 'parent_id' => '2'], // Self-reference
+            ['category_id' => '3', 'parent_id' => '0'],
+        ];
+
+        $result = $this->order->getCategoryTree($categories, 0);
+
+        // Should contain categories 1 and 3, but not 2 (self-referencing)
+        $this->assertArrayHasKey(1, $result);
+        $this->assertArrayHasKey(3, $result);
+        $this->assertArrayNotHasKey(2, $result);
+    }
+
+    public function testGetCategoryTreeWithCircularReference(): void
+    {
+        // Test case where categories have circular references: 2 -> 1 and 1 -> 2
+        // This simulates corrupt data where two categories reference each other as parent
+        $categories = [
+            ['category_id' => '1', 'parent_id' => '2'], // Category 1's parent is 2
+            ['category_id' => '2', 'parent_id' => '1'], // Category 2's parent is 1 (circular!)
+            ['category_id' => '3', 'parent_id' => '0'],
+        ];
+
+        $result = $this->order->getCategoryTree($categories, 0);
+
+        // Should handle the circular reference gracefully
+        // Only category 3 should appear at root level since 1 and 2 are in a circular loop
+        $this->assertArrayHasKey(3, $result);
+        $this->assertCount(1, $result);
+    }
+
+    public function testGetCategoryTreeWithComplexCircularReference(): void
+    {
+        // Test case: Category chain where the last one references an earlier one in the chain
+        // 1 (root) -> 2 -> 3 -> 4, but then 4 -> 2 (creates a loop: 2 -> 3 -> 4 -> 2)
+        $categories = [
+            ['category_id' => '1', 'parent_id' => '0'],
+            ['category_id' => '2', 'parent_id' => '1'],
+            ['category_id' => '3', 'parent_id' => '2'],
+            ['category_id' => '4', 'parent_id' => '3'],
+            ['category_id' => '5', 'parent_id' => '2'], // Another child of 2 (valid)
+            // Simulate someone incorrectly updating category 3 to make it a child of 4
+            // In real DB this would be an update, but we simulate by having it appear again
+            ['category_id' => '3', 'parent_id' => '4'], // This creates: 3 -> 4 -> (back to 3 via next entry)
+        ];
+
+        $result = $this->order->getCategoryTree($categories, 0);
+
+        // Should handle gracefully and not cause infinite recursion
+        // The tree should build up to the point where the circular reference is detected
+        $this->assertArrayHasKey(1, $result);
+        $this->assertArrayHasKey(2, $result[1]);
+        // Category 3 should be under 2, and when we try to add it again under 4, it's skipped
+        $this->assertArrayHasKey(3, $result[1][2]);
     }
 
     public function testGetParentIdFound(): void