diff --git a/resources/lang/en/validation.php b/resources/lang/en/validation.php index 42c4a87b24..0807579d1d 100644 --- a/resources/lang/en/validation.php +++ b/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.', diff --git a/src/Exceptions/FieldsetRecursionException.php b/src/Exceptions/FieldsetRecursionException.php new file mode 100644 index 0000000000..59b6838dcb --- /dev/null +++ b/src/Exceptions/FieldsetRecursionException.php @@ -0,0 +1,27 @@ +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."); + } +} diff --git a/src/Fields/Fields.php b/src/Fields/Fields.php index 32a97d22ea..d3719552a8 100644 --- a/src/Fields/Fields.php +++ b/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() diff --git a/src/Fields/Fieldset.php b/src/Fields/Fieldset.php index 4d685b2428..80cc21a2e9 100644 --- a/src/Fields/Fieldset.php +++ b/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', []); diff --git a/src/Fields/FieldsetRecursionStack.php b/src/Fields/FieldsetRecursionStack.php new file mode 100644 index 0000000000..682bc4290a --- /dev/null +++ b/src/Fields/FieldsetRecursionStack.php @@ -0,0 +1,28 @@ +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(); + } +} diff --git a/src/Http/Controllers/CP/Fields/FieldsetController.php b/src/Http/Controllers/CP/Fields/FieldsetController.php index b728d4097d..b398077959 100644 --- a/src/Http/Controllers/CP/Fields/FieldsetController.php +++ b/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); } diff --git a/src/Http/Controllers/CP/Fields/ManagesBlueprints.php b/src/Http/Controllers/CP/Fields/ManagesBlueprints.php index aa039412a7..d246a32796 100644 --- a/src/Http/Controllers/CP/Fields/ManagesBlueprints.php +++ b/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); diff --git a/src/Providers/AppServiceProvider.php b/src/Providers/AppServiceProvider.php index dc8eacbeb0..2bac85a9d3 100644 --- a/src/Providers/AppServiceProvider.php +++ b/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(), diff --git a/tests/Fakes/FakeFieldsetRepository.php b/tests/Fakes/FakeFieldsetRepository.php index 4954671322..bc33dca944 100644 --- a/tests/Fakes/FakeFieldsetRepository.php +++ b/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); } diff --git a/tests/Feature/Fieldsets/UpdateFieldsetTest.php b/tests/Feature/Fieldsets/UpdateFieldsetTest.php index 6aef24a5b2..ce6df944cb 100644 --- a/tests/Feature/Fieldsets/UpdateFieldsetTest.php +++ b/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', diff --git a/tests/Fields/FieldsTest.php b/tests/Fields/FieldsTest.php index ff2951077d..554382afec 100644 --- a/tests/Fields/FieldsTest.php +++ b/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', + ], + ]); + } }