From 5267cd4619f325a51fcef232a4fa5f3b3bc7684e Mon Sep 17 00:00:00 2001
From: Guy Sartorelli <guy.sartorelli@gmail.com>
Date: Tue, 24 Sep 2024 17:43:26 +1200
Subject: [PATCH] ENH Use symfony/validation logic

---
 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');