Skip to content

Commit

Permalink
[5.x] Prevents Recursive Fieldsets (#9539)
Browse files Browse the repository at this point in the history
  • Loading branch information
JohnathonKoster authored Apr 15, 2024
1 parent 17ec219 commit 54a0496
Show file tree
Hide file tree
Showing 11 changed files with 171 additions and 4 deletions.
1 change: 1 addition & 0 deletions resources/lang/en/validation.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.',
Expand Down
27 changes: 27 additions & 0 deletions src/Exceptions/FieldsetRecursionException.php
Original file line number Diff line number Diff line change
@@ -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.");
}
}
8 changes: 7 additions & 1 deletion src/Fields/Fields.php
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
}
Expand Down Expand Up @@ -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()
Expand Down
9 changes: 9 additions & 0 deletions src/Fields/Fieldset.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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', []);
Expand Down
28 changes: 28 additions & 0 deletions src/Fields/FieldsetRecursionStack.php
Original file line number Diff line number Diff line change
@@ -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();
}
}
8 changes: 7 additions & 1 deletion src/Http/Controllers/CP/Fields/FieldsetController.php
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ public function edit($fieldset)
{
$fieldset = Facades\Fieldset::find($fieldset);

$fieldset->validateRecursion();

$vue = [
'title' => $fieldset->title(),
'handle' => $fieldset->handle(),
Expand Down Expand Up @@ -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);
}
Expand Down
14 changes: 14 additions & 0 deletions src/Http/Controllers/CP/Fields/ManagesBlueprints.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand All @@ -75,6 +87,8 @@ private function updateBlueprint($request, $blueprint)
{
$this->setBlueprintContents($request, $blueprint);

$this->validateRecursion($blueprint);

$this->validateUniqueHandles($blueprint);

$this->validateReservedFieldHandles($blueprint);
Expand Down
3 changes: 3 additions & 0 deletions src/Providers/AppServiceProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(),
Expand Down
6 changes: 4 additions & 2 deletions tests/Fakes/FakeFieldsetRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [];

Expand All @@ -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);
}
Expand Down
3 changes: 3 additions & 0 deletions tests/Feature/Fieldsets/UpdateFieldsetTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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',
Expand Down
68 changes: 68 additions & 0 deletions tests/Fields/FieldsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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',
],
]);
}
}

0 comments on commit 54a0496

Please sign in to comment.