From 5c3b95ac8977f77e7d95d4da6333ca12b5ef465f Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Mon, 14 Jan 2019 09:17:06 +1300 Subject: [PATCH 1/2] 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 2/2] 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);