From 579986a69160ebfff63ead6d8c108ef8658933e5 Mon Sep 17 00:00:00 2001 From: Will Rossiter Date: Mon, 20 Nov 2023 09:49:44 +1300 Subject: [PATCH] FIX Handle exceptions when using /0 as a URL (#2825) --- code/Controllers/ModelAsController.php | 8 +++++--- code/Model/SiteTree.php | 11 ++++++++--- tests/php/Model/SiteTreeTest.php | 8 ++++++++ tests/php/Model/SiteTreeTest.yml | 3 +++ 4 files changed, 24 insertions(+), 6 deletions(-) diff --git a/code/Controllers/ModelAsController.php b/code/Controllers/ModelAsController.php index 8f9233ca65..5cd3f94081 100644 --- a/code/Controllers/ModelAsController.php +++ b/code/Controllers/ModelAsController.php @@ -118,8 +118,9 @@ public function handleRequest(HTTPRequest $request) public function getNestedController() { $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.'); } @@ -130,13 +131,14 @@ public function getNestedController() // url encode unless it's multibyte (already pre-encoded in the database) $filter = URLSegmentFilter::create(); + if (!$filter->getAllowMultibyte()) { - $URLSegment = rawurlencode($URLSegment ?? ''); + $urlSegment = rawurlencode($urlSegment ?? ''); } // Select child page $tableName = DataObject::singleton(SiteTree::class)->baseTable(); - $conditions = [sprintf('"%s"."URLSegment"', $tableName) => $URLSegment]; + $conditions = [sprintf('"%s"."URLSegment"', $tableName) => $urlSegment]; if (SiteTree::config()->get('nested_urls')) { $conditions[] = [sprintf('"%s"."ParentID"', $tableName) => 0]; } diff --git a/code/Model/SiteTree.php b/code/Model/SiteTree.php index 882d3c5ea5..60c799682d 100755 --- a/code/Model/SiteTree.php +++ b/code/Model/SiteTree.php @@ -456,7 +456,7 @@ public static function get_by_link($link, $cache = true) $parentIDExpr = sprintf('"%s"."ParentID"', $tableName); $link = trim(Director::makeRelative($link) ?? '', '/'); - if (!$link) { + if ($link === false || $link === null || $link === '') { $link = RootURLController::get_homepage_link(); } @@ -1633,6 +1633,11 @@ public function requireDefaultRecords() } } + private function hasURLSegment(): bool + { + return $this->URLSegment !== false && $this->URLSegment !== null && $this->URLSegment !== ''; + } + protected function onBeforeWrite() { parent::onBeforeWrite(); @@ -1653,7 +1658,7 @@ protected function onBeforeWrite() 'New {pagetype}', ['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); } elseif ($this->isChanged('URLSegment', 2)) { // Do a strict check on change level, to avoid double encoding caused by @@ -1661,7 +1666,7 @@ protected function onBeforeWrite() $filter = URLSegmentFilter::create(); $this->URLSegment = $filter->filter($this->URLSegment); // If after sanitising there is no URLSegment, give it a reasonable default - if (!$this->URLSegment) { + if (!$this->hasURLSegment()) { $this->URLSegment = "page-$this->ID"; } } diff --git a/tests/php/Model/SiteTreeTest.php b/tests/php/Model/SiteTreeTest.php index 5e45765e50..8f6952fa4c 100644 --- a/tests/php/Model/SiteTreeTest.php +++ b/tests/php/Model/SiteTreeTest.php @@ -133,6 +133,7 @@ public function testURLGeneration() 'object' => 'object', 'controller' => 'controller', 'numericonly' => '1930', + 'numeric0' => '0', ]; foreach ($expectedURLs as $fixture => $urlSegment) { @@ -459,6 +460,7 @@ public function testGetByLink() $about = $this->objFromFixture('Page', 'about'); $staff = $this->objFromFixture('Page', 'staff'); $product = $this->objFromFixture('Page', 'product1'); + $numeric0 = $this->objFromFixture('Page', 'numeric0'); SiteTree::config()->nested_urls = false; @@ -467,6 +469,7 @@ public function testGetByLink() $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($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); @@ -475,6 +478,7 @@ public function testGetByLink() $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($product->ID, SiteTree::get_by_link($product->Link(), false)->ID); + $this->assertEquals($numeric0->ID, SiteTree::get_by_link($numeric0->Link(), false)->ID); $this->assertEquals( $staff->ID, @@ -489,6 +493,7 @@ public function testGetByLinkAbsolute() $about = $this->objFromFixture('Page', 'about'); $staff = $this->objFromFixture('Page', 'staff'); $product = $this->objFromFixture('Page', 'product1'); + $numeric0 = $this->objFromFixture('Page', 'numeric0'); $base = 'https://example.test/'; $this->assertEquals($home->ID, SiteTree::get_by_link(Controller::join_links($base, '/'), false)->ID); @@ -496,12 +501,14 @@ public function testGetByLinkAbsolute() $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($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() { $about = $this->objFromFixture('Page', 'about'); $staff = $this->objFromFixture('Page', 'staff'); + $numeric0 = $this->objFromFixture('Page', 'numeric0'); Config::modify()->set(SiteTree::class, 'nested_urls', true); @@ -509,6 +516,7 @@ public function testRelativeLink() $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/tom&jerry', $about->RelativeLink('tom&jerry'), 'Doesnt url encode parameter'); + $this->assertEquals('0/', $numeric0->RelativeLink(), 'Matches URLSegment for segment = 0'); } public function testPageLevel() diff --git a/tests/php/Model/SiteTreeTest.yml b/tests/php/Model/SiteTreeTest.yml index eda4182d1f..22e1aff26c 100755 --- a/tests/php/Model/SiteTreeTest.yml +++ b/tests/php/Model/SiteTreeTest.yml @@ -112,6 +112,9 @@ Page: breadcrumbs5: Title: 'Breadcrumbs 5' Parent: =>Page.breadcrumbs4 + numeric0: + Title: 'urlsegment is 0' + URLSegment: '0' SilverStripe\CMS\Tests\Model\SiteTreeTest_Conflicted: parent: