Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.x] Prevents Recursive Fieldsets #9539

Merged
merged 32 commits into from
Apr 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
597c28a
[4.x] Include honeypot in Alpine.js form data (#9498)
duncanmcclean Feb 9, 2024
156debf
[4.x] Fix directory separator in Templates fieldtype on Windows (#9483)
duncanmcclean Feb 9, 2024
e749fc8
[4.x] Fix `$authenticatedUser` error with third-party addon events (#…
duncanmcclean Feb 9, 2024
377ff97
[4.x] Fix error from Code Fieldtype when switching sites in global (#…
duncanmcclean Feb 9, 2024
57079db
[4.x] Add GraphQL type for Group fieldtype (#9499)
duncanmcclean Feb 9, 2024
be31113
[4.x] Add clear value button to popover date fieldtype (#9478)
jacksleight Feb 9, 2024
247e7b7
[4.x] Translatable displays (#9450)
peimn Feb 9, 2024
3e771e9
[4.x] Clean up some event handlers (#9500)
jasonvarga Feb 9, 2024
7ef3e80
[4.x] Fix pagination with the `nocache` tag (#9394)
duncanmcclean Feb 9, 2024
a5e8fae
[4.x] Antlers: Allow number literals inside variable bindings (#9503)
JohnathonKoster Feb 12, 2024
cdf73c4
[4.x] Prevent warming redirect URLs (#9509)
duncanmcclean Feb 12, 2024
61e1567
[4.x] Fix scrolling in Inline Publish Form on Safari on iOS (#9510)
duncanmcclean Feb 12, 2024
f8bcd13
[4.x] Fix numbers not being cast in API filters (#9511)
jasonvarga Feb 12, 2024
2503f2b
[4.x] Prevent non-images being processed through source preset (#9517)
duncanmcclean Feb 13, 2024
a3cf965
[4.x] Fix table drag handles disappearing (#9522)
jasonvarga Feb 13, 2024
8f37cf7
[4.x] Support YouTube Shorts in embed_code modifier (#9521)
mnlmaier Feb 13, 2024
c807f9a
[4.x] Add PHP `fieldPathPrefix` method (#9080)
jacksleight Feb 13, 2024
dc49183
[4.x] Only search for descendants if multi-site mode is enabled (#9528)
helloiamlukas Feb 14, 2024
205c324
[4.x] PHPUnit 10 (#9529)
jasonvarga Feb 15, 2024
299f0d5
[4.x] Exclude super when using a custom field (#9536)
jasonvarga Feb 16, 2024
471825a
changelog
jasonvarga Feb 16, 2024
da8ef02
[4.x] Fix issues when saving entries with `JsonResource::withoutWrapp…
duncanmcclean Feb 16, 2024
49b1e58
[4.x] Antlers: stops double-initial execution of tags within conditio…
JohnathonKoster Feb 16, 2024
842b7ca
Prevent infinite loops
JohnathonKoster Feb 18, 2024
57fb234
Log warning; add test
JohnathonKoster Feb 18, 2024
21e7d09
Merge branch 'master' into pr/9539
duncanmcclean Feb 19, 2024
818f1ec
Merge branch 'master' into pr/9539
duncanmcclean Feb 19, 2024
30e2a6c
Merge branch 'master' into 1725--recursive-fieldsets-infinite-loop
jasonvarga Apr 15, 2024
e4c2f84
wip
jasonvarga Apr 15, 2024
e9e8612
wip
jasonvarga Apr 15, 2024
15fe3e6
delete file added through bad merge i guess
jasonvarga Apr 15, 2024
5cfd6d5
tweak tests so they pass now that fieldset->fields() gets called in c…
jasonvarga Apr 15, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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',
],
]);
}
}
Loading