Skip to content

Commit 4f385ef

Browse files
zonky2zonky2
committed
Detect circular visible conditions before they exhaust the call stack
1 parent db7171d commit 4f385ef

2 files changed

Lines changed: 148 additions & 1 deletion

File tree

‎src/DataDefinition/Palette/Property.php‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
use ContaoCommunityAlliance\DcGeneral\Data\PropertyValueBag;
2727
use ContaoCommunityAlliance\DcGeneral\Data\PropertyValueBagInterface;
2828
use ContaoCommunityAlliance\DcGeneral\DataDefinition\Palette\Condition\Property\PropertyConditionInterface;
29+
use ContaoCommunityAlliance\DcGeneral\Exception\DcGeneralRuntimeException;
2930

3031
/**
3132
* A property contained within a palette.
@@ -55,6 +56,15 @@ class Property implements PropertyInterface
5556
*/
5657
protected $editableCondition = null;
5758

59+
/**
60+
* The property names currently being resolved for visibility, keyed by object id, in call
61+
* order. Detects a visible-condition cycle (property A depends on B depends on A) before it
62+
* exhausts the call stack - see contao-community-alliance/dc-general#528.
63+
*
64+
* @var array<int, string>
65+
*/
66+
private static array $visibilityGuard = [];
67+
5868
/**
5969
* Create a new instance.
6070
*
@@ -97,7 +107,19 @@ public function isVisible(
97107
if ($this->visibleCondition) {
98108
// We should have defined the interfaces back in 2013... :/
99109
assert($input === null || $input instanceof PropertyValueBag);
100-
return $this->visibleCondition->match($model, $input, $this, $legend);
110+
111+
$key = spl_object_id($this);
112+
if (isset(self::$visibilityGuard[$key])) {
113+
$chain = implode('" -> "', [...array_values(self::$visibilityGuard), $this->name]);
114+
throw new DcGeneralRuntimeException('Circular visible condition detected: "' . $chain . '"');
115+
}
116+
117+
self::$visibilityGuard[$key] = $this->name;
118+
try {
119+
return $this->visibleCondition->match($model, $input, $this, $legend);
120+
} finally {
121+
unset(self::$visibilityGuard[$key]);
122+
}
101123
}
102124

103125
return true;

‎tests/DataDefinition/Palette/PropertyTest.php‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
<?php
2+
3+
/**
4+
* This file is part of contao-community-alliance/dc-general.
5+
*
6+
* (c) 2013-2026 Contao Community Alliance.
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*
11+
* This project is provided in good faith and hope to be usable by anyone.
12+
*
13+
* @package contao-community-alliance/dc-general
14+
* @author Ingolf Steinhardt <info@e-spin.de>
15+
* @copyright 2013-2026 Contao Community Alliance.
16+
* @license https://github.com/contao-community-alliance/dc-general/blob/master/LICENSE LGPL-3.0-or-later
17+
* @filesource
18+
*/
19+
20+
namespace ContaoCommunityAlliance\DcGeneral\Test\DataDefinition\Palette;
21+
22+
use ContaoCommunityAlliance\DcGeneral\DataDefinition\Palette\Condition\Property\PropertyVisibleCondition;
23+
use ContaoCommunityAlliance\DcGeneral\DataDefinition\Palette\Legend;
24+
use ContaoCommunityAlliance\DcGeneral\DataDefinition\Palette\Property;
25+
use ContaoCommunityAlliance\DcGeneral\Test\TestCase;
26+
use PHPUnit\Framework\Attributes\CoversClass;
27+
28+
#[CoversClass(Property::class)]
29+
final class PropertyTest extends TestCase
30+
{
31+
/**
32+
* Two properties whose visible conditions refer to each other used to recurse until the
33+
* PHP engine aborted with "Infinite recursion?" - see
34+
* contao-community-alliance/dc-general#528.
35+
*/
36+
public function testDirectCycleResolvesToInvisibleInsteadOfExhaustingTheStack(): void
37+
{
38+
$legend = new Legend('test');
39+
$propertyA = new Property('feld_a');
40+
$propertyB = new Property('feld_b');
41+
42+
$propertyA->setVisibleCondition(new PropertyVisibleCondition('feld_b'));
43+
$propertyB->setVisibleCondition(new PropertyVisibleCondition('feld_a'));
44+
45+
$legend->addProperty($propertyA);
46+
$legend->addProperty($propertyB);
47+
48+
self::assertFalse($propertyA->isVisible(null, null, $legend));
49+
}
50+
51+
/**
52+
* The cycle may close over any number of intermediate properties, not only two.
53+
*/
54+
public function testIndirectCycleResolvesToInvisible(): void
55+
{
56+
$legend = new Legend('test');
57+
$propertyA = new Property('feld_a');
58+
$propertyB = new Property('feld_b');
59+
$propertyC = new Property('feld_c');
60+
61+
$propertyA->setVisibleCondition(new PropertyVisibleCondition('feld_b'));
62+
$propertyB->setVisibleCondition(new PropertyVisibleCondition('feld_c'));
63+
$propertyC->setVisibleCondition(new PropertyVisibleCondition('feld_a'));
64+
65+
$legend->addProperty($propertyA);
66+
$legend->addProperty($propertyB);
67+
$legend->addProperty($propertyC);
68+
69+
self::assertFalse($propertyA->isVisible(null, null, $legend));
70+
}
71+
72+
/**
73+
* The cycle guard must not leave state behind that would poison an unrelated, later
74+
* evaluation - neither for the properties involved in the cycle nor for others.
75+
*/
76+
public function testGuardIsClearedAfterACycleWasDetected(): void
77+
{
78+
$legend = new Legend('test');
79+
$propertyA = new Property('feld_a');
80+
$propertyB = new Property('feld_b');
81+
82+
$propertyA->setVisibleCondition(new PropertyVisibleCondition('feld_b'));
83+
$propertyB->setVisibleCondition(new PropertyVisibleCondition('feld_a'));
84+
85+
$legend->addProperty($propertyA);
86+
$legend->addProperty($propertyB);
87+
88+
self::assertFalse($propertyA->isVisible(null, null, $legend));
89+
// Re-evaluating afterwards must behave exactly the same, not throw due to leftover guard
90+
// state from the previous, unrelated call.
91+
self::assertFalse($propertyA->isVisible(null, null, $legend));
92+
self::assertFalse($propertyB->isVisible(null, null, $legend));
93+
}
94+
95+
/**
96+
* A property referring to itself is the smallest possible cycle.
97+
*/
98+
public function testSelfReferenceResolvesToInvisible(): void
99+
{
100+
$legend = new Legend('test');
101+
$propertyA = new Property('feld_a');
102+
$propertyA->setVisibleCondition(new PropertyVisibleCondition('feld_a'));
103+
104+
$legend->addProperty($propertyA);
105+
106+
self::assertFalse($propertyA->isVisible(null, null, $legend));
107+
}
108+
109+
/**
110+
* Chained, non-cyclic conditions must keep working as before.
111+
*/
112+
public function testNonCyclicChainStillResolves(): void
113+
{
114+
$legend = new Legend('test');
115+
$propertyA = new Property('feld_a');
116+
$propertyB = new Property('feld_b');
117+
118+
$propertyA->setVisibleCondition(new PropertyVisibleCondition('feld_b'));
119+
120+
$legend->addProperty($propertyA);
121+
$legend->addProperty($propertyB);
122+
123+
self::assertTrue($propertyA->isVisible(null, null, $legend));
124+
}
125+
}

0 commit comments

Comments
 (0)