Merge pull request #7196 from magento-arcticfoxes/B2B-2069

B2B-2069: Images from remote directory is throwing errors on lib/internal/Magento/Framework/Filesystem/Directory/Write.php
magento · Nov 11, 2021 · 13d8ccd · 13d8ccd
2 parents f95efe4 + b8d69bf
commit 13d8ccd
Show file tree

Hide file tree

Showing 9 changed files with 166 additions and 103 deletions.
diff --git a/app/code/Magento/AwsS3/Driver/AwsS3.php b/app/code/Magento/AwsS3/Driver/AwsS3.php
@@ -25,6 +25,7 @@
  * Driver for AWS S3 IO operations.
  *
  * @SuppressWarnings(PHPMD.ExcessiveClassComplexity)
+ * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  */
 class AwsS3 implements RemoteDriverInterface
 {
@@ -258,6 +259,7 @@ public function filePutContents($path, $content, $mode = null): int
         $path = $this->normalizeRelativePath($path, true);
         $config = self::CONFIG;
 
+        // phpcs:ignore Generic.PHP.NoSilencedErrors.Discouraged
         if (false !== ($imageSize = @getimagesizefromstring($content))) {
             $config['Metadata'] = [
                 'image-width' => $imageSize[0],
@@ -295,19 +297,20 @@ public function readDirectory($path): array
      */
     public function getRealPathSafety($path)
     {
+        //Removing redundant directory separators
+        $path = preg_replace(
+            '~(?<!:)\/\/+~',
+            '/',
+            $path
+        );
+
         if (strpos($path, '/.') === false) {
             return $path;
         }
 
         $isAbsolute = strpos($path, $this->normalizeAbsolutePath('')) === 0;
         $path = $this->normalizeRelativePath($path);
 
-        //Removing redundant directory separators.
-        $path = preg_replace(
-            '/\\/\\/+/',
-            '/',
-            $path
-        );
         $pathParts = explode('/', $path);
         if (end($pathParts) === '.') {
             $pathParts[count($pathParts) - 1] = '';
@@ -488,8 +491,7 @@ public function getRelativePath($basePath, $path = null): string
         $basePath = (string)$basePath;
         $path = (string)$path;
 
-        if (
-            ($basePath && $path)
+        if ($basePath && $path
             && ($basePath === $path . '/' || strpos($path, $basePath) === 0)
         ) {
             $result = substr($path, strlen($basePath));
@@ -722,6 +724,7 @@ public function fileGetCsv($resource, $length = 0, $delimiter = ',', $enclosure
      */
     public function fileTell($resource): int
     {
+        // phpcs:ignore Magento2.Functions.DiscouragedFunction, Generic.PHP.NoSilencedErrors.Discouraged
         $result = @ftell($resource);
         if ($result === null) {
             throw new FileSystemException(
@@ -737,6 +740,7 @@ public function fileTell($resource): int
      */
     public function fileSeek($resource, $offset, $whence = SEEK_SET): int
     {
+        // phpcs:ignore Magento2.Functions.DiscouragedFunction, Generic.PHP.NoSilencedErrors.Discouraged
         $result = @fseek($resource, $offset, $whence);
         if ($result === -1) {
             throw new FileSystemException(
@@ -755,6 +759,7 @@ public function fileSeek($resource, $offset, $whence = SEEK_SET): int
      */
     public function endOfFile($resource): bool
     {
+        // phpcs:ignore Magento2.Functions.DiscouragedFunction.DiscouragedWithAlternative
         return feof($resource);
     }
 
@@ -772,6 +777,7 @@ public function filePutCsv($resource, array $data, $delimiter = ',', $enclosure
      */
     public function fileFlush($resource): bool
     {
+        // phpcs:ignore Magento2.Functions.DiscouragedFunction, Generic.PHP.NoSilencedErrors.Discouraged
         $result = @fflush($resource);
         if (!$result) {
             throw new FileSystemException(
@@ -790,6 +796,7 @@ public function fileFlush($resource): bool
      */
     public function fileLock($resource, $lockMode = LOCK_EX): bool
     {
+        // phpcs:ignore Magento2.Functions.DiscouragedFunction, Generic.PHP.NoSilencedErrors.Discouraged
         $result = @flock($resource, $lockMode);
         if (!$result) {
             throw new FileSystemException(
@@ -808,6 +815,7 @@ public function fileLock($resource, $lockMode = LOCK_EX): bool
      */
     public function fileUnlock($resource): bool
     {
+        // phpcs:ignore Magento2.Functions.DiscouragedFunction, Generic.PHP.NoSilencedErrors.Discouraged
         $result = @flock($resource, LOCK_UN);
         if (!$result) {
             throw new FileSystemException(
@@ -851,13 +859,14 @@ public function fileClose($resource): bool
         //phpcs:enable
 
         foreach ($this->streams as $path => $stream) {
-            // phpcs:ignore Magento2.Functions.DiscouragedFunction
+            // phpcs:ignore
             if (stream_get_meta_data($stream)['uri'] === $resourcePath) {
                 $this->adapter->writeStream($path, $resource, new Config(self::CONFIG));
 
                 // Remove path from streams after
                 unset($this->streams[$path]);
 
+                // phpcs:ignore Magento2.Functions.DiscouragedFunction.DiscouragedWithAlternative
                 return fclose($stream);
             }
         }
@@ -931,6 +940,7 @@ private function readPath(string $path, $isRecursive = false): array
             if (!empty($path)
                 && $path !== $relativePath
                 && (!$relativePath || strpos($path, $relativePath) === 0)) {
+                //phpcs:ignore Magento2.Functions.DiscouragedFunction
                 $itemsList[] = $this->getAbsolutePath(dirname($path), $path);
             }
         }

diff --git a/app/code/Magento/AwsS3/Test/Unit/Driver/AwsS3Test.php b/app/code/Magento/AwsS3/Test/Unit/Driver/AwsS3Test.php
@@ -64,6 +64,7 @@ public function testGetAbsolutePath($basePath, $path, string $expected): void
 
     /**
      * @return array
+     * @SuppressWarnings(PHPMD.ExcessiveMethodLength)
      */
     public function getAbsolutePathDataProvider(): array
     {
@@ -407,6 +408,18 @@ public function getRealPathSafetyDataProvider(): array
             [
                 'test/test/../test.txt',
                 'test/test.txt'
+            ],
+            [
+                'test//test/../test.txt',
+                'test/test.txt'
+            ],
+            [
+                'test1///test2/..//test3//test.txt',
+                'test1/test3/test.txt'
+            ],
+            [
+                self::URL . '/test1///test2/..//test3//test.txt',
+                self::URL . 'test1/test3/test.txt'
             ]
         ];
     }

diff --git a/app/code/Magento/Customer/Model/Metadata/Form/Image.php b/app/code/Magento/Customer/Model/Metadata/Form/Image.php
@@ -22,6 +22,7 @@
 use Magento\Framework\Exception\LocalizedException;
 use Magento\Framework\File\UploaderFactory;
 use Magento\Framework\Filesystem;
+use Magento\Framework\Filesystem\Directory\ReadInterface;
 use Magento\Framework\Filesystem\Directory\WriteFactory;
 use Magento\Framework\Filesystem\Directory\WriteInterface;
 use Magento\Framework\Filesystem\Io\File as IoFileSystem;
@@ -48,10 +49,15 @@ class Image extends File
      */
     private $ioFileSystem;
 
+    /**
+     * @var ReadInterface
+     */
+    private $mediaEntityTmpReadDirectory;
+
     /**
      * @var WriteInterface
      */
-    private $mediaEntityTmpDirectory;
+    private $mediaWriteDirectory;
 
     /**
      * @param TimezoneInterface $localeDate
@@ -71,6 +77,7 @@ class Image extends File
      * @param DirectoryList|null $directoryList
      * @param WriteFactory|null $writeFactory
      * @SuppressWarnings(PHPMD.ExcessiveParameterList)
+     * @SuppressWarnings(PHPMD.UnusedFormalParameter)
      * @throws FileSystemException
      */
     public function __construct(
@@ -109,12 +116,10 @@ public function __construct(
             ->get(ImageContentInterfaceFactory::class);
         $this->ioFileSystem = $ioFileSystem ?: ObjectManager::getInstance()
             ->get(IoFileSystem::class);
-        $writeFactory = $writeFactory ?? ObjectManager::getInstance()->get(WriteFactory::class);
-        $directoryList = $directoryList ?? ObjectManager::getInstance()->get(DirectoryList::class);
-        $this->mediaEntityTmpDirectory = $writeFactory->create(
-            $directoryList->getPath($directoryList::MEDIA)
-            . '/' . $this->_entityTypeCode
-            . '/' . FileProcessor::TMP_DIR
+        $this->mediaWriteDirectory = $fileSystem->getDirectoryWrite(DirectoryList::MEDIA);
+        $this->mediaEntityTmpReadDirectory = $fileSystem->getDirectoryReadByPath(
+            $this->mediaWriteDirectory->getAbsolutePath() . $this->_entityTypeCode
+            . DIRECTORY_SEPARATOR . FileProcessor::TMP_DIR . DIRECTORY_SEPARATOR
         );
     }
 
@@ -135,6 +140,7 @@ protected function _validateByRules($value)
         $rules = $this->getAttribute()->getValidationRules();
 
         try {
+            // phpcs:ignore Magento2.Functions.DiscouragedFunction
             $imageProp = getimagesize($value['tmp_name']);
         } catch (\Throwable $e) {
             $imageProp = false;
@@ -224,18 +230,18 @@ protected function processUiComponentValue(array $value)
      */
     protected function processCustomerAddressValue(array $value)
     {
-        $fileName = $this->mediaEntityTmpDirectory
+        $fileName = $this->mediaWriteDirectory
             ->getDriver()
             ->getRealPathSafety(
-                $this->mediaEntityTmpDirectory->getAbsolutePath(
+                $this->mediaEntityTmpReadDirectory->getAbsolutePath(
                     ltrim(
                         $value['file'],
                         '/'
                     )
                 )
             );
         return $this->getFileProcessor()->moveTemporaryFile(
-            $this->mediaEntityTmpDirectory->getRelativePath($fileName)
+            $this->mediaEntityTmpReadDirectory->getRelativePath($fileName)
         );
     }
 
@@ -249,7 +255,7 @@ protected function processCustomerAddressValue(array $value)
     protected function processCustomerValue(array $value)
     {
         $file = ltrim($value['file'], '/');
-        if ($this->mediaEntityTmpDirectory->isExist($file)) {
+        if ($this->mediaEntityTmpReadDirectory->isExist($file)) {
             $temporaryFile = FileProcessor::TMP_DIR . '/' . $file;
             $base64EncodedData = $this->getFileProcessor()->getBase64EncodedData($temporaryFile);
             /** @var ImageContentInterface $imageContentDataObject */

diff --git a/app/code/Magento/Customer/Test/Unit/Model/Metadata/Form/ImageTest.php b/app/code/Magento/Customer/Test/Unit/Model/Metadata/Form/ImageTest.php
@@ -15,13 +15,12 @@
 use Magento\Customer\Model\Metadata\Form\Image;
 use Magento\Framework\Api\Data\ImageContentInterface;
 use Magento\Framework\Api\Data\ImageContentInterfaceFactory;
-use Magento\Framework\App\Filesystem\DirectoryList;
 use Magento\Framework\App\Request\Http;
 use Magento\Framework\Exception\FileSystemException;
 use Magento\Framework\Exception\LocalizedException;
 use Magento\Framework\File\UploaderFactory;
 use Magento\Framework\Filesystem;
-use Magento\Framework\Filesystem\Directory\WriteFactory;
+use Magento\Framework\Filesystem\Directory\ReadInterface;
 use Magento\Framework\Filesystem\Directory\Write;
 use Magento\Framework\Filesystem\Driver\File as Driver;
 use Magento\Framework\Filesystem\Io\File;
@@ -82,19 +81,14 @@ class ImageTest extends AbstractFormTestCase
     private $ioFileSystemMock;
 
     /**
-     * @var DirectoryList|\PHPUnit\Framework\MockObject\MockObject
+     * @var ReadInterface|\PHPUnit\Framework\MockObject\MockObject
      */
-    private $directoryListMock;
-
-    /**
-     * @var WriteFactory|\PHPUnit\Framework\MockObject\MockObject
-     */
-    private $writeFactoryMock;
+    private $readDirectoryMock;
 
     /**
      * @var Write|\PHPUnit\Framework\MockObject\MockObject
      */
-    private $mediaEntityTmpDirectoryMock;
+    private $mediaWriteDirectoryMock;
 
     /**
      * @var Driver|\PHPUnit\Framework\MockObject\MockObject
@@ -140,23 +134,24 @@ protected function setUp(): void
         $this->ioFileSystemMock = $this->getMockBuilder(File::class)
             ->disableOriginalConstructor()
             ->getMock();
-        $this->directoryListMock = $this->getMockBuilder(DirectoryList::class)
-            ->disableOriginalConstructor()
-            ->getMock();
-        $this->writeFactoryMock = $this->getMockBuilder(WriteFactory::class)
-            ->setMethods(['create'])
-            ->disableOriginalConstructor()
-            ->getMock();
-        $this->mediaEntityTmpDirectoryMock = $this->getMockBuilder(Write::class)
+
+        $this->readDirectoryMock = $this->getMockBuilder(ReadInterface::class)
+            ->getMockForAbstractClass();
+        $this->fileSystemMock->expects($this->once())
+            ->method('getDirectoryReadByPath')
+            ->willReturn($this->readDirectoryMock);
+
+        $this->mediaWriteDirectoryMock = $this->getMockBuilder(Write::class)
             ->disableOriginalConstructor()
             ->getMock();
+        $this->fileSystemMock->expects($this->once())
+            ->method('getDirectoryWrite')
+            ->willReturn($this->mediaWriteDirectoryMock);
+
         $this->driverMock = $this->getMockBuilder(Driver::class)
             ->disableOriginalConstructor()
             ->getMock();
-        $this->writeFactoryMock->expects($this->any())
-            ->method('create')
-            ->willReturn($this->mediaEntityTmpDirectoryMock);
-        $this->mediaEntityTmpDirectoryMock->expects($this->any())
+        $this->mediaWriteDirectoryMock->expects($this->any())
             ->method('getDriver')
             ->willReturn($this->driverMock);
     }
@@ -184,9 +179,7 @@ private function initialize(array $data): Image
             $this->uploaderFactoryMock,
             $this->fileProcessorFactoryMock,
             $this->imageContentFactory,
-            $this->ioFileSystemMock,
-            $this->directoryListMock,
-            $this->writeFactoryMock
+            $this->ioFileSystemMock
         );
     }
 
@@ -470,10 +463,10 @@ public function testCompactValueUiComponentAddress()
             ->method('getRealPathSafety')
             ->with($value['file'])
             ->willReturn($value['file']);
-        $this->mediaEntityTmpDirectoryMock->expects($this->once())
+        $this->readDirectoryMock->expects($this->once())
             ->method('getAbsolutePath')
             ->willReturn($value['file']);
-        $this->mediaEntityTmpDirectoryMock->expects($this->once())
+        $this->readDirectoryMock->expects($this->once())
             ->method('getRelativePath')
             ->willReturn($value['file']);
         $this->fileProcessorMock->expects($this->once())
@@ -505,7 +498,7 @@ public function testCompactValueUiComponentCustomer()
 
         $base64EncodedData = 'encoded_data';
 
-        $this->mediaEntityTmpDirectoryMock->expects($this->once())
+        $this->readDirectoryMock->expects($this->once())
             ->method('isExist')
             ->with($value['file'])
             ->willReturn(true);
@@ -561,7 +554,7 @@ public function testCompactValueUiComponentCustomerNotExists()
             'type' => 'image',
         ];
 
-        $this->mediaEntityTmpDirectoryMock->expects($this->once())
+        $this->readDirectoryMock->expects($this->once())
             ->method('isExist')
             ->with($value['file'])
             ->willReturn(false);

diff --git a/...ts/integration/testsuite/Magento/Catalog/Controller/Adminhtml/Product/Save/ImagesTest.php b/...ts/integration/testsuite/Magento/Catalog/Controller/Adminhtml/Product/Save/ImagesTest.php
@@ -138,8 +138,10 @@ private function assertSuccessfulImageSave(array $expectation): void
         $this->assertEquals($expectation['small_image'], $product->getData('small_image'));
         $this->assertEquals($expectation['thumbnail'], $product->getData('thumbnail'));
         $this->assertEquals($expectation['swatch_image'], $product->getData('swatch_image'));
-        $this->assertFileExists(
-            $this->mediaDirectory->getAbsolutePath($this->config->getBaseMediaPath() . $expectation['image'])
+        $this->assertTrue(
+            $this->mediaDirectory->isExist(
+                $this->mediaDirectory->getAbsolutePath($this->config->getBaseMediaPath() . $expectation['image'])
+            )
         );
     }
 }
diff --git a/...te/Magento/ConfigurableProduct/Controller/Adminhtml/Product/Initialization/HelperTest.php b/...te/Magento/ConfigurableProduct/Controller/Adminhtml/Product/Initialization/HelperTest.php
@@ -315,8 +315,8 @@ private function assertChildProductImages(array $expectedImages): void
                 foreach ($roles as $role) {
                     $this->assertEquals($image, $simpleProduct->getData($role));
                 }
-                $this->assertFileExists(
-                    $this->mediaDirectory->getAbsolutePath($this->config->getBaseMediaPath() . $image)
+                $this->assertTrue(
+                    $this->mediaDirectory->isExist($this->config->getBaseMediaPath() . $image)
                 );
             }
         }