mirror of
https://github.com/silverstripe/silverstripe-userforms.git
synced 2024-10-22 17:05:42 +02:00
FIX Prevent infinite recursion when field display rules are co-dependent
This commit is contained in:
parent
f2a3b074ee
commit
bf49cab678
@ -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;
|
||||
|
@ -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));
|
||||
}
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user