ENGCOM-6297: Prevent saving upload media files in root directory #25546

 - Merge Pull Request #25546 from kenboy/magento2:upload-media-files-fix
 - Merged commits:
   1. e6e6769
   2. 4b79d4d
   3. d8e1a71

Author: magento-engcom-team

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

@@ -1198,7 +1198,7 @@ protected function _initTypeModels()
             // phpcs:disable Magento2.Performance.ForeachArrayMerge.ForeachArrayMerge
             $this->_fieldsMap = array_merge($this->_fieldsMap, $model->getCustomFieldsMapping());
             $this->_specialAttributes = array_merge($this->_specialAttributes, $model->getParticularAttributes());
-            // phpcs:enable 
+            // phpcs:enable
         }
         $this->_initErrorTemplates();
         // remove doubles
@@ -2035,9 +2035,9 @@ protected function _saveProductTierPrices(array $tierPriceData)
     protected function _getUploader()
     {
         if ($this->_fileUploader === null) {
-            $this->_fileUploader = $this->_uploaderFactory->create();
+            $fileUploader = $this->_uploaderFactory->create();
 
-            $this->_fileUploader->init();
+            $fileUploader->init();
 
             $dirConfig = DirectoryList::getDefaultConfig();
             $dirAddon = $dirConfig[DirectoryList::MEDIA][DirectoryList::PATH];
@@ -2048,7 +2048,7 @@ protected function _getUploader()
                 $tmpPath = $dirAddon . '/' . $this->_mediaDirectory->getRelativePath('import');
             }
 
-            if (!$this->_fileUploader->setTmpDir($tmpPath)) {
+            if (!$fileUploader->setTmpDir($tmpPath)) {
                 throw new LocalizedException(
                     __('File directory \'%1\' is not readable.', $tmpPath)
                 );
@@ -2057,11 +2057,13 @@ protected function _getUploader()
             $destinationPath = $dirAddon . '/' . $this->_mediaDirectory->getRelativePath($destinationDir);
 
             $this->_mediaDirectory->create($destinationPath);
-            if (!$this->_fileUploader->setDestDir($destinationPath)) {
+            if (!$fileUploader->setDestDir($destinationPath)) {
                 throw new LocalizedException(
                     __('File directory \'%1\' is not writable.', $destinationPath)
                 );
             }
+
+            $this->_fileUploader = $fileUploader;
         }
         return $this->_fileUploader;
     }

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

@@ -284,9 +284,11 @@ protected function setUp()
                 ->getMock();
         $this->storeResolver =
             $this->getMockBuilder(\Magento\CatalogImportExport\Model\Import\Product\StoreResolver::class)
-                ->setMethods([
-                    'getStoreCodeToId',
-                ])
+                ->setMethods(
+                    [
+                        'getStoreCodeToId',
+                    ]
+                )
                 ->disableOriginalConstructor()
                 ->getMock();
         $this->skuProcessor =
@@ -410,7 +412,7 @@ protected function _objectConstructor()
         $this->_filesystem->expects($this->once())
             ->method('getDirectoryWrite')
             ->with(DirectoryList::ROOT)
-            ->will($this->returnValue(self::MEDIA_DIRECTORY));
+            ->willReturn($this->_mediaDirectory);
 
         $this->validator->expects($this->any())->method('init');
         return $this;
@@ -596,9 +598,13 @@ public function testGetMultipleValueSeparatorDefault()
     public function testGetMultipleValueSeparatorFromParameters()
     {
         $expectedSeparator = 'value';
-        $this->setPropertyValue($this->importProduct, '_parameters', [
-            \Magento\ImportExport\Model\Import::FIELD_FIELD_MULTIPLE_VALUE_SEPARATOR => $expectedSeparator,
-        ]);
+        $this->setPropertyValue(
+            $this->importProduct,
+            '_parameters',
+            [
+                \Magento\ImportExport\Model\Import::FIELD_FIELD_MULTIPLE_VALUE_SEPARATOR => $expectedSeparator,
+            ]
+        );
 
         $this->assertEquals(
             $expectedSeparator,
@@ -618,9 +624,13 @@ public function testGetEmptyAttributeValueConstantDefault()
     public function testGetEmptyAttributeValueConstantFromParameters()
     {
         $expectedSeparator = '__EMPTY__VALUE__TEST__';
-        $this->setPropertyValue($this->importProduct, '_parameters', [
-            \Magento\ImportExport\Model\Import::FIELD_EMPTY_ATTRIBUTE_VALUE_CONSTANT => $expectedSeparator,
-        ]);
+        $this->setPropertyValue(
+            $this->importProduct,
+            '_parameters',
+            [
+                \Magento\ImportExport\Model\Import::FIELD_EMPTY_ATTRIBUTE_VALUE_CONSTANT => $expectedSeparator,
+            ]
+        );
 
         $this->assertEquals(
             $expectedSeparator,
@@ -632,9 +642,12 @@ public function testDeleteProductsForReplacement()
     {
         $importProduct = $this->getMockBuilder(Product::class)
             ->disableOriginalConstructor()
-            ->setMethods([
-                'setParameters', '_deleteProducts'
-            ])
+            ->setMethods(
+                [
+                    'setParameters',
+                    '_deleteProducts'
+                ]
+            )
             ->getMock();
 
         $importProduct->expects($this->once())->method('setParameters')->with(
@@ -764,9 +777,13 @@ public function testGetProductWebsites()
             'key 3' => 'val',
         ];
         $expectedResult = array_keys($productValue);
-        $this->setPropertyValue($this->importProduct, 'websitesCache', [
-            $productSku => $productValue
-        ]);
+        $this->setPropertyValue(
+            $this->importProduct,
+            'websitesCache',
+            [
+                $productSku => $productValue
+            ]
+        );
 
         $actualResult = $this->importProduct->getProductWebsites($productSku);
 
@@ -785,9 +802,13 @@ public function testGetProductCategories()
             'key 3' => 'val',
         ];
         $expectedResult = array_keys($productValue);
-        $this->setPropertyValue($this->importProduct, 'categoriesCache', [
-            $productSku => $productValue
-        ]);
+        $this->setPropertyValue(
+            $this->importProduct,
+            'categoriesCache',
+            [
+                $productSku => $productValue
+            ]
+        );
 
         $actualResult = $this->importProduct->getProductCategories($productSku);
 
@@ -1112,9 +1133,13 @@ public function testValidateRowSetAttributeSetCodeIntoRowData()
             ->disableOriginalConstructor()
             ->getMock();
         $productType->expects($this->once())->method('isRowValid')->with($expectedRowData);
-        $this->setPropertyValue($importProduct, '_productTypeModels', [
-            $newSku['type_id'] => $productType
-        ]);
+        $this->setPropertyValue(
+            $importProduct,
+            '_productTypeModels',
+            [
+                $newSku['type_id'] => $productType
+            ]
+        );
 
         //suppress option validation
         $this->_rewriteGetOptionEntityInImportProduct($importProduct);
@@ -1229,6 +1254,56 @@ public function testParseAttributesWithWrappedValuesWillReturnsLowercasedAttribu
         $this->assertArrayNotHasKey('PARAM2', $attributes);
     }
 
+    /**
+     * @param bool $isRead
+     * @param bool $isWrite
+     * @param string $message
+     * @dataProvider fillUploaderObjectDataProvider
+     */
+    public function testFillUploaderObject($isRead, $isWrite, $message)
+    {
+        $fileUploaderMock = $this
+            ->getMockBuilder(\Magento\CatalogImportExport\Model\Import\Uploader::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $fileUploaderMock
+            ->method('setTmpDir')
+            ->with('pub/media/import')
+            ->willReturn($isRead);
+
+        $fileUploaderMock
+            ->method('setDestDir')
+            ->with('pub/media/catalog/product')
+            ->willReturn($isWrite);
+
+        $this->_mediaDirectory
+            ->method('getRelativePath')
+            ->willReturnMap(
+                [
+                    ['import', 'import'],
+                    ['catalog/product', 'catalog/product'],
+                ]
+            );
+
+        $this->_mediaDirectory
+            ->method('create')
+            ->with('pub/media/catalog/product');
+
+        $this->_uploaderFactory
+            ->expects($this->once())
+            ->method('create')
+            ->willReturn($fileUploaderMock);
+
+        try {
+            $this->importProduct->getUploader();
+            $this->assertNotNull($this->getPropertyValue($this->importProduct, '_fileUploader'));
+        } catch (\Magento\Framework\Exception\LocalizedException $e) {
+            $this->assertNull($this->getPropertyValue($this->importProduct, '_fileUploader'));
+            $this->assertEquals($message, $e->getMessage());
+        }
+    }
+
     /**
      * Test that errors occurred during importing images are logged.
      *
@@ -1275,6 +1350,20 @@ function ($name) use ($throwException, $exception) {
         );
     }
 
+    /**
+     * Data provider for testFillUploaderObject.
+     *
+     * @return array
+     */
+    public function fillUploaderObjectDataProvider()
+    {
+        return [
+            [false, true, 'File directory \'pub/media/import\' is not readable.'],
+            [true, false, 'File directory \'pub/media/catalog/product\' is not writable.'],
+            [true, true, ''],
+        ];
+    }
+
     /**
      * Data provider for testUploadMediaFiles.
      *