FIX Handle exceptions when using /0 as a URL

silverstripe · Nov 17, 2023 · fbc5470 · fbc5470
1 parent 0df19a8
commit fbc5470
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 6 deletions.
diff --git 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 === 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
@@ -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 === null || $link === '') {
             $link = RootURLController::get_homepage_link();
         }
 
@@ -1633,6 +1633,11 @@ public function requireDefaultRecords()
         }
     }
 
+    private function hasURLSegment(): bool
+    {
+        return $this->URLSegment !== null && $this->URLSegment !== '';
+    }
+
     protected function onBeforeWrite()
     {
         parent::onBeforeWrite();
@@ -1653,15 +1658,15 @@ 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
             // bogus changes through forceChange()
             $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
@@ -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,26 +493,30 @@ 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);
         $this->assertEquals($home->ID, SiteTree::get_by_link(Controller::join_links($base, '/home/'), 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($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);
 
         $this->assertEquals('about-us/', $about->RelativeLink(), 'Matches URLSegment on top 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/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
@@ -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: