ENH Use symfony/validation logic

code/Controllers/ModelAsController.php

@@ -60,7 +60,7 @@ class ModelAsController extends Controller implements NestedController
                 Director::absoluteBaseURL(),
                 'dev/build',
                 '?' . http_build_query([
-                    'returnURL' => isset($_GET['url']) ? $_GET['url'] : null,
+                    'BackURL' => isset($_GET['url']) ? $_GET['url'] : null,
                 ])
             ));
         }
@@ -81,7 +81,7 @@ class ModelAsController extends Controller implements NestedController
 
         // If the database has not yet been created, redirect to the build page.
         if (!DB::is_active() || !ClassInfo::hasTable('SiteTree')) {
-            $this->getResponse()->redirect(Controller::join_links(Director::absoluteBaseURL(), 'dev/build?returnURL=' . (isset($_GET['url']) ? urlencode($_GET['url']) : null)));
+            $this->getResponse()->redirect(Controller::join_links(Director::absoluteBaseURL(), 'dev/build?BackURL=' . (isset($_GET['url']) ? urlencode($_GET['url']) : null)));
             $this->popCurrent();
 
             return $this->getResponse();

code/Controllers/RootURLController.php

@@ -78,7 +78,7 @@ class RootURLController extends Controller implements Resettable
                 Director::absoluteBaseURL(),
                 'dev/build',
                 '?' . http_build_query([
-                    'returnURL' => isset($_GET['url']) ? $_GET['url'] : null,
+                    'BackURL' => isset($_GET['url']) ? $_GET['url'] : null,
                 ])
             ));
         }
@@ -91,7 +91,7 @@ class RootURLController extends Controller implements Resettable
 
         if (!$this->getResponse()->isFinished()) {
             if (!DB::is_active() || !ClassInfo::hasTable('SiteTree')) {
-                $this->getResponse()->redirect(Director::absoluteBaseURL() . 'dev/build?returnURL=' . (isset($_GET['url']) ? urlencode($_GET['url']) : null));
+                $this->getResponse()->redirect(Director::absoluteBaseURL() . 'dev/build?BackURL=' . (isset($_GET['url']) ? urlencode($_GET['url']) : null));
                 return $this->getResponse();
             }
 

code/Model/RedirectorPage.php

@@ -7,6 +7,7 @@ use SilverStripe\Assets\File;
 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 @@ class RedirectorPage extends Page
         }
     }
 
-    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',

tests/php/Model/RedirectorPageTest.php

@@ -121,33 +121,6 @@ class RedirectorPageTest extends FunctionalTest
         $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 @@ class RedirectorPageTest extends FunctionalTest
         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');