Merge pull request #6438 from magento-borg/2.3.7-bugfixes-120920

[CIA] Bugfixes

Author: dvoskoboinikov

app/code/Magento/Catalog/Model/Product/Gallery/UpdateHandler.php

@@ -34,12 +34,10 @@ protected function processDeletedImages($product, array &$images)
         foreach ($images as &$image) {
             if (!empty($image['removed'])) {
                 if (!empty($image['value_id'])) {
-                    if (preg_match('/\.\.(\\\|\/)/', $image['file'])) {
-                        continue;
-                    }
                     $recordsToDelete[] = $image['value_id'];
                     $catalogPath = $this->mediaConfig->getBaseMediaPath();
-                    $isFile = $this->mediaDirectory->isFile($catalogPath . $image['file']);
+                    $filePath = $this->mediaDirectory->getRelativePath($catalogPath . $image['file']);
+                    $isFile = $this->mediaDirectory->isFile($filePath);
                     // only delete physical files if they are not used by any other products and if this file exist
                     if ($isFile && !($this->resourceModel->countImageUses($image['file']) > 1)) {
                         $filesToDelete[] = ltrim($image['file'], '/');
@@ -116,6 +114,7 @@ protected function extractStoreIds($product)
      *
      * @param array $files
      * @return null
+     * @throws \Magento\Framework\Exception\FileSystemException
      * @since 101.0.0
      */
     protected function removeDeletedImages(array $files)
@@ -125,5 +124,7 @@ protected function removeDeletedImages(array $files)
         foreach ($files as $filePath) {
             $this->mediaDirectory->delete($catalogPath . '/' . $filePath);
         }
+
+        return null;
     }
 }

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

@@ -9,6 +9,8 @@
 
 /**
  * Wysiwyg Images Helper.
+ *
+ * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  */
 class Images extends \Magento\Framework\App\Helper\AbstractHelper
 {
@@ -64,6 +66,11 @@ class Images extends \Magento\Framework\App\Helper\AbstractHelper
      */
     protected $escaper;
 
+    /**
+     * @var \Magento\Framework\Filesystem\Directory\Read
+     */
+    private $_readDirectory;
+
     /**
      * Construct
      *
@@ -87,6 +94,7 @@ public function __construct(
 
         $this->_directory = $filesystem->getDirectoryWrite(DirectoryList::MEDIA);
         $this->_directory->create($this->getStorageRoot());
+        $this->_readDirectory = $filesystem->getDirectoryReadByPath($this->getStorageRoot());
     }
 
     /**
@@ -166,7 +174,9 @@ public function convertIdToPath($id)
             return $this->getStorageRoot();
         } else {
             $path = $this->getStorageRoot() . $this->idDecode($id);
-            if (preg_match('/\.\.(\\\|\/)/', $path)) {
+            try {
+                $this->_readDirectory->getAbsolutePath($path);
+            } catch (\Exception $e) {
                 throw new \InvalidArgumentException('Path is invalid');
             }
 
@@ -225,6 +235,7 @@ public function getImageHtmlDeclaration($filename, $renderAsTag = false)
 
     /**
      * Return path of the current selected directory or root directory for startup
+     *
      * Try to create target directory if it doesn't exist
      *
      * @return string
@@ -294,6 +305,7 @@ public function idEncode($string)
     public function idDecode($string)
     {
         $string = strtr($string, ':_-', '+/=');
+        //phpcs:ignore Magento2.Functions.DiscouragedFunction
         return base64_decode($string);
     }
 
@@ -315,7 +327,7 @@ public function getShortFilename($filename, $maxLength = 20)
     /**
      * Set user-traversable image directory subpath relative to media directory and relative to nested storage root
      *
-     * @var string $subpath
+     * @param string $subpath
      * @return void
      */
     public function setImageDirectorySubpath($subpath)

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

@@ -545,6 +545,10 @@ public function uploadFile($targetPath, $type = null)
     {
         $targetPath = $this->file->getRealPathSafety($targetPath);
 
+        if ($this->file->isDirectory($targetPath)) {
+            $targetPath = $targetPath . DIRECTORY_SEPARATOR;
+        }
+
         if (!$this->isPathAllowed($targetPath, $this->getConditionsForExcludeDirs()) || strlen($targetPath) > 255) {
             throw new \Magento\Framework\Exception\LocalizedException(
                 __('We can\'t upload the file to current folder right now. Please try another folder.')

app/code/Magento/Cms/Test/Unit/Helper/Wysiwyg/ImagesTest.php

@@ -6,6 +6,7 @@
 namespace Magento\Cms\Test\Unit\Helper\Wysiwyg;
 
 use Magento\Cms\Model\Wysiwyg\Config as WysiwygConfig;
+use Magento\Framework\Exception\ValidatorException;
 
 /**
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
@@ -32,6 +33,11 @@ class ImagesTest extends \PHPUnit\Framework\TestCase
      */
     protected $directoryWriteMock;
 
+    /**
+     * @var \Magento\Framework\Filesystem\Directory\Read|\PHPUnit_Framework_MockObject_MockObject
+     */
+    protected $directoryReadMock;
+
     /**
      * @var \Magento\Store\Model\StoreManagerInterface|\PHPUnit_Framework_MockObject_MockObject
      */
@@ -115,10 +121,18 @@ protected function setUp()
                 ]
             );
 
+        $this->directoryReadMock = $this->getMockBuilder(\Magento\Framework\Filesystem\Directory\Read::class)
+            ->setConstructorArgs(['path' => $this->path])
+            ->disableOriginalConstructor()
+            ->getMock();
+
         $this->filesystemMock = $this->createMock(\Magento\Framework\Filesystem::class);
         $this->filesystemMock->expects($this->once())
             ->method('getDirectoryWrite')
             ->willReturn($this->directoryWriteMock);
+        $this->filesystemMock->expects($this->once())
+            ->method('getDirectoryReadByPath')
+            ->willReturn($this->directoryReadMock);
 
         $this->storeManagerMock = $this->getMockBuilder(\Magento\Store\Model\StoreManagerInterface::class)
             ->setMethods(
@@ -240,12 +254,15 @@ public function testConvertIdToPathNodeRoot()
         $this->assertEquals($this->imagesHelper->getStorageRoot(), $this->imagesHelper->convertIdToPath($pathId));
     }
 
-    /**
-     * @expectedException \InvalidArgumentException
-     * @expectedExceptionMessage Path is invalid
-     */
     public function testConvertIdToPathInvalid()
     {
+        $this->expectException('InvalidArgumentException');
+        $this->expectExceptionMessage('Path is invalid');
+        $this->directoryReadMock->expects($this->any())
+            ->method('getAbsolutePath')
+            ->will(
+                $this->throwException(new ValidatorException(__("Error")))
+            );
         $this->imagesHelper->convertIdToPath('Ly4uLy4uLy4uLy4uLy4uL3dvcms-');
     }
 

app/code/Magento/ImportExport/Controller/Adminhtml/Export/File/Download.php

@@ -9,11 +9,12 @@
 
 use Magento\Backend\App\Action;
 use Magento\Framework\App\Action\HttpGetActionInterface;
+use Magento\Framework\App\Filesystem\DirectoryList;
 use Magento\Framework\App\Response\Http\FileFactory;
 use Magento\Framework\Exception\LocalizedException;
-use Magento\Framework\App\Filesystem\DirectoryList;
-use Magento\ImportExport\Controller\Adminhtml\Export as ExportController;
 use Magento\Framework\Filesystem;
+use Magento\ImportExport\Controller\Adminhtml\Export as ExportController;
+use Throwable;
 
 /**
  * Controller that download file by name.
@@ -60,7 +61,13 @@ public function __construct(
     public function execute()
     {
         $fileName = $this->getRequest()->getParam('filename');
-        if (empty($fileName) || preg_match('/\.\.(\\\|\/)/', $fileName) !== 0) {
+        $exportDirectory = $this->filesystem->getDirectoryRead(DirectoryList::VAR_EXPORT);
+        try {
+            $fileExist = $exportDirectory->isExist($fileName);
+        } catch (Throwable $e) {
+            $fileExist = false;
+        }
+        if (empty($fileName) || !$fileExist) {
             throw new LocalizedException(__('Please provide valid export file name'));
         }
         try {
@@ -72,8 +79,14 @@ public function execute()
                     $directory->readFile($path),
                     DirectoryList::VAR_DIR
                 );
+            } else {
+                $fileExist = false;
             }
         } catch (LocalizedException | \Exception $exception) {
+            $fileExist = false;
+        }
+
+        if (!$fileExist) {
             throw new LocalizedException(__('There are no export file with such name %1', $fileName));
         }
     }

app/code/Magento/ImportExport/Helper/Report.php

@@ -7,7 +7,9 @@
 namespace Magento\ImportExport\Helper;
 
 use Magento\Framework\App\Filesystem\DirectoryList;
+use Magento\Framework\Exception\ValidatorException;
 use Magento\ImportExport\Model\Import;
+use Magento\Framework\Filesystem\Directory\ReadInterface;
 
 /**
  * ImportExport history reports helper
@@ -27,6 +29,11 @@ class Report extends \Magento\Framework\App\Helper\AbstractHelper
      */
     protected $varDirectory;
 
+    /**
+     * @var ReadInterface
+     */
+    private $importHistoryDirectory;
+
     /**
      * Construct
      *
@@ -41,6 +48,8 @@ public function __construct(
     ) {
         $this->timeZone = $timeZone;
         $this->varDirectory = $filesystem->getDirectoryWrite(DirectoryList::VAR_DIR);
+        $importHistoryPath = $this->varDirectory->getAbsolutePath('import_history');
+        $this->importHistoryDirectory = $filesystem->getDirectoryReadByPath($importHistoryPath);
         parent::__construct($context);
     }
 
@@ -127,11 +136,12 @@ public function getReportSize($filename)
      */
     protected function getFilePath($filename)
     {
-        if (preg_match('/\.\.(\\\|\/)/', $filename)) {
-            throw new \InvalidArgumentException('Filename has not permitted symbols in it');
+        try {
+            $filePath = $this->varDirectory->getRelativePath($this->importHistoryDirectory->getAbsolutePath($filename));
+        } catch (ValidatorException $e) {
+            throw new \InvalidArgumentException('File not found');
         }
-
-        return $this->varDirectory->getRelativePath(Import::IMPORT_HISTORY_DIR . $filename);
+        return $filePath;
     }
 
     /**

app/code/Magento/ImportExport/Test/Unit/Controller/Adminhtml/Export/File/DownloadTest.php

@@ -7,16 +7,16 @@
 
 namespace Magento\ImportExport\Test\Unit\Controller\Adminhtml\Export\File;
 
-use Magento\Framework\Filesystem;
 use Magento\Backend\App\Action\Context;
+use Magento\Framework\App\Filesystem\DirectoryList;
 use Magento\Framework\App\Request\Http;
+use Magento\Framework\App\Response\Http\FileFactory;
 use Magento\Framework\App\ResponseInterface;
-use PHPUnit\Framework\MockObject\MockObject;
+use Magento\Framework\Filesystem;
 use Magento\Framework\Filesystem\Directory\Read;
-use Magento\Framework\App\Filesystem\DirectoryList;
-use Magento\Framework\App\Response\Http\FileFactory;
-use Magento\ImportExport\Controller\Adminhtml\Export\File\Download;
 use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper;
+use Magento\ImportExport\Controller\Adminhtml\Export\File\Download;
+use PHPUnit\Framework\MockObject\MockObject;
 
 /**
  * Unit tests for \Magento\ImportExport\Controller\Adminhtml\Export\File\Download.
@@ -56,7 +56,7 @@ class DownloadTest extends \PHPUnit\Framework\TestCase
     /**
      * @var Read|MockObject
      */
-    private $readMock;
+    private $directoryMock;
 
     /**
      * @inheritdoc
@@ -70,9 +70,11 @@ protected function setUp()
             ['getRequest', 'getObjectManager', 'getResultRedirectFactory']
         );
         $this->fileFactoryMock = $this->createPartialMock(FileFactory::class, ['create']);
-        $this->fileSystemMock = $this->createPartialMock(Filesystem::class, ['getDirectoryRead']);
+        $this->fileSystemMock = $this->getMockBuilder(Filesystem::class)
+            ->disableOriginalConstructor()
+            ->getMock();
         $this->requestMock = $this->createPartialMock(Http::class, ['getParam']);
-        $this->readMock = $this->createPartialMock(Read::class, ['isFile', 'readFile']);
+        $this->directoryMock = $this->createPartialMock(Read::class, ['isFile', 'readFile','isExist']);
 
         $this->contextMock->expects($this->once())->method('getRequest')->willReturn($this->requestMock);
 
@@ -98,7 +100,7 @@ public function testExecute(): void
         $fileContent = 'content';
 
         $this->processDownloadAction($fileName, $path);
-        $this->readMock->expects($this->once())->method('readFile')->with($path)->willReturn($fileContent);
+        $this->directoryMock->expects($this->once())->method('readFile')->with($path)->willReturn($fileContent);
         $response = $this->createMock(ResponseInterface::class);
         $this->fileFactoryMock->expects($this->once())
             ->method('create')
@@ -135,7 +137,7 @@ public function testExecuteWithNonExistanceFile(): void
         $path = 'export/' . $fileName;
 
         $this->processDownloadAction($fileName, $path);
-        $this->readMock->expects($this->once())
+        $this->directoryMock->expects($this->once())
             ->method('readFile')
             ->with($path)
             ->willThrowException(new \Exception('Message'));
@@ -153,10 +155,10 @@ public function testExecuteWithNonExistanceFile(): void
     private function processDownloadAction(string $fileName, string $path): void
     {
         $this->requestMock->expects($this->once())->method('getParam')->with('filename')->willReturn($fileName);
-        $this->fileSystemMock->expects($this->once())
+        $this->fileSystemMock->expects($this->any())
             ->method('getDirectoryRead')
-            ->with(DirectoryList::VAR_DIR)
-            ->willReturn($this->readMock);
-        $this->readMock->expects($this->once())->method('isFile')->with($path)->willReturn(true);
+            ->willReturn($this->directoryMock);
+        $this->directoryMock->expects($this->once())->method('isExist')->willReturn(true);
+        $this->directoryMock->expects($this->once())->method('isFile')->with($path)->willReturn(true);
     }
 }