From bd48b04731f0c83dd9af07d15bc444250d3a12dc Mon Sep 17 00:00:00 2001 From: Guy Sartorelli <36352093+GuySartorelli@users.noreply.github.com> Date: Wed, 2 Oct 2024 18:39:01 +1300 Subject: [PATCH] ENH Use symfony/validation logic (#3009) --- code/Model/RedirectorPage.php | 29 ++++---------------- tests/php/Model/RedirectorPageTest.php | 38 -------------------------- 2 files changed, 5 insertions(+), 62 deletions(-) diff --git a/code/Model/RedirectorPage.php b/code/Model/RedirectorPage.php index 97124f32ce..6eaec82686 100644 --- a/code/Model/RedirectorPage.php +++ b/code/Model/RedirectorPage.php @@ -7,6 +7,7 @@ use SilverStripe\Forms\FieldList; use SilverStripe\Forms\HeaderField; use SilverStripe\Forms\OptionsetField; +use SilverStripe\Forms\UrlField; use SilverStripe\Versioned\Versioned; /** @@ -47,6 +48,9 @@ class RedirectorPage extends Page 'RedirectionType', 'Content', ], + 'fieldClasses' => [ + 'ExternalURL' => UrlField::class, + ], ]; private static $table_name = 'RedirectorPage'; @@ -171,35 +175,12 @@ public function syncLinkTracking() } } - protected function onBeforeWrite() - { - parent::onBeforeWrite(); - - 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'], [ - 'http', - 'https', - ])) { - // we only allow http(s) urls - $this->ExternalURL = ''; - } - } else { - // malformed URL to reject - $this->ExternalURL = ''; - } - } - } - public function getCMSFields() { $this->beforeUpdateCMSFields(function (FieldList $fields) { // Remove all metadata fields, does not apply for redirector pages $fields->removeByName('Metadata'); + $fields->dataFieldByName('ExternalURL')?->setAllowRelativeProtocol(true); $fields->addFieldsToTab( 'Root.Main', diff --git a/tests/php/Model/RedirectorPageTest.php b/tests/php/Model/RedirectorPageTest.php index e811845867..2f6cf61212 100644 --- a/tests/php/Model/RedirectorPageTest.php +++ b/tests/php/Model/RedirectorPageTest.php @@ -121,33 +121,6 @@ public function testReflexiveAndTransitiveInternalRedirectors() $this->assertEquals(Director::absoluteURL('/redirection-dest'), $response->getHeader("Location")); } - public function testExternalURLGetsPrefixIfNotSet() - { - $page = $this->objFromFixture(RedirectorPage::class, 'externalnoprefix'); - $this->assertEquals($page->ExternalURL, 'http://google.com', 'onBeforeWrite has prefixed with http'); - $page->write(); - $this->assertEquals( - $page->ExternalURL, - 'http://google.com', - 'onBeforeWrite will not double prefix if written again!' - ); - } - - public function testAllowsProtocolRelative() - { - $noProtocol = new RedirectorPage(['ExternalURL' => 'mydomain.com']); - $noProtocol->write(); - $this->assertEquals('http://mydomain.com', $noProtocol->ExternalURL); - - $protocolAbsolute = new RedirectorPage(['ExternalURL' => 'http://mydomain.com']); - $protocolAbsolute->write(); - $this->assertEquals('http://mydomain.com', $protocolAbsolute->ExternalURL); - - $protocolRelative = new RedirectorPage(['ExternalURL' => '//mydomain.com']); - $protocolRelative->write(); - $this->assertEquals('//mydomain.com', $protocolRelative->ExternalURL); - } - /** * Test that we can trigger a redirection before RedirectorPageController::init() is called */ @@ -163,17 +136,6 @@ public function testRedirectRespectsFinishedResponse() RedirectorPageController::remove_extension(RedirectorPageTest_RedirectExtension::class); } - 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); - } - public function testFileRedirector() { $page = $this->objFromFixture(RedirectorPage::class, 'file');