tonyair/silverstripe-userforms
1
0
Fork 0
mirror of https://github.com/silverstripe/silverstripe-userforms.git synced 2024-10-22 17:05:42 +02:00
Code Issues Projects Releases Wiki Activity

Merge pull request #1214 from creative-commoners/pulls/5.15/recursion-protection

Browse Source 
FIX Prevent infinite recursion when field display rules are co-dependent
This commit is contained in:
Sabina Talipova 2023-06-01 15:31:01 +12:00 committed by GitHub
commit 3c836ebe0b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 74 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
code/Model/EditableFormField.php
View File

@ -187,6 +187,11 @@ class EditableFormField extends DataObject
private static $cascade_duplicates = false; private static $cascade_duplicates = false;
/**
* This is protected rather that private so that it's unit testable
*/
protected static $isDisplayedRecursionProtection = [];
/** /**
* @var bool * @var bool
*/ */
@ -1009,6 +1014,20 @@ class EditableFormField extends DataObject
return (count($result['selectors'] ?? [])) ? $result : null; 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. * Check if this EditableFormField is displayed based on its DisplayRules and the provided data.
* @param array $data * @param array $data
@ -1016,6 +1035,7 @@ class EditableFormField extends DataObject
*/ */
public function isDisplayed(array $data) public function isDisplayed(array $data)
{ {
static::$isDisplayedRecursionProtection[] = $this->ID;
$displayRules = $this->DisplayRules(); $displayRules = $this->DisplayRules();
if ($displayRules->count() === 0) { if ($displayRules->count() === 0) {
@ -1033,7 +1053,9 @@ class EditableFormField extends DataObject
$controllingField = $rule->ConditionField(); $controllingField = $rule->ConditionField();
// recursively check - if any of the dependant fields are hidden, assume the rule can not be satisfied // 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) { if ($conjunction === '||' && $ruleSatisfied) {
$conditionsSatisfied = true; $conditionsSatisfied = true;

51
tests/php/Model/EditableFormFieldTest.php
View File

@ -16,6 +16,7 @@ use SilverStripe\UserForms\Model\EditableFormField\EditableRadioField;
use SilverStripe\UserForms\Model\EditableFormField\EditableTextField; use SilverStripe\UserForms\Model\EditableFormField\EditableTextField;
use SilverStripe\UserForms\Model\UserDefinedForm; use SilverStripe\UserForms\Model\UserDefinedForm;
use SilverStripe\Dev\Deprecation; use SilverStripe\Dev\Deprecation;
use SilverStripe\UserForms\Model\EditableCustomRule;
/** /**
* @package userforms * @package userforms
@ -358,4 +359,54 @@ class EditableFormFieldTest extends FunctionalTest
$updatedField = EditableFormField::get()->byId($fieldId); $updatedField = EditableFormField::get()->byId($fieldId);
$this->assertFalse((bool)$updatedField->Required); $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));
}
} }