FIX Handle exceptions when using /0 as a URL (#2825)

This commit is contained in:
Will Rossiter 2023-11-20 09:49:44 +13:00 committed by GitHub
parent 0df19a874a
commit 579986a691
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 24 additions and 6 deletions

View File

@ -118,8 +118,9 @@ class ModelAsController extends Controller implements NestedController
public function getNestedController() public function getNestedController()
{ {
$request = $this->getRequest(); $request = $this->getRequest();
$urlSegment = $request->param('URLSegment');
if (!$URLSegment = $request->param('URLSegment')) { if ($urlSegment === false || $urlSegment === null || $urlSegment === '') {
throw new Exception('ModelAsController->getNestedController(): was not passed a URLSegment value.'); throw new Exception('ModelAsController->getNestedController(): was not passed a URLSegment value.');
} }
@ -130,13 +131,14 @@ class ModelAsController extends Controller implements NestedController
// url encode unless it's multibyte (already pre-encoded in the database) // url encode unless it's multibyte (already pre-encoded in the database)
$filter = URLSegmentFilter::create(); $filter = URLSegmentFilter::create();
if (!$filter->getAllowMultibyte()) { if (!$filter->getAllowMultibyte()) {
$URLSegment = rawurlencode($URLSegment ?? ''); $urlSegment = rawurlencode($urlSegment ?? '');
} }
// Select child page // Select child page
$tableName = DataObject::singleton(SiteTree::class)->baseTable(); $tableName = DataObject::singleton(SiteTree::class)->baseTable();
$conditions = [sprintf('"%s"."URLSegment"', $tableName) => $URLSegment]; $conditions = [sprintf('"%s"."URLSegment"', $tableName) => $urlSegment];
if (SiteTree::config()->get('nested_urls')) { if (SiteTree::config()->get('nested_urls')) {
$conditions[] = [sprintf('"%s"."ParentID"', $tableName) => 0]; $conditions[] = [sprintf('"%s"."ParentID"', $tableName) => 0];
} }

View File

@ -456,7 +456,7 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi
$parentIDExpr = sprintf('"%s"."ParentID"', $tableName); $parentIDExpr = sprintf('"%s"."ParentID"', $tableName);
$link = trim(Director::makeRelative($link) ?? '', '/'); $link = trim(Director::makeRelative($link) ?? '', '/');
if (!$link) { if ($link === false || $link === null || $link === '') {
$link = RootURLController::get_homepage_link(); $link = RootURLController::get_homepage_link();
} }
@ -1633,6 +1633,11 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi
} }
} }
private function hasURLSegment(): bool
{
return $this->URLSegment !== false && $this->URLSegment !== null && $this->URLSegment !== '';
}
protected function onBeforeWrite() protected function onBeforeWrite()
{ {
parent::onBeforeWrite(); parent::onBeforeWrite();
@ -1653,7 +1658,7 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi
'New {pagetype}', 'New {pagetype}',
['pagetype' => $this->i18n_singular_name()] ['pagetype' => $this->i18n_singular_name()]
)); ));
if ((!$this->URLSegment || $this->URLSegment == $defaultSegment) && $this->Title) { if ((!$this->hasURLSegment() || $this->URLSegment == $defaultSegment) && $this->Title) {
$this->URLSegment = $this->generateURLSegment($this->Title); $this->URLSegment = $this->generateURLSegment($this->Title);
} elseif ($this->isChanged('URLSegment', 2)) { } elseif ($this->isChanged('URLSegment', 2)) {
// Do a strict check on change level, to avoid double encoding caused by // Do a strict check on change level, to avoid double encoding caused by
@ -1661,7 +1666,7 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi
$filter = URLSegmentFilter::create(); $filter = URLSegmentFilter::create();
$this->URLSegment = $filter->filter($this->URLSegment); $this->URLSegment = $filter->filter($this->URLSegment);
// If after sanitising there is no URLSegment, give it a reasonable default // If after sanitising there is no URLSegment, give it a reasonable default
if (!$this->URLSegment) { if (!$this->hasURLSegment()) {
$this->URLSegment = "page-$this->ID"; $this->URLSegment = "page-$this->ID";
} }
} }

View File

@ -133,6 +133,7 @@ class SiteTreeTest extends SapphireTest
'object' => 'object', 'object' => 'object',
'controller' => 'controller', 'controller' => 'controller',
'numericonly' => '1930', 'numericonly' => '1930',
'numeric0' => '0',
]; ];
foreach ($expectedURLs as $fixture => $urlSegment) { foreach ($expectedURLs as $fixture => $urlSegment) {
@ -459,6 +460,7 @@ class SiteTreeTest extends SapphireTest
$about = $this->objFromFixture('Page', 'about'); $about = $this->objFromFixture('Page', 'about');
$staff = $this->objFromFixture('Page', 'staff'); $staff = $this->objFromFixture('Page', 'staff');
$product = $this->objFromFixture('Page', 'product1'); $product = $this->objFromFixture('Page', 'product1');
$numeric0 = $this->objFromFixture('Page', 'numeric0');
SiteTree::config()->nested_urls = false; SiteTree::config()->nested_urls = false;
@ -467,6 +469,7 @@ class SiteTreeTest extends SapphireTest
$this->assertEquals($about->ID, SiteTree::get_by_link($about->Link(), false)->ID); $this->assertEquals($about->ID, SiteTree::get_by_link($about->Link(), false)->ID);
$this->assertEquals($staff->ID, SiteTree::get_by_link($staff->Link(), false)->ID); $this->assertEquals($staff->ID, SiteTree::get_by_link($staff->Link(), false)->ID);
$this->assertEquals($product->ID, SiteTree::get_by_link($product->Link(), false)->ID); $this->assertEquals($product->ID, SiteTree::get_by_link($product->Link(), false)->ID);
$this->assertEquals($numeric0->ID, SiteTree::get_by_link($numeric0->Link(), false)->ID);
Config::modify()->set(SiteTree::class, 'nested_urls', true); Config::modify()->set(SiteTree::class, 'nested_urls', true);
@ -475,6 +478,7 @@ class SiteTreeTest extends SapphireTest
$this->assertEquals($about->ID, SiteTree::get_by_link($about->Link(), false)->ID); $this->assertEquals($about->ID, SiteTree::get_by_link($about->Link(), false)->ID);
$this->assertEquals($staff->ID, SiteTree::get_by_link($staff->Link(), false)->ID); $this->assertEquals($staff->ID, SiteTree::get_by_link($staff->Link(), false)->ID);
$this->assertEquals($product->ID, SiteTree::get_by_link($product->Link(), false)->ID); $this->assertEquals($product->ID, SiteTree::get_by_link($product->Link(), false)->ID);
$this->assertEquals($numeric0->ID, SiteTree::get_by_link($numeric0->Link(), false)->ID);
$this->assertEquals( $this->assertEquals(
$staff->ID, $staff->ID,
@ -489,6 +493,7 @@ class SiteTreeTest extends SapphireTest
$about = $this->objFromFixture('Page', 'about'); $about = $this->objFromFixture('Page', 'about');
$staff = $this->objFromFixture('Page', 'staff'); $staff = $this->objFromFixture('Page', 'staff');
$product = $this->objFromFixture('Page', 'product1'); $product = $this->objFromFixture('Page', 'product1');
$numeric0 = $this->objFromFixture('Page', 'numeric0');
$base = 'https://example.test/'; $base = 'https://example.test/';
$this->assertEquals($home->ID, SiteTree::get_by_link(Controller::join_links($base, '/'), false)->ID); $this->assertEquals($home->ID, SiteTree::get_by_link(Controller::join_links($base, '/'), false)->ID);
@ -496,12 +501,14 @@ class SiteTreeTest extends SapphireTest
$this->assertEquals($about->ID, SiteTree::get_by_link(Controller::join_links($base, $about->Link()), false)->ID); $this->assertEquals($about->ID, SiteTree::get_by_link(Controller::join_links($base, $about->Link()), false)->ID);
$this->assertEquals($staff->ID, SiteTree::get_by_link(Controller::join_links($base, $staff->Link()), false)->ID); $this->assertEquals($staff->ID, SiteTree::get_by_link(Controller::join_links($base, $staff->Link()), false)->ID);
$this->assertEquals($product->ID, SiteTree::get_by_link(Controller::join_links($base, $product->Link()), false)->ID); $this->assertEquals($product->ID, SiteTree::get_by_link(Controller::join_links($base, $product->Link()), false)->ID);
$this->assertEquals($numeric0->ID, SiteTree::get_by_link(Controller::join_links($base, $numeric0->Link()), false)->ID);
} }
public function testRelativeLink() public function testRelativeLink()
{ {
$about = $this->objFromFixture('Page', 'about'); $about = $this->objFromFixture('Page', 'about');
$staff = $this->objFromFixture('Page', 'staff'); $staff = $this->objFromFixture('Page', 'staff');
$numeric0 = $this->objFromFixture('Page', 'numeric0');
Config::modify()->set(SiteTree::class, 'nested_urls', true); Config::modify()->set(SiteTree::class, 'nested_urls', true);
@ -509,6 +516,7 @@ class SiteTreeTest extends SapphireTest
$this->assertEquals('about-us/my-staff/', $staff->RelativeLink(), 'Matches URLSegment plus parent on second level without parameters'); $this->assertEquals('about-us/my-staff/', $staff->RelativeLink(), 'Matches URLSegment plus parent on second level without parameters');
$this->assertEquals('about-us/edit', $about->RelativeLink('edit'), 'Matches URLSegment plus parameter on top level'); $this->assertEquals('about-us/edit', $about->RelativeLink('edit'), 'Matches URLSegment plus parameter on top level');
$this->assertEquals('about-us/tom&jerry', $about->RelativeLink('tom&jerry'), 'Doesnt url encode parameter'); $this->assertEquals('about-us/tom&jerry', $about->RelativeLink('tom&jerry'), 'Doesnt url encode parameter');
$this->assertEquals('0/', $numeric0->RelativeLink(), 'Matches URLSegment for segment = 0');
} }
public function testPageLevel() public function testPageLevel()

View File

@ -112,6 +112,9 @@ Page:
breadcrumbs5: breadcrumbs5:
Title: 'Breadcrumbs 5' Title: 'Breadcrumbs 5'
Parent: =>Page.breadcrumbs4 Parent: =>Page.breadcrumbs4
numeric0:
Title: 'urlsegment is 0'
URLSegment: '0'
SilverStripe\CMS\Tests\Model\SiteTreeTest_Conflicted: SilverStripe\CMS\Tests\Model\SiteTreeTest_Conflicted:
parent: parent: