From 645502c4da949351e0c414e893f08c8ef8d70911 Mon Sep 17 00:00:00 2001 From: Steve Boyd Date: Tue, 6 Aug 2024 14:03:39 +1200 Subject: [PATCH] FIX Do not suffix trailing slash to external links --- src/Control/Controller.php | 19 +- src/Control/Director.php | 6 +- tests/php/Control/ControllerTest.php | 332 +++++++++++++++--- tests/php/Control/DirectorTest.php | 30 +- tests/php/Control/HTTPTest.php | 1 + .../Middleware/CanonicalURLMiddlewareTest.php | 2 + 6 files changed, 316 insertions(+), 74 deletions(-) diff --git a/src/Control/Controller.php b/src/Control/Controller.php index 5b94f0ad694..3416bfc8a6b 100644 --- a/src/Control/Controller.php +++ b/src/Control/Controller.php @@ -680,6 +680,18 @@ public static function join_links($arg = null) */ public static function normaliseTrailingSlash(string $url): string { + // Do not normalise external urls + // Note that urls without a scheme such as www.example.com will be counted as a relative file + if (!Director::is_site_url($url)) { + return $url; + } + + // Do not modify files e.g. some/path/myfile.txt (exclude if $url is absolute_base_url) + $extension = pathinfo(rtrim($url), PATHINFO_EXTENSION); + if ($extension && $url !== Director::config()->get('alternate_base_url')) { + return $url; + } + $querystring = null; $fragmentIdentifier = null; @@ -694,14 +706,9 @@ public static function normaliseTrailingSlash(string $url): string // Normlise trailing slash $shouldHaveTrailingSlash = Controller::config()->uninherited('add_trailing_slash'); - if ($shouldHaveTrailingSlash - && !str_ends_with($url, '/') - && !preg_match('/^(.*)\.([^\/]*)$/', Director::makeRelative($url)) - ) { - // Add trailing slash if enabled and url does not end with a file extension + if ($shouldHaveTrailingSlash && !str_ends_with($url, '/')) { $url .= '/'; } elseif (!$shouldHaveTrailingSlash) { - // Remove trailing slash if it shouldn't be there $url = rtrim($url, '/'); } diff --git a/src/Control/Director.php b/src/Control/Director.php index 8318e5dd6bb..601a1cb8a02 100644 --- a/src/Control/Director.php +++ b/src/Control/Director.php @@ -821,7 +821,11 @@ public static function is_site_url($url) // Allow extensions to weigh in $isSiteUrl = false; - static::singleton()->extend('updateIsSiteUrl', $isSiteUrl, $url); + // not using static::singleton() here because it can, for unknown reasons, break + // functional tests such as those in HTTPCacheControlIntegrationTest + // extension hooks will still be called as expected + $director = new static(); + $director->extend('updateIsSiteUrl', $isSiteUrl, $url); if ($isSiteUrl) { return true; } diff --git a/tests/php/Control/ControllerTest.php b/tests/php/Control/ControllerTest.php index ee1222dda08..29d520eb6ae 100644 --- a/tests/php/Control/ControllerTest.php +++ b/tests/php/Control/ControllerTest.php @@ -276,14 +276,17 @@ public function testJoinLinks() { /* Controller::join_links() will reliably join two URL-segments together so that they will be * appropriately parsed by the URL parser */ + Director::config()->set('alternate_base_url', 'https://www.internal.com'); Controller::config()->set('add_trailing_slash', false); $this->assertEquals("admin/crm/MyForm", Controller::join_links("admin/crm", "MyForm")); $this->assertEquals("admin/crm/MyForm", Controller::join_links("admin/crm/", "MyForm")); - $this->assertEquals("https://www.test.com/admin/crm/MyForm", Controller::join_links("https://www.test.com", "admin/crm/", "MyForm")); + $this->assertEquals("https://www.internal.com/admin/crm/MyForm", Controller::join_links("https://www.internal.com", "admin/crm/", "MyForm")); + $this->assertEquals("https://www.external.com/admin/crm/MyForm", Controller::join_links("https://www.external.com", "admin/crm/", "MyForm")); Controller::config()->set('add_trailing_slash', true); $this->assertEquals("admin/crm/MyForm/", Controller::join_links("admin/crm", "MyForm")); $this->assertEquals("admin/crm/MyForm/", Controller::join_links("admin/crm/", "MyForm")); - $this->assertEquals("https://www.test.com/admin/crm/MyForm/", Controller::join_links("https://www.test.com", "admin/crm/", "MyForm")); + $this->assertEquals("https://www.internal.com/admin/crm/MyForm/", Controller::join_links("https://www.internal.com", "admin/crm/", "MyForm")); + $this->assertEquals("https://www.external.com/admin/crm/MyForm", Controller::join_links("https://www.external.com", "admin/crm/", "MyForm")); /* It will also handle appropriate combination of querystring variables */ Controller::config()->set('add_trailing_slash', false); @@ -293,7 +296,8 @@ public function testJoinLinks() "admin/crm/MyForm?field=1&other=1", Controller::join_links("admin/crm/?field=1", "MyForm?other=1") ); - $this->assertEquals("https://www.test.com/admin/crm/MyForm?flush=1", Controller::join_links("https://www.test.com", "admin/crm/", "MyForm?flush=1")); + $this->assertEquals("https://www.internal.com/admin/crm/MyForm?flush=1", Controller::join_links("https://www.internal.com", "admin/crm/", "MyForm?flush=1")); + $this->assertEquals("https://www.external.com/admin/crm/MyForm?flush=1", Controller::join_links("https://www.external.com", "admin/crm/", "MyForm?flush=1")); Controller::config()->set('add_trailing_slash', true); $this->assertEquals("admin/crm/MyForm/?flush=1", Controller::join_links("admin/crm/?flush=1", "MyForm")); $this->assertEquals("admin/crm/MyForm/?flush=1", Controller::join_links("admin/crm/", "MyForm?flush=1")); @@ -301,7 +305,8 @@ public function testJoinLinks() "admin/crm/MyForm/?field=1&other=1", Controller::join_links("admin/crm/?field=1", "MyForm?other=1") ); - $this->assertEquals("https://www.test.com/admin/crm/MyForm/?flush=1", Controller::join_links("https://www.test.com", "admin/crm/", "MyForm?flush=1")); + $this->assertEquals("https://www.internal.com/admin/crm/MyForm/?flush=1", Controller::join_links("https://www.internal.com", "admin/crm/", "MyForm?flush=1")); + $this->assertEquals("https://www.external.com/admin/crm/MyForm?flush=1", Controller::join_links("https://www.external.com", "admin/crm/", "MyForm?flush=1")); /* It can handle arbitrary numbers of components, and will ignore empty ones */ Controller::config()->set('add_trailing_slash', false); @@ -340,8 +345,12 @@ public function testJoinLinks() Controller::join_links("admin/crm?foo=1&bar=1&baz=1", "?foo=2&bar=3") ); $this->assertEquals( - "https://www.test.com/admin/crm?foo=2&bar=3&baz=1", - Controller::join_links("https://www.test.com", "admin/crm?foo=1&bar=1&baz=1", "?foo=2&bar=3") + "https://www.internal.com/admin/crm?foo=2&bar=3&baz=1", + Controller::join_links("https://www.internal.com", "admin/crm?foo=1&bar=1&baz=1", "?foo=2&bar=3") + ); + $this->assertEquals( + "https://www.external.com/admin/crm?foo=2&bar=3&baz=1", + Controller::join_links("https://www.external.com", "admin/crm?foo=1&bar=1&baz=1", "?foo=2&bar=3") ); Controller::config()->set('add_trailing_slash', true); $this->assertEquals( @@ -349,8 +358,12 @@ public function testJoinLinks() Controller::join_links("admin/crm?foo=1&bar=1&baz=1", "?foo=2&bar=3") ); $this->assertEquals( - "https://www.test.com/admin/crm/?foo=2&bar=3&baz=1", - Controller::join_links("https://www.test.com", "admin/crm?foo=1&bar=1&baz=1", "?foo=2&bar=3") + "https://www.internal.com/admin/crm/?foo=2&bar=3&baz=1", + Controller::join_links("https://www.internal.com", "admin/crm?foo=1&bar=1&baz=1", "?foo=2&bar=3") + ); + $this->assertEquals( + "https://www.external.com/admin/crm?foo=2&bar=3&baz=1", + Controller::join_links("https://www.external.com", "admin/crm?foo=1&bar=1&baz=1", "?foo=2&bar=3") ); Controller::config()->set('add_trailing_slash', false); @@ -361,8 +374,13 @@ public function testJoinLinks() ); $this->assertEquals('/admin/action', Controller::join_links('/admin', 'action')); $this->assertEquals( - 'https://www.test.com/admin/action', - Controller::join_links('https://www.test.com', '/', '/admin/', '/', '/action'), + 'https://www.internal.com/admin/action', + Controller::join_links('https://www.internal.com', '/', '/admin/', '/', '/action'), + 'Test that multiple slashes are trimmed.' + ); + $this->assertEquals( + 'https://www.external.com/admin/action', + Controller::join_links('https://www.external.com', '/', '/admin/', '/', '/action'), 'Test that multiple slashes are trimmed.' ); Controller::config()->set('add_trailing_slash', true); @@ -373,8 +391,13 @@ public function testJoinLinks() ); $this->assertEquals('/admin/action/', Controller::join_links('/admin', 'action')); $this->assertEquals( - 'https://www.test.com/admin/action/', - Controller::join_links('https://www.test.com', '/', '/admin/', '/', '/action'), + 'https://www.internal.com/admin/action/', + Controller::join_links('https://www.internal.com', '/', '/admin/', '/', '/action'), + 'Test that multiple slashes are trimmed.' + ); + $this->assertEquals( + 'https://www.external.com/admin/action', + Controller::join_links('https://www.external.com', '/', '/admin/', '/', '/action'), 'Test that multiple slashes are trimmed.' ); @@ -391,8 +414,12 @@ public function testJoinLinks() Controller::join_links("my-page#subsection", "?arg=var", "#second-section") ); $this->assertEquals( - "https://www.test.com/my-page?arg=var#second-section", - Controller::join_links("https://www.test.com", "my-page#subsection", "?arg=var", "#second-section") + "https://www.internal.com/my-page?arg=var#second-section", + Controller::join_links("https://www.internal.com", "my-page#subsection", "?arg=var", "#second-section") + ); + $this->assertEquals( + "https://www.external.com/my-page?arg=var#second-section", + Controller::join_links("https://www.external.com", "my-page#subsection", "?arg=var", "#second-section") ); Controller::config()->set('add_trailing_slash', true); $this->assertEquals( @@ -400,8 +427,12 @@ public function testJoinLinks() Controller::join_links("my-page#subsection", "?arg=var", "#second-section") ); $this->assertEquals( - "https://www.test.com/my-page/?arg=var#second-section", - Controller::join_links("https://www.test.com", "my-page#subsection", "?arg=var", "#second-section") + "https://www.internal.com/my-page/?arg=var#second-section", + Controller::join_links("https://www.internal.com", "my-page#subsection", "?arg=var", "#second-section") + ); + $this->assertEquals( + "https://www.external.com/my-page?arg=var#second-section", + Controller::join_links("https://www.external.com", "my-page#subsection", "?arg=var", "#second-section") ); /* Does type-safe checks for zero value */ @@ -413,60 +444,245 @@ public function testJoinLinks() // Test array args Controller::config()->set('add_trailing_slash', false); $this->assertEquals( - "https://www.test.com/admin/crm/MyForm?a=1&b=2&c=3", - Controller::join_links(["https://www.test.com", "?a=1", "admin/crm", "?b=2", "MyForm?c=3"]) + "https://www.internal.com/admin/crm/MyForm?a=1&b=2&c=3", + Controller::join_links(["https://www.internal.com", "?a=1", "admin/crm", "?b=2", "MyForm?c=3"]) + ); + $this->assertEquals( + "https://www.external.com/admin/crm/MyForm?a=1&b=2&c=3", + Controller::join_links(["https://www.external.com", "?a=1", "admin/crm", "?b=2", "MyForm?c=3"]) ); Controller::config()->set('add_trailing_slash', true); $this->assertEquals( - "https://www.test.com/admin/crm/MyForm/?a=1&b=2&c=3", - Controller::join_links(["https://www.test.com", "?a=1", "admin/crm", "?b=2", "MyForm?c=3"]) + "https://www.internal.com/admin/crm/MyForm/?a=1&b=2&c=3", + Controller::join_links(["https://www.internal.com", "?a=1", "admin/crm", "?b=2", "MyForm?c=3"]) + ); + $this->assertEquals( + "https://www.external.com/admin/crm/MyForm?a=1&b=2&c=3", + Controller::join_links(["https://www.external.com", "?a=1", "admin/crm", "?b=2", "MyForm?c=3"]) ); } - public function testNormaliseTrailingSlash() + public function provideNormaliseTrailingSlash(): array { - foreach ([true, false] as $withTrailingSlash) { - Controller::config()->set('add_trailing_slash', $withTrailingSlash); - $slash = $withTrailingSlash ? '/' : ''; - + // note 93.184.215.14 is the IP address for example.com + return [ // Correctly gives slash to a relative root path - $this->assertEquals('/', Controller::normaliseTrailingSlash('')); - $this->assertEquals('/', Controller::normaliseTrailingSlash('/')); - + [ + 'path' => '', + 'withSlash' => '/', + 'withoutSlash' => '/', + ], + [ + 'path' => '/', + 'withSlash' => '/', + 'withoutSlash' => '/', + ], // Correctly adds or removes trailing slash - $this->assertEquals("some/path{$slash}", Controller::normaliseTrailingSlash('some/path/')); - $this->assertEquals("some/path{$slash}", Controller::normaliseTrailingSlash('some/path')); - + [ + 'path' => 'some/path/', + 'withSlash' => 'some/path/', + 'withoutSlash' => 'some/path', + ], // Retains leading slash, if there is one - $this->assertEquals("/some/path{$slash}", Controller::normaliseTrailingSlash('/some/path/')); - $this->assertEquals("/some/path{$slash}", Controller::normaliseTrailingSlash('/some/path')); - - // Effectively treats absolute URL as relative - $this->assertEquals("https://www.google.com/some/path{$slash}", Controller::normaliseTrailingSlash('https://www.google.com/some/path/')); - $this->assertEquals("//www.google.com/some/path{$slash}", Controller::normaliseTrailingSlash('//www.google.com/some/path')); - $this->assertEquals("www.google.com/some/path{$slash}", Controller::normaliseTrailingSlash('www.google.com/some/path')); - $this->assertEquals("https://www.google.com{$slash}", Controller::normaliseTrailingSlash('https://www.google.com')); - $this->assertEquals("//www.google.com{$slash}", Controller::normaliseTrailingSlash('//www.google.com/')); - + [ + 'path' => '/some/path/', + 'withSlash' => '/some/path/', + 'withoutSlash' => '/some/path', + ], + // Treat absolute URLs pointing to the current site as relative + [ + 'path' => '/some/path/', + 'withSlash' => '/some/path/', + 'withoutSlash' => '/some/path', + ], + [ + 'path' => '/', + 'withSlash' => '/', + 'withoutSlash' => '', + ], + [ + 'path' => '', + 'withSlash' => '/', + 'withoutSlash' => '', + ], + // External links never get normalised + [ + 'path' => 'https://www.example.com/some/path', + 'withSlash' => 'https://www.example.com/some/path', + 'withoutSlash' => 'https://www.example.com/some/path', + ], + [ + 'path' => 'https://www.example.com/some/path/', + 'withSlash' => 'https://www.example.com/some/path/', + 'withoutSlash' => 'https://www.example.com/some/path/', + ], + [ + 'path' => 'https://www.example.com', + 'withSlash' => 'https://www.example.com', + 'withoutSlash' => 'https://www.example.com', + ], + [ + 'path' => 'https://www.example.com/', + 'withSlash' => 'https://www.example.com/', + 'withoutSlash' => 'https://www.example.com/', + ], + [ + 'path' => '//www.example.com/some/path', + 'withSlash' => '//www.example.com/some/path', + 'withoutSlash' => '//www.example.com/some/path', + ], + [ + 'path' => '//www.example.com/some/path/', + 'withSlash' => '//www.example.com/some/path/', + 'withoutSlash' => '//www.example.com/some/path/', + ], + [ + 'path' => '//www.example.com', + 'withSlash' => '//www.example.com', + 'withoutSlash' => '//www.example.com', + ], + [ + 'path' => '//www.example.com/', + 'withSlash' => '//www.example.com/', + 'withoutSlash' => '//www.example.com/', + ], + [ + 'path' => 'https://93.184.215.14/some/path', + 'withSlash' => 'https://93.184.215.14/some/path', + 'withoutSlash' => 'https://93.184.215.14/some/path', + ], + [ + 'path' => 'https://93.184.215.14/some/path/', + 'withSlash' => 'https://93.184.215.14/some/path/', + 'withoutSlash' => 'https://93.184.215.14/some/path/', + ], + // Links without a scheme with a path are treated as relative + // Note: content authors should be specifying a scheme in these cases themselves + [ + 'path' => 'www.example.com/some/path', + 'withSlash' => 'www.example.com/some/path/', + 'withoutSlash' => 'www.example.com/some/path', + ], + [ + 'path' => 'www.example.com/some/path/', + 'withSlash' => 'www.example.com/some/path/', + 'withoutSlash' => 'www.example.com/some/path', + ], + [ + 'path' => '93.184.215.14/some/path', + 'withSlash' => '93.184.215.14/some/path/', + 'withoutSlash' => '93.184.215.14/some/path', + ], + [ + 'path' => '93.184.215.14/some/path/', + 'withSlash' => '93.184.215.14/some/path/', + 'withoutSlash' => '93.184.215.14/some/path', + ], + // Links without a scheme or path are treated like files i.e. not altered + // Note: content authors should be specifying a scheme in these cases themselves + [ + 'path' => 'www.example.com', + 'withSlash' => 'www.example.com', + 'withoutSlash' => 'www.example.com', + ], + [ + 'path' => 'www.example.com/', + 'withSlash' => 'www.example.com/', + 'withoutSlash' => 'www.example.com/', + ], + [ + 'path' => '93.184.215.14', + 'withSlash' => '93.184.215.14', + 'withoutSlash' => '93.184.215.14', + ], + [ + 'path' => '93.184.215.14/', + 'withSlash' => '93.184.215.14/', + 'withoutSlash' => '93.184.215.14/', + ], // Retains query string and anchor if present - $this->assertEquals("some/path{$slash}?key=value&key2=value2", Controller::normaliseTrailingSlash('some/path/?key=value&key2=value2')); - $this->assertEquals("some/path{$slash}#some-id", Controller::normaliseTrailingSlash('some/path/#some-id')); - $this->assertEquals("some/path{$slash}?key=value&key2=value2#some-id", Controller::normaliseTrailingSlash('some/path/?key=value&key2=value2#some-id')); - $this->assertEquals("some/path{$slash}?key=value&key2=value2", Controller::normaliseTrailingSlash('some/path?key=value&key2=value2')); - $this->assertEquals("some/path{$slash}#some-id", Controller::normaliseTrailingSlash('some/path#some-id')); - $this->assertEquals("some/path{$slash}?key=value&key2=value2#some-id", Controller::normaliseTrailingSlash('some/path?key=value&key2=value2#some-id')); - + [ + 'path' => 'some/path/?key=value&key2=value2', + 'withSlash' => 'some/path/?key=value&key2=value2', + 'withoutSlash' => 'some/path?key=value&key2=value2', + ], + [ + 'path' => 'some/path/#some-id', + 'withSlash' => 'some/path/#some-id', + 'withoutSlash' => 'some/path#some-id', + ], + [ + 'path' => 'some/path?key=value&key2=value2#some-id', + 'withSlash' => 'some/path/?key=value&key2=value2#some-id', + 'withoutSlash' => 'some/path?key=value&key2=value2#some-id', + ], + [ + 'path' => 'some/path?key=value&key2=value2', + 'withSlash' => 'some/path/?key=value&key2=value2', + 'withoutSlash' => 'some/path?key=value&key2=value2', + ], + [ + 'path' => 'some/path#some-id', + 'withSlash' => 'some/path/#some-id', + 'withoutSlash' => 'some/path#some-id', + ], + [ + 'path' => 'some/path?key=value&key2=value2#some-id', + 'withSlash' => 'some/path/?key=value&key2=value2#some-id', + 'withoutSlash' => 'some/path?key=value&key2=value2#some-id', + ], // Don't ever add a trailing slash to the end of a URL that looks like a file - $this->assertEquals("https://www.google.com/some/file.txt", Controller::normaliseTrailingSlash('https://www.google.com/some/file.txt')); - $this->assertEquals("//www.google.com/some/file.txt", Controller::normaliseTrailingSlash('//www.google.com/some/file.txt')); - $this->assertEquals("www.google.com/some/file.txt", Controller::normaliseTrailingSlash('www.google.com/some/file.txt')); - $this->assertEquals("/some/file.txt", Controller::normaliseTrailingSlash('/some/file.txt')); - $this->assertEquals("some/file.txt", Controller::normaliseTrailingSlash('some/file.txt')); - $this->assertEquals("file.txt", Controller::normaliseTrailingSlash('file.txt')); - $this->assertEquals("some/file.txt?key=value&key2=value2#some-id", Controller::normaliseTrailingSlash('some/file.txt?key=value&key2=value2#some-id')); - // NOTE: `www.google.com` is already treated as "relative" by Director::makeRelative(), which means we can't tell that it's a host (and not a file). - $this->assertEquals("www.google.com", Controller::normaliseTrailingSlash('www.google.com')); - } + [ + 'path' => 'https://www.example.com/some/file.txt', + 'withSlash' => 'https://www.example.com/some/file.txt', + 'withoutSlash' => 'https://www.example.com/some/file.txt', + ], + [ + 'path' => '//www.example.com/some/file.txt', + 'withSlash' => '//www.example.com/some/file.txt', + 'withoutSlash' => '//www.example.com/some/file.txt', + ], + [ + 'path' => 'www.example.com/some/file.txt', + 'withSlash' => 'www.example.com/some/file.txt', + 'withoutSlash' => 'www.example.com/some/file.txt', + ], + [ + 'path' => '/some/file.txt', + 'withSlash' => '/some/file.txt', + 'withoutSlash' => '/some/file.txt', + ], + [ + 'path' => 'some/file.txt', + 'withSlash' => 'some/file.txt', + 'withoutSlash' => 'some/file.txt', + ], + [ + 'path' => 'file.txt', + 'withSlash' => 'file.txt', + 'withoutSlash' => 'file.txt', + ], + [ + 'path' => 'some/file.txt?key=value&key2=value2#some-id', + 'withSlash' => 'some/file.txt?key=value&key2=value2#some-id', + 'withoutSlash' => 'some/file.txt?key=value&key2=value2#some-id', + ], + ]; + } + + /** + * @dataProvider provideNormaliseTrailingSlash + */ + public function testNormaliseTrailingSlash(string $path, string $withSlash, string $withoutSlash): void + { + $absBaseUrlNoSlash = rtrim(Director::absoluteBaseURL(), '/'); + $path = str_replace('', $absBaseUrlNoSlash, $path); + $withSlash = str_replace('', $absBaseUrlNoSlash, $withSlash); + $withoutSlash = str_replace('', $absBaseUrlNoSlash, $withoutSlash); + Controller::config()->set('add_trailing_slash', true); + $this->assertEquals($withSlash, Controller::normaliseTrailingSlash($path), 'With trailing slash test'); + Controller::config()->set('add_trailing_slash', false); + $this->assertEquals($withoutSlash, Controller::normaliseTrailingSlash($path), 'Without trailing slash test'); } public function testLink() diff --git a/tests/php/Control/DirectorTest.php b/tests/php/Control/DirectorTest.php index f0332322e73..75b50b8152e 100644 --- a/tests/php/Control/DirectorTest.php +++ b/tests/php/Control/DirectorTest.php @@ -100,30 +100,30 @@ public function testAbsoluteURL() //test empty / local urls foreach (['', './', '.'] as $url) { $this->assertEquals("http://www.mysite.com:9090/mysite{$slash}", Director::absoluteURL($url, Director::BASE)); - $this->assertEquals("http://www.mysite.com:9090{$slash}", Director::absoluteURL($url, Director::ROOT)); + $this->assertEquals("http://www.mysite.com:9090/", Director::absoluteURL($url, Director::ROOT)); $this->assertEquals("http://www.mysite.com:9090/mysite/sub-page{$slash}", Director::absoluteURL($url, Director::REQUEST)); } // Test site root url - $this->assertEquals("http://www.mysite.com:9090{$slash}", Director::absoluteURL('/')); + $this->assertEquals("http://www.mysite.com:9090/", Director::absoluteURL('/')); // Test Director::BASE - $this->assertEquals("http://www.mysite.com:9090{$slash}", Director::absoluteURL('http://www.mysite.com:9090/', Director::BASE)); - $this->assertEquals("http://www.mytest.com{$slash}", Director::absoluteURL('http://www.mytest.com', Director::BASE)); + $this->assertEquals("http://www.mysite.com:9090/", Director::absoluteURL('http://www.mysite.com:9090/', Director::BASE)); + $this->assertEquals("http://www.mytest.com", Director::absoluteURL('http://www.mytest.com', Director::BASE)); $this->assertEquals("http://www.mysite.com:9090/test{$slash}", Director::absoluteURL("http://www.mysite.com:9090/test", Director::BASE)); $this->assertEquals("http://www.mysite.com:9090/root{$slash}", Director::absoluteURL("/root", Director::BASE)); $this->assertEquals("http://www.mysite.com:9090/root/url{$slash}", Director::absoluteURL("/root/url", Director::BASE)); // Test Director::ROOT - $this->assertEquals("http://www.mysite.com:9090{$slash}", Director::absoluteURL('http://www.mysite.com:9090/', Director::ROOT)); - $this->assertEquals("http://www.mytest.com{$slash}", Director::absoluteURL('http://www.mytest.com', Director::ROOT)); + $this->assertEquals("http://www.mysite.com:9090/", Director::absoluteURL('http://www.mysite.com:9090/', Director::ROOT)); + $this->assertEquals("http://www.mytest.com", Director::absoluteURL('http://www.mytest.com', Director::ROOT)); $this->assertEquals("http://www.mysite.com:9090/test{$slash}", Director::absoluteURL("http://www.mysite.com:9090/test", Director::ROOT)); $this->assertEquals("http://www.mysite.com:9090/root{$slash}", Director::absoluteURL("/root", Director::ROOT)); $this->assertEquals("http://www.mysite.com:9090/root/url{$slash}", Director::absoluteURL("/root/url", Director::ROOT)); // Test Director::REQUEST - $this->assertEquals("http://www.mysite.com:9090{$slash}", Director::absoluteURL('http://www.mysite.com:9090/', Director::REQUEST)); - $this->assertEquals("http://www.mytest.com{$slash}", Director::absoluteURL('http://www.mytest.com', Director::REQUEST)); + $this->assertEquals("http://www.mysite.com:9090/", Director::absoluteURL('http://www.mysite.com:9090/', Director::REQUEST)); + $this->assertEquals("http://www.mytest.com", Director::absoluteURL('http://www.mytest.com', Director::REQUEST)); $this->assertEquals("http://www.mysite.com:9090/test{$slash}", Director::absoluteURL("http://www.mysite.com:9090/test", Director::REQUEST)); $this->assertEquals("http://www.mysite.com:9090/root{$slash}", Director::absoluteURL("/root", Director::REQUEST)); $this->assertEquals("http://www.mysite.com:9090/root/url{$slash}", Director::absoluteURL("/root/url", Director::REQUEST)); @@ -175,7 +175,7 @@ public function testAlternativeBaseURL() $this->assertEquals("http://www.example.org/relativebase{$slash}", Director::absoluteBaseURL()); $this->assertEquals("http://www.example.org/relativebase/sub-page{$slash}", Director::absoluteURL('', Director::REQUEST)); $this->assertEquals("http://www.example.org/relativebase{$slash}", Director::absoluteURL('', Director::BASE)); - $this->assertEquals("http://www.example.org{$slash}", Director::absoluteURL('', Director::ROOT)); + $this->assertEquals("http://www.example.org/", Director::absoluteURL('', Director::ROOT)); $this->assertEquals( "http://www.example.org/relativebase/sub-page/subfolder/test{$slash}", Director::absoluteURL('subfolder/test', Director::REQUEST) @@ -565,6 +565,7 @@ public function providerTestTestRequestCarriesGlobals() */ public function testTestRequestCarriesGlobals($url, $fixture, $method) { + Controller::config()->set('add_trailing_slash', false); $getresponse = Director::test( $url, $fixture, @@ -602,6 +603,7 @@ public function testRouteParams() public function testForceWWW() { + Controller::config()->set('add_trailing_slash', false); $this->expectExceptionRedirect('http://www.mysite.com:9090/some-url'); Director::mockRequest(function ($request) { Injector::inst()->registerService($request, HTTPRequest::class); @@ -611,6 +613,7 @@ public function testForceWWW() public function testPromisedForceWWW() { + Controller::config()->set('add_trailing_slash', false); Director::forceWWW(); // Flag is set but not redirected yet @@ -632,6 +635,7 @@ public function testPromisedForceWWW() public function testForceSSLProtectsEntireSite() { + Controller::config()->set('add_trailing_slash', false); $this->expectExceptionRedirect('https://www.mysite.com:9090/some-url'); Director::mockRequest(function ($request) { Injector::inst()->registerService($request, HTTPRequest::class); @@ -641,6 +645,7 @@ public function testForceSSLProtectsEntireSite() public function testPromisedForceSSL() { + Controller::config()->set('add_trailing_slash', false); Director::forceSSL(); // Flag is set but not redirected yet @@ -663,6 +668,7 @@ public function testPromisedForceSSL() public function testForceSSLOnTopLevelPagePattern() { + Controller::config()->set('add_trailing_slash', false); // Expect admin to trigger redirect $this->expectExceptionRedirect('https://www.mysite.com:9090/admin'); Director::mockRequest(function (HTTPRequest $request) { @@ -673,6 +679,7 @@ public function testForceSSLOnTopLevelPagePattern() public function testForceSSLOnSubPagesPattern() { + Controller::config()->set('add_trailing_slash', false); // Expect to redirect to security login page $this->expectExceptionRedirect('https://www.mysite.com:9090/Security/login'); Director::mockRequest(function (HTTPRequest $request) { @@ -683,6 +690,7 @@ public function testForceSSLOnSubPagesPattern() public function testForceSSLWithPatternDoesNotMatchOtherPages() { + Controller::config()->set('add_trailing_slash', false); // Not on same url should not trigger redirect $response = Director::mockRequest(function (HTTPRequest $request) { Injector::inst()->registerService($request, HTTPRequest::class); @@ -723,6 +731,7 @@ public function testForceSSLAlternateDomainWithPort() */ public function testForceSSLandForceWWW() { + Controller::config()->set('add_trailing_slash', false); Director::forceWWW(); Director::forceSSL(); @@ -783,12 +792,14 @@ protected function runTest() public function testUnmatchedRequestReturns404() { + Controller::config()->set('add_trailing_slash', false); // Remove non-tested rules $this->assertEquals(404, Director::test('no-route')->getStatusCode()); } public function testIsHttps() { + Controller::config()->set('add_trailing_slash', false); // Trust all IPs for this test /** @var TrustedProxyMiddleware $trustedProxyMiddleware */ $trustedProxyMiddleware @@ -878,6 +889,7 @@ public function testIsHttps() public function testTestIgnoresHashes() { + Controller::config()->set('add_trailing_slash', false); //test that hashes are ignored $url = "TestController/returnGetValue?somekey=key"; $hash = "#test"; diff --git a/tests/php/Control/HTTPTest.php b/tests/php/Control/HTTPTest.php index ff83ea208e2..3ec25c1cfd6 100644 --- a/tests/php/Control/HTTPTest.php +++ b/tests/php/Control/HTTPTest.php @@ -154,6 +154,7 @@ public function testGetLinksIn() */ public function testSetGetVar() { + Controller::config()->set('add_trailing_slash', false); // Hackery to work around volatile URL formats in test invocation, // and the inability of Director::absoluteBaseURL() to produce consistent URLs. Director::mockRequest(function (HTTPRequest $request) { diff --git a/tests/php/Control/Middleware/CanonicalURLMiddlewareTest.php b/tests/php/Control/Middleware/CanonicalURLMiddlewareTest.php index 5689fb767d6..2770874ab88 100644 --- a/tests/php/Control/Middleware/CanonicalURLMiddlewareTest.php +++ b/tests/php/Control/Middleware/CanonicalURLMiddlewareTest.php @@ -8,6 +8,7 @@ use SilverStripe\Control\Middleware\CanonicalURLMiddleware; use SilverStripe\Core\Environment; use SilverStripe\Dev\SapphireTest; +use SilverStripe\Control\Director; class CanonicalURLMiddlewareTest extends SapphireTest { @@ -121,6 +122,7 @@ public function testRedirectTrailingSlash(bool $forceRedirect, bool $addTrailing private function performRedirectTest(string $requestURL, CanonicalURLMiddleware $middleware, bool $shouldRedirect, bool $addTrailingSlash) { + Director::config()->set('alternate_base_url', 'https://www.example.com'); Environment::setEnv('REQUEST_URI', $requestURL); $request = new HTTPRequest('GET', $requestURL); $request->setScheme('https');