Merge branch '5.15' into 6.0

silverstripe · Jun 1, 2023 · 6f699d0 · 6f699d0
2 parents 8a40ba9 + 3c836eb
commit 6f699d0
Show file tree

Hide file tree

Showing 2 changed files with 74 additions and 1 deletion.
diff --git a/code/Model/EditableFormField.php b/code/Model/EditableFormField.php
@@ -183,6 +183,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
      */
@@ -977,13 +982,28 @@ public function formatDisplayRules()
         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
      * @return bool
      */
     public function isDisplayed(array $data)
     {
+        static::$isDisplayedRecursionProtection[] = $this->ID;
         $displayRules = $this->DisplayRules();
 
         if ($displayRules->count() === 0) {
@@ -1001,7 +1021,9 @@ public function isDisplayed(array $data)
             $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
@@ -14,6 +14,7 @@
 use SilverStripe\UserForms\Model\EditableFormField\EditableOption;
 use SilverStripe\UserForms\Model\EditableFormField\EditableRadioField;
 use SilverStripe\UserForms\Model\EditableFormField\EditableTextField;
+use SilverStripe\UserForms\Model\EditableCustomRule;
 
 /**
  * @package userforms
@@ -324,4 +325,54 @@ public function testChangingDataFieldTypeToDatalessRemovesRequiredSetting()
         $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));
+    }
 }