From 81b642794215a15075631260da2933a4a65abd54 Mon Sep 17 00:00:00 2001 From: Brian Hanson Date: Tue, 7 Jul 2026 20:31:13 -0500 Subject: [PATCH] Reject self-referencing Content Block fields at save time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ContentBlock::ensureNoRecursion() resolved nested fields from the DB via the field layout's custom fields, so on a field's first save a nested layout element referencing the field's own UID couldn't be resolved, the FieldNotFoundException was swallowed, and the self-reference passed validation — persisting a field whose nested layout references itself. Iterate the layout's CustomField elements directly instead, comparing each element's raw fieldUid against the field's own uid when the nested field can't be resolved yet, and track ancestor UIDs through the recursion so indirect cycles are caught at any depth and already- corrupted data can't cause infinite recursion. Co-Authored-By: Claude Fable 5 --- src/Field/ContentBlock.php | 40 ++++++++++++- tests/Feature/Field/FieldsTest.php | 95 ++++++++++++++++++++++++++++++ 2 files changed, 133 insertions(+), 2 deletions(-) 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();