From ff9997e0f26266e47b49e3682edbafa1c1ae04c7 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Fri, 19 Oct 2018 20:51:43 +0200 Subject: [PATCH 1/4] FIX Ignore ports when matching domain for subsite --- src/Model/Subsite.php | 4 ++++ tests/php/SubsiteTest.php | 16 ++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/src/Model/Subsite.php b/src/Model/Subsite.php index 229604d..80b7645 100644 --- a/src/Model/Subsite.php +++ b/src/Model/Subsite.php @@ -257,6 +257,10 @@ class Subsite extends DataObject $host = $_SERVER['HTTP_HOST']; } + // Remove ports, we aren't concerned with them in terms of detecting subsites via domains + $hostParts = explode(':', $host); + $host = reset($hostParts); + $matchingDomains = null; $cacheKey = null; if ($host) { diff --git a/tests/php/SubsiteTest.php b/tests/php/SubsiteTest.php index 8e360cb..b3a6a3c 100644 --- a/tests/php/SubsiteTest.php +++ b/tests/php/SubsiteTest.php @@ -191,12 +191,23 @@ class SubsiteTest extends BaseSubsiteTest Subsite::getSubsiteIDForDomain('example.org'), 'Exact matches without strict checking when not using www prefix' ); + $this->assertEquals( + $subsite1->ID, + Subsite::getSubsiteIDForDomain('example.org:1123'), + 'Exact matches without strict checking when not using www prefix and ignores port' + ); $this->assertEquals( $subsite1->ID, Subsite::getSubsiteIDForDomain('www.example.org'), 'Matches without strict checking when using www prefix, ' .'still matching first domain regardless of www prefix (falling back to subsite primary key ordering)' ); + $this->assertEquals( + $subsite1->ID, + Subsite::getSubsiteIDForDomain('www.example.org:9923'), + 'Matches without strict checking when using www prefix, ' + .'still matching first domain without prefix (falling back to primary key ordering and ignoring port)' + ); $this->assertEquals( $subsite1->ID, Subsite::getSubsiteIDForDomain('www.example.com'), @@ -215,6 +226,11 @@ class SubsiteTest extends BaseSubsiteTest Subsite::getSubsiteIDForDomain('example.org'), 'Matches with strict checking when not using www prefix' ); + $this->assertEquals( + $subsite1->ID, + Subsite::getSubsiteIDForDomain('example.org:123'), + 'Matches with strict checking when not using www prefix and ignores port' + ); $this->assertEquals( $subsite2->ID, // not 1 Subsite::getSubsiteIDForDomain('www.example.org'), From 5e79abdbbc9b7aea53fcf846a7714c9ec8f34b57 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Fri, 19 Oct 2018 21:51:32 +0200 Subject: [PATCH 2/4] Update testDomainProtocol to use a dataProvider This might help with test state leakage --- tests/php/SubsiteTest.php | 70 ++++++++++++++++++--------------------- 1 file changed, 33 insertions(+), 37 deletions(-) diff --git a/tests/php/SubsiteTest.php b/tests/php/SubsiteTest.php index 8e360cb..8a9955f 100644 --- a/tests/php/SubsiteTest.php +++ b/tests/php/SubsiteTest.php @@ -291,46 +291,42 @@ class SubsiteTest extends BaseSubsiteTest /** * Tests that Subsite and SubsiteDomain both respect http protocol correctly + * + * @param string $class Fixture class name + * @param string $identifier Fixture identifier + * @param bool $currentIsSecure Whether the current base URL should be secure + * @param string $expected The expected base URL for the subsite or subsite domain + * @dataProvider domainProtocolProvider */ - public function testDomainProtocol() + public function testDomainProtocol($class, $identifier, $currentIsSecure, $expected) { - // domaintest2 has 'protocol' - $subsite2 = $this->objFromFixture(Subsite::class, 'domaintest2'); - $domain2a = $this->objFromFixture(SubsiteDomain::class, 'dt2a'); - $domain2b = $this->objFromFixture(SubsiteDomain::class, 'dt2b'); + /** @var Subsite|SubsiteDomain $model */ + $model = $this->objFromFixture($class, $identifier); + $protocol = $currentIsSecure ? 'https' : 'http'; + Config::modify()->set(Director::class, 'alternate_base_url', $protocol . '://www.mysite.com'); + $this->assertSame($expected, $model->absoluteBaseURL()); + } - // domaintest4 is 'https' (primary only) - $subsite4 = $this->objFromFixture(Subsite::class, 'domaintest4'); - $domain4a = $this->objFromFixture(SubsiteDomain::class, 'dt4a'); - $domain4b = $this->objFromFixture(SubsiteDomain::class, 'dt4b'); // secondary domain is http only though - - // domaintest5 is 'http' - $subsite5 = $this->objFromFixture(Subsite::class, 'domaintest5'); - $domain5a = $this->objFromFixture(SubsiteDomain::class, 'dt5'); - - // Check protocol when current protocol is http:// - Config::modify()->set(Director::class, 'alternate_base_url', 'http://www.mysite.com'); - - $this->assertEquals('http://two.mysite.com/', $subsite2->absoluteBaseURL()); - $this->assertEquals('http://two.mysite.com/', $domain2a->absoluteBaseURL()); - $this->assertEquals('http://subsite.mysite.com/', $domain2b->absoluteBaseURL()); - $this->assertEquals('https://www.primary.com/', $subsite4->absoluteBaseURL()); - $this->assertEquals('https://www.primary.com/', $domain4a->absoluteBaseURL()); - $this->assertEquals('http://www.secondary.com/', $domain4b->absoluteBaseURL()); - $this->assertEquals('http://www.tertiary.com/', $subsite5->absoluteBaseURL()); - $this->assertEquals('http://www.tertiary.com/', $domain5a->absoluteBaseURL()); - - // Check protocol when current protocol is https:// - Config::modify()->set(Director::class, 'alternate_base_url', 'https://www.mysite.com'); - - $this->assertEquals('https://two.mysite.com/', $subsite2->absoluteBaseURL()); - $this->assertEquals('https://two.mysite.com/', $domain2a->absoluteBaseURL()); - $this->assertEquals('https://subsite.mysite.com/', $domain2b->absoluteBaseURL()); - $this->assertEquals('https://www.primary.com/', $subsite4->absoluteBaseURL()); - $this->assertEquals('https://www.primary.com/', $domain4a->absoluteBaseURL()); - $this->assertEquals('http://www.secondary.com/', $domain4b->absoluteBaseURL()); - $this->assertEquals('http://www.tertiary.com/', $subsite5->absoluteBaseURL()); - $this->assertEquals('http://www.tertiary.com/', $domain5a->absoluteBaseURL()); + public function domainProtocolProvider() + { + return [ + [Subsite::class, 'domaintest2', false, 'http://two.mysite.com/'], + [SubsiteDomain::class, 'dt2a', false, 'http://two.mysite.com/'], + [SubsiteDomain::class, 'dt2b', false, 'http://subsite.mysite.com/'], + [Subsite::class, 'domaintest4', false, 'https://www.primary.com/'], + [SubsiteDomain::class, 'dt4a', false, 'https://www.primary.com/'], + [SubsiteDomain::class, 'dt4b', false, 'http://www.secondary.com/'], + [Subsite::class, 'domaintest5', false, 'http://www.tertiary.com/'], + [SubsiteDomain::class, 'dt5', false, 'http://www.tertiary.com/'], + [Subsite::class, 'domaintest2', true, 'https://two.mysite.com/'], + [SubsiteDomain::class, 'dt2a', true, 'https://two.mysite.com/'], + [SubsiteDomain::class, 'dt2b', true, 'https://subsite.mysite.com/'], + [Subsite::class, 'domaintest4', true, 'https://www.primary.com/'], + [SubsiteDomain::class, 'dt4a', true, 'https://www.primary.com/'], + [SubsiteDomain::class, 'dt4b', true, 'http://www.secondary.com/'], + [Subsite::class, 'domaintest5', true, 'http://www.tertiary.com/'], + [SubsiteDomain::class, 'dt5', true, 'http://www.tertiary.com/'], + ]; } public function testAllSites() From 1fa549886f1f42992c266078f57f80fca45301e9 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Sat, 20 Oct 2018 23:15:32 +0200 Subject: [PATCH 3/4] Define explode limit when removing port --- src/Model/Subsite.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Model/Subsite.php b/src/Model/Subsite.php index 80b7645..c12721b 100644 --- a/src/Model/Subsite.php +++ b/src/Model/Subsite.php @@ -258,7 +258,7 @@ class Subsite extends DataObject } // Remove ports, we aren't concerned with them in terms of detecting subsites via domains - $hostParts = explode(':', $host); + $hostParts = explode(':', $host, 2); $host = reset($hostParts); $matchingDomains = null; From 2a35a5c70a09a6990796cd6ff9b66f0879389f6e Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Sun, 28 Oct 2018 21:41:32 +0000 Subject: [PATCH 4/4] FIX Replace Convert JSON methods with json_* methods, deprecated from SilverStripe 4.4 --- src/Extensions/SiteTreeSubsites.php | 2 +- tests/php/SiteTreeSubsitesTest.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Extensions/SiteTreeSubsites.php b/src/Extensions/SiteTreeSubsites.php index ecef3a5..68c5ca6 100644 --- a/src/Extensions/SiteTreeSubsites.php +++ b/src/Extensions/SiteTreeSubsites.php @@ -545,7 +545,7 @@ class SiteTreeSubsites extends DataExtension $subsite = Subsite::currentSubsite(); if ($subsite && $subsite->exists() && $subsite->PageTypeBlacklist) { // SS 4.1: JSON encoded. SS 4.0, comma delimited - $blacklist = Convert::json2array($subsite->PageTypeBlacklist); + $blacklist = json_decode($subsite->PageTypeBlacklist, true); if ($blacklist === false) { $blacklist = explode(',', $subsite->PageTypeBlacklist); } diff --git a/tests/php/SiteTreeSubsitesTest.php b/tests/php/SiteTreeSubsitesTest.php index efb16f8..d165336 100644 --- a/tests/php/SiteTreeSubsitesTest.php +++ b/tests/php/SiteTreeSubsitesTest.php @@ -268,7 +268,7 @@ class SiteTreeSubsitesTest extends BaseSubsiteTest Subsite::changeSubsite($s1); $cmsmain = CMSMain::create(); - $hints = Convert::json2array($cmsmain->SiteTreeHints()); + $hints = json_decode($cmsmain->SiteTreeHints(), true); $classes = $hints['Root']['disallowedChildren']; $this->assertContains(ErrorPage::class, $classes); $this->assertContains(TestClassA::class, $classes); @@ -279,7 +279,7 @@ class SiteTreeSubsitesTest extends BaseSubsiteTest if ($cmsmain->hasMethod('getHintsCache')) { $cmsmain->getHintsCache()->clear(); } - $hints = Convert::json2array($cmsmain->SiteTreeHints()); + $hints = json_decode($cmsmain->SiteTreeHints(), true); $classes = $hints['Root']['disallowedChildren']; $this->assertNotContains(ErrorPage::class, $classes);