Merge remote-tracking branch 'origin/MAGETWO-42123' into goinc-bugsfixing

Author: Michail Slabko

app/code/Magento/Catalog/Model/Product/Option/Type/File/ValidatorFile.php

@@ -21,21 +21,21 @@ class ValidatorFile extends Validator
      *
      * @var string
      */
-    protected $path = '/custom_options';
+    protected $path = 'custom_options';
 
     /**
      * Relative path for quote folder
      *
      * @var string
      */
-    protected $quotePath = '/custom_options/quote';
+    protected $quotePath = 'custom_options/quote';
 
     /**
      * Relative path for order folder
      *
      * @var string
      */
-    protected $orderPath = '/custom_options/order';
+    protected $orderPath = 'custom_options/order';
 
     /**
      * @var \Magento\Framework\Filesystem\Directory\WriteInterface
@@ -175,12 +175,12 @@ public function validate($processingParams, $option)
                     $_height = $imageSize[1];
                 }
             }
-            $uri = $this->filesystem->getUri(DirectoryList::MEDIA);
+
             $userValue = [
                 'type' => $fileInfo['type'],
                 'title' => $fileInfo['name'],
-                'quote_path' => $uri . $this->quotePath . $filePath,
-                'order_path' => $uri . $this->orderPath . $filePath,
+                'quote_path' => $this->quotePath . $filePath,
+                'order_path' => $this->orderPath . $filePath,
                 'fullpath' => $fileFullPath,
                 'size' => $fileInfo['size'],
                 'width' => $_width,

app/code/Magento/Sales/Model/Download.php

@@ -6,6 +6,7 @@
 namespace Magento\Sales\Model;
 
 use Magento\Framework\App\Filesystem\DirectoryList;
+use Magento\Framework\Exception\LocalizedException;
 
 class Download
 {
@@ -29,19 +30,27 @@ class Download
      */
     protected $_fileFactory;
 
+    /**
+     * @var string
+     */
+    protected $rootDirBasePath;
+
     /**
      * @param \Magento\Framework\Filesystem $filesystem
      * @param \Magento\MediaStorage\Helper\File\Storage\Database $fileStorageDatabase
      * @param \Magento\MediaStorage\Model\File\Storage\DatabaseFactory $storageDatabaseFactory
      * @param \Magento\Framework\App\Response\Http\FileFactory $fileFactory
+     * @param string $rootDirBasePath
      */
     public function __construct(
         \Magento\Framework\Filesystem $filesystem,
         \Magento\MediaStorage\Helper\File\Storage\Database $fileStorageDatabase,
         \Magento\MediaStorage\Model\File\Storage\DatabaseFactory $storageDatabaseFactory,
-        \Magento\Framework\App\Response\Http\FileFactory $fileFactory
+        \Magento\Framework\App\Response\Http\FileFactory $fileFactory,
+        $rootDirBasePath = DirectoryList::MEDIA
     ) {
-        $this->_rootDir = $filesystem->getDirectoryWrite(DirectoryList::ROOT);
+        $this->rootDirBasePath = $rootDirBasePath;
+        $this->_rootDir = $filesystem->getDirectoryWrite($this->rootDirBasePath);
         $this->_fileStorageDatabase = $fileStorageDatabase;
         $this->_storageDatabaseFactory = $storageDatabaseFactory;
         $this->_fileFactory = $fileFactory;
@@ -57,18 +66,19 @@ public function __construct(
     public function downloadFile($info)
     {
         $relativePath = $info['order_path'];
-        if ($this->_isCanProcessed($relativePath)) {
+        if (!$this->_isCanProcessed($relativePath)) {
             //try get file from quote
             $relativePath = $info['quote_path'];
-            if ($this->_isCanProcessed($relativePath)) {
-                throw new \Exception();
+            if (!$this->_isCanProcessed($relativePath)) {
+                throw new LocalizedException(
+                    __('Path "%1" is not part of allowed directory "%2"', $relativePath, $this->rootDirBasePath)
+                );
             }
         }
-
         $this->_fileFactory->create(
             $info['title'],
             ['value' => $this->_rootDir->getRelativePath($relativePath), 'type' => 'filename'],
-            DirectoryList::ROOT
+            $this->rootDirBasePath
         );
     }
 
@@ -79,32 +89,28 @@ public function downloadFile($info)
     protected function _isCanProcessed($relativePath)
     {
         $filePath = $this->_rootDir->getAbsolutePath($relativePath);
-        return (!$this->_rootDir->isFile(
-            $relativePath
-        ) || !$this->_rootDir->isReadable(
-            $relativePath
-        )) && !$this->_processDatabaseFile(
-            $filePath
-        );
+        return (strpos($this->_rootDir->getDriver()->getRealPath($filePath), $relativePath) !== false
+            && $this->_rootDir->isFile($relativePath) && $this->_rootDir->isReadable($relativePath))
+            || $this->_processDatabaseFile($filePath, $relativePath);
     }
 
     /**
      * Check file in database storage if needed and place it on file system
      *
      * @param string $filePath
+     * @param string $relativePath
      * @return bool
      */
-    protected function _processDatabaseFile($filePath)
+    protected function _processDatabaseFile($filePath, $relativePath)
     {
         if (!$this->_fileStorageDatabase->checkDbUsage()) {
             return false;
         }
-        $relativePath = $this->_fileStorageDatabase->getMediaRelativePath($filePath);
         $file = $this->_storageDatabaseFactory->create()->loadByFilename($relativePath);
         if (!$file->getId()) {
             return false;
         }
-        $stream = $this->_rootDir->openFile($filePath, 'w+');
+        $stream = $this->_rootDir->openFile($relativePath, 'w+');
         $stream->lock();
         $stream->write($filePath, $file->getContent());
         $stream->unlock();

app/code/Magento/Sales/Test/Unit/Model/DownloadTest.php

@@ -39,6 +39,11 @@ class DownloadTest extends \PHPUnit_Framework_TestCase
      */
     protected $writeDirectoryMock;
 
+    /**
+     * @var \PHPUnit_Framework_MockObject_MockObject
+     */
+    protected $driverMock;
+
     protected function setUp()
     {
         $this->writeDirectoryMock = $this->getMockBuilder('Magento\Framework\Filesystem\Directory\Write')
@@ -49,9 +54,10 @@ protected function setUp()
             ->getMock();
         $this->filesystemMock->expects($this->any())
             ->method('getDirectoryWrite')
-            ->with(DirectoryList::ROOT)
+            ->with(DirectoryList::MEDIA)
             ->will($this->returnValue($this->writeDirectoryMock));
 
+        $this->driverMock = $this->getMockForAbstractClass('Magento\Framework\Filesystem\DriverInterface');
         $this->storageMock = $this->getMockBuilder('Magento\MediaStorage\Helper\File\Storage\Database')
             ->disableOriginalConstructor()
             ->getMock();
@@ -83,17 +89,23 @@ public function testInstanceOf()
     }
 
     /**
-     * @expectedException \Exception
+     * @param $realPatchCheck
+     * @param $isFile
+     * @param $isReadable
+     * @expectedException \Magento\Framework\Exception\LocalizedException
+     * @dataProvider dataProviderForTestDownloadFileException
      */
-    public function testDownloadFileException()
+    public function testDownloadFileException($realPatchCheck, $isFile, $isReadable)
     {
         $info = ['order_path' => 'test/path', 'quote_path' => 'test/path2', 'title' => 'test title'];
-        $isFile = true;
-        $isReadable = false;
 
         $this->writeDirectoryMock->expects($this->any())
             ->method('getAbsolutePath')
             ->will($this->returnArgument(0));
+        $this->writeDirectoryMock->expects($this->any())
+            ->method('getDriver')
+            ->willReturn($this->driverMock);
+        $this->driverMock->expects($this->any())->method('getRealPath')->willReturn($realPatchCheck);
         $this->writeDirectoryMock->expects($this->any())
             ->method('isFile')
             ->will($this->returnValue($isFile));
@@ -104,12 +116,25 @@ public function testDownloadFileException()
         $this->storageFactoryMock->expects($this->any())
             ->method('checkDbUsage')
             ->will($this->returnValue(false));
+        $this->httpFileFactoryMock->expects($this->never())->method('create');
 
         $this->model->downloadFile($info);
     }
 
     /**
-     * @expectedException \Exception
+     * @return array
+     */
+    public function dataProviderForTestDownloadFileException()
+    {
+        return [
+            [1, true, false],
+            [1, false, true],
+            [false, true, true],
+        ];
+    }
+
+    /**
+     * @expectedException \Magento\Framework\Exception\LocalizedException
      */
     public function testDownloadFileNoStorage()
     {
@@ -120,6 +145,11 @@ public function testDownloadFileNoStorage()
         $this->writeDirectoryMock->expects($this->any())
             ->method('getAbsolutePath')
             ->will($this->returnArgument(0));
+        $this->writeDirectoryMock->expects($this->any())
+            ->method('getDriver')
+            ->willReturn($this->driverMock);
+        $this->driverMock->expects($this->any())->method('getRealPath')->willReturn(true);
+
         $this->writeDirectoryMock->expects($this->any())
             ->method('isFile')
             ->will($this->returnValue($isFile));
@@ -130,9 +160,6 @@ public function testDownloadFileNoStorage()
         $this->storageMock->expects($this->any())
             ->method('checkDbUsage')
             ->will($this->returnValue(true));
-        $this->storageMock->expects($this->any())
-            ->method('getMediaRelativePath')
-            ->will($this->returnArgument(0));
 
         $storageDatabaseMock = $this->getMockBuilder('Magento\MediaStorage\Model\File\Storage\Database')
             ->disableOriginalConstructor()
@@ -153,6 +180,7 @@ public function testDownloadFileNoStorage()
         $this->storageFactoryMock->expects($this->any())
             ->method('create')
             ->will($this->returnValue($storageDatabaseMock));
+        $this->httpFileFactoryMock->expects($this->never())->method('create');
 
         $this->model->downloadFile($info);
     }
@@ -178,6 +206,11 @@ public function testDownloadFile()
         $this->writeDirectoryMock->expects($this->any())
             ->method('getAbsolutePath')
             ->will($this->returnArgument(0));
+        $this->writeDirectoryMock->expects($this->any())
+            ->method('getDriver')
+            ->willReturn($this->driverMock);
+        $this->driverMock->expects($this->any())->method('getRealPath')->willReturn(true);
+
         $this->writeDirectoryMock->expects($this->any())
             ->method('isFile')
             ->will($this->returnValue($isFile));
@@ -195,9 +228,6 @@ public function testDownloadFile()
         $this->storageMock->expects($this->any())
             ->method('checkDbUsage')
             ->will($this->returnValue(true));
-        $this->storageMock->expects($this->any())
-            ->method('getMediaRelativePath')
-            ->will($this->returnArgument(0));
 
         $storageDatabaseMock = $this->getMockBuilder('Magento\MediaStorage\Model\File\Storage\Database')
             ->disableOriginalConstructor()
@@ -220,7 +250,7 @@ public function testDownloadFile()
             ->with(
                 $info['title'],
                 ['value' => $info['order_path'], 'type' => 'filename'],
-                DirectoryList::ROOT,
+                DirectoryList::MEDIA,
                 'application/octet-stream',
                 null
             );

dev/tests/integration/testsuite/Magento/Catalog/Model/Product/Option/Type/File/ValidatorFileTest.php

@@ -241,8 +241,8 @@ protected function expectedValidate()
         return [
             'type' => 'image/jpeg',
             'title' => 'test.jpg',
-            'quote_path' => 'pub/media/custom_options/quote/t/e/e1d601731b4b1a84163cd0e9370a4fcb.jpg',
-            'order_path' => 'pub/media/custom_options/order/t/e/e1d601731b4b1a84163cd0e9370a4fcb.jpg',
+            'quote_path' => 'custom_options/quote/t/e/e1d601731b4b1a84163cd0e9370a4fcb.jpg',
+            'order_path' => 'custom_options/order/t/e/e1d601731b4b1a84163cd0e9370a4fcb.jpg',
             'size' => '3300',
             'width' => 136,
             'height' => 131,