Merge pull request #4101 from magento-arcticfoxes/2.1.18-develop-pr

[arcticfoxes] Bug Fixes
magento · Apr 24, 2019 · 5cba861 · 5cba861
2 parents ade1293 + bb0e4ef
commit 5cba861
Show file tree

Hide file tree

Showing 15 changed files with 412 additions and 91 deletions.
diff --git a/app/code/Magento/Checkout/Model/ShippingInformationManagement.php b/app/code/Magento/Checkout/Model/ShippingInformationManagement.php
@@ -18,6 +18,8 @@
 use Magento\Framework\App\ObjectManager;
 
 /**
+ * Class ShippingInformationManagement
+ *
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  */
 class ShippingInformationManagement implements \Magento\Checkout\Api\ShippingInformationManagementInterface
@@ -149,31 +151,32 @@ public function saveAddressInformation(
         $cartId,
         \Magento\Checkout\Api\Data\ShippingInformationInterface $addressInformation
     ) {
-        $address = $addressInformation->getShippingAddress();
-        $billingAddress = $addressInformation->getBillingAddress();
-        $carrierCode = $addressInformation->getShippingCarrierCode();
-        $methodCode = $addressInformation->getShippingMethodCode();
-
-        if (!$address->getCustomerAddressId()) {
-            $address->setCustomerAddressId(null);
-        }
-
-        if (!$address->getCountryId()) {
-            throw new StateException(__('Shipping address is not set'));
-        }
-
         /** @var \Magento\Quote\Model\Quote $quote */
         $quote = $this->quoteRepository->getActive($cartId);
-        $address->setLimitCarrier($carrierCode);
-        $quote = $this->prepareShippingAssignment($quote, $address, $carrierCode . '_' . $methodCode);
         $this->validateQuote($quote);
-        $quote->setIsMultiShipping(false);
 
-        if ($billingAddress) {
-            $quote->setBillingAddress($billingAddress);
+        $address = $addressInformation->getShippingAddress();
+        if (!$address || !$address->getCountryId()) {
+            throw new StateException(__('Shipping address is not set'));
+        }
+        if (!$address->getCustomerAddressId()) {
+            $address->setCustomerAddressId(null);
         }
 
         try {
+            $billingAddress = $addressInformation->getBillingAddress();
+            if ($billingAddress) {
+                $this->addressValidator->validateForCart($quote, $billingAddress);
+                $quote->setBillingAddress($billingAddress);
+            }
+
+            $this->addressValidator->validateForCart($quote, $address);
+            $carrierCode = $addressInformation->getShippingCarrierCode();
+            $address->setLimitCarrier($carrierCode);
+            $methodCode = $addressInformation->getShippingMethodCode();
+            $quote = $this->prepareShippingAssignment($quote, $address, $carrierCode . '_' . $methodCode);
+            $quote->setIsMultiShipping(false);
+
             $this->quoteRepository->save($quote);
         } catch (\Exception $e) {
             $this->logger->critical($e);
@@ -211,6 +214,8 @@ protected function validateQuote(\Magento\Quote\Model\Quote $quote)
     }
 
     /**
+     * Prepare shipping assignment.
+     *
      * @param CartInterface $quote
      * @param AddressInterface $address
      * @param string $method

diff --git a/app/code/Magento/Checkout/Test/Unit/Model/ShippingInformationManagementTest.php b/app/code/Magento/Checkout/Test/Unit/Model/ShippingInformationManagementTest.php
@@ -5,6 +5,8 @@
  */
 namespace Magento\Checkout\Test\Unit\Model;
 
+use Magento\Quote\Model\QuoteAddressValidator;
+
 /**
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  * @SuppressWarnings(PHPMD.TooManyFields)
@@ -149,6 +151,7 @@ protected function setUp()
             $this->getMock(\Magento\Quote\Api\Data\CartExtensionFactory::class, ['create'], [], '', false);
         $this->shippingFactoryMock =
             $this->getMock(\Magento\Quote\Model\ShippingFactory::class, ['create'], [], '', false);
+        $this->addressValidatorMock = $this->getMock(QuoteAddressValidator::class, [], [], '', false);
 
         $this->model = $this->objectManager->getObject(
             \Magento\Checkout\Model\ShippingInformationManagement::class,
@@ -157,6 +160,7 @@ protected function setUp()
                 'paymentDetailsFactory' => $this->paymentDetailsFactoryMock,
                 'cartTotalsRepository' => $this->cartTotalsRepositoryMock,
                 'quoteRepository' => $this->quoteRepositoryMock,
+                'addressValidator' => $this->addressValidatorMock,
             ]
         );
         $this->objectManager->setBackwardCompatibleProperty(
@@ -183,22 +187,8 @@ protected function setUp()
     public function testSaveAddressInformationIfCartIsEmpty()
     {
         $cartId = 100;
-        $carrierCode = 'carrier_code';
-        $shippingMethod = 'shipping_method';
         $addressInformationMock = $this->getMock(\Magento\Checkout\Api\Data\ShippingInformationInterface::class);
 
-        $billingAddress = $this->getMock(\Magento\Quote\Api\Data\AddressInterface::class);
-        $addressInformationMock->expects($this->once())
-            ->method('getShippingAddress')
-            ->willReturn($this->shippingAddressMock);
-        $addressInformationMock->expects($this->once())->method('getBillingAddress')->willReturn($billingAddress);
-        $addressInformationMock->expects($this->once())->method('getShippingCarrierCode')->willReturn($carrierCode);
-        $addressInformationMock->expects($this->once())->method('getShippingMethodCode')->willReturn($shippingMethod);
-
-        $this->shippingAddressMock->expects($this->once())->method('getCountryId')->willReturn('USA');
-
-        $this->setShippingAssignmentsMocks($carrierCode . '_' . $shippingMethod);
-
         $this->quoteMock->expects($this->once())->method('getItemsCount')->willReturn(0);
         $this->quoteRepositoryMock->expects($this->once())
             ->method('getActive')
@@ -271,21 +261,19 @@ private function setShippingAssignmentsMocks($shippingMethod)
     public function testSaveAddressInformationIfShippingAddressNotSet()
     {
         $cartId = 100;
-        $carrierCode = 'carrier_code';
-        $shippingMethod = 'shipping_method';
         $addressInformationMock = $this->getMock(\Magento\Checkout\Api\Data\ShippingInformationInterface::class);
-
         $addressInformationMock->expects($this->once())
             ->method('getShippingAddress')
             ->willReturn($this->shippingAddressMock);
-        $addressInformationMock->expects($this->once())->method('getShippingCarrierCode')->willReturn($carrierCode);
-        $addressInformationMock->expects($this->once())->method('getShippingMethodCode')->willReturn($shippingMethod);
-
-        $billingAddress = $this->getMock(\Magento\Quote\Api\Data\AddressInterface::class);
-        $addressInformationMock->expects($this->once())->method('getBillingAddress')->willReturn($billingAddress);
 
         $this->shippingAddressMock->expects($this->once())->method('getCountryId')->willReturn(null);
 
+        $this->quoteRepositoryMock->expects($this->once())
+            ->method('getActive')
+            ->with($cartId)
+            ->willReturn($this->quoteMock);
+        $this->quoteMock->expects($this->once())->method('getItemsCount')->willReturn(100);
+
         $this->model->saveAddressInformation($cartId, $addressInformationMock);
     }
 
@@ -300,6 +288,9 @@ public function testSaveAddressInformationIfCanNotSaveQuote()
         $shippingMethod = 'shipping_method';
         $addressInformationMock = $this->getMock(\Magento\Checkout\Api\Data\ShippingInformationInterface::class);
 
+        $this->addressValidatorMock->expects($this->exactly(2))
+            ->method('validateForCart');
+
         $this->quoteRepositoryMock->expects($this->once())
             ->method('getActive')
             ->with($cartId)
@@ -341,6 +332,9 @@ public function testSaveAddressInformationIfCarrierCodeIsInvalid()
         $shippingMethod = 'shipping_method';
         $addressInformationMock = $this->getMock(\Magento\Checkout\Api\Data\ShippingInformationInterface::class);
 
+        $this->addressValidatorMock->expects($this->exactly(2))
+            ->method('validateForCart');
+
         $this->quoteRepositoryMock->expects($this->once())
             ->method('getActive')
             ->with($cartId)
@@ -382,6 +376,9 @@ public function testSaveAddressInformation()
         $shippingMethod = 'shipping_method';
         $addressInformationMock = $this->getMock(\Magento\Checkout\Api\Data\ShippingInformationInterface::class);
 
+        $this->addressValidatorMock->expects($this->exactly(2))
+            ->method('validateForCart');
+
         $this->quoteRepositoryMock->expects($this->once())
             ->method('getActive')
             ->with($cartId)

diff --git a/app/code/Magento/Cms/Helper/Wysiwyg/Images.php b/app/code/Magento/Cms/Helper/Wysiwyg/Images.php
@@ -27,11 +27,11 @@ class Images extends \Magento\Framework\App\Helper\AbstractHelper
     protected $_currentUrl;
 
     /**
-     * Currenty selected store ID if applicable
+     * Currently selected store ID if applicable
      *
      * @var int
      */
-    protected $_storeId = null;
+    protected $_storeId;
 
     /**
      * @var \Magento\Framework\Filesystem\Directory\Write
@@ -71,7 +71,7 @@ public function __construct(
         $this->_storeManager = $storeManager;
 
         $this->_directory = $filesystem->getDirectoryWrite(DirectoryList::MEDIA);
-        $this->_directory->create(\Magento\Cms\Model\Wysiwyg\Config::IMAGE_DIRECTORY);
+        $this->_directory->create($this->getStorageRoot());
     }
 
     /**
@@ -93,7 +93,17 @@ public function setStoreId($store)
      */
     public function getStorageRoot()
     {
-        return $this->_directory->getAbsolutePath(\Magento\Cms\Model\Wysiwyg\Config::IMAGE_DIRECTORY);
+        return $this->_directory->getAbsolutePath($this->getStorageRootSubpath());
+    }
+
+    /**
+     * Get image storage root subpath.  User is unable to traverse outside of this subpath in media gallery
+     *
+     * @return string
+     */
+    public function getStorageRootSubpath()
+    {
+        return '';
     }
 
     /**
@@ -141,7 +151,7 @@ public function convertIdToPath($id)
             return $this->getStorageRoot();
         } else {
             $path = $this->getStorageRoot() . $this->idDecode($id);
-            if (strpos($path, DIRECTORY_SEPARATOR . '..' . DIRECTORY_SEPARATOR) !== false) {
+            if (preg_match('/\.\.(\\\|\/)/', $path)) {
                 throw new \InvalidArgumentException('Path is invalid');
             }
 
@@ -208,7 +218,7 @@ public function getImageHtmlDeclaration($filename, $renderAsTag = false)
     public function getCurrentPath()
     {
         if (!$this->_currentPath) {
-            $currentPath = $this->_directory->getAbsolutePath() . \Magento\Cms\Model\Wysiwyg\Config::IMAGE_DIRECTORY;
+            $currentPath = $this->getStorageRoot();
             $path = $this->_getRequest()->getParam($this->getTreeNodeName());
             if ($path) {
                 $path = $this->convertIdToPath($path);
@@ -244,7 +254,7 @@ public function getCurrentUrl()
             )->getBaseUrl(
                 \Magento\Framework\UrlInterface::URL_TYPE_MEDIA
             );
-            $this->_currentUrl = $mediaUrl . $this->_directory->getRelativePath($path) . '/';
+            $this->_currentUrl = rtrim($mediaUrl . $this->_directory->getRelativePath($path), '/') . '/';
         }
         return $this->_currentUrl;
     }

diff --git a/app/code/Magento/Cms/Model/Wysiwyg/Images/Storage.php b/app/code/Magento/Cms/Model/Wysiwyg/Images/Storage.php
@@ -238,10 +238,12 @@ protected function getConditionsForExcludeDirs()
     protected function removeItemFromCollection($collection, $conditions)
     {
         $regExp = $conditions['reg_exp'] ? '~' . implode('|', array_keys($conditions['reg_exp'])) . '~i' : null;
-        $storageRootLength = strlen($this->_cmsWysiwygImages->getStorageRoot());
+        $storageRoot = $this->_cmsWysiwygImages->getStorageRoot();
+        $storageRootLength = strlen($storageRoot);
 
         foreach ($collection as $key => $value) {
-            $rootChildParts = explode('/', substr($value->getFilename(), $storageRootLength));
+            $mediaSubPathname = substr($value->getFilename(), $storageRootLength);
+            $rootChildParts = explode('/', '/' . ltrim($mediaSubPathname, '/'));
 
             if (array_key_exists($rootChildParts[1], $conditions['plain'])
                 || ($regExp && preg_match($regExp, $value->getFilename()))) {
@@ -316,6 +318,8 @@ public function getFilesCollection($path, $type = null)
             $item->setName($item->getBasename());
             $item->setShortName($this->_cmsWysiwygImages->getShortFilename($item->getBasename()));
             $item->setUrl($this->_cmsWysiwygImages->getCurrentUrl() . $item->getBasename());
+            $item->setSize(filesize($item->getFilename()));
+            $item->setMimeType(\mime_content_type($item->getFilename()));
 
             if ($this->isImage($item->getBasename())) {
                 $thumbUrl = $this->getThumbnailUrl($item->getFilename(), true);
@@ -407,7 +411,7 @@ public function createDirectory($name, $path)
     /**
      * Recursively delete directory from storage
      *
-     * @param string $path Target dir
+     * @param string $path Absolute path to target directory
      * @return void
      * @throws \Magento\Framework\Exception\LocalizedException
      */
@@ -416,12 +420,19 @@ public function deleteDirectory($path)
         if ($this->_coreFileStorageDb->checkDbUsage()) {
             $this->_directoryDatabaseFactory->create()->deleteDirectory($path);
         }
+        if (!$this->isPathAllowed($path, $this->getConditionsForExcludeDirs())) {
+            throw new \Magento\Framework\Exception\LocalizedException(
+                __('We cannot delete directory %1.', $this->_getRelativePathToRoot($path))
+            );
+        }
         try {
             $this->_deleteByPath($path);
             $path = $this->getThumbnailRoot() . $this->_getRelativePathToRoot($path);
             $this->_deleteByPath($path);
         } catch (\Magento\Framework\Exception\FileSystemException $e) {
-            throw new \Magento\Framework\Exception\LocalizedException(__('We cannot delete directory %1.', $path));
+            throw new \Magento\Framework\Exception\LocalizedException(
+                __('We cannot delete directory %1.', $this->_getRelativePathToRoot($path))
+            );
         }
     }
 
@@ -468,14 +479,18 @@ public function deleteFile($target)
     /**
      * Upload and resize new file.
      *
-     * @param string $targetPath Target directory
+     * @param string $targetPath Absolute path to target directory
      * @param string $type Type of storage, e.g. image, media etc.
      * @return array File info Array
      * @throws \Magento\Framework\Exception\LocalizedException
-     * @throws \Exception
      */
     public function uploadFile($targetPath, $type = null)
     {
+        if (!$this->isPathAllowed($targetPath, $this->getConditionsForExcludeDirs())) {
+            throw new \Magento\Framework\Exception\LocalizedException(
+                __('We can\'t upload the file to current folder right now. Please try another folder.')
+            );
+        }
         /** @var \Magento\MediaStorage\Model\File\Uploader $uploader */
         $uploader = $this->_uploaderFactory->create(['fileId' => 'image']);
         $allowed = $this->getAllowedExtensions($type);
@@ -725,7 +740,7 @@ protected function _validatePath($path)
      */
     protected function _sanitizePath($path)
     {
-        return rtrim(preg_replace('~[/\\\]+~', '/', $this->_directory->getDriver()->getRealPath($path)), '/');
+        return rtrim(preg_replace('~[/\\\]+~', '/', $this->_directory->getDriver()->getRealPathSafety($path)), '/');
     }
 
     /**
@@ -771,4 +786,29 @@ private function getExtensionsList($type = null)
 
         return $allowed;
     }
+
+    /**
+     * Check if path is not in excluded dirs.
+     *
+     * @param string $path Absolute path
+     * @param array $conditions Exclude conditions
+     * @return bool
+     */
+    private function isPathAllowed($path, $conditions)
+    {
+        $isAllowed = true;
+        $regExp = $conditions['reg_exp'] ? '~' . implode('|', array_keys($conditions['reg_exp'])) . '~i' : null;
+        $storageRoot = $this->_cmsWysiwygImages->getStorageRoot();
+        $storageRootLength = strlen($storageRoot);
+
+        $mediaSubPathname = substr($path, $storageRootLength);
+        $rootChildParts = explode('/', '/' . ltrim($mediaSubPathname, '/'));
+
+        if (array_key_exists($rootChildParts[1], $conditions['plain'])
+            || ($regExp && preg_match($regExp, $path))) {
+            $isAllowed = false;
+        }
+
+        return $isAllowed;
+    }
 }