diff --git a/code/Model/EditableFormField.php b/code/Model/EditableFormField.php index 4cd748d..4f60370 100755 --- a/code/Model/EditableFormField.php +++ b/code/Model/EditableFormField.php @@ -187,6 +187,11 @@ class EditableFormField extends DataObject private static $cascade_duplicates = false; + /** + * This is protected rather that private so that it's unit testable + */ + protected static $isDisplayedRecursionProtection = []; + /** * @var bool */ @@ -1009,6 +1014,20 @@ class EditableFormField extends DataObject return (count($result['selectors'] ?? [])) ? $result : null; } + /** + * Used to prevent infinite recursion when checking a CMS user has setup two or more fields to have + * their display rules dependent on one another + * + * There will be several thousand calls to isDisplayed before memory is likely to be hit, so 100 + * calls is a reasonable limit that ensures that this doesn't prevent legit use cases from being + * identified as recursion + */ + private function checkIsDisplayedRecursionProtection(): bool + { + $count = count(array_filter(static::$isDisplayedRecursionProtection, fn($id) => $id === $this->ID)); + return $count < 100; + } + /** * Check if this EditableFormField is displayed based on its DisplayRules and the provided data. * @param array $data @@ -1016,6 +1035,7 @@ class EditableFormField extends DataObject */ public function isDisplayed(array $data) { + static::$isDisplayedRecursionProtection[] = $this->ID; $displayRules = $this->DisplayRules(); if ($displayRules->count() === 0) { @@ -1033,7 +1053,9 @@ class EditableFormField extends DataObject $controllingField = $rule->ConditionField(); // recursively check - if any of the dependant fields are hidden, assume the rule can not be satisfied - $ruleSatisfied = $controllingField->isDisplayed($data) && $rule->validateAgainstFormData($data); + $ruleSatisfied = $this->checkIsDisplayedRecursionProtection() + && $controllingField->isDisplayed($data) + && $rule->validateAgainstFormData($data); if ($conjunction === '||' && $ruleSatisfied) { $conditionsSatisfied = true; diff --git a/tests/php/Model/EditableFormFieldTest.php b/tests/php/Model/EditableFormFieldTest.php index 3ebe75d..d2be2b7 100644 --- a/tests/php/Model/EditableFormFieldTest.php +++ b/tests/php/Model/EditableFormFieldTest.php @@ -16,6 +16,7 @@ use SilverStripe\UserForms\Model\EditableFormField\EditableRadioField; use SilverStripe\UserForms\Model\EditableFormField\EditableTextField; use SilverStripe\UserForms\Model\UserDefinedForm; use SilverStripe\Dev\Deprecation; +use SilverStripe\UserForms\Model\EditableCustomRule; /** * @package userforms @@ -358,4 +359,54 @@ class EditableFormFieldTest extends FunctionalTest $updatedField = EditableFormField::get()->byId($fieldId); $this->assertFalse((bool)$updatedField->Required); } + + public function testRecursionProtection() + { + $radioOne = EditableRadioField::create(); + $radioOneID = $radioOne->write(); + $optionOneOne = EditableOption::create(); + $optionOneOne->Value = 'yes'; + $optionOneOne->ParentID = $radioOneID; + $optionOneTwo = EditableOption::create(); + $optionOneTwo->Value = 'no'; + $optionOneTwo->ParentID = $radioOneID; + + $radioTwo = EditableRadioField::create(); + $radioTwoID = $radioTwo->write(); + $optionTwoOne = EditableOption::create(); + $optionTwoOne->Value = 'yes'; + $optionTwoOne->ParentID = $radioOneID; + $optionTwoTwo = EditableOption::create(); + $optionTwoTwo->Value = 'no'; + $optionTwoTwo->ParentID = $radioTwoID; + + $conditionOne = EditableCustomRule::create(); + $conditionOne->ParentID = $radioOneID; + $conditionOne->ConditionFieldID = $radioTwoID; + $conditionOne->ConditionOption = 'HasValue'; + $conditionOne->FieldValue = 'yes'; + $conditionOne->write(); + $radioOne->DisplayRules()->add($conditionOne); + + $conditionTwo = EditableCustomRule::create(); + $conditionTwo->ParentID = $radioTwoID; + $conditionTwo->ConditionFieldID = $radioOneID; + $conditionTwo->ConditionOption = 'IsNotBlank'; + $conditionTwo->write(); + $radioTwo->DisplayRules()->add($conditionTwo); + + $testField = new class extends EditableFormField + { + public function countIsDisplayedRecursionProtection(int $fieldID) + { + return count(array_filter(static::$isDisplayedRecursionProtection, function ($id) use ($fieldID) { + return $id === $fieldID; + })); + } + }; + + $this->assertSame(0, $testField->countIsDisplayedRecursionProtection($radioOneID)); + $radioOne->isDisplayed([]); + $this->assertSame(100, $testField->countIsDisplayedRecursionProtection($radioOneID)); + } }