ENGCOM-4591: Fix Issue #19872 - checking if image is in media directory #21131

 - Merge Pull Request #21131 from Bartlomiejsz/magento2:feature/fix_19872_category_image_path
 - Merged commits:
   1. ed43407
   2. 619475d
   3. 17ed8d4
   4. ae61b70
   5. 112476a
   6. 8522a14
   7. 07a296a
   8. a142ef2

Author: magento-engcom-team

app/code/Magento/Catalog/Model/Category/FileInfo.php

@@ -43,6 +43,11 @@ class FileInfo
      */
     private $baseDirectory;
 
+    /**
+     * @var ReadInterface
+     */
+    private $pubDirectory;
+
     /**
      * @param Filesystem $filesystem
      * @param Mime $mime
@@ -82,6 +87,20 @@ private function getBaseDirectory()
         return $this->baseDirectory;
     }
 
+    /**
+     * Get Pub Directory read instance
+     *
+     * @return ReadInterface
+     */
+    private function getPubDirectory()
+    {
+        if (!isset($this->pubDirectory)) {
+            $this->pubDirectory = $this->filesystem->getDirectoryRead(DirectoryList::PUB);
+        }
+
+        return $this->pubDirectory;
+    }
+
     /**
      * Retrieve MIME type of requested file
      *
@@ -135,7 +154,7 @@ private function getFilePath($fileName)
     {
         $filePath = ltrim($fileName, '/');
 
-        $mediaDirectoryRelativeSubpath = $this->getMediaDirectoryPathRelativeToBaseDirectoryPath();
+        $mediaDirectoryRelativeSubpath = $this->getMediaDirectoryPathRelativeToBaseDirectoryPath($filePath);
         $isFileNameBeginsWithMediaDirectoryPath = $this->isBeginsWithMediaDirectoryPath($fileName);
 
         // if the file is not using a relative path, it resides in the catalog/category media directory
@@ -160,7 +179,7 @@ public function isBeginsWithMediaDirectoryPath($fileName)
     {
         $filePath = ltrim($fileName, '/');
 
-        $mediaDirectoryRelativeSubpath = $this->getMediaDirectoryPathRelativeToBaseDirectoryPath();
+        $mediaDirectoryRelativeSubpath = $this->getMediaDirectoryPathRelativeToBaseDirectoryPath($filePath);
         $isFileNameBeginsWithMediaDirectoryPath = strpos($filePath, $mediaDirectoryRelativeSubpath) === 0;
 
         return $isFileNameBeginsWithMediaDirectoryPath;
@@ -169,14 +188,22 @@ public function isBeginsWithMediaDirectoryPath($fileName)
     /**
      * Get media directory subpath relative to base directory path
      *
+     * @param string $filePath
      * @return string
      */
-    private function getMediaDirectoryPathRelativeToBaseDirectoryPath()
+    private function getMediaDirectoryPathRelativeToBaseDirectoryPath(string $filePath = '')
     {
-        $baseDirectoryPath = $this->getBaseDirectory()->getAbsolutePath();
+        $baseDirectory = $this->getBaseDirectory();
+        $baseDirectoryPath = $baseDirectory->getAbsolutePath();
         $mediaDirectoryPath = $this->getMediaDirectory()->getAbsolutePath();
+        $pubDirectoryPath = $this->getPubDirectory()->getAbsolutePath();
 
         $mediaDirectoryRelativeSubpath = substr($mediaDirectoryPath, strlen($baseDirectoryPath));
+        $pubDirectory = $baseDirectory->getRelativePath($pubDirectoryPath);
+
+        if (strpos($mediaDirectoryRelativeSubpath, $pubDirectory) === 0 && strpos($filePath, $pubDirectory) !== 0) {
+            $mediaDirectoryRelativeSubpath = substr($mediaDirectoryRelativeSubpath, strlen($pubDirectory));
+        }
 
         return $mediaDirectoryRelativeSubpath;
     }

app/code/Magento/Catalog/Test/Unit/Model/Category/FileInfoTest.php

@@ -11,29 +11,36 @@
 use Magento\Framework\Filesystem;
 use Magento\Framework\Filesystem\Directory\WriteInterface;
 use Magento\Framework\Filesystem\Directory\ReadInterface;
+use PHPUnit\Framework\MockObject\MockObject;
+use PHPUnit\Framework\TestCase;
 
-class FileInfoTest extends \PHPUnit\Framework\TestCase
+class FileInfoTest extends TestCase
 {
     /**
-     * @var Filesystem|\PHPUnit_Framework_MockObject_MockObject
+     * @var Filesystem|MockObject
      */
     private $filesystem;
 
     /**
-     * @var Mime|\PHPUnit_Framework_MockObject_MockObject
+     * @var Mime|MockObject
      */
     private $mime;
 
     /**
-     * @var WriteInterface|\PHPUnit_Framework_MockObject_MockObject
+     * @var WriteInterface|MockObject
      */
     private $mediaDirectory;
 
     /**
-     * @var ReadInterface|\PHPUnit_Framework_MockObject_MockObject
+     * @var ReadInterface|MockObject
      */
     private $baseDirectory;
 
+    /**
+     * @var ReadInterface|MockObject
+     */
+    private $pubDirectory;
+
     /**
      * @var FileInfo
      */
@@ -44,30 +51,41 @@ protected function setUp()
         $this->mediaDirectory = $this->getMockBuilder(WriteInterface::class)
             ->getMockForAbstractClass();
 
-        $this->baseDirectory = $this->getMockBuilder(ReadInterface::class)
+        $this->baseDirectory = $baseDirectory = $this->getMockBuilder(ReadInterface::class)
+            ->getMockForAbstractClass();
+
+        $this->pubDirectory = $pubDirectory = $this->getMockBuilder(ReadInterface::class)
             ->getMockForAbstractClass();
 
         $this->filesystem = $this->getMockBuilder(Filesystem::class)
             ->disableOriginalConstructor()
             ->getMock();
-        $this->filesystem->expects($this->any())
-            ->method('getDirectoryWrite')
+
+        $this->filesystem->method('getDirectoryWrite')
             ->with(DirectoryList::MEDIA)
             ->willReturn($this->mediaDirectory);
 
-        $this->filesystem->expects($this->any())
-            ->method('getDirectoryRead')
-            ->with(DirectoryList::ROOT)
-            ->willReturn($this->baseDirectory);
+        $this->filesystem->method('getDirectoryRead')
+            ->willReturnCallback(function ($arg) use ($baseDirectory, $pubDirectory) {
+                if ($arg === DirectoryList::PUB) {
+                    return $pubDirectory;
+                }
+                return $baseDirectory;
+            });
 
         $this->mime = $this->getMockBuilder(Mime::class)
             ->disableOriginalConstructor()
             ->getMock();
 
-        $this->baseDirectory->expects($this->any())
-            ->method('getAbsolutePath')
-            ->with(null)
-            ->willReturn('/a/b/c');
+        $this->baseDirectory->method('getAbsolutePath')
+            ->willReturn('/a/b/c/');
+
+        $this->baseDirectory->method('getRelativePath')
+            ->with('/a/b/c/pub/')
+            ->willReturn('pub/');
+
+        $this->pubDirectory->method('getAbsolutePath')
+            ->willReturn('/a/b/c/pub/');
 
         $this->model = new FileInfo(
             $this->filesystem,
@@ -85,12 +103,12 @@ public function testGetMimeType()
         $this->mediaDirectory->expects($this->at(0))
             ->method('getAbsolutePath')
             ->with(null)
-            ->willReturn('/a/b/c/pub/media');
+            ->willReturn('/a/b/c/pub/media/');
 
         $this->mediaDirectory->expects($this->at(1))
             ->method('getAbsolutePath')
             ->with(null)
-            ->willReturn('/a/b/c/pub/media');
+            ->willReturn('/a/b/c/pub/media/');
 
         $this->mediaDirectory->expects($this->at(2))
             ->method('getAbsolutePath')
@@ -113,13 +131,11 @@ public function testGetStat()
 
         $expected = ['size' => 1];
 
-        $this->mediaDirectory->expects($this->any())
-            ->method('getAbsolutePath')
+        $this->mediaDirectory->method('getAbsolutePath')
             ->with(null)
-            ->willReturn('/a/b/c/pub/media');
+            ->willReturn('/a/b/c/pub/media/');
 
-        $this->mediaDirectory->expects($this->once())
-            ->method('stat')
+        $this->mediaDirectory->method('stat')
             ->with($mediaPath . $fileName)
             ->willReturn($expected);
 
@@ -130,22 +146,52 @@ public function testGetStat()
         $this->assertEquals(1, $result['size']);
     }
 
-    public function testIsExist()
+    /**
+     * @param $fileName
+     * @param $fileMediaPath
+     * @dataProvider isExistProvider
+     */
+    public function testIsExist($fileName, $fileMediaPath)
     {
-        $mediaPath = '/catalog/category';
-
-        $fileName = '/filename.ext1';
+        $this->mediaDirectory->method('getAbsolutePath')
+            ->willReturn('/a/b/c/pub/media/');
 
-        $this->mediaDirectory->expects($this->any())
-            ->method('getAbsolutePath')
-            ->with(null)
-            ->willReturn('/a/b/c/pub/media');
-
-        $this->mediaDirectory->expects($this->once())
-            ->method('isExist')
-            ->with($mediaPath . $fileName)
+        $this->mediaDirectory->method('isExist')
+            ->with($fileMediaPath)
             ->willReturn(true);
 
         $this->assertTrue($this->model->isExist($fileName));
     }
+
+    public function isExistProvider()
+    {
+        return [
+            ['/filename.ext1', '/catalog/category/filename.ext1'],
+            ['/pub/media/filename.ext1', 'filename.ext1'],
+            ['/media/filename.ext1', 'filename.ext1']
+        ];
+    }
+
+    /**
+     * @param $fileName
+     * @param $expected
+     * @dataProvider isBeginsWithMediaDirectoryPathProvider
+     */
+    public function testIsBeginsWithMediaDirectoryPath($fileName, $expected)
+    {
+        $this->mediaDirectory->method('getAbsolutePath')
+            ->willReturn('/a/b/c/pub/media/');
+
+        $this->assertEquals($expected, $this->model->isBeginsWithMediaDirectoryPath($fileName));
+    }
+
+    public function isBeginsWithMediaDirectoryPathProvider()
+    {
+        return [
+            ['/pub/media/test/filename.ext1', true],
+            ['/media/test/filename.ext1', true],
+            ['/test/filename.ext1', false],
+            ['test2/filename.ext1', false]
+        ];
+    }
 }