diff --git a/code/Model/RedirectorPage.php b/code/Model/RedirectorPage.php index 01c86485..645a6a42 100644 --- a/code/Model/RedirectorPage.php +++ b/code/Model/RedirectorPage.php @@ -138,12 +138,23 @@ class RedirectorPage extends Page { parent::onBeforeWrite(); - // Prefix the URL with "http://" if no prefix is found - if ($this->ExternalURL - && !parse_url($this->ExternalURL, PHP_URL_SCHEME) - && !preg_match('#^//#', $this->ExternalURL) - ) { - $this->ExternalURL = 'http://' . $this->ExternalURL; + if ($this->ExternalURL && substr($this->ExternalURL, 0, 2) !== '//') { + $urlParts = parse_url($this->ExternalURL); + if ($urlParts) { + if (empty($urlParts['scheme'])) { + // no scheme, assume http + $this->ExternalURL = 'http://' . $this->ExternalURL; + } elseif (!in_array($urlParts['scheme'], array( + 'http', + 'https', + ))) { + // we only allow http(s) urls + $this->ExternalURL = ''; + } + } else { + // malformed URL to reject + $this->ExternalURL = ''; + } } } diff --git a/code/Model/SiteTree.php b/code/Model/SiteTree.php index 4d70a6b5..55592efb 100755 --- a/code/Model/SiteTree.php +++ b/code/Model/SiteTree.php @@ -777,7 +777,9 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi /** @var SiteTree $child */ $sort = 0; foreach ($children as $child) { - $childClone = $child->duplicateWithChildren(); + $childClone = method_exists($child, 'duplicateWithChildren') + ? $child->duplicateWithChildren() + : $child->duplicate(); $childClone->ParentID = $clone->ID; //retain sort order by manually setting sort values $childClone->Sort = ++$sort; @@ -1812,7 +1814,7 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi 'New {pagetype}', array('pagetype' => $this->i18n_singular_name()) ))); - $helpText = (self::config()->nested_urls && $this->Children()->count()) + $helpText = (self::config()->nested_urls && $this->numChildren()) ? $this->fieldLabel('LinkChangeNote') : ''; if (!Config::inst()->get('SilverStripe\\View\\Parsers\\URLSegmentFilter', 'default_allow_multibyte')) { diff --git a/tests/model/RedirectorPageTest.php b/tests/model/RedirectorPageTest.php index d60b455a..676489fb 100644 --- a/tests/model/RedirectorPageTest.php +++ b/tests/model/RedirectorPageTest.php @@ -53,7 +53,7 @@ class RedirectorPageTest extends FunctionalTest $this->assertContains('message-setupWithoutRedirect', $content); /* Transitive redirectors are those that point to another redirector page. They should send people to the URLSegment - * of the destination page - the middle-stop, so to speak. That should redirect to the final destination */ + * of the destination page - the middle-stop, so to speak. That should redirect to the final destination */ $page = $this->objFromFixture(RedirectorPage::class, 'transitive'); $this->assertEquals('/good-internal/', $page->Link()); @@ -99,4 +99,15 @@ class RedirectorPageTest extends FunctionalTest RedirectorPageController::remove_extension('RedirectorPageTest_RedirectExtension'); } + + public function testNoJSLinksAllowed() + { + $page = new RedirectorPage(); + $js = 'javascript:alert("hello world")'; + $page->ExternalURL = $js; + $this->assertEquals($js, $page->ExternalURL); + + $page->write(); + $this->assertEmpty($page->ExternalURL); + } }