From 90b800f417fb607460f61ea83b1f92274fb9f1f5 Mon Sep 17 00:00:00 2001
From: Bohdan Korablov <bkorablov@magento.com>
Date: Wed, 19 Oct 2016 18:12:30 +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 | 62 +++++++------------
.../Magento/Framework/Url/SimpleValidator.php | 60 ++++++++++++++++++
.../InstallStoreConfigurationCommand.php | 15 +++--
.../src/Magento/Setup/Controller/UrlCheck.php | 18 +++++-
.../InstallStoreConfigurationCommandTest.php | 17 +++--
.../Test/Unit/Controller/UrlCheckTest.php | 50 ++++++++++-----
6 files changed, 154 insertions(+), 68 deletions(-)
create mode 100644 lib/internal/Magento/Framework/Url/SimpleValidator.php
diff --git a/app/code/Magento/Config/Model/Config/Backend/Baseurl.php b/app/code/Magento/Config/Model/Config/Backend/Baseurl.php
index 75d626e0045..85f0c92e159 100644
--- a/app/code/Magento/Config/Model/Config/Backend/Baseurl.php
+++ b/app/code/Magento/Config/Model/Config/Backend/Baseurl.php
@@ -5,6 +5,9 @@
*/
namespace Magento\Config\Model\Config\Backend;
+use Magento\Framework\Url\SimpleValidator;
+use Magento\Framework\App\ObjectManager;
+
class Baseurl extends \Magento\Framework\App\Config\Value
{
/**
@@ -13,9 +16,9 @@ class Baseurl extends \Magento\Framework\App\Config\Value
protected $_mergeService;
/**
- * @var array
+ * @var SimpleValidator
*/
- private $allowedSchemes = ['http', 'https'];
+ private $simpleUrlValidator;
/**
* @param \Magento\Framework\Model\Context $context
@@ -190,38 +193,6 @@ class Baseurl extends \Magento\Framework\App\Config\Value
}
}
- /**
- * Check that URL contains allowed symbols
- *
- * @param string $value
- * @return void
- * @throws \Magento\Framework\Exception\LocalizedException
- */
- private function validateUrl($value)
- {
- if (!filter_var($value, FILTER_VALIDATE_URL)) {
- throw new \Magento\Framework\Exception\LocalizedException(
- __('Domain Name can contain only letters, digits and hyphen.')
- );
- }
- }
-
- /**
- * Check that scheme there is in list of allowed schemes
- *
- * @param string $value
- * @return void
- * @throws \Magento\Framework\Exception\LocalizedException
- */
- private function validateSchemes($value)
- {
- if (!in_array($value, $this->allowedSchemes)) {
- throw new \Magento\Framework\Exception\LocalizedException(
- __('You can use only following schemes: %1', implode(', ', $this->allowedSchemes))
- );
- }
- }
-
/**
* Whether the provided value can be considered as a fully qualified URL
*
@@ -233,13 +204,8 @@ class Baseurl extends \Magento\Framework\App\Config\Value
{
$url = parse_url($value);
- $result = isset($url['scheme']) && isset($url['host']) && preg_match('/\/$/', $value);
- if ($result) {
- $this->validateSchemes($url['scheme']);
- $this->validateUrl($value);
- }
-
- return $result;
+ return isset($url['scheme']) && isset($url['host']) && preg_match('/\/$/', $value)
+ && $this->getSimpleUrlValidator()->isValid($value);
}
/**
@@ -261,4 +227,18 @@ class Baseurl extends \Magento\Framework\App\Config\Value
}
return parent::afterSave();
}
+
+ /**
+ * Get link interface factory
+ *
+ * @deprecated
+ * @return SimpleValidator
+ */
+ private function getSimpleUrlValidator()
+ {
+ if (!$this->simpleUrlValidator) {
+ $this->simpleUrlValidator = ObjectManager::getInstance()->get(SimpleValidator::class);
+ }
+ return $this->simpleUrlValidator;
+ }
}
diff --git a/lib/internal/Magento/Framework/Url/SimpleValidator.php b/lib/internal/Magento/Framework/Url/SimpleValidator.php
new file mode 100644
index 00000000000..a7ce3b53611
--- /dev/null
+++ b/lib/internal/Magento/Framework/Url/SimpleValidator.php
@@ -0,0 +1,60 @@
+<?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
+ */
+ public function setAllowedSchemes(array $allowedSchemes)
+ {
+ if (!empty($allowedSchemes)) {
+ $this->allowedSchemes = $allowedSchemes;
+ }
+ }
+
+ /**
+ * 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/setup/src/Magento/Setup/Console/Command/InstallStoreConfigurationCommand.php b/setup/src/Magento/Setup/Console/Command/InstallStoreConfigurationCommand.php
index 6ebd387d409..db7db14e866 100644
--- a/setup/src/Magento/Setup/Console/Command/InstallStoreConfigurationCommand.php
+++ b/setup/src/Magento/Setup/Console/Command/InstallStoreConfigurationCommand.php
@@ -229,7 +229,7 @@ class InstallStoreConfigurationCommand extends AbstractSetupCommand
if ($errorMsg !== '') {
$errors[] = $errorMsg;
}
- if (empty($errorMsg) && strpos($value, 'https:') === false) {
+ if (empty($errorMsg) && strpos($value, 'https:') !== 0) {
throw new LocalizedException(new \Magento\Framework\Phrase("Invalid secure URL."));
}
} catch (LocalizedException $e) {
@@ -309,13 +309,20 @@ class InstallStoreConfigurationCommand extends AbstractSetupCommand
*/
private function validateUrl($url, $option)
{
+ /** @var \Magento\Framework\Url\SimpleValidator $validator */
+ $validator = $this->objectManager->get(\Magento\Framework\Url\SimpleValidator::class);
+
$errorMsg = '';
- if (!filter_var($url, FILTER_VALIDATE_URL)) {
+ if (!$validator->isValid($url)) {
+ $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>';
$errorMsg = sprintf(
- '<error>Command option \'%s\': Invalid URL \'%s\'.</error>',
+ $errorTemplate,
$option,
- $url
+ $url,
+ implode(', ', $validator->getAllowedSchemes())
);
}
diff --git a/setup/src/Magento/Setup/Controller/UrlCheck.php b/setup/src/Magento/Setup/Controller/UrlCheck.php
index c27764d37aa..5885b721a9c 100644
--- a/setup/src/Magento/Setup/Controller/UrlCheck.php
+++ b/setup/src/Magento/Setup/Controller/UrlCheck.php
@@ -8,9 +8,23 @@ namespace Magento\Setup\Controller;
use Zend\Mvc\Controller\AbstractActionController;
use Zend\View\Model\JsonModel;
use Zend\Json\Json;
+use Magento\Framework\Url\SimpleValidator;
class UrlCheck extends AbstractActionController
{
+ /**
+ * @var SimpleValidator
+ */
+ private $simpleUrlValidator;
+
+ /**
+ * @param SimpleValidator $simpleUrlValidator
+ */
+ public function __construct(SimpleValidator $simpleUrlValidator)
+ {
+ $this->simpleUrlValidator = $simpleUrlValidator;
+ }
+
/**
* Validate URL
*
@@ -27,13 +41,13 @@ class UrlCheck extends AbstractActionController
$hasSecureFrontUrl = !empty($params['https']['front']);
// Validating of Base URL
- if ($hasBaseUrl && filter_var($params['address']['actual_base_url'], FILTER_VALIDATE_URL)) {
+ if ($hasBaseUrl && $this->simpleUrlValidator->isValid($params['address']['actual_base_url'])) {
$result['successUrl'] = true;
}
// Validating of Secure Base URL
if ($hasSecureAdminUrl || $hasSecureFrontUrl) {
- if (!($hasSecureBaseUrl && filter_var($params['https']['text'], FILTER_VALIDATE_URL))) {
+ if (!($hasSecureBaseUrl && $this->simpleUrlValidator->isValid($params['https']['text']))) {
$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 c9de37ce9d1..0204946dede 100644
--- a/setup/src/Magento/Setup/Test/Unit/Console/Command/InstallStoreConfigurationCommandTest.php
+++ b/setup/src/Magento/Setup/Test/Unit/Console/Command/InstallStoreConfigurationCommandTest.php
@@ -11,6 +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;
/**
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
@@ -104,6 +105,11 @@ 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->expects($this->any())->method('getAllowedSchemes')->willReturn(['http', 'https']);
+
$localeLists= $this->getMock(\Magento\Framework\Validator\Locale::class, [], [], '', false);
$localeLists->expects($this->any())->method('isValid')->will($this->returnValue(false));
$timezoneLists= $this->getMock(\Magento\Framework\Validator\Timezone::class, [], [], '', false);
@@ -112,6 +118,10 @@ class InstallStoreConfigurationCommandTest extends \PHPUnit_Framework_TestCase
$currencyLists->expects($this->any())->method('isValid')->will($this->returnValue(false));
$returnValueMapOM = [
+ [
+ \Magento\Framework\Url\SimpleValidator::class,
+ $validator
+ ],
[
\Magento\Framework\Validator\Locale::class,
$localeLists
@@ -135,7 +145,7 @@ class InstallStoreConfigurationCommandTest extends \PHPUnit_Framework_TestCase
->method('create');
$commandTester = new CommandTester($this->command);
$commandTester->execute($option);
- $this->assertEquals($error . PHP_EOL, $commandTester->getDisplay());
+ $this->assertContains($error, $commandTester->getDisplay());
}
/**
@@ -183,11 +193,6 @@ class InstallStoreConfigurationCommandTest extends \PHPUnit_Framework_TestCase
'Command option \'' . StoreConfigurationDataMapper::KEY_BASE_URL_SECURE
. '\': Invalid secure URL.'
],
- [
- ['--' . StoreConfigurationDataMapper::KEY_BASE_URL_SECURE => 'https://sample.com_test'],
- 'Command option \'' . StoreConfigurationDataMapper::KEY_BASE_URL_SECURE
- . '\': Invalid URL \'https://sample.com_test\'.'
- ],
[
['--' . StoreConfigurationDataMapper::KEY_IS_SECURE_ADMIN => 'invalidValue'],
'Command option \'' . StoreConfigurationDataMapper::KEY_IS_SECURE_ADMIN
diff --git a/setup/src/Magento/Setup/Test/Unit/Controller/UrlCheckTest.php b/setup/src/Magento/Setup/Test/Unit/Controller/UrlCheckTest.php
index bad4ce0f035..e77d3f8466c 100644
--- a/setup/src/Magento/Setup/Test/Unit/Controller/UrlCheckTest.php
+++ b/setup/src/Magento/Setup/Test/Unit/Controller/UrlCheckTest.php
@@ -8,12 +8,13 @@ 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\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper;
class UrlCheckTest extends \PHPUnit_Framework_TestCase
{
/**
- * @param string $requestJson
+ * @param array $requestJson
* @param array $expectedResult
* @dataProvider indexActionDataProvider
*/
@@ -22,14 +23,33 @@ class UrlCheckTest extends \PHPUnit_Framework_TestCase
/** @var ObjectManagerHelper $objectManagerHelper */
$objectManagerHelper = new ObjectManagerHelper($this);
+ $returnMap = [];
+ if (isset($requestJson['address']['actual_base_url'])) {
+ $returnMap[] = [$requestJson['address']['actual_base_url'], $expectedResult['successUrl']];
+ }
+ if (isset($requestJson['https']['text'])) {
+ $returnMap[] = [$requestJson['https']['text'], $expectedResult['successSecureUrl']];
+ }
+
+ /** @var SimpleValidator|\PHPUnit_Framework_MockObject_MockObject $validator */
+ $validator = $this->getMockBuilder(SimpleValidator::class)
+ ->disableOriginalConstructor()
+ ->getMock();
+ $validator->expects($this->any())
+ ->method('isValid')
+ ->willReturnMap($returnMap);
+
/** @var RequestInterface|\PHPUnit_Framework_MockObject_MockObject $requestMock */
$requestMock = $this->getMockBuilder(RequestInterface::class)
->getMockForAbstractClass();
$requestMock->expects($this->once())
->method('getContent')
- ->willReturn($requestJson);
+ ->willReturn(json_encode($requestJson));
- $controller = $objectManagerHelper->getObject(UrlCheck::class);
+ $controller = $objectManagerHelper->getObject(
+ UrlCheck::class,
+ ['simpleUrlValidator' => $validator]
+ );
$objectManagerHelper->setBackwardCompatibleProperty($controller, 'request', $requestMock);
$this->assertEquals(new JsonModel($expectedResult), $controller->indexAction());
@@ -42,23 +62,23 @@ class UrlCheckTest extends \PHPUnit_Framework_TestCase
{
return [
[
- 'requestJson' => json_encode([
+ 'requestJson' => [
'address' => [
'actual_base_url' => 'http://localhost'
]
- ]),
+ ],
'expectedResult' => ['successUrl' => true, 'successSecureUrl' => true]
],
[
- 'requestJson' => json_encode([
+ 'requestJson' => [
'address' => [
'actual_base_url' => 'http://localhost.com_test'
]
- ]),
+ ],
'expectedResult' => ['successUrl' => false, 'successSecureUrl' => true]
],
[
- 'requestJson' => json_encode([
+ 'requestJson' => [
'address' => [
'actual_base_url' => 'http://localhost.com_test'
],
@@ -67,11 +87,11 @@ class UrlCheckTest extends \PHPUnit_Framework_TestCase
'front' => false,
'text' => ''
]
- ]),
+ ],
'expectedResult' => ['successUrl' => false, 'successSecureUrl' => true]
],
[
- 'requestJson' => json_encode([
+ 'requestJson' => [
'address' => [
'actual_base_url' => 'http://localhost.com:8080'
],
@@ -80,11 +100,11 @@ class UrlCheckTest extends \PHPUnit_Framework_TestCase
'front' => false,
'text' => 'https://example.com.ua/'
]
- ]),
+ ],
'expectedResult' => ['successUrl' => true, 'successSecureUrl' => true]
],
[
- 'requestJson' => json_encode([
+ 'requestJson' => [
'address' => [
'actual_base_url' => 'http://localhost.com:8080/folder_name/'
],
@@ -93,11 +113,11 @@ class UrlCheckTest extends \PHPUnit_Framework_TestCase
'front' => true,
'text' => 'https://example.com.ua/'
]
- ]),
+ ],
'expectedResult' => ['successUrl' => true, 'successSecureUrl' => true]
],
[
- 'requestJson' => json_encode([
+ 'requestJson' => [
'address' => [
'actual_base_url' => 'http://localhost.com:8080/folder_name/'
],
@@ -106,7 +126,7 @@ class UrlCheckTest extends \PHPUnit_Framework_TestCase
'front' => true,
'text' => 'https://example.com.ua:8090/folder_name/'
]
- ]),
+ ],
'expectedResult' => ['successUrl' => true, 'successSecureUrl' => true]
],
];
--
GitLab