Skip to content

Commit b6e48be

Browse files
VladimirZaetsVladimirZaets
authored
ENGCOM-6297: Prevent saving upload media files in root directory #25546 
2 parents 148d6cc + 855638a commit b6e48be

File tree

2 files changed

+118
-27
lines changed

2 files changed

+118
-27
lines changed

‎app/code/Magento/CatalogImportExport/Model/Import/Product.php‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1198,7 +1198,7 @@ protected function _initTypeModels()
11981198
// phpcs:disable Magento2.Performance.ForeachArrayMerge.ForeachArrayMerge
11991199
$this->_fieldsMap = array_merge($this->_fieldsMap, $model->getCustomFieldsMapping());
12001200
$this->_specialAttributes = array_merge($this->_specialAttributes, $model->getParticularAttributes());
1201-
// phpcs:enable
1201+
// phpcs:enable
12021202
}
12031203
$this->_initErrorTemplates();
12041204
// remove doubles
@@ -2035,9 +2035,9 @@ protected function _saveProductTierPrices(array $tierPriceData)
20352035
protected function _getUploader()
20362036
{
20372037
if ($this->_fileUploader === null) {
2038-
$this->_fileUploader = $this->_uploaderFactory->create();
2038+
$fileUploader = $this->_uploaderFactory->create();
20392039

2040-
$this->_fileUploader->init();
2040+
$fileUploader->init();
20412041

20422042
$dirConfig = DirectoryList::getDefaultConfig();
20432043
$dirAddon = $dirConfig[DirectoryList::MEDIA][DirectoryList::PATH];
@@ -2048,7 +2048,7 @@ protected function _getUploader()
20482048
$tmpPath = $dirAddon . '/' . $this->_mediaDirectory->getRelativePath('import');
20492049
}
20502050

2051-
if (!$this->_fileUploader->setTmpDir($tmpPath)) {
2051+
if (!$fileUploader->setTmpDir($tmpPath)) {
20522052
throw new LocalizedException(
20532053
__('File directory \'%1\' is not readable.', $tmpPath)
20542054
);
@@ -2057,11 +2057,13 @@ protected function _getUploader()
20572057
$destinationPath = $dirAddon . '/' . $this->_mediaDirectory->getRelativePath($destinationDir);
20582058

20592059
$this->_mediaDirectory->create($destinationPath);
2060-
if (!$this->_fileUploader->setDestDir($destinationPath)) {
2060+
if (!$fileUploader->setDestDir($destinationPath)) {
20612061
throw new LocalizedException(
20622062
__('File directory \'%1\' is not writable.', $destinationPath)
20632063
);
20642064
}
2065+
2066+
$this->_fileUploader = $fileUploader;
20652067
}
20662068
return $this->_fileUploader;
20672069
}

‎app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/ProductTest.php‎

Lines changed: 111 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -284,9 +284,11 @@ protected function setUp()
284284
->getMock();
285285
$this->storeResolver =
286286
$this->getMockBuilder(\Magento\CatalogImportExport\Model\Import\Product\StoreResolver::class)
287-
->setMethods([
288-
'getStoreCodeToId',
289-
])
287+
->setMethods(
288+
[
289+
'getStoreCodeToId',
290+
]
291+
)
290292
->disableOriginalConstructor()
291293
->getMock();
292294
$this->skuProcessor =
@@ -410,7 +412,7 @@ protected function _objectConstructor()
410412
$this->_filesystem->expects($this->once())
411413
->method('getDirectoryWrite')
412414
->with(DirectoryList::ROOT)
413-
->will($this->returnValue(self::MEDIA_DIRECTORY));
415+
->willReturn($this->_mediaDirectory);
414416

415417
$this->validator->expects($this->any())->method('init');
416418
return $this;
@@ -596,9 +598,13 @@ public function testGetMultipleValueSeparatorDefault()
596598
public function testGetMultipleValueSeparatorFromParameters()
597599
{
598600
$expectedSeparator = 'value';
599-
$this->setPropertyValue($this->importProduct, '_parameters', [
600-
\Magento\ImportExport\Model\Import::FIELD_FIELD_MULTIPLE_VALUE_SEPARATOR => $expectedSeparator,
601-
]);
601+
$this->setPropertyValue(
602+
$this->importProduct,
603+
'_parameters',
604+
[
605+
\Magento\ImportExport\Model\Import::FIELD_FIELD_MULTIPLE_VALUE_SEPARATOR => $expectedSeparator,
606+
]
607+
);
602608

603609
$this->assertEquals(
604610
$expectedSeparator,
@@ -618,9 +624,13 @@ public function testGetEmptyAttributeValueConstantDefault()
618624
public function testGetEmptyAttributeValueConstantFromParameters()
619625
{
620626
$expectedSeparator = '__EMPTY__VALUE__TEST__';
621-
$this->setPropertyValue($this->importProduct, '_parameters', [
622-
\Magento\ImportExport\Model\Import::FIELD_EMPTY_ATTRIBUTE_VALUE_CONSTANT => $expectedSeparator,
623-
]);
627+
$this->setPropertyValue(
628+
$this->importProduct,
629+
'_parameters',
630+
[
631+
\Magento\ImportExport\Model\Import::FIELD_EMPTY_ATTRIBUTE_VALUE_CONSTANT => $expectedSeparator,
632+
]
633+
);
624634

625635
$this->assertEquals(
626636
$expectedSeparator,
@@ -632,9 +642,12 @@ public function testDeleteProductsForReplacement()
632642
{
633643
$importProduct = $this->getMockBuilder(Product::class)
634644
->disableOriginalConstructor()
635-
->setMethods([
636-
'setParameters', '_deleteProducts'
637-
])
645+
->setMethods(
646+
[
647+
'setParameters',
648+
'_deleteProducts'
649+
]
650+
)
638651
->getMock();
639652

640653
$importProduct->expects($this->once())->method('setParameters')->with(
@@ -764,9 +777,13 @@ public function testGetProductWebsites()
764777
'key 3' => 'val',
765778
];
766779
$expectedResult = array_keys($productValue);
767-
$this->setPropertyValue($this->importProduct, 'websitesCache', [
768-
$productSku => $productValue
769-
]);
780+
$this->setPropertyValue(
781+
$this->importProduct,
782+
'websitesCache',
783+
[
784+
$productSku => $productValue
785+
]
786+
);
770787

771788
$actualResult = $this->importProduct->getProductWebsites($productSku);
772789

@@ -785,9 +802,13 @@ public function testGetProductCategories()
785802
'key 3' => 'val',
786803
];
787804
$expectedResult = array_keys($productValue);
788-
$this->setPropertyValue($this->importProduct, 'categoriesCache', [
789-
$productSku => $productValue
790-
]);
805+
$this->setPropertyValue(
806+
$this->importProduct,
807+
'categoriesCache',
808+
[
809+
$productSku => $productValue
810+
]
811+
);
791812

792813
$actualResult = $this->importProduct->getProductCategories($productSku);
793814

@@ -1112,9 +1133,13 @@ public function testValidateRowSetAttributeSetCodeIntoRowData()
11121133
->disableOriginalConstructor()
11131134
->getMock();
11141135
$productType->expects($this->once())->method('isRowValid')->with($expectedRowData);
1115-
$this->setPropertyValue($importProduct, '_productTypeModels', [
1116-
$newSku['type_id'] => $productType
1117-
]);
1136+
$this->setPropertyValue(
1137+
$importProduct,
1138+
'_productTypeModels',
1139+
[
1140+
$newSku['type_id'] => $productType
1141+
]
1142+
);
11181143

11191144
//suppress option validation
11201145
$this->_rewriteGetOptionEntityInImportProduct($importProduct);
@@ -1229,6 +1254,56 @@ public function testParseAttributesWithWrappedValuesWillReturnsLowercasedAttribu
12291254
$this->assertArrayNotHasKey('PARAM2', $attributes);
12301255
}
12311256

1257+
/**
1258+
* @param bool $isRead
1259+
* @param bool $isWrite
1260+
* @param string $message
1261+
* @dataProvider fillUploaderObjectDataProvider
1262+
*/
1263+
public function testFillUploaderObject($isRead, $isWrite, $message)
1264+
{
1265+
$fileUploaderMock = $this
1266+
->getMockBuilder(\Magento\CatalogImportExport\Model\Import\Uploader::class)
1267+
->disableOriginalConstructor()
1268+
->getMock();
1269+
1270+
$fileUploaderMock
1271+
->method('setTmpDir')
1272+
->with('pub/media/import')
1273+
->willReturn($isRead);
1274+
1275+
$fileUploaderMock
1276+
->method('setDestDir')
1277+
->with('pub/media/catalog/product')
1278+
->willReturn($isWrite);
1279+
1280+
$this->_mediaDirectory
1281+
->method('getRelativePath')
1282+
->willReturnMap(
1283+
[
1284+
['import', 'import'],
1285+
['catalog/product', 'catalog/product'],
1286+
]
1287+
);
1288+
1289+
$this->_mediaDirectory
1290+
->method('create')
1291+
->with('pub/media/catalog/product');
1292+
1293+
$this->_uploaderFactory
1294+
->expects($this->once())
1295+
->method('create')
1296+
->willReturn($fileUploaderMock);
1297+
1298+
try {
1299+
$this->importProduct->getUploader();
1300+
$this->assertNotNull($this->getPropertyValue($this->importProduct, '_fileUploader'));
1301+
} catch (\Magento\Framework\Exception\LocalizedException $e) {
1302+
$this->assertNull($this->getPropertyValue($this->importProduct, '_fileUploader'));
1303+
$this->assertEquals($message, $e->getMessage());
1304+
}
1305+
}
1306+
12321307
/**
12331308
* Test that errors occurred during importing images are logged.
12341309
*
@@ -1275,6 +1350,20 @@ function ($name) use ($throwException, $exception) {
12751350
);
12761351
}
12771352

1353+
/**
1354+
* Data provider for testFillUploaderObject.
1355+
*
1356+
* @return array
1357+
*/
1358+
public function fillUploaderObjectDataProvider()
1359+
{
1360+
return [
1361+
[false, true, 'File directory \'pub/media/import\' is not readable.'],
1362+
[true, false, 'File directory \'pub/media/catalog/product\' is not writable.'],
1363+
[true, true, ''],
1364+
];
1365+
}
1366+
12781367
/**
12791368
* Data provider for testUploadMediaFiles.
12801369
*

0 commit comments

Comments
 (0)