From 7f88bcfd332e4e53a17c447a013b40d9a83c82a7 Mon Sep 17 00:00:00 2001 From: Clement Beudot Date: Wed, 25 Jan 2017 13:05:22 +0100 Subject: [PATCH 1/4] bug #8277 fixing bug with https downloading. --- .../Model/Import/Uploader.php | 25 +++++++- .../Test/Unit/Model/Import/UploaderTest.php | 57 +++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/app/code/Magento/CatalogImportExport/Model/Import/Uploader.php b/app/code/Magento/CatalogImportExport/Model/Import/Uploader.php index a95c5c864089f..851097f32240a 100644 --- a/app/code/Magento/CatalogImportExport/Model/Import/Uploader.php +++ b/app/code/Magento/CatalogImportExport/Model/Import/Uploader.php @@ -6,6 +6,7 @@ namespace Magento\CatalogImportExport\Model\Import; use Magento\Framework\App\Filesystem\DirectoryList; +use Magento\Framework\Exception\LocalizedException; use Magento\Framework\Filesystem\DriverPool; /** @@ -15,6 +16,18 @@ */ class Uploader extends \Magento\MediaStorage\Model\File\Uploader { + + /** + * HTTP scheme + * used to compare against the filename and select the proper DriverPool adapter + */ + const HTTP_SCHEME = 'http://'; + /** + * HTTPS scheme + * used to compare against the filename and select the proper DriverPool adapter + */ + const HTTPS_SCHEME = 'https://'; + /** * Temp directory. * @@ -145,7 +158,17 @@ public function move($fileName, $renameFileOff = false) } if (preg_match('/\bhttps?:\/\//i', $fileName, $matches)) { $url = str_replace($matches[0], '', $fileName); - $read = $this->_readFactory->create($url, DriverPool::HTTP); + + if ($matches[0] === self::HTTP_SCHEME) { + $read = $this->_readFactory->create($url, DriverPool::HTTP); + } elseif ($matches[0] === self::HTTPS_SCHEME) { + $read = $this->_readFactory->create($url, DriverPool::HTTPS); + } else { + throw new LocalizedException( + __('No adapted DriverPool for File \'%1\'.', $fileName) + ); + } + $fileName = preg_replace('/[^a-z0-9\._-]+/i', '', $fileName); $this->_directory->writeFile( $this->_directory->getRelativePath($this->getTmpDir() . '/' . $fileName), diff --git a/app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/UploaderTest.php b/app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/UploaderTest.php index d56044894c96b..d84d280fa56cb 100644 --- a/app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/UploaderTest.php +++ b/app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/UploaderTest.php @@ -152,6 +152,63 @@ public function testMoveFileName() $this->assertEquals(['name' => $fileName], $this->uploader->move($fileName)); } + /** + * @dataProvider moveFileUrlDriverPoolDataProvider + */ + public function testMoveFileUrlDrivePool($fileUrl, $expectedHost, $expectedDriverPool, $expectedScheme) + { + + $driverPool = $this->getMock('Magento\Framework\Filesystem\DriverPool', ['getDriver']); + $driverMock = $this->getMock($expectedDriverPool, ['readAll']); + $driverMock->expects($this->any())->method('isExists')->willReturn(true); + $driverMock->expects($this->any())->method('readAll')->willReturn(null); + $driverPool->expects($this->any())->method('getDriver')->willReturn($driverMock); + + $readFactory = $this->getMockBuilder('Magento\Framework\Filesystem\File\ReadFactory') + ->setConstructorArgs( + [ + $driverPool, + ] + ) + ->setMethods(['create']) + ->getMock(); + + $readFactory->expects($this->any())->method('create')->with($expectedHost, $expectedScheme)->willReturn($driverMock); + + + $uploaderMock = $this->getMockBuilder('\Magento\CatalogImportExport\Model\Import\Uploader') + ->setConstructorArgs([ + $this->coreFileStorageDb, + $this->coreFileStorage, + $this->imageFactory, + $this->validator, + $this->filesystem, + $readFactory, + ]) + ->getMock(); + + + $uploaderMock->move($fileUrl); + } + + public function moveFileUrlDriverPoolDataProvider() + { + return [ + [ + '$fileUrl' => 'http://test_uploader_file', + '$expectedHost' => 'test_uploader_file', + '$expectedDriverPool' => \Magento\Framework\Filesystem\Driver\Http::class, + '$expectedScheme' => \Magento\Framework\Filesystem\DriverPool::HTTP, + ], + [ + '$fileUrl' => 'https://!:^&`;file', + '$expectedHost' => '!:^&`;file', + '$expectedDriverPool' => \Magento\Framework\Filesystem\Driver\Https::class, + '$expectedScheme' => \Magento\Framework\Filesystem\DriverPool::HTTPS, + ], + ]; + } + public function moveFileUrlDataProvider() { return [ From 9dbab51a228dc2665b8968fb651a766d56d913b8 Mon Sep 17 00:00:00 2001 From: Clement Beudot Date: Wed, 25 Jan 2017 13:21:47 +0100 Subject: [PATCH 2/4] bug #8277 fixing bug with https downloading and fixed errors from build. --- .../CatalogImportExport/Model/Import/Uploader.php | 1 + .../Test/Unit/Model/Import/UploaderTest.php | 12 ++++++------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/app/code/Magento/CatalogImportExport/Model/Import/Uploader.php b/app/code/Magento/CatalogImportExport/Model/Import/Uploader.php index 851097f32240a..ea695a734b844 100644 --- a/app/code/Magento/CatalogImportExport/Model/Import/Uploader.php +++ b/app/code/Magento/CatalogImportExport/Model/Import/Uploader.php @@ -22,6 +22,7 @@ class Uploader extends \Magento\MediaStorage\Model\File\Uploader * used to compare against the filename and select the proper DriverPool adapter */ const HTTP_SCHEME = 'http://'; + /** * HTTPS scheme * used to compare against the filename and select the proper DriverPool adapter diff --git a/app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/UploaderTest.php b/app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/UploaderTest.php index d84d280fa56cb..ac5d173f91ef5 100644 --- a/app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/UploaderTest.php +++ b/app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/UploaderTest.php @@ -158,13 +158,13 @@ public function testMoveFileName() public function testMoveFileUrlDrivePool($fileUrl, $expectedHost, $expectedDriverPool, $expectedScheme) { - $driverPool = $this->getMock('Magento\Framework\Filesystem\DriverPool', ['getDriver']); + $driverPool = $this->getMock(\Magento\Framework\Filesystem\DriverPool::class, ['getDriver']); $driverMock = $this->getMock($expectedDriverPool, ['readAll']); $driverMock->expects($this->any())->method('isExists')->willReturn(true); $driverMock->expects($this->any())->method('readAll')->willReturn(null); $driverPool->expects($this->any())->method('getDriver')->willReturn($driverMock); - $readFactory = $this->getMockBuilder('Magento\Framework\Filesystem\File\ReadFactory') + $readFactory = $this->getMockBuilder(\Magento\Framework\Filesystem\File\ReadFactory::class) ->setConstructorArgs( [ $driverPool, @@ -173,10 +173,11 @@ public function testMoveFileUrlDrivePool($fileUrl, $expectedHost, $expectedDrive ->setMethods(['create']) ->getMock(); - $readFactory->expects($this->any())->method('create')->with($expectedHost, $expectedScheme)->willReturn($driverMock); + $readFactory->expects($this->any())->method('create') + ->with($expectedHost, $expectedScheme) + ->willReturn($driverMock); - - $uploaderMock = $this->getMockBuilder('\Magento\CatalogImportExport\Model\Import\Uploader') + $uploaderMock = $this->getMockBuilder(\Magento\CatalogImportExport\Model\Import\Uploader::class) ->setConstructorArgs([ $this->coreFileStorageDb, $this->coreFileStorage, @@ -187,7 +188,6 @@ public function testMoveFileUrlDrivePool($fileUrl, $expectedHost, $expectedDrive ]) ->getMock(); - $uploaderMock->move($fileUrl); } From a3a0002940b0509dded2bebec8ec175b782cc4db Mon Sep 17 00:00:00 2001 From: Clement Beudot Date: Wed, 25 Jan 2017 14:27:57 +0100 Subject: [PATCH 3/4] bug #8277 removed useless exception throws as there is already a preg_match on https? --- .../Magento/CatalogImportExport/Model/Import/Uploader.php | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/app/code/Magento/CatalogImportExport/Model/Import/Uploader.php b/app/code/Magento/CatalogImportExport/Model/Import/Uploader.php index ea695a734b844..bfe0802b2e864 100644 --- a/app/code/Magento/CatalogImportExport/Model/Import/Uploader.php +++ b/app/code/Magento/CatalogImportExport/Model/Import/Uploader.php @@ -148,7 +148,7 @@ public function init() /** * Proceed moving a file from TMP to destination folder * - * @param string $fileName + * @param $fileName * @param bool $renameFileOff * @return array */ @@ -162,12 +162,8 @@ public function move($fileName, $renameFileOff = false) if ($matches[0] === self::HTTP_SCHEME) { $read = $this->_readFactory->create($url, DriverPool::HTTP); - } elseif ($matches[0] === self::HTTPS_SCHEME) { - $read = $this->_readFactory->create($url, DriverPool::HTTPS); } else { - throw new LocalizedException( - __('No adapted DriverPool for File \'%1\'.', $fileName) - ); + $read = $this->_readFactory->create($url, DriverPool::HTTPS); } $fileName = preg_replace('/[^a-z0-9\._-]+/i', '', $fileName); From 4ff373609922ce9a5abc817ca05196b59eb6c2a2 Mon Sep 17 00:00:00 2001 From: Clement Beudot Date: Wed, 25 Jan 2017 16:09:16 +0100 Subject: [PATCH 4/4] bug #8277 Fixing static code analysis failure. --- app/code/Magento/CatalogImportExport/Model/Import/Uploader.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/CatalogImportExport/Model/Import/Uploader.php b/app/code/Magento/CatalogImportExport/Model/Import/Uploader.php index bfe0802b2e864..24ceeb3cffa35 100644 --- a/app/code/Magento/CatalogImportExport/Model/Import/Uploader.php +++ b/app/code/Magento/CatalogImportExport/Model/Import/Uploader.php @@ -148,7 +148,7 @@ public function init() /** * Proceed moving a file from TMP to destination folder * - * @param $fileName + * @param string $fileName * @param bool $renameFileOff * @return array */