diff --git a/app/code/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/DeleteFiles.php b/app/code/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/DeleteFiles.php index 19dc989620b89..890c9bf5eae52 100644 --- a/app/code/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/DeleteFiles.php +++ b/app/code/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/DeleteFiles.php @@ -7,6 +7,9 @@ use Magento\Framework\App\Filesystem\DirectoryList; +/** + * Delete image files. + */ class DeleteFiles extends \Magento\Cms\Controller\Adminhtml\Wysiwyg\Images { /** @@ -19,6 +22,11 @@ class DeleteFiles extends \Magento\Cms\Controller\Adminhtml\Wysiwyg\Images */ protected $resultRawFactory; + /** + * @var \Magento\Framework\App\Filesystem\DirectoryResolver + */ + private $directoryResolver; + /** * Constructor * @@ -26,22 +34,28 @@ class DeleteFiles extends \Magento\Cms\Controller\Adminhtml\Wysiwyg\Images * @param \Magento\Framework\Registry $coreRegistry * @param \Magento\Framework\Controller\Result\JsonFactory $resultJsonFactory * @param \Magento\Framework\Controller\Result\RawFactory $resultRawFactory + * @param \Magento\Framework\App\Filesystem\DirectoryResolver|null $directoryResolver */ public function __construct( \Magento\Backend\App\Action\Context $context, \Magento\Framework\Registry $coreRegistry, \Magento\Framework\Controller\Result\JsonFactory $resultJsonFactory, - \Magento\Framework\Controller\Result\RawFactory $resultRawFactory + \Magento\Framework\Controller\Result\RawFactory $resultRawFactory, + \Magento\Framework\App\Filesystem\DirectoryResolver $directoryResolver = null ) { + parent::__construct($context, $coreRegistry); + $this->resultRawFactory = $resultRawFactory; $this->resultJsonFactory = $resultJsonFactory; - parent::__construct($context, $coreRegistry); + $this->directoryResolver = $directoryResolver + ?: $this->_objectManager->get(\Magento\Framework\App\Filesystem\DirectoryResolver::class); } /** - * Delete file from media storage + * Delete file from media storage. * * @return \Magento\Framework\Controller\ResultInterface + * @throws \Magento\Framework\Exception\LocalizedException */ public function execute() { @@ -54,6 +68,11 @@ public function execute() /** @var $helper \Magento\Cms\Helper\Wysiwyg\Images */ $helper = $this->_objectManager->get(\Magento\Cms\Helper\Wysiwyg\Images::class); $path = $this->getStorage()->getSession()->getCurrentPath(); + if (!$this->directoryResolver->validatePath($path, DirectoryList::MEDIA)) { + throw new \Magento\Framework\Exception\LocalizedException( + __('Directory %1 is not under storage root path.', $path) + ); + } foreach ($files as $file) { $file = $helper->idDecode($file); /** @var \Magento\Framework\Filesystem $filesystem */ @@ -64,11 +83,13 @@ public function execute() $this->getStorage()->deleteFile($filePath); } } + return $this->resultRawFactory->create(); } catch (\Exception $e) { $result = ['error' => true, 'message' => $e->getMessage()]; /** @var \Magento\Framework\Controller\Result\Json $resultJson */ $resultJson = $this->resultJsonFactory->create(); + return $resultJson->setData($result); } } diff --git a/app/code/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/DeleteFolder.php b/app/code/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/DeleteFolder.php index 8a89de87a6f85..a1de11c3c462e 100644 --- a/app/code/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/DeleteFolder.php +++ b/app/code/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/DeleteFolder.php @@ -6,6 +6,11 @@ */ namespace Magento\Cms\Controller\Adminhtml\Wysiwyg\Images; +use Magento\Framework\App\Filesystem\DirectoryList; + +/** + * Delete image folder. + */ class DeleteFolder extends \Magento\Cms\Controller\Adminhtml\Wysiwyg\Images { /** @@ -18,38 +23,55 @@ class DeleteFolder extends \Magento\Cms\Controller\Adminhtml\Wysiwyg\Images */ protected $resultRawFactory; + /** + * @var \Magento\Framework\App\Filesystem\DirectoryResolver + */ + private $directoryResolver; + /** * @param \Magento\Backend\App\Action\Context $context * @param \Magento\Framework\Registry $coreRegistry * @param \Magento\Framework\Controller\Result\JsonFactory $resultJsonFactory * @param \Magento\Framework\Controller\Result\RawFactory $resultRawFactory + * @param \Magento\Framework\App\Filesystem\DirectoryResolver|null $directoryResolver */ public function __construct( \Magento\Backend\App\Action\Context $context, \Magento\Framework\Registry $coreRegistry, \Magento\Framework\Controller\Result\JsonFactory $resultJsonFactory, - \Magento\Framework\Controller\Result\RawFactory $resultRawFactory + \Magento\Framework\Controller\Result\RawFactory $resultRawFactory, + \Magento\Framework\App\Filesystem\DirectoryResolver $directoryResolver = null ) { + parent::__construct($context, $coreRegistry); $this->resultRawFactory = $resultRawFactory; $this->resultJsonFactory = $resultJsonFactory; - parent::__construct($context, $coreRegistry); + $this->directoryResolver = $directoryResolver + ?: $this->_objectManager->get(\Magento\Framework\App\Filesystem\DirectoryResolver::class); } /** - * Delete folder action + * Delete folder action. * * @return \Magento\Framework\Controller\ResultInterface + * @throws \Magento\Framework\Exception\LocalizedException */ public function execute() { try { $path = $this->getStorage()->getCmsWysiwygImages()->getCurrentPath(); + if (!$this->directoryResolver->validatePath($path, DirectoryList::MEDIA)) { + throw new \Magento\Framework\Exception\LocalizedException( + __('Directory %1 is not under storage root path.', $path) + ); + } $this->getStorage()->deleteDirectory($path); + return $this->resultRawFactory->create(); } catch (\Exception $e) { $result = ['error' => true, 'message' => $e->getMessage()]; /** @var \Magento\Framework\Controller\Result\Json $resultJson */ $resultJson = $this->resultJsonFactory->create(); + return $resultJson->setData($result); } } diff --git a/app/code/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/NewFolder.php b/app/code/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/NewFolder.php index 2124bdabe6009..a7f49e8a431a4 100644 --- a/app/code/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/NewFolder.php +++ b/app/code/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/NewFolder.php @@ -6,6 +6,11 @@ */ namespace Magento\Cms\Controller\Adminhtml\Wysiwyg\Images; +use Magento\Framework\App\Filesystem\DirectoryList; + +/** + * Creates new folder. + */ class NewFolder extends \Magento\Cms\Controller\Adminhtml\Wysiwyg\Images { /** @@ -13,24 +18,34 @@ class NewFolder extends \Magento\Cms\Controller\Adminhtml\Wysiwyg\Images */ protected $resultJsonFactory; + /** + * @var \Magento\Framework\App\Filesystem\DirectoryResolver + */ + private $directoryResolver; + /** * @param \Magento\Backend\App\Action\Context $context * @param \Magento\Framework\Registry $coreRegistry * @param \Magento\Framework\Controller\Result\JsonFactory $resultJsonFactory + * @param \Magento\Framework\App\Filesystem\DirectoryResolver|null $directoryResolver */ public function __construct( \Magento\Backend\App\Action\Context $context, \Magento\Framework\Registry $coreRegistry, - \Magento\Framework\Controller\Result\JsonFactory $resultJsonFactory + \Magento\Framework\Controller\Result\JsonFactory $resultJsonFactory, + \Magento\Framework\App\Filesystem\DirectoryResolver $directoryResolver = null ) { - $this->resultJsonFactory = $resultJsonFactory; parent::__construct($context, $coreRegistry); + $this->resultJsonFactory = $resultJsonFactory; + $this->directoryResolver = $directoryResolver + ?: $this->_objectManager->get(\Magento\Framework\App\Filesystem\DirectoryResolver::class); } /** - * New folder action + * New folder action. * * @return \Magento\Framework\Controller\ResultInterface + * @throws \Magento\Framework\Exception\LocalizedException */ public function execute() { @@ -38,12 +53,18 @@ public function execute() $this->_initAction(); $name = $this->getRequest()->getPost('name'); $path = $this->getStorage()->getSession()->getCurrentPath(); + if (!$this->directoryResolver->validatePath($path, DirectoryList::MEDIA)) { + throw new \Magento\Framework\Exception\LocalizedException( + __('Directory %1 is not under storage root path.', $path) + ); + } $result = $this->getStorage()->createDirectory($name, $path); } catch (\Exception $e) { $result = ['error' => true, 'message' => $e->getMessage()]; } /** @var \Magento\Framework\Controller\Result\Json $resultJson */ $resultJson = $this->resultJsonFactory->create(); + return $resultJson->setData($result); } } diff --git a/app/code/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/Upload.php b/app/code/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/Upload.php index 7a94c4ab6aa12..5c9aa2243bc6d 100644 --- a/app/code/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/Upload.php +++ b/app/code/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/Upload.php @@ -6,6 +6,11 @@ */ namespace Magento\Cms\Controller\Adminhtml\Wysiwyg\Images; +use Magento\Framework\App\Filesystem\DirectoryList; + +/** + * Upload image. + */ class Upload extends \Magento\Cms\Controller\Adminhtml\Wysiwyg\Images { /** @@ -13,36 +18,52 @@ class Upload extends \Magento\Cms\Controller\Adminhtml\Wysiwyg\Images */ protected $resultJsonFactory; + /** + * @var \Magento\Framework\App\Filesystem\DirectoryResolver + */ + private $directoryResolver; + /** * @param \Magento\Backend\App\Action\Context $context * @param \Magento\Framework\Registry $coreRegistry * @param \Magento\Framework\Controller\Result\JsonFactory $resultJsonFactory + * @param \Magento\Framework\App\Filesystem\DirectoryResolver|null $directoryResolver */ public function __construct( \Magento\Backend\App\Action\Context $context, \Magento\Framework\Registry $coreRegistry, - \Magento\Framework\Controller\Result\JsonFactory $resultJsonFactory + \Magento\Framework\Controller\Result\JsonFactory $resultJsonFactory, + \Magento\Framework\App\Filesystem\DirectoryResolver $directoryResolver = null ) { - $this->resultJsonFactory = $resultJsonFactory; parent::__construct($context, $coreRegistry); + $this->resultJsonFactory = $resultJsonFactory; + $this->directoryResolver = $directoryResolver + ?: $this->_objectManager->get(\Magento\Framework\App\Filesystem\DirectoryResolver::class); } /** - * Files upload processing + * Files upload processing. * * @return \Magento\Framework\Controller\ResultInterface + * @throws \Magento\Framework\Exception\LocalizedException */ public function execute() { try { $this->_initAction(); - $targetPath = $this->getStorage()->getSession()->getCurrentPath(); - $result = $this->getStorage()->uploadFile($targetPath, $this->getRequest()->getParam('type')); + $path = $this->getStorage()->getSession()->getCurrentPath(); + if (!$this->directoryResolver->validatePath($path, DirectoryList::MEDIA)) { + throw new \Magento\Framework\Exception\LocalizedException( + __('Directory %1 is not under storage root path.', $path) + ); + } + $result = $this->getStorage()->uploadFile($path, $this->getRequest()->getParam('type')); } catch (\Exception $e) { $result = ['error' => $e->getMessage(), 'errorcode' => $e->getCode()]; } /** @var \Magento\Framework\Controller\Result\Json $resultJson */ $resultJson = $this->resultJsonFactory->create(); + return $resultJson->setData($result); } } diff --git a/app/code/Magento/Cms/Helper/Wysiwyg/Images.php b/app/code/Magento/Cms/Helper/Wysiwyg/Images.php index 3e410799906a1..b055572795210 100644 --- a/app/code/Magento/Cms/Helper/Wysiwyg/Images.php +++ b/app/code/Magento/Cms/Helper/Wysiwyg/Images.php @@ -8,7 +8,9 @@ use Magento\Framework\App\Filesystem\DirectoryList; /** - * Wysiwyg Images Helper + * Wysiwyg Images Helper. + * + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class Images extends \Magento\Framework\App\Helper\AbstractHelper { @@ -156,17 +158,23 @@ public function convertPathToId($path) } /** - * Decode HTML element id + * Decode HTML element id. * * @param string $id * @return string + * @throws \InvalidArgumentException When path contains restricted symbols. */ public function convertIdToPath($id) { if ($id === \Magento\Theme\Helper\Storage::NODE_ROOT) { return $this->getStorageRoot(); } else { - return $this->getStorageRoot() . $this->idDecode($id); + $path = $this->getStorageRoot() . $this->idDecode($id); + if (preg_match('/\.\.(\\\|\/)/', $path)) { + throw new \InvalidArgumentException('Path is invalid'); + } + + return $path; } } diff --git a/app/code/Magento/Cms/Test/Unit/Helper/Wysiwyg/ImagesTest.php b/app/code/Magento/Cms/Test/Unit/Helper/Wysiwyg/ImagesTest.php index fc5eaf998c748..c434808d0b03f 100644 --- a/app/code/Magento/Cms/Test/Unit/Helper/Wysiwyg/ImagesTest.php +++ b/app/code/Magento/Cms/Test/Unit/Helper/Wysiwyg/ImagesTest.php @@ -110,7 +110,7 @@ protected function setUp() ->willReturnMap( [ [WysiwygConfig::IMAGE_DIRECTORY, null, $this->getAbsolutePath(WysiwygConfig::IMAGE_DIRECTORY)], - [null, null, $this->getAbsolutePath(null)] + [null, null, $this->getAbsolutePath(null)], ] ); @@ -124,7 +124,7 @@ protected function setUp() [ 'clearWebsiteCache', 'getDefaultStoreView', 'getGroup', 'getGroups', 'getStore', 'getStores', 'getWebsite', 'getWebsites', 'hasSingleStore', - 'isSingleStoreMode', 'reinitStores', 'setCurrentStore', 'setIsSingleStoreModeAllowed' + 'isSingleStoreMode', 'reinitStores', 'setCurrentStore', 'setIsSingleStoreModeAllowed', ] ) ->disableOriginalConstructor() @@ -229,7 +229,7 @@ public function providerConvertIdToPath() { return [ ['', ''], - ['/test_path', 'L3Rlc3RfcGF0aA--'] + ['/test_path', 'L3Rlc3RfcGF0aA--'], ]; } @@ -239,6 +239,15 @@ public function testConvertIdToPathNodeRoot() $this->assertEquals($this->imagesHelper->getStorageRoot(), $this->imagesHelper->convertIdToPath($pathId)); } + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage Path is invalid + */ + public function testConvertIdToPathInvalid() + { + $this->imagesHelper->convertIdToPath('Ly4uLy4uLy4uLy4uLy4uL3dvcms-'); + } + /** * @param string $fileName * @param int $maxLength @@ -258,7 +267,7 @@ public function providerShortFilename() return [ ['test', 3, 'tes...'], ['test', 4, 'test'], - ['test', 20, 'test'] + ['test', 20, 'test'], ]; } @@ -280,7 +289,7 @@ public function providerShortFilenameDefaultMaxLength() return [ ['Mini text', 'Mini text'], ['20 symbols are here', '20 symbols are here'], - ['Some text for this unit test', 'Some text for this u...'] + ['Some text for this unit test', 'Some text for this u...'], ]; } @@ -319,7 +328,7 @@ public function providerIsUsingStaticUrlsAllowed() { return [ [true], - [false] + [false], ]; } @@ -346,7 +355,7 @@ public function testGetCurrentPath($pathId, $expectedPath, $isExist) [ ['/../wysiwyg/test_path', true], ['/../wysiwyg/my.jpg', false], - ['/../wysiwyg', true] + ['/../wysiwyg', true], ] ); $this->directoryWriteMock->expects($this->any()) @@ -397,7 +406,7 @@ public function providerGetCurrentPath() [null, 'PATH/wysiwyg', true], ['L3Rlc3RfcGF0aA--', 'PATH/wysiwyg/test_path', false], ['L215LmpwZw--', 'PATH/wysiwyg', false], - [null, 'PATH/wysiwyg', false] + [null, 'PATH/wysiwyg', false], ]; } @@ -450,15 +459,15 @@ public function providerGetImageHtmlDeclarationRenderingAsTag() 'test.png', true, null, - '' + '', ], [ 'http://localhost', 'test.png', false, '{{media url="/test.png"}}', - '' - ] + '', + ], ]; } @@ -492,7 +501,7 @@ public function providerGetImageHtmlDeclaration() { return [ ['http://localhost', 'test.png', true, 'http://localhost/test.png'], - ['http://localhost', 'test.png', false, '{{media url="/test.png"}}'] + ['http://localhost', 'test.png', false, '{{media url="/test.png"}}'], ]; } diff --git a/app/code/Magento/Cms/Test/Unit/Model/Wysiwyg/Images/StorageTest.php b/app/code/Magento/Cms/Test/Unit/Model/Wysiwyg/Images/StorageTest.php index a2178489e1298..805f6d042f94a 100644 --- a/app/code/Magento/Cms/Test/Unit/Model/Wysiwyg/Images/StorageTest.php +++ b/app/code/Magento/Cms/Test/Unit/Model/Wysiwyg/Images/StorageTest.php @@ -127,7 +127,7 @@ protected function setUp() $this->directoryMock = $this->createPartialMock( \Magento\Framework\Filesystem\Directory\Write::class, - ['delete', 'getDriver', 'create'] + ['delete', 'getDriver', 'create', 'getRelativePath', 'isExist'] ); $this->directoryMock->expects( $this->any() @@ -151,7 +151,7 @@ protected function setUp() $this->adapterFactoryMock = $this->createMock(\Magento\Framework\Image\AdapterFactory::class); $this->imageHelperMock = $this->createPartialMock( \Magento\Cms\Helper\Wysiwyg\Images::class, - ['getStorageRoot'] + ['getStorageRoot', 'getCurrentPath'] ); $this->imageHelperMock->expects( $this->any() @@ -182,7 +182,10 @@ protected function setUp() $this->uploaderFactoryMock = $this->getMockBuilder(\Magento\MediaStorage\Model\File\UploaderFactory::class) ->disableOriginalConstructor() ->getMock(); - $this->sessionMock = $this->createMock(\Magento\Backend\Model\Session::class); + $this->sessionMock = $this->getMockBuilder(\Magento\Backend\Model\Session::class) + ->setMethods(['getCurrentPath']) + ->disableOriginalConstructor() + ->getMock(); $this->backendUrlMock = $this->createMock(\Magento\Backend\Model\Url::class); $this->coreFileStorageMock = $this->getMockBuilder(\Magento\MediaStorage\Helper\File\Storage\Database::class) diff --git a/app/code/Magento/Config/etc/module.xml b/app/code/Magento/Config/etc/module.xml index 19051a96a434c..2fd4255a2bc0c 100644 --- a/app/code/Magento/Config/etc/module.xml +++ b/app/code/Magento/Config/etc/module.xml @@ -6,5 +6,9 @@ */ --> - + + + + + diff --git a/app/code/Magento/Downloadable/Controller/Adminhtml/Downloadable/Product/Edit/Link.php b/app/code/Magento/Downloadable/Controller/Adminhtml/Downloadable/Product/Edit/Link.php index 8ee93c37a7775..8d5f64e02be47 100644 --- a/app/code/Magento/Downloadable/Controller/Adminhtml/Downloadable/Product/Edit/Link.php +++ b/app/code/Magento/Downloadable/Controller/Adminhtml/Downloadable/Product/Edit/Link.php @@ -7,6 +7,7 @@ namespace Magento\Downloadable\Controller\Adminhtml\Downloadable\Product\Edit; use Magento\Downloadable\Helper\Download as DownloadHelper; +use Magento\Framework\App\Response\Http as HttpResponse; class Link extends \Magento\Catalog\Controller\Adminhtml\Product\Edit { @@ -42,7 +43,9 @@ protected function _processDownload($resource, $resourceType) $fileName = $helper->getFilename(); $contentType = $helper->getContentType(); - $this->getResponse()->setHttpResponseCode( + /** @var HttpResponse $response */ + $response = $this->getResponse(); + $response->setHttpResponseCode( 200 )->setHeader( 'Pragma', @@ -59,16 +62,22 @@ protected function _processDownload($resource, $resourceType) ); if ($fileSize = $helper->getFileSize()) { - $this->getResponse()->setHeader('Content-Length', $fileSize); + $response->setHeader('Content-Length', $fileSize); } - - if ($contentDisposition = $helper->getContentDisposition()) { - $this->getResponse() - ->setHeader('Content-Disposition', $contentDisposition . '; filename=' . $fileName); + //Setting disposition as state in the config or forcing it for HTML. + /** @var string|null $contentDisposition */ + $contentDisposition = $helper->getContentDisposition(); + if (!$contentDisposition || $contentType === 'text/html') { + $contentDisposition = 'attachment'; } - - $this->getResponse()->clearBody(); - $this->getResponse()->sendHeaders(); + $response->setHeader( + 'Content-Disposition', + $contentDisposition . '; filename=' . $fileName + ); + //Rendering + $response->clearBody(); + $response->sendHeaders(); + $helper->output(); } diff --git a/app/code/Magento/Downloadable/Test/Unit/Controller/Adminhtml/Downloadable/Product/Edit/LinkTest.php b/app/code/Magento/Downloadable/Test/Unit/Controller/Adminhtml/Downloadable/Product/Edit/LinkTest.php index e125ddee9c5d8..aa8b774ab5511 100644 --- a/app/code/Magento/Downloadable/Test/Unit/Controller/Adminhtml/Downloadable/Product/Edit/LinkTest.php +++ b/app/code/Magento/Downloadable/Test/Unit/Controller/Adminhtml/Downloadable/Product/Edit/LinkTest.php @@ -22,7 +22,7 @@ class LinkTest extends \PHPUnit\Framework\TestCase protected $request; /** - * @var \Magento\Framework\App\ResponseInterface + * @var \Magento\Framework\App\ResponseInterface|\PHPUnit_Framework_MockObject_MockObject */ protected $response; @@ -109,6 +109,8 @@ protected function setUp() */ public function testExecuteFile($fileType) { + $fileSize = 58493; + $fileName = 'link.jpg'; $this->request->expects($this->at(0))->method('getParam')->with('id', 0) ->will($this->returnValue(1)); $this->request->expects($this->at(1))->method('getParam')->with('type', 0) @@ -117,7 +119,20 @@ public function testExecuteFile($fileType) ->will($this->returnSelf()); $this->response->expects($this->once())->method('clearBody') ->will($this->returnSelf()); - $this->response->expects($this->any())->method('setHeader') + $this->response + ->expects($this->any()) + ->method('setHeader') + ->withConsecutive( + ['Pragma', 'public', true], + [ + 'Cache-Control', + 'must-revalidate, post-check=0, pre-check=0', + true, + ], + ['Content-type', 'text/html'], + ['Content-Length', $fileSize], + ['Content-Disposition', 'attachment; filename=' . $fileName] + ) ->will($this->returnSelf()); $this->response->expects($this->once())->method('sendHeaders') ->will($this->returnSelf()); @@ -132,13 +147,13 @@ public function testExecuteFile($fileType) $this->downloadHelper->expects($this->once())->method('setResource') ->will($this->returnSelf()); $this->downloadHelper->expects($this->once())->method('getFilename') - ->will($this->returnValue('link.jpg')); + ->will($this->returnValue($fileName)); $this->downloadHelper->expects($this->once())->method('getContentType') - ->will($this->returnSelf('file')); + ->willReturn('text/html'); $this->downloadHelper->expects($this->once())->method('getFileSize') - ->will($this->returnValue(null)); + ->will($this->returnValue($fileSize)); $this->downloadHelper->expects($this->once())->method('getContentDisposition') - ->will($this->returnValue(null)); + ->will($this->returnValue('inline')); $this->downloadHelper->expects($this->once())->method('output') ->will($this->returnSelf()); $this->linkModel->expects($this->once())->method('load') diff --git a/app/code/Magento/Review/view/adminhtml/web/js/rating.js b/app/code/Magento/Review/view/adminhtml/web/js/rating.js index cc72d386dc053..b8d1b1b241b8f 100644 --- a/app/code/Magento/Review/view/adminhtml/web/js/rating.js +++ b/app/code/Magento/Review/view/adminhtml/web/js/rating.js @@ -27,7 +27,7 @@ define([ _bind: function () { this._labels.on({ click: $.proxy(function (e) { - $('[id="' + $(e.currentTarget).attr('for') + '"]').prop('checked', true); + $(e.currentTarget).prev().prop('checked', true); this._updateRating(); }, this), diff --git a/app/code/Magento/Sales/Model/Order/Email/Sender.php b/app/code/Magento/Sales/Model/Order/Email/Sender.php index 8ada6a3f321d2..6d4480c4c45e0 100644 --- a/app/code/Magento/Sales/Model/Order/Email/Sender.php +++ b/app/code/Magento/Sales/Model/Order/Email/Sender.php @@ -84,6 +84,8 @@ protected function checkAndSend(Order $order) $sender->sendCopyTo(); } catch (\Exception $e) { $this->logger->error($e->getMessage()); + + return false; } return true; diff --git a/app/code/Magento/Sales/Test/Unit/Model/Order/Email/Sender/OrderSenderTest.php b/app/code/Magento/Sales/Test/Unit/Model/Order/Email/Sender/OrderSenderTest.php index 411dd9e1433d7..46c44c03b1514 100644 --- a/app/code/Magento/Sales/Test/Unit/Model/Order/Email/Sender/OrderSenderTest.php +++ b/app/code/Magento/Sales/Test/Unit/Model/Order/Email/Sender/OrderSenderTest.php @@ -53,10 +53,11 @@ protected function setUp() * @param int $configValue * @param bool|null $forceSyncMode * @param bool|null $emailSendingResult - * @dataProvider sendDataProvider + * @param $senderSendException * @return void + * @dataProvider sendDataProvider */ - public function testSend($configValue, $forceSyncMode, $emailSendingResult) + public function testSend($configValue, $forceSyncMode, $emailSendingResult, $senderSendException) { $address = 'address_test'; $configPath = 'sales_email/general/async_sending'; @@ -110,19 +111,23 @@ public function testSend($configValue, $forceSyncMode, $emailSendingResult) $this->senderMock->expects($this->once())->method('send'); - $this->senderMock->expects($this->once())->method('sendCopyTo'); + if ($senderSendException) { + $this->checkSenderSendExceptionCase(); + } else { + $this->senderMock->expects($this->once())->method('sendCopyTo'); - $this->orderMock->expects($this->once()) - ->method('setEmailSent') - ->with(true); + $this->orderMock->expects($this->once()) + ->method('setEmailSent') + ->with(true); - $this->orderResourceMock->expects($this->once()) - ->method('saveAttribute') - ->with($this->orderMock, ['send_email', 'email_sent']); + $this->orderResourceMock->expects($this->once()) + ->method('saveAttribute') + ->with($this->orderMock, ['send_email', 'email_sent']); - $this->assertTrue( - $this->sender->send($this->orderMock) - ); + $this->assertTrue( + $this->sender->send($this->orderMock) + ); + } } else { $this->orderResourceMock->expects($this->once()) ->method('saveAttribute') @@ -146,19 +151,42 @@ public function testSend($configValue, $forceSyncMode, $emailSendingResult) } } + /** + * Methods check case when method "send" in "senderMock" throw exception. + * + * @return void + */ + protected function checkSenderSendExceptionCase() + { + $this->senderMock->expects($this->once()) + ->method('send') + ->willThrowException(new \Exception('exception')); + + $this->orderResourceMock->expects($this->once()) + ->method('saveAttribute') + ->with($this->orderMock, 'send_email'); + + $this->assertFalse( + $this->sender->send($this->orderMock) + ); + } + /** * @return array */ public function sendDataProvider() { return [ - [0, 0, true], - [0, 0, true], - [0, 0, false], - [0, 0, false], - [0, 1, true], - [0, 1, true], - [1, null, null, null] + [0, 0, true, false], + [0, 0, true, false], + [0, 0, true, true], + [0, 0, false, false], + [0, 0, false, false], + [0, 0, false, true], + [0, 1, true, false], + [0, 1, true, false], + [0, 1, true, false], + [1, null, null, false] ]; } diff --git a/app/code/Magento/Shipping/Model/Info.php b/app/code/Magento/Shipping/Model/Info.php index 31d39fb80410b..ed4c1c3f6d127 100644 --- a/app/code/Magento/Shipping/Model/Info.php +++ b/app/code/Magento/Shipping/Model/Info.php @@ -114,7 +114,7 @@ protected function _initOrder() /** @var \Magento\Sales\Model\Order $order */ $order = $this->_orderFactory->create()->load($this->getOrderId()); - if (!$order->getId() || $this->getProtectCode() != $order->getProtectCode()) { + if (!$order->getId() || $this->getProtectCode() !== $order->getProtectCode()) { return false; } @@ -130,7 +130,7 @@ protected function _initShipment() { /* @var $model Shipment */ $ship = $this->shipmentRepository->get($this->getShipId()); - if (!$ship->getEntityId() || $this->getProtectCode() != $ship->getProtectCode()) { + if (!$ship->getEntityId() || $this->getProtectCode() !== $ship->getProtectCode()) { return false; } @@ -195,7 +195,7 @@ public function getTrackingInfoByTrackId() { /** @var \Magento\Shipping\Model\Order\Track $track */ $track = $this->_trackFactory->create()->load($this->getTrackId()); - if ($track->getId() && $this->getProtectCode() == $track->getProtectCode()) { + if ($track->getId() && $this->getProtectCode() === $track->getProtectCode()) { $this->_trackingInfo = [[$track->getNumberDetail()]]; } return $this->_trackingInfo; diff --git a/app/code/Magento/Shipping/Test/Unit/Model/InfoTest.php b/app/code/Magento/Shipping/Test/Unit/Model/InfoTest.php new file mode 100644 index 0000000000000..6bc95993bfde6 --- /dev/null +++ b/app/code/Magento/Shipping/Test/Unit/Model/InfoTest.php @@ -0,0 +1,312 @@ +helper = $this->getMockBuilder(\Magento\Shipping\Helper\Data::class) + ->disableOriginalConstructor() + ->getMock(); + $this->orderFactory = $this->getMockBuilder(\Magento\Sales\Model\OrderFactory::class) + ->disableOriginalConstructor() + ->setMethods(['create']) + ->getMock(); + $this->shipmentRepository = $this->getMockBuilder(\Magento\Sales\Api\ShipmentRepositoryInterface::class) + ->disableOriginalConstructor() + ->getMock(); + $this->trackFactory = $this->getMockBuilder(\Magento\Shipping\Model\Order\TrackFactory::class) + ->disableOriginalConstructor() + ->setMethods(['create']) + ->getMock(); + $this->trackCollectionFactory = $this->getMockBuilder(CollectionFactory::class) + ->disableOriginalConstructor() + ->setMethods(['create']) + ->getMock(); + + $objectManagerHelper = new ObjectManager($this); + $this->info = $objectManagerHelper->getObject( + Info::class, + [ + 'shippingData' => $this->helper, + 'orderFactory' => $this->orderFactory, + 'shipmentRepository' => $this->shipmentRepository, + 'trackFactory' => $this->trackFactory, + 'trackCollectionFactory' => $this->trackCollectionFactory, + ] + ); + } + + public function testLoadByHashWithOrderId() + { + $hash = strtr(base64_encode('order_id:1:protected_code'), '+/=', '-_,'); + $decodedHash = [ + 'key' => 'order_id', + 'id' => 1, + 'hash' => 'protected_code', + ]; + $shipmentId = 1; + $shipmentIncrementId = 3; + $trackDetails = 'track_details'; + + $this->helper->expects($this->atLeastOnce()) + ->method('decodeTrackingHash') + ->with($hash) + ->willReturn($decodedHash); + $shipmentCollection = $this->getMockBuilder(\Magento\Sales\Model\ResourceModel\Order\Shipment\Collection::class) + ->disableOriginalConstructor() + ->setMethods(['getIterator']) + ->getMock(); + + $order = $this->getMockBuilder(\Magento\Sales\Model\Order::class) + ->disableOriginalConstructor() + ->setMethods(['load', 'getId', 'getProtectCode', 'getShipmentsCollection']) + ->getMock(); + $order->expects($this->atLeastOnce())->method('load')->with($decodedHash['id'])->willReturnSelf(); + $order->expects($this->atLeastOnce())->method('getId')->willReturn($decodedHash['id']); + $order->expects($this->atLeastOnce())->method('getProtectCode')->willReturn($decodedHash['hash']); + $order->expects($this->atLeastOnce())->method('getShipmentsCollection')->willReturn($shipmentCollection); + $this->orderFactory->expects($this->atLeastOnce())->method('create')->willReturn($order); + + $shipment = $this->getMockBuilder(\Magento\Sales\Model\Order\Shipment::class) + ->disableOriginalConstructor() + ->setMethods(['getIncrementId', 'getId']) + ->getMock(); + $shipment->expects($this->atLeastOnce())->method('getIncrementId')->willReturn($shipmentIncrementId); + $shipment->expects($this->atLeastOnce())->method('getId')->willReturn($shipmentId); + $shipmentCollection->expects($this->any())->method('getIterator')->willReturn(new \ArrayIterator([$shipment])); + + $track = $this->getMockBuilder(\Magento\Sales\Model\Order\Shipment\Track::class) + ->disableOriginalConstructor() + ->setMethods(['setShipment', 'getNumberDetail']) + ->getMock(); + $track->expects($this->atLeastOnce())->method('setShipment')->with($shipment)->willReturnSelf(); + $track->expects($this->atLeastOnce())->method('getNumberDetail')->willReturn($trackDetails); + $trackCollection = $this->getMockBuilder(\Magento\Shipping\Model\ResourceModel\Order\Track\Collection::class) + ->disableOriginalConstructor() + ->setMethods(['getIterator', 'setShipmentFilter']) + ->getMock(); + $trackCollection->expects($this->atLeastOnce()) + ->method('setShipmentFilter') + ->with($shipmentId) + ->willReturnSelf(); + $trackCollection->expects($this->atLeastOnce()) + ->method('getIterator') + ->willReturn(new \ArrayIterator([$track])); + + $this->trackCollectionFactory->expects($this->atLeastOnce())->method('create')->willReturn($trackCollection); + $this->info->loadByHash($hash); + + $this->assertEquals([$shipmentIncrementId => [$trackDetails]], $this->info->getTrackingInfo()); + } + + public function testLoadByHashWithOrderIdWrongCode() + { + $hash = strtr(base64_encode('order_id:1:0'), '+/=', '-_,'); + $decodedHash = [ + 'key' => 'order_id', + 'id' => 1, + 'hash' => '0', + ]; + $this->helper->expects($this->atLeastOnce()) + ->method('decodeTrackingHash') + ->with($hash) + ->willReturn($decodedHash); + $order = $this->getMockBuilder(\Magento\Sales\Model\Order::class) + ->disableOriginalConstructor() + ->setMethods(['load', 'getId', 'getProtectCode']) + ->getMock(); + $order->expects($this->atLeastOnce())->method('load')->with($decodedHash['id'])->willReturnSelf(); + $order->expects($this->atLeastOnce())->method('getId')->willReturn($decodedHash['id']); + $order->expects($this->atLeastOnce())->method('getProtectCode')->willReturn('0e123123123'); + $this->orderFactory->expects($this->atLeastOnce())->method('create')->willReturn($order); + $this->info->loadByHash($hash); + + $this->assertEmpty($this->info->getTrackingInfo()); + } + + public function testLoadByHashWithShipmentId() + { + $hash = strtr(base64_encode('ship_id:1:protected_code'), '+/=', '-_,'); + $decodedHash = [ + 'key' => 'ship_id', + 'id' => 1, + 'hash' => 'protected_code', + ]; + $shipmentIncrementId = 3; + $trackDetails = 'track_details'; + + $this->helper->expects($this->atLeastOnce()) + ->method('decodeTrackingHash') + ->with($hash) + ->willReturn($decodedHash); + $shipment = $this->getMockBuilder(\Magento\Sales\Model\Order\Shipment::class) + ->disableOriginalConstructor() + ->setMethods(['getEntityId', 'getProtectCode', 'getIncrementId', 'getId']) + ->getMock(); + $shipment->expects($this->atLeastOnce())->method('getIncrementId')->willReturn($shipmentIncrementId); + $shipment->expects($this->atLeastOnce())->method('getId')->willReturn($decodedHash['id']); + $shipment->expects($this->atLeastOnce())->method('getEntityId')->willReturn(3); + $shipment->expects($this->atLeastOnce())->method('getProtectCode')->willReturn($decodedHash['hash']); + $this->shipmentRepository->expects($this->atLeastOnce()) + ->method('get') + ->with($decodedHash['id']) + ->willReturn($shipment); + $track = $this->getMockBuilder(\Magento\Sales\Model\Order\Shipment\Track::class) + ->disableOriginalConstructor() + ->setMethods(['setShipment', 'getNumberDetail']) + ->getMock(); + $track->expects($this->atLeastOnce())->method('setShipment')->with($shipment)->willReturnSelf(); + $track->expects($this->atLeastOnce())->method('getNumberDetail')->willReturn($trackDetails); + $trackCollection = $this->getMockBuilder(\Magento\Shipping\Model\ResourceModel\Order\Track\Collection::class) + ->disableOriginalConstructor() + ->setMethods(['getIterator', 'setShipmentFilter']) + ->getMock(); + $trackCollection->expects($this->atLeastOnce()) + ->method('setShipmentFilter') + ->with($decodedHash['id']) + ->willReturnSelf(); + $trackCollection->expects($this->atLeastOnce()) + ->method('getIterator') + ->willReturn(new \ArrayIterator([$track])); + $this->trackCollectionFactory->expects($this->atLeastOnce())->method('create')->willReturn($trackCollection); + + $this->info->loadByHash($hash); + + $this->assertEquals([$shipmentIncrementId => [$trackDetails]], $this->info->getTrackingInfo()); + } + + public function testLoadByHashWithShipmentIdWrongCode() + { + $hash = strtr(base64_encode('ship_id:1:0'), '+/=', '-_,'); + $decodedHash = [ + 'key' => 'ship_id', + 'id' => 1, + 'hash' => '0', + ]; + $this->helper->expects($this->atLeastOnce()) + ->method('decodeTrackingHash') + ->with($hash) + ->willReturn($decodedHash); + $shipment = $this->getMockBuilder(\Magento\Sales\Model\Order\Shipment::class) + ->disableOriginalConstructor() + ->setMethods(['getEntityId', 'getProtectCode']) + ->getMock(); + $shipment->expects($this->atLeastOnce())->method('getEntityId')->willReturn(3); + $shipment->expects($this->atLeastOnce())->method('getProtectCode')->willReturn('0e123123123'); + $this->shipmentRepository->expects($this->atLeastOnce()) + ->method('get') + ->with($decodedHash['id']) + ->willReturn($shipment); + + $this->info->loadByHash($hash); + + $this->assertEmpty($this->info->getTrackingInfo()); + } + + /** + * @dataProvider loadByHashWithTrackIdDataProvider + * @param string $protectCodeHash + * @param string $protectCode + * @param string $numberDetail + * @param array $trackDetails + * @return void + */ + public function testLoadByHashWithTrackId( + string $protectCodeHash, + string $protectCode, + string $numberDetail, + array $trackDetails + ) { + $hash = base64_encode('hash'); + $decodedHash = [ + 'key' => 'track_id', + 'id' => 1, + 'hash' => $protectCodeHash, + ]; + $this->helper->expects($this->atLeastOnce()) + ->method('decodeTrackingHash') + ->with($hash) + ->willReturn($decodedHash); + $track = $this->getMockBuilder(\Magento\Sales\Model\Order\Shipment\Track::class) + ->disableOriginalConstructor() + ->setMethods(['load', 'getId', 'getProtectCode', 'getNumberDetail']) + ->getMock(); + $track->expects($this->atLeastOnce())->method('load')->with($decodedHash['id'])->willReturnSelf(); + $track->expects($this->atLeastOnce())->method('getId')->willReturn($decodedHash['id']); + $track->expects($this->atLeastOnce())->method('getProtectCode')->willReturn($protectCode); + $track->expects($this->any())->method('getNumberDetail')->willReturn($numberDetail); + + $this->trackFactory->expects($this->atLeastOnce())->method('create')->willReturn($track); + $this->info->loadByHash($hash); + + $this->assertEquals($trackDetails, $this->info->getTrackingInfo()); + } + + /** + * @return array + */ + public function loadByHashWithTrackIdDataProvider() + { + return [ + [ + 'hash' => 'protected_code', + 'protect_code' => 'protected_code', + 'number_detail' => 'track_details', + 'track_details' => [['track_details']], + ], + [ + 'hash' => '0', + 'protect_code' => '0e6640', + 'number_detail' => '', + 'track_details' => [], + ], + ]; + } +} diff --git a/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/DeleteFilesTest.php b/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/DeleteFilesTest.php new file mode 100644 index 0000000000000..6509ed62101d0 --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/DeleteFilesTest.php @@ -0,0 +1,114 @@ +get(\Magento\Framework\Filesystem::class); + /** @var \Magento\Cms\Helper\Wysiwyg\Images $imagesHelper */ + $this->imagesHelper = $objectManager->get(\Magento\Cms\Helper\Wysiwyg\Images::class); + $this->mediaDirectory = $filesystem->getDirectoryWrite(DirectoryList::MEDIA); + $this->fullDirectoryPath = $this->imagesHelper->getStorageRoot() . '/' . $directoryName; + $this->mediaDirectory->create($this->mediaDirectory->getRelativePath($this->fullDirectoryPath)); + $filePath = $this->fullDirectoryPath . DIRECTORY_SEPARATOR . $this->fileName; + $fixtureDir = realpath(__DIR__ . '/../../../../../Catalog/_files'); + copy($fixtureDir . '/' . $this->fileName, $filePath); + $this->model = $objectManager->get(\Magento\Cms\Controller\Adminhtml\Wysiwyg\Images\DeleteFiles::class); + } + + /** + * Execute method with correct directory path and file name to check that files under WYSIWYG media directory + * can be removed. + * + * @return void + */ + public function testExecute() + { + $this->model->getRequest()->setMethod('POST') + ->setPostValue('files', [$this->imagesHelper->idEncode($this->fileName)]); + $this->model->getStorage()->getSession()->setCurrentPath($this->fullDirectoryPath); + $this->model->execute(); + + $this->assertFalse( + $this->mediaDirectory->isExist( + $this->mediaDirectory->getRelativePath($this->fullDirectoryPath . '/' . $this->fileName) + ) + ); + } + + /** + * Execute method with traversal file path to check that there is no ability to remove file which is not + * under media directory. + * + * @return void + */ + public function testExecuteWithWrongFileName() + { + $fileName = '/../../../../etc/env.php'; + $this->model->getRequest()->setMethod('POST') + ->setPostValue('files', [$this->imagesHelper->idEncode($fileName)]); + $this->model->getStorage()->getSession()->setCurrentPath($this->fullDirectoryPath); + $this->model->execute(); + + $this->assertTrue( + $this->mediaDirectory->isExist( + $this->mediaDirectory->getRelativePath($this->fullDirectoryPath . $fileName) + ) + ); + } + + /** + * @inheritdoc + */ + public static function tearDownAfterClass() + { + $filesystem = \Magento\TestFramework\Helper\Bootstrap::getObjectManager() + ->get(\Magento\Framework\Filesystem::class); + /** @var \Magento\Framework\Filesystem\Directory\WriteInterface $directory */ + $directory = $filesystem->getDirectoryWrite(DirectoryList::MEDIA); + if ($directory->isExist('wysiwyg')) { + $directory->delete('wysiwyg'); + } + } +} diff --git a/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/DeleteFolderTest.php b/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/DeleteFolderTest.php new file mode 100644 index 0000000000000..48280d35e38fb --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/DeleteFolderTest.php @@ -0,0 +1,106 @@ +get(\Magento\Framework\Filesystem::class); + $this->mediaDirectory = $filesystem->getDirectoryWrite(DirectoryList::MEDIA); + /** @var \Magento\Cms\Helper\Wysiwyg\Images $imagesHelper */ + $this->imagesHelper = $objectManager->get(\Magento\Cms\Helper\Wysiwyg\Images::class); + $this->fullDirectoryPath = $this->imagesHelper->getStorageRoot(); + $this->model = $objectManager->get(\Magento\Cms\Controller\Adminhtml\Wysiwyg\Images\DeleteFolder::class); + } + + /** + * Execute method with correct directory path to check that directories under WYSIWYG media directory + * can be removed. + * + * @return void + */ + public function testExecute() + { + $directoryName = DIRECTORY_SEPARATOR . 'NewDirectory'; + $this->mediaDirectory->create( + $this->mediaDirectory->getRelativePath($this->fullDirectoryPath . $directoryName) + ); + $this->model->getRequest()->setParams(['node' => $this->imagesHelper->idEncode($directoryName)]); + $this->model->execute(); + + $this->assertFalse( + $this->mediaDirectory->isExist( + $this->mediaDirectory->getRelativePath( + $this->fullDirectoryPath . $directoryName + ) + ) + ); + } + + /** + * Execute method with traversal directory path to check that there is no ability to remove folder which is not + * under media directory. + * + * @return void + */ + public function testExecuteWithWrongDirectoryName() + { + $directoryName = '/../../../etc/'; + $this->model->getRequest()->setParams(['node' => $this->imagesHelper->idEncode($directoryName)]); + $this->model->execute(); + + $this->assertTrue( + $this->mediaDirectory->isExist( + $this->mediaDirectory->getRelativePath($this->fullDirectoryPath . $directoryName) + ) + ); + } + + /** + * @inheritdoc + */ + public static function tearDownAfterClass() + { + $filesystem = \Magento\TestFramework\Helper\Bootstrap::getObjectManager() + ->get(\Magento\Framework\Filesystem::class); + /** @var \Magento\Framework\Filesystem\Directory\WriteInterface $directory */ + $directory = $filesystem->getDirectoryWrite(DirectoryList::MEDIA); + if ($directory->isExist('wysiwyg')) { + $directory->delete('wysiwyg'); + } + } +} diff --git a/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/NewFolderTest.php b/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/NewFolderTest.php new file mode 100644 index 0000000000000..b20b5a04a2a50 --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/NewFolderTest.php @@ -0,0 +1,105 @@ +get(\Magento\Framework\Filesystem::class); + $this->mediaDirectory = $filesystem->getDirectoryWrite(DirectoryList::MEDIA); + /** @var \Magento\Cms\Helper\Wysiwyg\Images $imagesHelper */ + $imagesHelper = $objectManager->get(\Magento\Cms\Helper\Wysiwyg\Images::class); + $this->fullDirectoryPath = $imagesHelper->getStorageRoot(); + $this->model = $objectManager->get(\Magento\Cms\Controller\Adminhtml\Wysiwyg\Images\NewFolder::class); + } + + /** + * Execute method with correct directory path to check that new folder can be created under WYSIWYG media directory. + * + * @return void + */ + public function testExecute() + { + $this->model->getRequest()->setMethod('POST') + ->setPostValue('name', $this->dirName); + $this->model->getStorage()->getSession()->setCurrentPath($this->fullDirectoryPath); + $this->model->execute(); + + $this->assertTrue( + $this->mediaDirectory->isExist( + $this->mediaDirectory->getRelativePath( + $this->fullDirectoryPath . DIRECTORY_SEPARATOR . $this->dirName + ) + ) + ); + } + + /** + * Execute method with traversal directory path to check that there is no ability to create new folder not + * under media directory. + * + * @return void + */ + public function testExecuteWithWrongPath() + { + $dirPath = '/../../../'; + $this->model->getRequest()->setMethod('POST') + ->setPostValue('name', $this->dirName); + $this->model->getStorage()->getSession()->setCurrentPath($this->fullDirectoryPath . $dirPath); + $this->model->execute(); + + $this->assertFalse( + $this->mediaDirectory->isExist( + $this->mediaDirectory->getRelativePath($this->fullDirectoryPath . $dirPath . $this->dirName) + ) + ); + } + + /** + * @inheritdoc + */ + public static function tearDownAfterClass() + { + $filesystem = \Magento\TestFramework\Helper\Bootstrap::getObjectManager() + ->get(\Magento\Framework\Filesystem::class); + /** @var \Magento\Framework\Filesystem\Directory\WriteInterface $directory */ + $directory = $filesystem->getDirectoryWrite(DirectoryList::MEDIA); + if ($directory->isExist('wysiwyg')) { + $directory->delete('wysiwyg'); + } + } +} diff --git a/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/UploadTest.php b/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/UploadTest.php new file mode 100644 index 0000000000000..ba2527e3e449e --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/UploadTest.php @@ -0,0 +1,140 @@ +get(\Magento\Framework\Filesystem::class); + /** @var \Magento\Cms\Helper\Wysiwyg\Images $imagesHelper */ + $imagesHelper = $objectManager->get(\Magento\Cms\Helper\Wysiwyg\Images::class); + $this->mediaDirectory = $filesystem->getDirectoryWrite(DirectoryList::MEDIA); + $this->fullDirectoryPath = $imagesHelper->getStorageRoot() . DIRECTORY_SEPARATOR . $directoryName; + $this->mediaDirectory->create($this->mediaDirectory->getRelativePath($this->fullDirectoryPath)); + $this->model = $objectManager->get(\Magento\Cms\Controller\Adminhtml\Wysiwyg\Images\Upload::class); + $fixtureDir = realpath(__DIR__ . '/../../../../../Catalog/_files'); + $tmpFile = __DIR__ . DIRECTORY_SEPARATOR . $this->fileName; + copy($fixtureDir . DIRECTORY_SEPARATOR . $this->fileName, $tmpFile); + $_FILES = [ + 'image' => [ + 'name' => $this->fileName, + 'type' => 'image/png', + 'tmp_name' => $tmpFile, + 'error' => 0, + 'size' => filesize($fixtureDir), + ], + ]; + } + + /** + * Execute method with correct directory path and file name to check that file can be uploaded to the directory + * located under WYSIWYG media. + * + * @return void + */ + public function testExecute() + { + $this->model->getRequest()->setParams(['type' => 'image/png']); + $this->model->getStorage()->getSession()->setCurrentPath($this->fullDirectoryPath); + $this->model->execute(); + + $this->assertTrue( + $this->mediaDirectory->isExist( + $this->mediaDirectory->getRelativePath( + $this->fullDirectoryPath . DIRECTORY_SEPARATOR . $this->fileName + ) + ) + ); + } + + /** + * Execute method with traversal directory path to check that there is no ability to create file not + * under media directory. + * + * @return void + */ + public function testExecuteWithWrongPath() + { + $dirPath = '/../../../etc/'; + $this->model->getRequest()->setParams(['type' => 'image/png']); + $this->model->getStorage()->getSession()->setCurrentPath($dirPath); + $this->model->execute(); + + $this->assertFalse( + $this->mediaDirectory->isExist( + $this->mediaDirectory->getRelativePath($this->fullDirectoryPath . $dirPath . $this->fileName) + ) + ); + } + + /** + * Execute method with traversal file path to check that there is no ability to create file not + * under media directory. + * + * @return void + */ + public function testExecuteWithWrongFileName() + { + $newFilename = '/../../../../etc/new_file.png'; + $_FILES['image']['name'] = $newFilename; + $_FILES['image']['tmp_name'] = __DIR__ . DIRECTORY_SEPARATOR . $this->fileName; + $this->model->getRequest()->setParams(['type' => 'image/png']); + $this->model->getStorage()->getSession()->setCurrentPath($this->fullDirectoryPath); + $this->model->execute(); + + $this->assertFalse( + $this->mediaDirectory->isExist( + $this->mediaDirectory->getRelativePath($this->fullDirectoryPath . $newFilename) + ) + ); + } + + /** + * @inheritdoc + */ + public static function tearDownAfterClass() + { + $filesystem = \Magento\TestFramework\Helper\Bootstrap::getObjectManager() + ->get(\Magento\Framework\Filesystem::class); + /** @var \Magento\Framework\Filesystem\Directory\WriteInterface $directory */ + $directory = $filesystem->getDirectoryWrite(DirectoryList::MEDIA); + if ($directory->isExist('wysiwyg')) { + $directory->delete('wysiwyg'); + } + } +} diff --git a/dev/tests/integration/testsuite/Magento/Framework/App/Filesystem/DirectoryResolverTest.php b/dev/tests/integration/testsuite/Magento/Framework/App/Filesystem/DirectoryResolverTest.php new file mode 100644 index 0000000000000..278c08fbd2b07 --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/Framework/App/Filesystem/DirectoryResolverTest.php @@ -0,0 +1,87 @@ +objectManager = Bootstrap::getObjectManager(); + $this->directoryResolver = $this->objectManager + ->create(\Magento\Framework\App\Filesystem\DirectoryResolver::class); + /** @var \Magento\Framework\Filesystem $filesystem */ + $filesystem = $this->objectManager->create(\Magento\Framework\Filesystem::class); + $this->directory = $filesystem->getDirectoryWrite(\Magento\Framework\App\Filesystem\DirectoryList::MEDIA); + } + + /** + * @param string $path + * @param string $directoryConfig + * @param bool $expectation + * @dataProvider validatePathDataProvider + * @magentoAppIsolation enabled + * @return void + */ + public function testValidatePath($path, $directoryConfig, $expectation) + { + $path = $this->directory->getAbsolutePath($path); + $this->assertEquals($expectation, $this->directoryResolver->validatePath($path, $directoryConfig)); + } + + /** + * @expectedException \Magento\Framework\Exception\FileSystemException + * @magentoAppIsolation enabled + * @return void + */ + public function testValidatePathWithException() + { + $path = $this->directory->getAbsolutePath(); + $this->directoryResolver->validatePath($path, 'wrong_dir'); + } + + /** + * @return array + */ + public function validatePathDataProvider() + { + return [ + [ + '/', + DirectoryList::MEDIA, + true, + ], + [ + '/../../pub/', + DirectoryList::MEDIA, + false, + ], + ]; + } +} diff --git a/lib/internal/Magento/Framework/App/Filesystem/DirectoryResolver.php b/lib/internal/Magento/Framework/App/Filesystem/DirectoryResolver.php new file mode 100644 index 0000000000000..4b62476ae8445 --- /dev/null +++ b/lib/internal/Magento/Framework/App/Filesystem/DirectoryResolver.php @@ -0,0 +1,46 @@ +directoryList = $directoryList; + } + + /** + * Validate path. + * + * Gets real path for directory provided in parameters and compares it with specified root directory. + * Will return TRUE if real path of provided value contains root directory path and FALSE if not. + * Throws the \Magento\Framework\Exception\FileSystemException in case when directory path is absent + * in Directories configuration. + * + * @param string $path + * @param string $directoryConfig + * @return bool + * @throws \Magento\Framework\Exception\FileSystemException + */ + public function validatePath($path, $directoryConfig = DirectoryList::MEDIA) + { + $realPath = realpath($path); + $root = $this->directoryList->getPath($directoryConfig); + + return strpos($realPath, $root) === 0; + } +} diff --git a/lib/internal/Magento/Framework/App/StaticResource.php b/lib/internal/Magento/Framework/App/StaticResource.php index c554e1b49bccc..575074fdb58ac 100644 --- a/lib/internal/Magento/Framework/App/StaticResource.php +++ b/lib/internal/Magento/Framework/App/StaticResource.php @@ -165,9 +165,11 @@ protected function parsePath($path) { $path = ltrim($path, '/'); $parts = explode('/', $path, 6); - if (count($parts) < 5) { + if (count($parts) < 5 || preg_match('/\.\.(\\\|\/)/', $path)) { + //Checking that path contains all required parts and is not above static folder. throw new \InvalidArgumentException("Requested path '$path' is wrong."); } + $result = []; $result['area'] = $parts[0]; $result['theme'] = $parts[1] . '/' . $parts[2]; diff --git a/lib/internal/Magento/Framework/App/Test/Unit/StaticResourceTest.php b/lib/internal/Magento/Framework/App/Test/Unit/StaticResourceTest.php index 2edd0ffd3b1b5..d69ce4956ce8c 100644 --- a/lib/internal/Magento/Framework/App/Test/Unit/StaticResourceTest.php +++ b/lib/internal/Magento/Framework/App/Test/Unit/StaticResourceTest.php @@ -284,4 +284,22 @@ public function testCatchExceptionDeveloperMode() ->method('sendResponse'); $this->assertTrue($this->object->catchException($bootstrap, $exception)); } + + /** + * @expectedException \InvalidArgumentException + */ + public function testLaunchPathAbove() + { + $path = 'frontend/..\..\folder_above/././Magento_Ui/template/messages.html'; + $this->stateMock->expects($this->once()) + ->method('getMode') + ->will($this->returnValue(State::MODE_DEVELOPER)); + $this->requestMock->expects($this->once()) + ->method('get') + ->with('resource') + ->willReturn('frontend/..\..\folder_above/././Magento_Ui/template/messages.html'); + $this->expectExceptionMessage("Requested path '$path' is wrong."); + + $this->object->launch(); + } } diff --git a/lib/internal/Magento/Framework/Filesystem/DirectoryList.php b/lib/internal/Magento/Framework/Filesystem/DirectoryList.php index 2685ca13ac4ba..20874f60791c1 100644 --- a/lib/internal/Magento/Framework/Filesystem/DirectoryList.php +++ b/lib/internal/Magento/Framework/Filesystem/DirectoryList.php @@ -202,6 +202,7 @@ public function getRoot() * * @param string $code * @return string + * @throws \Magento\Framework\Exception\FileSystemException */ public function getPath($code) { diff --git a/lib/internal/Magento/Framework/Filter/RemoveTags.php b/lib/internal/Magento/Framework/Filter/RemoveTags.php index 63528dd4234c3..ca581176de368 100644 --- a/lib/internal/Magento/Framework/Filter/RemoveTags.php +++ b/lib/internal/Magento/Framework/Filter/RemoveTags.php @@ -34,7 +34,8 @@ public function filter($value) [$this, '_convertEntities'], $value ); - $value = strip_tags($value); - return htmlspecialchars_decode($value); + $value = htmlspecialchars_decode($value); + + return strip_tags($value); } } diff --git a/lib/internal/Magento/Framework/Filter/Test/Unit/RemoveTagsTest.php b/lib/internal/Magento/Framework/Filter/Test/Unit/RemoveTagsTest.php index 7ac9a7fe583c4..8fc9685aa7c42 100644 --- a/lib/internal/Magento/Framework/Filter/Test/Unit/RemoveTagsTest.php +++ b/lib/internal/Magento/Framework/Filter/Test/Unit/RemoveTagsTest.php @@ -5,6 +5,9 @@ */ namespace Magento\Framework\Filter\Test\Unit; +/** + * Test for \Magento\Framework\Filter\RemoveTags + */ class RemoveTagsTest extends \PHPUnit\Framework\TestCase { /** @@ -19,4 +22,13 @@ public function testRemoveTags() $expected = '10 < 11 > 10'; $this->assertSame($expected, $actual); } + + public function testFilterEncodedValue() + { + $input = '"><script>alert("website")</script><br a="'; + $removeTags = new \Magento\Framework\Filter\RemoveTags(); + $actual = $removeTags->filter($input); + $expected = '">alert("website")'; + $this->assertSame($expected, $actual); + } } diff --git a/setup/src/Magento/Setup/Console/Command/DiCompileCommand.php b/setup/src/Magento/Setup/Console/Command/DiCompileCommand.php index 904840869fdeb..f918eef9d5d55 100644 --- a/setup/src/Magento/Setup/Console/Command/DiCompileCommand.php +++ b/setup/src/Magento/Setup/Console/Command/DiCompileCommand.php @@ -239,7 +239,7 @@ private function getExcludedModulePaths(array $modulePaths) $vendorPathsRegExps[] = $vendorDir . '/(?:' . join('|', $vendorModules) . ')'; } - $basePathsRegExps[] = $basePath + $basePathsRegExps[] = preg_quote($basePath, '#') . '/(?:' . join('|', $vendorPathsRegExps) . ')'; } @@ -258,6 +258,10 @@ private function getExcludedModulePaths(array $modulePaths) */ private function getExcludedLibraryPaths(array $libraryPaths) { + $libraryPaths = array_map(function ($libraryPath) { + return preg_quote($libraryPath, '#'); + }, $libraryPaths); + $excludedLibraryPaths = [ '#^(?:' . join('|', $libraryPaths) . ')/([\\w]+/)?Test#', '#^(?:' . join('|', $libraryPaths) . ')/([\\w]+/)?tests#', @@ -274,7 +278,7 @@ private function getExcludedLibraryPaths(array $libraryPaths) private function getExcludedSetupPaths($setupPath) { return [ - '#^(?:' . $setupPath . ')(/[\\w]+)*/Test#' + '#^(?:' . preg_quote($setupPath, '#') . ')(/[\\w]+)*/Test#' ]; } diff --git a/setup/src/Magento/Setup/Test/Unit/Console/Command/DiCompileCommandTest.php b/setup/src/Magento/Setup/Test/Unit/Console/Command/DiCompileCommandTest.php index 388aa0670e069..20bbb5902d4d2 100644 --- a/setup/src/Magento/Setup/Test/Unit/Console/Command/DiCompileCommandTest.php +++ b/setup/src/Magento/Setup/Test/Unit/Console/Command/DiCompileCommandTest.php @@ -7,6 +7,7 @@ use Magento\Framework\Component\ComponentRegistrar; use Magento\Setup\Console\Command\DiCompileCommand; +use Magento\Setup\Module\Di\App\Task\OperationFactory; use Symfony\Component\Console\Tester\CommandTester; /** @@ -61,6 +62,10 @@ public function setUp() $this->managerMock = $this->createMock(\Magento\Setup\Module\Di\App\Task\Manager::class); $this->directoryListMock = $this->createMock(\Magento\Framework\App\Filesystem\DirectoryList::class); + $this->directoryListMock->expects($this->any())->method('getPath')->willReturnMap([ + [\Magento\Framework\App\Filesystem\DirectoryList::SETUP, '/path (1)/to/setup/'], + ]); + $this->filesystemMock = $this->getMockBuilder(\Magento\Framework\Filesystem::class) ->disableOriginalConstructor() ->getMock(); @@ -70,8 +75,8 @@ public function setUp() ->getMock(); $this->componentRegistrarMock = $this->createMock(\Magento\Framework\Component\ComponentRegistrar::class); $this->componentRegistrarMock->expects($this->any())->method('getPaths')->willReturnMap([ - [ComponentRegistrar::MODULE, ['/path/to/module/one', '/path/to/module/two']], - [ComponentRegistrar::LIBRARY, ['/path/to/library/one', '/path/to/library/two']], + [ComponentRegistrar::MODULE, ['/path/to/module/one', '/path (1)/to/module/two']], + [ComponentRegistrar::LIBRARY, ['/path/to/library/one', '/path (1)/to/library/two']], ]); $this->command = new DiCompileCommand( @@ -128,7 +133,28 @@ public function testExecute() ->method('create') ->with(\Symfony\Component\Console\Helper\ProgressBar::class) ->willReturn($progressBar); - $this->managerMock->expects($this->exactly(7))->method('addOperation'); + + $this->managerMock->expects($this->exactly(7))->method('addOperation') + ->withConsecutive( + [OperationFactory::PROXY_GENERATOR, []], + [OperationFactory::REPOSITORY_GENERATOR, $this->anything()], + [OperationFactory::DATA_ATTRIBUTES_GENERATOR, []], + [OperationFactory::APPLICATION_CODE_GENERATOR, $this->callback(function ($subject) { + $this->assertEmpty(array_diff($subject['excludePatterns'], [ + "#^(?:/path \(1\)/to/setup/)(/[\w]+)*/Test#", + "#^(?:/path/to/library/one|/path \(1\)/to/library/two)/([\w]+/)?Test#", + "#^(?:/path/to/library/one|/path \(1\)/to/library/two)/([\w]+/)?tests#", + "#^(?:/path/to/(?:module/(?:one))|/path \(1\)/to/(?:module/(?:two)))/Test#", + "#^(?:/path/to/(?:module/(?:one))|/path \(1\)/to/(?:module/(?:two)))/tests#" + ])); + return true; + })], + [OperationFactory::INTERCEPTION, $this->anything()], + [OperationFactory::AREA_CONFIG_GENERATOR, $this->anything()], + [OperationFactory::INTERCEPTION_CACHE, $this->anything()] + ) + ; + $this->managerMock->expects($this->once())->method('process'); $tester = new CommandTester($this->command); $tester->execute([]);