Skip to content

Commit 5cba861

Browse files
joanhejoanhe
authored
Merge pull request #4101 from magento-arcticfoxes/2.1.18-develop-pr
[arcticfoxes] Bug Fixes
2 parents ade1293 + bb0e4ef commit 5cba861

File tree

15 files changed

+412
-91
lines changed

15 files changed

+412
-91
lines changed

‎app/code/Magento/Checkout/Model/ShippingInformationManagement.php

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
use Magento\Framework\App\ObjectManager;
1919

2020
/**
21+
* Class ShippingInformationManagement
22+
*
2123
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
2224
*/
2325
class ShippingInformationManagement implements \Magento\Checkout\Api\ShippingInformationManagementInterface
@@ -149,31 +151,32 @@ public function saveAddressInformation(
149151
$cartId,
150152
\Magento\Checkout\Api\Data\ShippingInformationInterface $addressInformation
151153
) {
152-
$address = $addressInformation->getShippingAddress();
153-
$billingAddress = $addressInformation->getBillingAddress();
154-
$carrierCode = $addressInformation->getShippingCarrierCode();
155-
$methodCode = $addressInformation->getShippingMethodCode();
156-
157-
if (!$address->getCustomerAddressId()) {
158-
$address->setCustomerAddressId(null);
159-
}
160-
161-
if (!$address->getCountryId()) {
162-
throw new StateException(__('Shipping address is not set'));
163-
}
164-
165154
/** @var \Magento\Quote\Model\Quote $quote */
166155
$quote = $this->quoteRepository->getActive($cartId);
167-
$address->setLimitCarrier($carrierCode);
168-
$quote = $this->prepareShippingAssignment($quote, $address, $carrierCode . '_' . $methodCode);
169156
$this->validateQuote($quote);
170-
$quote->setIsMultiShipping(false);
171157

172-
if ($billingAddress) {
173-
$quote->setBillingAddress($billingAddress);
158+
$address = $addressInformation->getShippingAddress();
159+
if (!$address || !$address->getCountryId()) {
160+
throw new StateException(__('Shipping address is not set'));
161+
}
162+
if (!$address->getCustomerAddressId()) {
163+
$address->setCustomerAddressId(null);
174164
}
175165

176166
try {
167+
$billingAddress = $addressInformation->getBillingAddress();
168+
if ($billingAddress) {
169+
$this->addressValidator->validateForCart($quote, $billingAddress);
170+
$quote->setBillingAddress($billingAddress);
171+
}
172+
173+
$this->addressValidator->validateForCart($quote, $address);
174+
$carrierCode = $addressInformation->getShippingCarrierCode();
175+
$address->setLimitCarrier($carrierCode);
176+
$methodCode = $addressInformation->getShippingMethodCode();
177+
$quote = $this->prepareShippingAssignment($quote, $address, $carrierCode . '_' . $methodCode);
178+
$quote->setIsMultiShipping(false);
179+
177180
$this->quoteRepository->save($quote);
178181
} catch (\Exception $e) {
179182
$this->logger->critical($e);
@@ -211,6 +214,8 @@ protected function validateQuote(\Magento\Quote\Model\Quote $quote)
211214
}
212215

213216
/**
217+
* Prepare shipping assignment.
218+
*
214219
* @param CartInterface $quote
215220
* @param AddressInterface $address
216221
* @param string $method

‎app/code/Magento/Checkout/Test/Unit/Model/ShippingInformationManagementTest.php

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
*/
66
namespace Magento\Checkout\Test\Unit\Model;
77

8+
use Magento\Quote\Model\QuoteAddressValidator;
9+
810
/**
911
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
1012
* @SuppressWarnings(PHPMD.TooManyFields)
@@ -149,6 +151,7 @@ protected function setUp()
149151
$this->getMock(\Magento\Quote\Api\Data\CartExtensionFactory::class, ['create'], [], '', false);
150152
$this->shippingFactoryMock =
151153
$this->getMock(\Magento\Quote\Model\ShippingFactory::class, ['create'], [], '', false);
154+
$this->addressValidatorMock = $this->getMock(QuoteAddressValidator::class, [], [], '', false);
152155

153156
$this->model = $this->objectManager->getObject(
154157
\Magento\Checkout\Model\ShippingInformationManagement::class,
@@ -157,6 +160,7 @@ protected function setUp()
157160
'paymentDetailsFactory' => $this->paymentDetailsFactoryMock,
158161
'cartTotalsRepository' => $this->cartTotalsRepositoryMock,
159162
'quoteRepository' => $this->quoteRepositoryMock,
163+
'addressValidator' => $this->addressValidatorMock,
160164
]
161165
);
162166
$this->objectManager->setBackwardCompatibleProperty(
@@ -183,22 +187,8 @@ protected function setUp()
183187
public function testSaveAddressInformationIfCartIsEmpty()
184188
{
185189
$cartId = 100;
186-
$carrierCode = 'carrier_code';
187-
$shippingMethod = 'shipping_method';
188190
$addressInformationMock = $this->getMock(\Magento\Checkout\Api\Data\ShippingInformationInterface::class);
189191

190-
$billingAddress = $this->getMock(\Magento\Quote\Api\Data\AddressInterface::class);
191-
$addressInformationMock->expects($this->once())
192-
->method('getShippingAddress')
193-
->willReturn($this->shippingAddressMock);
194-
$addressInformationMock->expects($this->once())->method('getBillingAddress')->willReturn($billingAddress);
195-
$addressInformationMock->expects($this->once())->method('getShippingCarrierCode')->willReturn($carrierCode);
196-
$addressInformationMock->expects($this->once())->method('getShippingMethodCode')->willReturn($shippingMethod);
197-
198-
$this->shippingAddressMock->expects($this->once())->method('getCountryId')->willReturn('USA');
199-
200-
$this->setShippingAssignmentsMocks($carrierCode . '_' . $shippingMethod);
201-
202192
$this->quoteMock->expects($this->once())->method('getItemsCount')->willReturn(0);
203193
$this->quoteRepositoryMock->expects($this->once())
204194
->method('getActive')
@@ -271,21 +261,19 @@ private function setShippingAssignmentsMocks($shippingMethod)
271261
public function testSaveAddressInformationIfShippingAddressNotSet()
272262
{
273263
$cartId = 100;
274-
$carrierCode = 'carrier_code';
275-
$shippingMethod = 'shipping_method';
276264
$addressInformationMock = $this->getMock(\Magento\Checkout\Api\Data\ShippingInformationInterface::class);
277-
278265
$addressInformationMock->expects($this->once())
279266
->method('getShippingAddress')
280267
->willReturn($this->shippingAddressMock);
281-
$addressInformationMock->expects($this->once())->method('getShippingCarrierCode')->willReturn($carrierCode);
282-
$addressInformationMock->expects($this->once())->method('getShippingMethodCode')->willReturn($shippingMethod);
283-
284-
$billingAddress = $this->getMock(\Magento\Quote\Api\Data\AddressInterface::class);
285-
$addressInformationMock->expects($this->once())->method('getBillingAddress')->willReturn($billingAddress);
286268

287269
$this->shippingAddressMock->expects($this->once())->method('getCountryId')->willReturn(null);
288270

271+
$this->quoteRepositoryMock->expects($this->once())
272+
->method('getActive')
273+
->with($cartId)
274+
->willReturn($this->quoteMock);
275+
$this->quoteMock->expects($this->once())->method('getItemsCount')->willReturn(100);
276+
289277
$this->model->saveAddressInformation($cartId, $addressInformationMock);
290278
}
291279

@@ -300,6 +288,9 @@ public function testSaveAddressInformationIfCanNotSaveQuote()
300288
$shippingMethod = 'shipping_method';
301289
$addressInformationMock = $this->getMock(\Magento\Checkout\Api\Data\ShippingInformationInterface::class);
302290

291+
$this->addressValidatorMock->expects($this->exactly(2))
292+
->method('validateForCart');
293+
303294
$this->quoteRepositoryMock->expects($this->once())
304295
->method('getActive')
305296
->with($cartId)
@@ -341,6 +332,9 @@ public function testSaveAddressInformationIfCarrierCodeIsInvalid()
341332
$shippingMethod = 'shipping_method';
342333
$addressInformationMock = $this->getMock(\Magento\Checkout\Api\Data\ShippingInformationInterface::class);
343334

335+
$this->addressValidatorMock->expects($this->exactly(2))
336+
->method('validateForCart');
337+
344338
$this->quoteRepositoryMock->expects($this->once())
345339
->method('getActive')
346340
->with($cartId)
@@ -382,6 +376,9 @@ public function testSaveAddressInformation()
382376
$shippingMethod = 'shipping_method';
383377
$addressInformationMock = $this->getMock(\Magento\Checkout\Api\Data\ShippingInformationInterface::class);
384378

379+
$this->addressValidatorMock->expects($this->exactly(2))
380+
->method('validateForCart');
381+
385382
$this->quoteRepositoryMock->expects($this->once())
386383
->method('getActive')
387384
->with($cartId)

‎app/code/Magento/Cms/Helper/Wysiwyg/Images.php

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,11 @@ class Images extends \Magento\Framework\App\Helper\AbstractHelper
2727
protected $_currentUrl;
2828

2929
/**
30-
* Currenty selected store ID if applicable
30+
* Currently selected store ID if applicable
3131
*
3232
* @var int
3333
*/
34-
protected $_storeId = null;
34+
protected $_storeId;
3535

3636
/**
3737
* @var \Magento\Framework\Filesystem\Directory\Write
@@ -71,7 +71,7 @@ public function __construct(
7171
$this->_storeManager = $storeManager;
7272

7373
$this->_directory = $filesystem->getDirectoryWrite(DirectoryList::MEDIA);
74-
$this->_directory->create(\Magento\Cms\Model\Wysiwyg\Config::IMAGE_DIRECTORY);
74+
$this->_directory->create($this->getStorageRoot());
7575
}
7676

7777
/**
@@ -93,7 +93,17 @@ public function setStoreId($store)
9393
*/
9494
public function getStorageRoot()
9595
{
96-
return $this->_directory->getAbsolutePath(\Magento\Cms\Model\Wysiwyg\Config::IMAGE_DIRECTORY);
96+
return $this->_directory->getAbsolutePath($this->getStorageRootSubpath());
97+
}
98+
99+
/**
100+
* Get image storage root subpath. User is unable to traverse outside of this subpath in media gallery
101+
*
102+
* @return string
103+
*/
104+
public function getStorageRootSubpath()
105+
{
106+
return '';
97107
}
98108

99109
/**
@@ -141,7 +151,7 @@ public function convertIdToPath($id)
141151
return $this->getStorageRoot();
142152
} else {
143153
$path = $this->getStorageRoot() . $this->idDecode($id);
144-
if (strpos($path, DIRECTORY_SEPARATOR . '..' . DIRECTORY_SEPARATOR) !== false) {
154+
if (preg_match('/\.\.(\\\|\/)/', $path)) {
145155
throw new \InvalidArgumentException('Path is invalid');
146156
}
147157

@@ -208,7 +218,7 @@ public function getImageHtmlDeclaration($filename, $renderAsTag = false)
208218
public function getCurrentPath()
209219
{
210220
if (!$this->_currentPath) {
211-
$currentPath = $this->_directory->getAbsolutePath() . \Magento\Cms\Model\Wysiwyg\Config::IMAGE_DIRECTORY;
221+
$currentPath = $this->getStorageRoot();
212222
$path = $this->_getRequest()->getParam($this->getTreeNodeName());
213223
if ($path) {
214224
$path = $this->convertIdToPath($path);
@@ -244,7 +254,7 @@ public function getCurrentUrl()
244254
)->getBaseUrl(
245255
\Magento\Framework\UrlInterface::URL_TYPE_MEDIA
246256
);
247-
$this->_currentUrl = $mediaUrl . $this->_directory->getRelativePath($path) . '/';
257+
$this->_currentUrl = rtrim($mediaUrl . $this->_directory->getRelativePath($path), '/') . '/';
248258
}
249259
return $this->_currentUrl;
250260
}

‎app/code/Magento/Cms/Model/Wysiwyg/Images/Storage.php

Lines changed: 47 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -238,10 +238,12 @@ protected function getConditionsForExcludeDirs()
238238
protected function removeItemFromCollection($collection, $conditions)
239239
{
240240
$regExp = $conditions['reg_exp'] ? '~' . implode('|', array_keys($conditions['reg_exp'])) . '~i' : null;
241-
$storageRootLength = strlen($this->_cmsWysiwygImages->getStorageRoot());
241+
$storageRoot = $this->_cmsWysiwygImages->getStorageRoot();
242+
$storageRootLength = strlen($storageRoot);
242243

243244
foreach ($collection as $key => $value) {
244-
$rootChildParts = explode('/', substr($value->getFilename(), $storageRootLength));
245+
$mediaSubPathname = substr($value->getFilename(), $storageRootLength);
246+
$rootChildParts = explode('/', '/' . ltrim($mediaSubPathname, '/'));
245247

246248
if (array_key_exists($rootChildParts[1], $conditions['plain'])
247249
|| ($regExp && preg_match($regExp, $value->getFilename()))) {
@@ -316,6 +318,8 @@ public function getFilesCollection($path, $type = null)
316318
$item->setName($item->getBasename());
317319
$item->setShortName($this->_cmsWysiwygImages->getShortFilename($item->getBasename()));
318320
$item->setUrl($this->_cmsWysiwygImages->getCurrentUrl() . $item->getBasename());
321+
$item->setSize(filesize($item->getFilename()));
322+
$item->setMimeType(\mime_content_type($item->getFilename()));
319323

320324
if ($this->isImage($item->getBasename())) {
321325
$thumbUrl = $this->getThumbnailUrl($item->getFilename(), true);
@@ -407,7 +411,7 @@ public function createDirectory($name, $path)
407411
/**
408412
* Recursively delete directory from storage
409413
*
410-
* @param string $path Target dir
414+
* @param string $path Absolute path to target directory
411415
* @return void
412416
* @throws \Magento\Framework\Exception\LocalizedException
413417
*/
@@ -416,12 +420,19 @@ public function deleteDirectory($path)
416420
if ($this->_coreFileStorageDb->checkDbUsage()) {
417421
$this->_directoryDatabaseFactory->create()->deleteDirectory($path);
418422
}
423+
if (!$this->isPathAllowed($path, $this->getConditionsForExcludeDirs())) {
424+
throw new \Magento\Framework\Exception\LocalizedException(
425+
__('We cannot delete directory %1.', $this->_getRelativePathToRoot($path))
426+
);
427+
}
419428
try {
420429
$this->_deleteByPath($path);
421430
$path = $this->getThumbnailRoot() . $this->_getRelativePathToRoot($path);
422431
$this->_deleteByPath($path);
423432
} catch (\Magento\Framework\Exception\FileSystemException $e) {
424-
throw new \Magento\Framework\Exception\LocalizedException(__('We cannot delete directory %1.', $path));
433+
throw new \Magento\Framework\Exception\LocalizedException(
434+
__('We cannot delete directory %1.', $this->_getRelativePathToRoot($path))
435+
);
425436
}
426437
}
427438

@@ -468,14 +479,18 @@ public function deleteFile($target)
468479
/**
469480
* Upload and resize new file.
470481
*
471-
* @param string $targetPath Target directory
482+
* @param string $targetPath Absolute path to target directory
472483
* @param string $type Type of storage, e.g. image, media etc.
473484
* @return array File info Array
474485
* @throws \Magento\Framework\Exception\LocalizedException
475-
* @throws \Exception
476486
*/
477487
public function uploadFile($targetPath, $type = null)
478488
{
489+
if (!$this->isPathAllowed($targetPath, $this->getConditionsForExcludeDirs())) {
490+
throw new \Magento\Framework\Exception\LocalizedException(
491+
__('We can\'t upload the file to current folder right now. Please try another folder.')
492+
);
493+
}
479494
/** @var \Magento\MediaStorage\Model\File\Uploader $uploader */
480495
$uploader = $this->_uploaderFactory->create(['fileId' => 'image']);
481496
$allowed = $this->getAllowedExtensions($type);
@@ -725,7 +740,7 @@ protected function _validatePath($path)
725740
*/
726741
protected function _sanitizePath($path)
727742
{
728-
return rtrim(preg_replace('~[/\\\]+~', '/', $this->_directory->getDriver()->getRealPath($path)), '/');
743+
return rtrim(preg_replace('~[/\\\]+~', '/', $this->_directory->getDriver()->getRealPathSafety($path)), '/');
729744
}
730745

731746
/**
@@ -771,4 +786,29 @@ private function getExtensionsList($type = null)
771786

772787
return $allowed;
773788
}
789+
790+
/**
791+
* Check if path is not in excluded dirs.
792+
*
793+
* @param string $path Absolute path
794+
* @param array $conditions Exclude conditions
795+
* @return bool
796+
*/
797+
private function isPathAllowed($path, $conditions)
798+
{
799+
$isAllowed = true;
800+
$regExp = $conditions['reg_exp'] ? '~' . implode('|', array_keys($conditions['reg_exp'])) . '~i' : null;
801+
$storageRoot = $this->_cmsWysiwygImages->getStorageRoot();
802+
$storageRootLength = strlen($storageRoot);
803+
804+
$mediaSubPathname = substr($path, $storageRootLength);
805+
$rootChildParts = explode('/', '/' . ltrim($mediaSubPathname, '/'));
806+
807+
if (array_key_exists($rootChildParts[1], $conditions['plain'])
808+
|| ($regExp && preg_match($regExp, $path))) {
809+
$isAllowed = false;
810+
}
811+
812+
return $isAllowed;
813+
}
774814
}

0 commit comments

Comments
 (0)