diff --git a/phpmyfaq/src/phpMyFAQ/Category/Order.php b/phpmyfaq/src/phpMyFAQ/Category/Order.php index b44ab0161d..509222e0ce 100644 --- a/phpmyfaq/src/phpMyFAQ/Category/Order.php +++ b/phpmyfaq/src/phpMyFAQ/Category/Order.php @@ -118,19 +118,35 @@ public function setCategoryTree( /** * Returns the category tree. * - * @param stdClass[] $categories + * @param array $categories + * @param int $parentId + * @param array $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; diff --git a/tests/phpMyFAQ/Category/OrderTest.php b/tests/phpMyFAQ/Category/OrderTest.php index beb10e9599..0d36bdcee8 100644 --- a/tests/phpMyFAQ/Category/OrderTest.php +++ b/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