ENH Use symfony/validation logic

silverstripe · Sep 30, 2024 · 5267cd4 · 5267cd4
1 parent 6194844
commit 5267cd4
Show file tree

Hide file tree

Showing 2 changed files with 5 additions and 62 deletions.
diff --git 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
@@ -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');