[5.x] Prevents Recursive Fieldsets (#9539)

resources/lang/en/validation.php

@@ -169,6 +169,7 @@
  'duplicate_field_handle' => 'A field with a handle of :handle already exists.',
  'duplicate_uri' => 'Duplicate URI :value',
  'email_available' => 'A user with this email already exists.',
+ 'fieldset_imported_recursively' => 'Fieldset :handle is being imported recursively.',
  'one_site_without_origin' => 'At least one site must not have an origin.',
  'options_require_keys' => 'All options must have keys.',
  'origin_cannot_be_disabled' => 'Cannot select a disabled origin.',

src/Exceptions/FieldsetRecursionException.php

@@ -0,0 +1,27 @@
+<?php
+
+namespace Statamic\Exceptions;
+
+use Exception;
+use Spatie\Ignition\Contracts\BaseSolution;
+use Spatie\Ignition\Contracts\ProvidesSolution;
+use Spatie\Ignition\Contracts\Solution;
+
+class FieldsetRecursionException extends Exception implements ProvidesSolution
+{
+ public function __construct(private string $fieldset, private string $target)
+ {
+ parent::__construct("Fieldset [$fieldset] is being imported recursively.");
+ }
+
+ public function getFieldset()
+ {
+ return $this->fieldset;
+ }
+
+ public function getSolution(): Solution
+ {
+ return BaseSolution::create('Avoid infinite recursion')
+ ->setSolutionDescription("The fieldset `$this->fieldset` is being imported into `$this->target`, however it has already been imported elsewhere. This is causing infinite recursion.");
+ }
+}

src/Fields/Fields.php

@@ -268,9 +268,11 @@ private function getReferencedField(array $config): Field
 
  private function getImportedFields(array $config): array
  {
+ $recursion = tap(app(FieldsetRecursionStack::class))->push($config['import']);
+
  $blink = 'blueprint-imported-fields-'.md5(json_encode($config));
 
- return Blink::once($blink, function () use ($config) {
+ $imported = Blink::once($blink, function () use ($config) {
  if (! $fieldset = FieldsetRepository::find($config['import'])) {
  throw new FieldsetNotFoundException($config['import']);
  }
@@ -299,6 +301,10 @@ private function getImportedFields(array $config): array
  ->setParent($this->parent)
  ->setParentField($this->parentField, $this->parentIndex);
  })->all();
+
+ $recursion->pop();
+
+ return $imported;
  }
 
  public function meta()

src/Fields/Fieldset.php

@@ -8,6 +8,7 @@
 use Statamic\Events\FieldsetDeleting;
 use Statamic\Events\FieldsetSaved;
 use Statamic\Events\FieldsetSaving;
+use Statamic\Exceptions\FieldsetRecursionException;
 use Statamic\Facades;
 use Statamic\Facades\AssetContainer;
 use Statamic\Facades\Collection;
@@ -85,6 +86,14 @@ public function title()
  return $this->contents['title'] ?? Str::humanize(Str::of($this->handle)->after('::')->afterLast('.'));
  }
 
+ /**
+ * @throws FieldsetRecursionException
+ */
+ public function validateRecursion()
+ {
+ $this->fields();
+ }
+
  public function fields(): Fields
  {
  $fields = Arr::get($this->contents, 'fields', []);

src/Fields/FieldsetRecursionStack.php

@@ -0,0 +1,28 @@
+<?php
+
+namespace Statamic\Fields;
+
+use Illuminate\Support\Collection;
+use Statamic\Exceptions\FieldsetRecursionException;
+
+class FieldsetRecursionStack
+{
+ public function __construct(private Collection $stack)
+ {
+ $this->stack = collect();
+ }
+
+ public function push(string $import)
+ {
+ if ($this->stack->contains($import)) {
+ throw new FieldsetRecursionException($import, $this->stack->last());
+ }
+
+ $this->stack->push($import);
+ }
+
+ public function pop()
+ {
+ $this->stack->pop();
+ }
+}

src/Http/Controllers/CP/Fields/FieldsetController.php

@@ -74,6 +74,8 @@ public function edit($fieldset)
  {
  $fieldset = Facades\Fieldset::find($fieldset);
 
+ $fieldset->validateRecursion();
+
  $vue = [
  'title' => $fieldset->title(),
  'handle' => $fieldset->handle(),
@@ -102,7 +104,11 @@ public function update(Request $request, $fieldset)
  'fields' => collect($request->fields)->map(function ($field) {
  return FieldTransformer::fromVue($field);
  })->all(),
- ]))->save();
+ ]));
+
+ $fieldset->validateRecursion();
+
+ $fieldset->save();
 
  return response('', 204);
  }

src/Http/Controllers/CP/Fields/ManagesBlueprints.php

@@ -6,6 +6,7 @@
 use Illuminate\Support\Collection;
 use Illuminate\Validation\ValidationException;
 use Statamic\Exceptions\DuplicateFieldException;
+use Statamic\Exceptions\FieldsetRecursionException;
 use Statamic\Facades;
 use Statamic\Fields\Blueprint;
 use Statamic\Fields\FieldTransformer;
@@ -49,6 +50,17 @@ private function setBlueprintContents(Request $request, Blueprint $blueprint)
  return $blueprint;
  }
 
+ private function validateRecursion($blueprint)
+ {
+ try {
+ $blueprint->fields();
+ } catch (FieldsetRecursionException $exception) {
+ throw ValidationException::withMessages([
+ 'tabs' => __('statamic::validation.fieldset_imported_recursively', ['handle' => $exception->getFieldset()]),
+ ]);
+ }
+ }
+
  private function validateUniqueHandles($blueprint)
  {
  try {
@@ -75,6 +87,8 @@ private function updateBlueprint($request, $blueprint)
  {
  $this->setBlueprintContents($request, $blueprint);
 
+ $this->validateRecursion($blueprint);
+
  $this->validateUniqueHandles($blueprint);
 
  $this->validateReservedFieldHandles($blueprint);

src/Providers/AppServiceProvider.php

@@ -13,6 +13,7 @@
 use Statamic\Facades\Preference;
 use Statamic\Facades\Site;
 use Statamic\Facades\Token;
+use Statamic\Fields\FieldsetRecursionStack;
 use Statamic\Sites\Sites;
 use Statamic\Statamic;
 use Statamic\Tokens\Handlers\LivePreview;
@@ -152,6 +153,8 @@ public function register()
  ->setDirectory(resource_path('fieldsets'));
  });
 
+ $this->app->singleton(FieldsetRecursionStack::class);
+
  collect([
  'entries' => fn () => Facades\Entry::query(),
  'form-submissions' => fn () => Facades\FormSubmission::query(),

tests/Fakes/FakeFieldsetRepository.php

@@ -2,10 +2,12 @@
 
 namespace Tests\Fakes;
 
+use Illuminate\Support\Collection;
 use Statamic\Fields\Fieldset;
+use Statamic\Fields\FieldsetRepository;
 use Statamic\Support\Arr;
 
-class FakeFieldsetRepository
+class FakeFieldsetRepository extends FieldsetRepository
 {
  protected $fieldsets = [];
 
@@ -24,7 +26,7 @@ public function save(Fieldset $fieldset)
  $this->fieldsets[$fieldset->handle()] = $fieldset;
  }
 
- public function all()
+ public function all(): Collection
  {
  return collect($this->fieldsets);
  }

tests/Feature/Fieldsets/UpdateFieldsetTest.php

@@ -2,8 +2,10 @@
 
 namespace Tests\Feature\Fieldsets;
 
+use Facades\Statamic\Fields\FieldRepository;
 use Statamic\Facades;
 use Statamic\Facades\Fieldset as FieldsetRepository;
+use Statamic\Fields\Field;
 use Statamic\Fields\Fieldset;
 use Tests\Fakes\FakeFieldsetRepository;
 use Tests\FakesRoles;
@@ -44,6 +46,7 @@ public function it_denies_access_if_you_dont_have_permission()
  public function fieldset_gets_saved()
  {
  $this->withoutExceptionHandling();
+ FieldRepository::shouldReceive('find')->with('somefieldset.somefield')->andReturn(new Field('somefield', []));
  $user = tap(Facades\User::make()->makeSuper())->save();
  $fieldset = (new Fieldset)->setHandle('test')->setContents([
  'title' => 'Test',

tests/Fields/FieldsTest.php

@@ -6,6 +6,7 @@
 use Facades\Statamic\Fields\FieldtypeRepository;
 use Facades\Statamic\Fields\Validator;
 use Illuminate\Support\Collection;
+use Statamic\Exceptions\FieldsetRecursionException;
 use Statamic\Facades\Fieldset as FieldsetRepository;
 use Statamic\Fields\Field;
 use Statamic\Fields\Fields;
@@ -1057,4 +1058,71 @@ public function it_sets_the_parentfield_and_parentindex_on_referenced_fields()
  $this->assertEquals($parentField, $collection['bar']->parentField());
  $this->assertEquals(1, $collection['bar']->parentIndex());
  }
+
+ /** @test */
+ public function it_does_not_allow_recursive_imports()
+ {
+ $this->expectException(FieldsetRecursionException::class);
+
+ $one = (new Fieldset)->setHandle('one')->setContents([
+ 'fields' => [
+ [
+ 'import' => 'two',
+ ],
+ ],
+ ]);
+
+ $two = (new Fieldset)->setHandle('two')->setContents([
+ 'fields' => [
+ [
+ 'import' => 'one',
+ ],
+ ],
+ ]);
+
+ FieldsetRepository::shouldReceive('find')->with('one')->zeroOrMoreTimes()->andReturn($one);
+ FieldsetRepository::shouldReceive('find')->with('two')->zeroOrMoreTimes()->andReturn($two);
+
+ new Fields([
+ [
+ 'import' => 'one',
+ ],
+ ]);
+ }
+
+ /** @test */
+ public function import_recursion_check_should_reset_across_instances()
+ {
+ $one = (new Fieldset)->setHandle('one')->setContents([
+ 'fields' => [
+ [
+ 'import' => 'two',
+ ],
+ ],
+ ]);
+
+ $two = (new Fieldset)->setHandle('two')->setContents([
+ 'fields' => [
+ [
+ 'handle' => 'foo',
+ 'field' => ['type' => 'text'],
+ ],
+ ],
+ ]);
+
+ FieldsetRepository::shouldReceive('find')->with('one')->zeroOrMoreTimes()->andReturn($one);
+ FieldsetRepository::shouldReceive('find')->with('two')->zeroOrMoreTimes()->andReturn($two);
+
+ new Fields([
+ [
+ 'import' => 'one',
+ ],
+ ]);
+
+ new Fields([
+ [
+ 'import' => 'two',
+ ],
+ ]);
+ }
 }