From 99c7f0f1f509f1d8cfc679b471c9431481da5d57 Mon Sep 17 00:00:00 2001 From: Bohdan Korablov <bkorablov@magento.com> Date: Thu, 20 Oct 2016 12:23:52 +0300 Subject: [PATCH] MAGETWO-56317: [GitHub] -[2.1.0] underscore in site url breaks admin redirect - The page isn't redirecting properly #5809 --- .../Config/Model/Config/Backend/Baseurl.php | 20 ++-- .../Magento/Setup/Controller/UrlCheckTest.php | 98 +++++++++++++++++++ .../Magento/Framework/Url/SimpleValidator.php | 63 ------------ .../Test/Unit/UrlTest.php} | 30 +++--- .../Magento/Framework/Validator/Url.php | 37 +++++++ .../InstallStoreConfigurationCommand.php | 27 +++-- .../src/Magento/Setup/Controller/UrlCheck.php | 17 ++-- .../InstallStoreConfigurationCommandTest.php | 25 +++-- .../Test/Unit/Controller/UrlCheckTest.php | 21 ++-- 9 files changed, 215 insertions(+), 123 deletions(-) create mode 100644 dev/tests/integration/testsuite/Magento/Setup/Controller/UrlCheckTest.php delete mode 100644 lib/internal/Magento/Framework/Url/SimpleValidator.php rename lib/internal/Magento/Framework/{Url/Test/Unit/SimpleValidatorTest.php => Validator/Test/Unit/UrlTest.php} (72%) create mode 100644 lib/internal/Magento/Framework/Validator/Url.php diff --git a/app/code/Magento/Config/Model/Config/Backend/Baseurl.php b/app/code/Magento/Config/Model/Config/Backend/Baseurl.php index 46599f32bcc..f08c2f8e044 100644 --- a/app/code/Magento/Config/Model/Config/Backend/Baseurl.php +++ b/app/code/Magento/Config/Model/Config/Backend/Baseurl.php @@ -5,7 +5,7 @@ */ namespace Magento\Config\Model\Config\Backend; -use Magento\Framework\Url\SimpleValidator; +use Magento\Framework\Validator\Url as UrlValidator; use Magento\Framework\App\ObjectManager; class Baseurl extends \Magento\Framework\App\Config\Value @@ -16,9 +16,9 @@ class Baseurl extends \Magento\Framework\App\Config\Value protected $_mergeService; /** - * @var SimpleValidator + * @var UrlValidator */ - private $simpleUrlValidator; + private $urlValidator; /** * @param \Magento\Framework\Model\Context $context @@ -201,7 +201,7 @@ class Baseurl extends \Magento\Framework\App\Config\Value */ private function _isFullyQualifiedUrl($value) { - return preg_match('/\/$/', $value) && $this->getSimpleUrlValidator()->isValid($value); + return preg_match('/\/$/', $value) && $this->getUrlValidator()->isValid($value, ['http', 'https']); } /** @@ -225,16 +225,16 @@ class Baseurl extends \Magento\Framework\App\Config\Value } /** - * Get Simple URL Validator + * Get URL Validator * * @deprecated - * @return SimpleValidator + * @return UrlValidator */ - private function getSimpleUrlValidator() + private function getUrlValidator() { - if (!$this->simpleUrlValidator) { - $this->simpleUrlValidator = ObjectManager::getInstance()->get(SimpleValidator::class); + if (!$this->urlValidator) { + $this->urlValidator = ObjectManager::getInstance()->get(UrlValidator::class); } - return $this->simpleUrlValidator; + return $this->urlValidator; } } diff --git a/dev/tests/integration/testsuite/Magento/Setup/Controller/UrlCheckTest.php b/dev/tests/integration/testsuite/Magento/Setup/Controller/UrlCheckTest.php new file mode 100644 index 00000000000..6fe9ec2ee3e --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/Setup/Controller/UrlCheckTest.php @@ -0,0 +1,98 @@ +<?php +/** + * Copyright © 2016 Magento. All rights reserved. + * See COPYING.txt for license details. + */ +namespace Magento\Setup\Controller; + +use Magento\TestFramework\Helper\Bootstrap; +use Zend\Stdlib\RequestInterface as Request; +use Zend\View\Model\JsonModel; + +class UrlCheckTest extends \PHPUnit_Framework_TestCase +{ + /** + * @var UrlCheck + */ + private $controller; + + protected function setUp() + { + $this->controller = Bootstrap::getObjectManager()->create(UrlCheck::class); + } + + /** + * @param array $requestContent + * @param bool $successUrl + * @param bool $successSecureUrl + * @return void + * @dataProvider indexActionDataProvider + */ + public function testIndexAction($requestContent, $successUrl, $successSecureUrl) + { + $requestMock = $this->getMockBuilder(Request::class) + ->getMockForAbstractClass(); + $requestMock->expects($this->once()) + ->method('getContent') + ->willReturn(json_encode($requestContent)); + + $requestProperty = new \ReflectionProperty(get_class($this->controller), 'request'); + $requestProperty->setAccessible(true); + $requestProperty->setValue($this->controller, $requestMock); + + $resultModel = new JsonModel(['successUrl' => $successUrl, 'successSecureUrl' => $successSecureUrl]); + + $this->assertEquals($resultModel, $this->controller->indexAction()); + } + + /** + * @return array + */ + public function indexActionDataProvider() + { + return [ + [ + 'requestContent' => [ + 'address' => [ + 'actual_base_url' => 'http://example.com/' + ], + 'https' => [ + 'text' => 'https://example.com/', + 'admin' => true, + 'front' => false + ], + ], + 'successUrl' => true, + 'successSecureUrl' => true + ], + [ + 'requestContent' => [ + 'address' => [ + 'actual_base_url' => 'http://example.com/folder/' + ], + 'https' => [ + 'text' => 'https://example.com/folder_name/', + 'admin' => false, + 'front' => true + ], + ], + 'successUrl' => true, + 'successSecureUrl' => true + ], + [ + 'requestContent' => [ + 'address' => [ + 'actual_base_url' => 'ftp://example.com/' + ], + 'https' => [ + 'text' => 'https://example.com_test/', + 'admin' => true, + 'front' => true + ], + ], + 'successUrl' => false, + 'successSecureUrl' => false + ], + ]; + } +} diff --git a/lib/internal/Magento/Framework/Url/SimpleValidator.php b/lib/internal/Magento/Framework/Url/SimpleValidator.php deleted file mode 100644 index 298a90a44e4..00000000000 --- a/lib/internal/Magento/Framework/Url/SimpleValidator.php +++ /dev/null @@ -1,63 +0,0 @@ -<?php -/** - * Copyright © 2016 Magento. All rights reserved. - * See COPYING.txt for license details. - */ -namespace Magento\Framework\Url; - -class SimpleValidator -{ - /** - * @var array - */ - private $allowedSchemes = ['http', 'https']; - - /** - * Set allowed Schemes - * - * @param array $allowedSchemes - * @return $this - */ - public function setAllowedSchemes(array $allowedSchemes) - { - if (!empty($allowedSchemes)) { - $this->allowedSchemes = $allowedSchemes; - } - - return $this; - } - - /** - * Get allowed Schemes - * - * @return array - */ - public function getAllowedSchemes() - { - return $this->allowedSchemes; - } - - /** - * Check that URL contains allowed symbols and use allowed scheme - * - * @param string $value - * @return bool - */ - public function isValid($value) - { - $isValid = true; - - // Check that URL contains allowed symbols - if (!filter_var($value, FILTER_VALIDATE_URL)) { - $isValid = false; - } - - // Check that scheme there is in list of allowed schemes - $url = parse_url($value); - if (empty($url['scheme']) || !in_array($url['scheme'], $this->allowedSchemes)) { - $isValid = false; - } - - return $isValid; - } -} diff --git a/lib/internal/Magento/Framework/Url/Test/Unit/SimpleValidatorTest.php b/lib/internal/Magento/Framework/Validator/Test/Unit/UrlTest.php similarity index 72% rename from lib/internal/Magento/Framework/Url/Test/Unit/SimpleValidatorTest.php rename to lib/internal/Magento/Framework/Validator/Test/Unit/UrlTest.php index 39b1334702d..d66f8428942 100644 --- a/lib/internal/Magento/Framework/Url/Test/Unit/SimpleValidatorTest.php +++ b/lib/internal/Magento/Framework/Validator/Test/Unit/UrlTest.php @@ -3,20 +3,20 @@ * Copyright © 2016 Magento. All rights reserved. * See COPYING.txt for license details. */ -namespace Magento\Framework\Url\Test\Unit; +namespace Magento\Framework\Validator\Test\Unit; -use Magento\Framework\Url\SimpleValidator; +use Magento\Framework\Validator\Url as UrlValidator; -class SimpleValidatorTest extends \PHPUnit_Framework_TestCase +class UrlTest extends \PHPUnit_Framework_TestCase { /** - * @var SimpleValidator + * @var UrlValidator */ private $validator; protected function setUp() { - $this->validator = new SimpleValidator(); + $this->validator = new UrlValidator(); } /** @@ -27,8 +27,7 @@ class SimpleValidatorTest extends \PHPUnit_Framework_TestCase */ public function testIsValid(array $allowedSchemes, $url, $expectedResult) { - $this->validator->setAllowedSchemes($allowedSchemes); - $this->assertSame($expectedResult, $this->validator->isValid($url)); + $this->assertSame($expectedResult, $this->validator->isValid($url, $allowedSchemes)); } /** @@ -42,29 +41,34 @@ class SimpleValidatorTest extends \PHPUnit_Framework_TestCase 'url' => 'http://example.com', 'expectedResult' => true, ], + [ + 'allowedSchemes' => ['http'], + 'url' => 'http://example.com', + 'expectedResult' => true, + ], [ 'allowedSchemes' => [], 'url' => 'https://example.com', 'expectedResult' => true, ], [ - 'allowedSchemes' => [], - 'url' => 'http://example.com_test', - 'expectedResult' => false, + 'allowedSchemes' => ['https'], + 'url' => 'https://example.com', + 'expectedResult' => true, ], [ 'allowedSchemes' => [], - 'url' => 'ftp://example.com', + 'url' => 'http://example.com_test', 'expectedResult' => false, ], [ - 'allowedSchemes' => ['ftp'], + 'allowedSchemes' => [], 'url' => 'ftp://example.com', 'expectedResult' => true, ], [ 'allowedSchemes' => ['ftp'], - 'url' => 'ftp://example.com_test', + 'url' => 'ftp://example.com', 'expectedResult' => true, ], ]; diff --git a/lib/internal/Magento/Framework/Validator/Url.php b/lib/internal/Magento/Framework/Validator/Url.php new file mode 100644 index 00000000000..aacccba6221 --- /dev/null +++ b/lib/internal/Magento/Framework/Validator/Url.php @@ -0,0 +1,37 @@ +<?php +/** + * Copyright © 2016 Magento. All rights reserved. + * See COPYING.txt for license details. + */ +namespace Magento\Framework\Validator; + +/** + * Class Url validates URL and checks that it has allowed scheme + */ +class Url +{ + /** + * Validate URL and check that it has allowed scheme + * + * @param string $value + * @param array $allowedSchemes + * @return bool + */ + public function isValid($value, array $allowedSchemes = []) + { + $isValid = true; + + if (!filter_var($value, FILTER_VALIDATE_URL)) { + $isValid = false; + } + + if (!empty($allowedSchemes)) { + $url = parse_url($value); + if (empty($url['scheme']) || !in_array($url['scheme'], $allowedSchemes)) { + $isValid = false; + } + } + + return $isValid; + } +} diff --git a/setup/src/Magento/Setup/Console/Command/InstallStoreConfigurationCommand.php b/setup/src/Magento/Setup/Console/Command/InstallStoreConfigurationCommand.php index db7db14e866..253c731d1c2 100644 --- a/setup/src/Magento/Setup/Console/Command/InstallStoreConfigurationCommand.php +++ b/setup/src/Magento/Setup/Console/Command/InstallStoreConfigurationCommand.php @@ -20,6 +20,7 @@ use Magento\Store\Model\Store; use Magento\Framework\Validator\Locale; use Magento\Framework\Validator\Timezone; use Magento\Framework\Validator\Currency; +use Magento\Framework\Validator\Url; /** * @SuppressWarnings(PHPMD.CouplingBetweenObjects) @@ -182,7 +183,11 @@ class InstallStoreConfigurationCommand extends AbstractSetupCommand if (strcmp($value, '{{base_url}}') == 0) { break; } - $errorMsg = $this->validateUrl($value, StoreConfigurationDataMapper::KEY_BASE_URL); + $errorMsg = $this->validateUrl( + $value, + StoreConfigurationDataMapper::KEY_BASE_URL, + ['http', 'https'] + ); if ($errorMsg !== '') { $errors[] = $errorMsg; } @@ -225,13 +230,14 @@ class InstallStoreConfigurationCommand extends AbstractSetupCommand break; case StoreConfigurationDataMapper::KEY_BASE_URL_SECURE: try { - $errorMsg = $this->validateUrl($value, StoreConfigurationDataMapper::KEY_BASE_URL_SECURE); + $errorMsg = $this->validateUrl( + $value, + StoreConfigurationDataMapper::KEY_BASE_URL_SECURE, + ['https'] + ); if ($errorMsg !== '') { $errors[] = $errorMsg; } - if (empty($errorMsg) && strpos($value, 'https:') !== 0) { - throw new LocalizedException(new \Magento\Framework\Phrase("Invalid secure URL.")); - } } catch (LocalizedException $e) { $errors[] = '<error>' . 'Command option \'' . StoreConfigurationDataMapper::KEY_BASE_URL_SECURE . '\': ' . $e->getLogMessage() .'</error>'; @@ -305,16 +311,17 @@ class InstallStoreConfigurationCommand extends AbstractSetupCommand * * @param string $url * @param string $option + * @param array $allowedSchemes * @return string */ - private function validateUrl($url, $option) + private function validateUrl($url, $option, array $allowedSchemes) { - /** @var \Magento\Framework\Url\SimpleValidator $validator */ - $validator = $this->objectManager->get(\Magento\Framework\Url\SimpleValidator::class); + /** @var Url $validator */ + $validator = $this->objectManager->get(Url::class); $errorMsg = ''; - if (!$validator->isValid($url)) { + if (!$validator->isValid($url, $allowedSchemes)) { $errorTemplate = '<error>Command option \'%s\': Invalid URL \'%s\'.' . ' Domain Name should contain only letters, digits and hyphen.' . ' And you should use only following schemes: \'%s\'.</error>'; @@ -322,7 +329,7 @@ class InstallStoreConfigurationCommand extends AbstractSetupCommand $errorTemplate, $option, $url, - implode(', ', $validator->getAllowedSchemes()) + implode(', ', $allowedSchemes) ); } diff --git a/setup/src/Magento/Setup/Controller/UrlCheck.php b/setup/src/Magento/Setup/Controller/UrlCheck.php index b0fb1ac7296..08a0d50404c 100644 --- a/setup/src/Magento/Setup/Controller/UrlCheck.php +++ b/setup/src/Magento/Setup/Controller/UrlCheck.php @@ -8,21 +8,21 @@ namespace Magento\Setup\Controller; use Zend\Mvc\Controller\AbstractActionController; use Zend\View\Model\JsonModel; use Zend\Json\Json; -use Magento\Framework\Url\SimpleValidator; +use Magento\Framework\Validator\Url as UrlValidator; class UrlCheck extends AbstractActionController { /** - * @var SimpleValidator + * @var UrlValidator */ - private $simpleUrlValidator; + private $urlValidator; /** - * @param SimpleValidator $simpleUrlValidator + * @param UrlValidator $urlValidator */ - public function __construct(SimpleValidator $simpleUrlValidator) + public function __construct(UrlValidator $urlValidator) { - $this->simpleUrlValidator = $simpleUrlValidator; + $this->urlValidator = $urlValidator; } /** @@ -39,15 +39,16 @@ class UrlCheck extends AbstractActionController $hasSecureBaseUrl = isset($params['https']['text']); $hasSecureAdminUrl = !empty($params['https']['admin']); $hasSecureFrontUrl = !empty($params['https']['front']); + $schemes = ['http', 'https']; // Validating of Base URL - if ($hasBaseUrl && $this->simpleUrlValidator->isValid($params['address']['actual_base_url'])) { + if ($hasBaseUrl && $this->urlValidator->isValid($params['address']['actual_base_url'], $schemes)) { $result['successUrl'] = true; } // Validating of Secure Base URL if ($hasSecureAdminUrl || $hasSecureFrontUrl) { - if (!($hasSecureBaseUrl && $this->simpleUrlValidator->isValid($params['https']['text']))) { + if (!($hasSecureBaseUrl && $this->urlValidator->isValid($params['https']['text'], $schemes))) { $result['successSecureUrl'] = false; } } diff --git a/setup/src/Magento/Setup/Test/Unit/Console/Command/InstallStoreConfigurationCommandTest.php b/setup/src/Magento/Setup/Test/Unit/Console/Command/InstallStoreConfigurationCommandTest.php index 0204946dede..63855509e18 100644 --- a/setup/src/Magento/Setup/Test/Unit/Console/Command/InstallStoreConfigurationCommandTest.php +++ b/setup/src/Magento/Setup/Test/Unit/Console/Command/InstallStoreConfigurationCommandTest.php @@ -11,7 +11,7 @@ use Symfony\Component\Console\Tester\CommandTester; use Magento\Setup\Model\Installer; use Magento\Framework\ObjectManagerInterface; use Magento\Setup\Model\StoreConfigurationDataMapper; -use Magento\Framework\Url\ExtendedValidator; +use Magento\Framework\Validator\Url as UrlValidator; /** * @SuppressWarnings(PHPMD.CouplingBetweenObjects) @@ -105,9 +105,8 @@ class InstallStoreConfigurationCommandTest extends \PHPUnit_Framework_TestCase */ public function testExecuteInvalidData(array $option, $error) { - $isSecureUrl = isset($option['--' . StoreConfigurationDataMapper::KEY_BASE_URL_SECURE]); - $validator = $this->getMock(\Magento\Framework\Url\SimpleValidator::class, [], [], '', false); - $validator->expects($this->any())->method('isValid')->willReturn($isSecureUrl); + $validator = $this->getMock(UrlValidator::class, [], [], '', false); + $validator->expects($this->any())->method('isValid')->willReturn(false); $validator->expects($this->any())->method('getAllowedSchemes')->willReturn(['http', 'https']); $localeLists= $this->getMock(\Magento\Framework\Validator\Locale::class, [], [], '', false); @@ -119,7 +118,7 @@ class InstallStoreConfigurationCommandTest extends \PHPUnit_Framework_TestCase $returnValueMapOM = [ [ - \Magento\Framework\Url\SimpleValidator::class, + UrlValidator::class, $validator ], [ @@ -166,42 +165,42 @@ class InstallStoreConfigurationCommandTest extends \PHPUnit_Framework_TestCase [ ['--' . StoreConfigurationDataMapper::KEY_LANGUAGE => 'sampleLanguage'], 'Command option \'' . StoreConfigurationDataMapper::KEY_LANGUAGE - . '\': Invalid value. To see possible values, run command \'bin/magento info:language:list\'.' + . '\': Invalid value. To see possible values, run command \'bin/magento info:language:list\'.' ], [ ['--' . StoreConfigurationDataMapper::KEY_TIMEZONE => 'sampleTimezone'], 'Command option \'' . StoreConfigurationDataMapper::KEY_TIMEZONE - . '\': Invalid value. To see possible values, run command \'bin/magento info:timezone:list\'.' + . '\': Invalid value. To see possible values, run command \'bin/magento info:timezone:list\'.' ], [ ['--' . StoreConfigurationDataMapper::KEY_CURRENCY => 'sampleLanguage'], 'Command option \'' . StoreConfigurationDataMapper::KEY_CURRENCY - . '\': Invalid value. To see possible values, run command \'bin/magento info:currency:list\'.' + . '\': Invalid value. To see possible values, run command \'bin/magento info:currency:list\'.' ], [ ['--' . StoreConfigurationDataMapper::KEY_USE_SEF_URL => 'invalidValue'], 'Command option \'' . StoreConfigurationDataMapper::KEY_USE_SEF_URL - . '\': Invalid value. Possible values (0|1).' + . '\': Invalid value. Possible values (0|1).' ], [ ['--' . StoreConfigurationDataMapper::KEY_IS_SECURE => 'invalidValue'], 'Command option \'' . StoreConfigurationDataMapper::KEY_IS_SECURE - . '\': Invalid value. Possible values (0|1).' + . '\': Invalid value. Possible values (0|1).' ], [ ['--' . StoreConfigurationDataMapper::KEY_BASE_URL_SECURE => 'http://www.sample.com'], 'Command option \'' . StoreConfigurationDataMapper::KEY_BASE_URL_SECURE - . '\': Invalid secure URL.' + . '\': Invalid URL \'http://www.sample.com\'.' ], [ ['--' . StoreConfigurationDataMapper::KEY_IS_SECURE_ADMIN => 'invalidValue'], 'Command option \'' . StoreConfigurationDataMapper::KEY_IS_SECURE_ADMIN - . '\': Invalid value. Possible values (0|1).' + . '\': Invalid value. Possible values (0|1).' ], [ ['--' . StoreConfigurationDataMapper::KEY_ADMIN_USE_SECURITY_KEY => 'invalidValue'], 'Command option \'' . StoreConfigurationDataMapper::KEY_ADMIN_USE_SECURITY_KEY - . '\': Invalid value. Possible values (0|1).' + . '\': Invalid value. Possible values (0|1).' ], ]; diff --git a/setup/src/Magento/Setup/Test/Unit/Controller/UrlCheckTest.php b/setup/src/Magento/Setup/Test/Unit/Controller/UrlCheckTest.php index e77d3f8466c..0b44887b586 100644 --- a/setup/src/Magento/Setup/Test/Unit/Controller/UrlCheckTest.php +++ b/setup/src/Magento/Setup/Test/Unit/Controller/UrlCheckTest.php @@ -8,7 +8,7 @@ namespace Magento\Setup\Test\Unit\Controller; use Magento\Setup\Controller\UrlCheck; use Zend\Stdlib\RequestInterface; use Zend\View\Model\JsonModel; -use Magento\Framework\Url\SimpleValidator; +use Magento\Framework\Validator\Url as UrlValidator; use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper; class UrlCheckTest extends \PHPUnit_Framework_TestCase @@ -23,16 +23,25 @@ class UrlCheckTest extends \PHPUnit_Framework_TestCase /** @var ObjectManagerHelper $objectManagerHelper */ $objectManagerHelper = new ObjectManagerHelper($this); + $allowedSchemes = ['http', 'https']; $returnMap = []; if (isset($requestJson['address']['actual_base_url'])) { - $returnMap[] = [$requestJson['address']['actual_base_url'], $expectedResult['successUrl']]; + $returnMap[] = [ + $requestJson['address']['actual_base_url'], + $allowedSchemes, + $expectedResult['successUrl'], + ]; } if (isset($requestJson['https']['text'])) { - $returnMap[] = [$requestJson['https']['text'], $expectedResult['successSecureUrl']]; + $returnMap[] = [ + $requestJson['https']['text'], + $allowedSchemes, + $expectedResult['successSecureUrl'], + ]; } - /** @var SimpleValidator|\PHPUnit_Framework_MockObject_MockObject $validator */ - $validator = $this->getMockBuilder(SimpleValidator::class) + /** @var UrlValidator|\PHPUnit_Framework_MockObject_MockObject $validator */ + $validator = $this->getMockBuilder(UrlValidator::class) ->disableOriginalConstructor() ->getMock(); $validator->expects($this->any()) @@ -48,7 +57,7 @@ class UrlCheckTest extends \PHPUnit_Framework_TestCase $controller = $objectManagerHelper->getObject( UrlCheck::class, - ['simpleUrlValidator' => $validator] + ['urlValidator' => $validator] ); $objectManagerHelper->setBackwardCompatibleProperty($controller, 'request', $requestMock); -- GitLab