Skip to content

Commit 4a71ce8

Browse files
committed
Guard against circular parent chains on AbstractPage
Adds an Assert\Callback validator that walks the parentPage/parentPageData chain upward using a visited-ID set, blocking self-references and cycles that mix Page and AbstractPageData parents (e.g. A→B→A across types). Three Behat scenarios cover: self-reference, same-type cycle, mixed-type cycle.
1 parent 7e34755 commit 4a71ce8

3 files changed

Lines changed: 102 additions & 0 deletions

File tree

features/bootstrap/DoctrineContext.php

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,47 @@ public function theComponentGroupHasTheAllowedComponents(string $allowedComponen
436436
$this->manager->clear();
437437
}
438438

439+
/**
440+
* @Given there is a page with parent page :parentRef
441+
*/
442+
public function thereIsAPageWithParentPage(string $parentRef): void
443+
{
444+
/** @var Page $parent */
445+
$parent = $this->iriConverter->getResourceFromIri($this->restContext->resources[$parentRef]);
446+
447+
$page = new Page();
448+
$page->isTemplate = true;
449+
$page->reference = 'child-page';
450+
$page->setParentPage($parent);
451+
$this->timestampedHelper->persistTimestampedFields($page, true);
452+
$this->manager->persist($page);
453+
$this->manager->flush();
454+
$this->restContext->resources['child_page'] = $this->iriConverter->getIriFromResource($page);
455+
}
456+
457+
/**
458+
* @Given there is a page data with parent page :parentRef
459+
*/
460+
public function thereIsAPageDataWithParentPage(string $parentRef): void
461+
{
462+
/** @var Page $parent */
463+
$parent = $this->iriConverter->getResourceFromIri($this->restContext->resources[$parentRef]);
464+
465+
$templatePage = new Page();
466+
$templatePage->isTemplate = true;
467+
$templatePage->reference = 'page-data-template';
468+
$this->timestampedHelper->persistTimestampedFields($templatePage, true);
469+
$this->manager->persist($templatePage);
470+
471+
$pageData = new PageData();
472+
$pageData->page = $templatePage;
473+
$pageData->setParentPage($parent);
474+
$this->timestampedHelper->persistTimestampedFields($pageData, true);
475+
$this->manager->persist($pageData);
476+
$this->manager->flush();
477+
$this->restContext->resources['page_data'] = $this->iriConverter->getIriFromResource($pageData);
478+
}
479+
439480
/**
440481
* @Given there is a Page
441482
*/

features/main/page.feature

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,35 @@ Feature: Page resources
4848
Then the response status code should be 200
4949
And the JSON node "parentPage" should be equal to the IRI of the resource "page"
5050

51+
@loginAdmin
52+
Scenario: I cannot set a page as its own parent
53+
Given there is a Page
54+
When I send a "PATCH" request to the resource "page" with data:
55+
| parentPage |
56+
| resource[page] |
57+
Then the response status code should be 422
58+
And the JSON should be valid according to the schema file "validation_errors_object.schema.json"
59+
60+
@loginAdmin
61+
Scenario: I cannot create a circular parent chain between two pages
62+
Given there is a Page
63+
And there is a page with parent page "page"
64+
When I send a "PATCH" request to the resource "page" with data:
65+
| parentPage |
66+
| resource[child_page] |
67+
Then the response status code should be 422
68+
And the JSON should be valid according to the schema file "validation_errors_object.schema.json"
69+
70+
@loginAdmin
71+
Scenario: I cannot create a circular parent chain across mixed Page and PageData parents
72+
Given there is a Page
73+
And there is a page data with parent page "page"
74+
When I send a "PATCH" request to the resource "page" with data:
75+
| parentPageData |
76+
| resource[page_data] |
77+
Then the response status code should be 422
78+
And the JSON should be valid according to the schema file "validation_errors_object.schema.json"
79+
5180
@loginUser
5281
Scenario: I cannot set both a parent Page and a parent PageData on a page
5382
Given there is a Layout

src/Entity/Core/AbstractPage.php

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use Symfony\Bridge\Doctrine\Validator\Constraints\UniqueEntity;
1919
use Symfony\Component\Serializer\Annotation\Groups;
2020
use Symfony\Component\Validator\Constraints as Assert;
21+
use Symfony\Component\Validator\Context\ExecutionContextInterface;
2122

2223
/**
2324
* @author Daniel West <daniel@silverback.is>
@@ -93,6 +94,37 @@ public function setParentPageData(?AbstractPageData $parentPageData): self
9394
return $this;
9495
}
9596

97+
#[Assert\Callback]
98+
public function validateNoCircularParent(ExecutionContextInterface $context): void
99+
{
100+
$parent = $this->parentPage ?? $this->parentPageData;
101+
if ($parent === null) {
102+
return;
103+
}
104+
105+
$visitedIds = [];
106+
if ($this->id !== null) {
107+
$visitedIds[] = $this->id->toString();
108+
}
109+
110+
while ($parent !== null) {
111+
$parentId = $parent->getId();
112+
if ($parentId !== null) {
113+
$parentIdStr = $parentId->toString();
114+
if (\in_array($parentIdStr, $visitedIds, true)) {
115+
$field = $this->parentPage !== null ? 'parentPage' : 'parentPageData';
116+
$context->buildViolation('Setting this parent would create a circular reference.')
117+
->atPath($field)
118+
->addViolation();
119+
120+
return;
121+
}
122+
$visitedIds[] = $parentIdStr;
123+
}
124+
$parent = $parent->getParentPage() ?? $parent->getParentPageData();
125+
}
126+
}
127+
96128
public function getParentPageRoute(): ?Route
97129
{
98130
return $this->parentPage?->getRoute() ?? $this->parentPageData?->getRoute();

0 commit comments

Comments
 (0)