From ce3a1ce91307424f643a15f0c292e16b35f35873 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Thu, 27 Sep 2018 14:07:42 +0200 Subject: [PATCH 1/4] FIX Use correct subsites namespace in SiteTree and test classes --- code/Model/SiteTree.php | 13 ++++++------- tests/php/Model/SiteTreePermissionsTest.php | 9 +++++---- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/code/Model/SiteTree.php b/code/Model/SiteTree.php index 7c23abf3..c2f2fad6 100755 --- a/code/Model/SiteTree.php +++ b/code/Model/SiteTree.php @@ -63,6 +63,7 @@ use SilverStripe\Security\PermissionChecker; use SilverStripe\Security\PermissionProvider; use SilverStripe\Security\Security; use SilverStripe\SiteConfig\SiteConfig; +use SilverStripe\Subsites\Model\Subsite; use SilverStripe\Versioned\RecursivePublishable; use SilverStripe\Versioned\Versioned; use SilverStripe\View\ArrayData; @@ -70,7 +71,6 @@ use SilverStripe\View\HTML; use SilverStripe\View\Parsers\ShortcodeParser; use SilverStripe\View\Parsers\URLSegmentFilter; use SilverStripe\View\SSViewer; -use Subsite; /** * Basic data-object representing all pages within the site tree. All page types that live within the hierarchy should @@ -1740,7 +1740,7 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi */ public function DependentPages($includeVirtuals = true) { - if (class_exists('Subsite')) { + if (class_exists(Subsite::class)) { $origDisableSubsiteFilter = Subsite::$disable_subsite_filter; Subsite::disable_subsite_filter(true); } @@ -1785,7 +1785,7 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi $items->merge($redirectorList); } - if (class_exists('Subsite')) { + if (class_exists(Subsite::class)) { Subsite::disable_subsite_filter($origDisableSubsiteFilter); } @@ -1804,9 +1804,8 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi // Disable subsite filter for these pages if ($pages instanceof DataList) { return $pages->setDataQueryParam('Subsite.filter', false); - } else { - return $pages; } + return $pages; } /** @@ -1875,8 +1874,8 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi 'AbsoluteLink' => _t(__CLASS__.'.DependtPageColumnURL', 'URL'), 'DependentLinkType' => _t(__CLASS__.'.DependtPageColumnLinkType', 'Link type'), ); - if (class_exists('Subsite')) { - $dependentColumns['Subsite.Title'] = singleton('Subsite')->i18n_singular_name(); + if (class_exists(Subsite::class)) { + $dependentColumns['Subsite.Title'] = Subsite::singleton()->i18n_singular_name(); } $dependentNote = new LiteralField('DependentNote', '

' . _t(__CLASS__.'.DEPENDENT_NOTE', 'The following pages depend on this page. This includes virtual pages, redirector pages, and pages with content links.') . '

'); diff --git a/tests/php/Model/SiteTreePermissionsTest.php b/tests/php/Model/SiteTreePermissionsTest.php index db50fe2c..9e595b1a 100644 --- a/tests/php/Model/SiteTreePermissionsTest.php +++ b/tests/php/Model/SiteTreePermissionsTest.php @@ -10,6 +10,7 @@ use SilverStripe\Security\Group; use SilverStripe\Security\Member; use SilverStripe\Security\Security; use SilverStripe\SiteConfig\SiteConfig; +use SilverStripe\Subsites\Extensions\SiteTreeSubsites; use SilverStripe\Versioned\Versioned; /** @@ -20,11 +21,11 @@ class SiteTreePermissionsTest extends FunctionalTest { protected static $fixture_file = "SiteTreePermissionsTest.yml"; - protected static $illegal_extensions = array( - SiteTree::class => array('SiteTreeSubsites') - ); + protected static $illegal_extensions = [ + SiteTree::class => [SiteTreeSubsites::class], + ]; - public function setUp() + protected function setUp() { parent::setUp(); From ff458810ea48153b6af09a834c2d6ad7e7291f67 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Tue, 5 Feb 2019 16:45:11 +0300 Subject: [PATCH 2/4] Automated phpcs linting updates --- tests/php/Controllers/CMSMainTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/php/Controllers/CMSMainTest.php b/tests/php/Controllers/CMSMainTest.php index 850abb72..c4a05d5d 100644 --- a/tests/php/Controllers/CMSMainTest.php +++ b/tests/php/Controllers/CMSMainTest.php @@ -28,7 +28,7 @@ class CMSMainTest extends FunctionalTest { protected static $fixture_file = 'CMSMainTest.yml'; - static protected $orig = array(); + protected static $orig = array(); public function setUp() { From 5c3b95ac8977f77e7d95d4da6333ca12b5ef465f Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Mon, 14 Jan 2019 09:17:06 +1300 Subject: [PATCH 3/4] FIX Multibyte URL routing Was double url encoding (once in database value, then again in request) Fixes https://github.com/silverstripe/silverstripe-framework/issues/8723 --- code/Controllers/ContentController.php | 8 ++++- code/Controllers/ModelAsController.php | 9 ++++- .../php/Controllers/ModelAsControllerTest.php | 34 +++++++++++++++++++ 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/code/Controllers/ContentController.php b/code/Controllers/ContentController.php index 503946c6..e0a78186 100644 --- a/code/Controllers/ContentController.php +++ b/code/Controllers/ContentController.php @@ -25,6 +25,7 @@ use SilverStripe\Security\Security; use SilverStripe\SiteConfig\SiteConfig; use SilverStripe\Versioned\Versioned; use SilverStripe\View\ArrayData; +use SilverStripe\View\Parsers\URLSegmentFilter; use SilverStripe\View\Requirements; use SilverStripe\View\SSViewer; use Translatable; @@ -193,11 +194,16 @@ class ContentController extends Controller if (class_exists('Translatable')) { Translatable::disable_locale_filter(); } + + $filter = URLSegmentFilter::create(); + // look for a page with this URLSegment $child = SiteTree::get()->filter([ 'ParentID' => $this->ID, - 'URLSegment' => rawurlencode($action), + // url encode unless it's multibyte (already pre-encoded in the database) + 'URLSegment' => $filter->getAllowMultibyte() ? $action : rawurlencode($action), ])->first(); + if (class_exists('Translatable')) { Translatable::enable_locale_filter(); } diff --git a/code/Controllers/ModelAsController.php b/code/Controllers/ModelAsController.php index bd828ae2..630ce5e1 100644 --- a/code/Controllers/ModelAsController.php +++ b/code/Controllers/ModelAsController.php @@ -16,6 +16,7 @@ use SilverStripe\Core\Injector\Injector; use SilverStripe\Dev\Debug; use SilverStripe\ORM\DataObject; use SilverStripe\ORM\DB; +use SilverStripe\View\Parsers\URLSegmentFilter; use Translatable; /** @@ -127,8 +128,14 @@ class ModelAsController extends Controller implements NestedController Translatable::disable_locale_filter(); } + // url encode unless it's multibyte (already pre-encoded in the database) + $filter = URLSegmentFilter::create(); + if (!$filter->getAllowMultibyte()) { + $URLSegment = rawurlencode($URLSegment); + } + // Select child page - $conditions = array('"SiteTree"."URLSegment"' => rawurlencode($URLSegment)); + $conditions = array('"SiteTree"."URLSegment"' => $URLSegment); if (SiteTree::config()->get('nested_urls')) { $conditions[] = array('"SiteTree"."ParentID"' => 0); } diff --git a/tests/php/Controllers/ModelAsControllerTest.php b/tests/php/Controllers/ModelAsControllerTest.php index 1dfefd16..88fdf01a 100644 --- a/tests/php/Controllers/ModelAsControllerTest.php +++ b/tests/php/Controllers/ModelAsControllerTest.php @@ -11,6 +11,7 @@ use SilverStripe\Control\Director; use SilverStripe\Control\Controller; use SilverStripe\Dev\FunctionalTest; use Page; +use SilverStripe\View\Parsers\URLSegmentFilter; class ModelAsControllerTest extends FunctionalTest { @@ -321,4 +322,37 @@ class ModelAsControllerTest extends FunctionalTest 'The page should not be found since its parent has not been published, in this case http:///root/sub-root or http:///sub-root' ); } + + public function testAllowMultibyte() + { + Config::modify()->set(URLSegmentFilter::class, 'default_allow_multibyte', true); + + $parent = new Page(); + $parent->Title = 'Multibyte test'; + $parent->URLSegment = 'بلاگ'; + $parent->write(); + $parent->copyVersionToStage(Versioned::DRAFT, Versioned::LIVE); + + $child = new Page(); + $child->Title = 'Multibyte test'; + $child->URLSegment = 'فضة'; + $child->ParentID = $parent->ID; + $child->write(); + $child->copyVersionToStage(Versioned::DRAFT, Versioned::LIVE); + + // Emulate browser behaviour around multibyte URL encodings + $response = $this->get(rawurlencode('بلاگ')); + $this->assertEquals( + $response->getStatusCode(), + 200, + 'Routes toplevel paths' + ); + + $response = $this->get(join('/', [rawurlencode('بلاگ'), rawurlencode('فضة')])); + $this->assertEquals( + $response->getStatusCode(), + 200, + 'Routes nested paths' + ); + } } From f8a8b38a0a823a4ce596133f918b343a01eeb05f Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Mon, 4 Feb 2019 22:04:48 +1300 Subject: [PATCH 4/4] Consistent check of allow_multibyte It uses an injected instance everywhere else. See https://github.com/silverstripe/silverstripe-cms/pull/2365#discussion_r247383809 --- code/Model/SiteTree.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/Model/SiteTree.php b/code/Model/SiteTree.php index 0467d6b6..52ff0575 100755 --- a/code/Model/SiteTree.php +++ b/code/Model/SiteTree.php @@ -1842,7 +1842,7 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi $helpText = (self::config()->nested_urls && $this->numChildren()) ? $this->fieldLabel('LinkChangeNote') : ''; - if (!Config::inst()->get('SilverStripe\\View\\Parsers\\URLSegmentFilter', 'default_allow_multibyte')) { + if (!URLSegmentFilter::create()->getAllowMultibyte()) { $helpText .= _t('SilverStripe\\CMS\\Forms\\SiteTreeURLSegmentField.HelpChars', ' Special characters are automatically converted or removed.'); } $urlsegment->setHelpText($helpText);