diff --git a/src/Field/ContentBlock.php b/src/Field/ContentBlock.php index 37d4695f375..173086c23d1 100644 --- a/src/Field/ContentBlock.php +++ b/src/Field/ContentBlock.php @@ -18,9 +18,11 @@ use CraftCms\Cms\Field\Contracts\ElementContainerFieldInterface; use CraftCms\Cms\Field\Elements\ContentBlock as ContentBlockElement; use CraftCms\Cms\Field\Enums\TranslationMethod; +use CraftCms\Cms\Field\Exceptions\FieldNotFoundException; use CraftCms\Cms\Field\Exceptions\InvalidFieldException; use CraftCms\Cms\FieldLayout\Contracts\FieldLayoutProviderInterface; use CraftCms\Cms\FieldLayout\FieldLayout; +use CraftCms\Cms\FieldLayout\LayoutElements\CustomField as CustomFieldElement; use CraftCms\Cms\Gql\GqlHelper as Gql; use CraftCms\Cms\Gql\Resolvers\Elements\ContentBlock as ContentBlockResolver; use CraftCms\Cms\Gql\Types\Generators\ContentBlock as ContentBlockGenerator; @@ -148,9 +150,43 @@ public function afterValidate(?Validator $validator = null): void } } - private function ensureNoRecursion(self $field): bool + private function ensureNoRecursion(self $field, array $ancestorUids = []): bool { - return array_all($field->getFieldLayout()->getCustomFields(), fn ($customField) => ! ($customField instanceof self && ($customField->id === $this->id || ! $this->ensureNoRecursion($customField)))); + if ($field->uid !== null) { + $ancestorUids[] = $field->uid; + } + + /** @var CustomFieldElement $layoutElement */ + foreach ($field->getFieldLayout()->getElementsByType(CustomFieldElement::class) as $layoutElement) { + try { + $customField = $layoutElement->getField(); + } catch (FieldNotFoundException) { + // The referenced field may not be saved yet (e.g. this field's own first save), + // so the raw UUID is all there is to compare against + if (in_array($layoutElement->getFieldUid(), $ancestorUids, true)) { + return false; + } + + continue; + } + + if (! $customField instanceof self) { + continue; + } + + if ( + ($customField->id !== null && $customField->id === $this->id) || + in_array($customField->uid, $ancestorUids, true) + ) { + return false; + } + + if (! $this->ensureNoRecursion($customField, $ancestorUids)) { + return false; + } + } + + return true; } public function getFieldLayoutProviders(): array diff --git a/tests/Feature/Field/FieldsTest.php b/tests/Feature/Field/FieldsTest.php index abd92528482..ab54f581417 100644 --- a/tests/Feature/Field/FieldsTest.php +++ b/tests/Feature/Field/FieldsTest.php @@ -6,6 +6,7 @@ use CraftCms\Cms\Entry\Elements\Entry as EntryElement; use CraftCms\Cms\Entry\Models\EntryType; use CraftCms\Cms\Field\Color; +use CraftCms\Cms\Field\ContentBlock; use CraftCms\Cms\Field\Entries; use CraftCms\Cms\Field\Enums\TranslationMethod; use CraftCms\Cms\Field\Events\CompatibleFieldTypesResolving; @@ -344,6 +345,100 @@ class CustomNestedEntryField extends Field {} ->and($this->fields->getLayoutById($layout->id))->toBeNull(); }); +it('rejects saving a content block field whose nested layout references itself', function () { + // On a first save the field doesn't exist in the database yet, so the + // self-reference can't be caught by resolving the nested field by UID — + // the raw fieldUid has to be compared against the field's own uid. + $fieldUid = (string) Str::uuid(); + + $field = $this->fields->createField([ + 'type' => ContentBlock::class, + 'name' => 'Recursive Block', + 'handle' => 'recursiveBlock', + 'uid' => $fieldUid, + 'settings' => [ + 'viewMode' => 'grouped', + 'fieldLayouts' => [ + (string) Str::uuid() => [ + 'tabs' => [[ + 'uid' => (string) Str::uuid(), + 'elements' => [[ + 'type' => CustomFieldElement::class, + 'uid' => (string) Str::uuid(), + 'width' => 100, + 'required' => false, + 'fieldUid' => $fieldUid, + ]], + ]], + ], + ], + ], + ]); + + expect($this->fields->saveField($field))->toBeFalse() + ->and($field->errors()->first('fieldLayout'))->toBe('Including a Content Block field recursively is not allowed.') + ->and(FieldModel::count())->toBe(0); +}); + +it('rejects re-saving a content block field with its own layout element added', function () { + $field = $this->fields->createField([ + 'type' => ContentBlock::class, + 'name' => 'Recursive Block', + 'handle' => 'recursiveBlock', + ]); + + expect($this->fields->saveField($field))->toBeTrue(); + + $field->setFieldLayouts([ + (string) Str::uuid() => [ + 'tabs' => [[ + 'uid' => (string) Str::uuid(), + 'elements' => [[ + 'type' => CustomFieldElement::class, + 'uid' => (string) Str::uuid(), + 'fieldUid' => $field->uid, + ]], + ]], + ], + ]); + + expect($this->fields->saveField($field))->toBeFalse() + ->and($field->errors()->first('fieldLayout'))->toBe('Including a Content Block field recursively is not allowed.'); +}); + +it('allows nesting a different content block field', function () { + $innerField = $this->fields->createField([ + 'type' => ContentBlock::class, + 'name' => 'Inner Block', + 'handle' => 'innerBlock', + ]); + + expect($this->fields->saveField($innerField))->toBeTrue(); + + $outerField = $this->fields->createField([ + 'type' => ContentBlock::class, + 'name' => 'Outer Block', + 'handle' => 'outerBlock', + 'settings' => [ + 'fieldLayouts' => [ + (string) Str::uuid() => [ + 'tabs' => [[ + 'uid' => (string) Str::uuid(), + 'elements' => [[ + 'type' => CustomFieldElement::class, + 'uid' => (string) Str::uuid(), + 'fieldUid' => $innerField->uid, + ]], + ]], + ], + ], + ], + ]); + + expect($this->fields->saveField($outerField))->toBeTrue() + ->and(FieldModel::count())->toBe(2); +}); + it('can find field usages', function () { expect($this->fields->findFieldUsages(new PlainText))->toBeEmpty();