diff --git a/code/Model/SiteTree.php b/code/Model/SiteTree.php index 49d2c3ad..66a907a1 100755 --- a/code/Model/SiteTree.php +++ b/code/Model/SiteTree.php @@ -1559,6 +1559,22 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi */ public function validURLSegment() { + $excludes = Director::config()->get('rules'); + $excludes = array_keys($excludes); + $disallowedSegments = array_map(function ($key) { + $route = explode('/', $key); + if (!empty($route) && strpos($route[0], '$') === false) { + return $route[0]; + } + return; + }, $excludes); + + if (!$this->ParentID && in_array($this->URLSegment, $disallowedSegments)) { + // Default to '-2', onBeforeWrite takes care of further possible clashes + return false; + } + + if (self::config()->nested_urls && $parent = $this->Parent()) { if ($controller = ModelAsController::controller_for($parent)) { if ($controller instanceof Controller && $controller->hasAction($this->URLSegment)) { @@ -1611,17 +1627,17 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi public function generateURLSegment($title) { $filter = URLSegmentFilter::create(); - $t = $filter->filter($title); + $filteredTitle = $filter->filter($title); // Fallback to generic page name if path is empty (= no valid, convertable characters) - if (!$t || $t == '-' || $t == '-1') { - $t = "page-$this->ID"; + if (!$filteredTitle || $filteredTitle == '-' || $filteredTitle == '-1') { + $filteredTitle = "page-$this->ID"; } // Hook for extensions - $this->extend('updateURLSegment', $t, $title); + $this->extend('updateURLSegment', $filteredTitle, $title); - return $t; + return $filteredTitle; } /** diff --git a/tests/php/Model/SiteTreeTest.php b/tests/php/Model/SiteTreeTest.php index 1ad29818..97382e30 100644 --- a/tests/php/Model/SiteTreeTest.php +++ b/tests/php/Model/SiteTreeTest.php @@ -46,6 +46,15 @@ class SiteTreeTest extends SapphireTest SiteTreeTest_DataObject::class, ); + public function reservedSegmentsProvider() + { + return [ + ['Admin', 'admin-2'], + ['Dev', 'dev-2'], + ['Robots in disguise', 'robots-in-disguise'] + ]; + } + public function testCreateDefaultpages() { $remove = SiteTree::get(); @@ -101,6 +110,33 @@ class SiteTreeTest extends SapphireTest } } + /** + * Check if reserved URL's are properly appended with a number at top level + * @dataProvider reservedSegmentsProvider + */ + public function testDisallowedURLGeneration($title, $urlSegment) + { + $page = Page::create(['Title' => $title]); + $id = $page->write(); + $page = Page::get()->byID($id); + $this->assertEquals($urlSegment, $page->URLSegment); + } + + /** + * Check if reserved URL's are not appended with a number on a child page + * It's okay to have a URL like domain.com/my-page/admin as it won't interfere with domain.com/admin + * @dataProvider reservedSegmentsProvider + */ + public function testDisallowedChildURLGeneration($title, $urlSegment) + { + // Using the same dataprovider, strip out the -2 from the admin and dev segment + $urlSegment = str_replace('-2', '', $urlSegment); + $page = Page::create(['Title' => $title, 'ParentID' => 1]); + $id = $page->write(); + $page = Page::get()->byID($id); + $this->assertEquals($urlSegment, $page->URLSegment); + } + /** * Test that publication copies data to SiteTree_Live */ @@ -820,6 +856,14 @@ class SiteTreeTest extends SapphireTest $this->assertTrue($ceo->isSection()); } + public function testURLSegmentReserved() + { + $siteTree = SiteTree::create(['URLSegment' => 'admin']); + $segment = $siteTree->validURLSegment(); + + $this->assertFalse($segment); + } + public function testURLSegmentAutoUpdate() { $sitetree = new SiteTree();