Skip to content

Commit fcfc381

Browse files
OlgaVasyltsunOlgaVasyltsun
committed
MAGETWO-90070: Image uploader improvements
1 parent 2697952 commit fcfc381

File tree

14 files changed

+536
-123
lines changed

14 files changed

+536
-123
lines changed

‎app/code/Magento/Catalog/Model/ImageUploader.php

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,18 @@ class ImageUploader
6464
*/
6565
protected $allowedExtensions;
6666

67+
/**
68+
* List of allowed image mime types
69+
*
70+
* @var array
71+
*/
72+
private $allowedMimeTypes = [
73+
'image/jpg',
74+
'image/jpeg',
75+
'image/gif',
76+
'image/png',
77+
];
78+
6779
/**
6880
* ImageUploader constructor
6981
*
@@ -218,6 +230,7 @@ public function moveFileFromTmp($imageName)
218230
* @return string[]
219231
*
220232
* @throws \Magento\Framework\Exception\LocalizedException
233+
* @throws \Exception
221234
*/
222235
public function saveFileToTmpDir($fileId)
223236
{
@@ -227,7 +240,9 @@ public function saveFileToTmpDir($fileId)
227240
$uploader = $this->uploaderFactory->create(['fileId' => $fileId]);
228241
$uploader->setAllowedExtensions($this->getAllowedExtensions());
229242
$uploader->setAllowRenameFiles(true);
230-
243+
if (!$uploader->checkMimeType($this->allowedMimeTypes)) {
244+
throw new \Magento\Framework\Exception\LocalizedException(__('File validation failed.'));
245+
}
231246
$result = $uploader->save($this->mediaDirectory->getAbsolutePath($baseTmpPath));
232247
unset($result['path']);
233248

‎app/code/Magento/Catalog/Test/Unit/Model/ImageUploaderTest.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,12 @@ protected function setUp()
114114
public function testSaveFileToTmpDir()
115115
{
116116
$fileId = 'file.jpg';
117+
$allowedMimeTypes = [
118+
'image/jpg',
119+
'image/jpeg',
120+
'image/gif',
121+
'image/png',
122+
];
117123
/** @var \Magento\MediaStorage\Model\File\Uploader|\PHPUnit_Framework_MockObject_MockObject $uploader */
118124
$uploader = $this->createMock(\Magento\MediaStorage\Model\File\Uploader::class);
119125
$this->uploaderFactoryMock->expects($this->once())->method('create')->willReturn($uploader);
@@ -123,6 +129,7 @@ public function testSaveFileToTmpDir()
123129
->willReturn($this->basePath);
124130
$uploader->expects($this->once())->method('save')->with($this->basePath)
125131
->willReturn(['tmp_name' => $this->baseTmpPath, 'file' => $fileId, 'path' => $this->basePath]);
132+
$uploader->expects($this->atLeastOnce())->method('checkMimeType')->with($allowedMimeTypes)->willReturn(true);
126133
$storeMock = $this->createPartialMock(
127134
\Magento\Store\Model\Store::class,
128135
['getBaseUrl']

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

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,7 @@ public function deleteFile($target)
478478
* @param string $type Type of storage, e.g. image, media etc.
479479
* @return array File info Array
480480
* @throws \Magento\Framework\Exception\LocalizedException
481+
* @throws \Exception
481482
*/
482483
public function uploadFile($targetPath, $type = null)
483484
{
@@ -489,6 +490,9 @@ public function uploadFile($targetPath, $type = null)
489490
}
490491
$uploader->setAllowRenameFiles(true);
491492
$uploader->setFilesDispersion(false);
493+
if (!$uploader->checkMimeType($this->getAllowedMimeTypes($type))) {
494+
throw new \Magento\Framework\Exception\LocalizedException(__('File validation failed.'));
495+
}
492496
$result = $uploader->save($targetPath);
493497

494498
if (!$result) {
@@ -645,11 +649,7 @@ public function getSession()
645649
*/
646650
public function getAllowedExtensions($type = null)
647651
{
648-
if (is_string($type) && array_key_exists("{$type}_allowed", $this->_extensions)) {
649-
$allowed = $this->_extensions["{$type}_allowed"];
650-
} else {
651-
$allowed = $this->_extensions['allowed'];
652-
}
652+
$allowed = $this->getExtensionsList($type);
653653

654654
return array_keys(array_filter($allowed));
655655
}
@@ -755,4 +755,33 @@ protected function _getRelativePathToRoot($path)
755755
strlen($this->_sanitizePath($this->_cmsWysiwygImages->getStorageRoot()))
756756
);
757757
}
758+
759+
/**
760+
* Prepare mime types config settings
761+
*
762+
* @param string|null $type Type of storage, e.g. image, media etc.
763+
* @return array Array of allowed file extensions
764+
*/
765+
private function getAllowedMimeTypes($type = null): array
766+
{
767+
$allowed = $this->getExtensionsList($type);
768+
769+
return array_values(array_filter($allowed));
770+
}
771+
772+
/**
773+
* Get list of allowed file extensions with mime type in values
774+
*
775+
* @param string|null $type
776+
* @return array
777+
*/
778+
private function getExtensionsList($type = null): array
779+
{
780+
if (is_string($type) && array_key_exists("{$type}_allowed", $this->_extensions)) {
781+
$allowed = $this->_extensions["{$type}_allowed"];
782+
} else {
783+
$allowed = $this->_extensions['allowed'];
784+
}
785+
return $allowed;
786+
}
758787
}

‎app/code/Magento/Cms/Test/Unit/Model/Wysiwyg/Images/StorageTest.php

Lines changed: 133 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,13 @@ class StorageTest extends \PHPUnit\Framework\TestCase
107107
*/
108108
protected $objectManagerHelper;
109109

110+
private $allowedImageExtensions = [
111+
'jpg' => 'image/jpg',
112+
'jpeg' => 'image/jpeg',
113+
'png' => 'image/png',
114+
'gif' => 'image/png',
115+
];
116+
110117
/**
111118
* @return void
112119
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
@@ -120,7 +127,7 @@ protected function setUp()
120127

121128
$this->directoryMock = $this->createPartialMock(
122129
\Magento\Framework\Filesystem\Directory\Write::class,
123-
['delete', 'getDriver', 'create', 'getRelativePath', 'isExist']
130+
['delete', 'getDriver', 'create', 'getRelativePath', 'isExist', 'isFile']
124131
);
125132
$this->directoryMock->expects(
126133
$this->any()
@@ -176,14 +183,27 @@ protected function setUp()
176183
->disableOriginalConstructor()
177184
->getMock();
178185
$this->sessionMock = $this->getMockBuilder(\Magento\Backend\Model\Session::class)
179-
->setMethods(['getCurrentPath'])
186+
->setMethods(
187+
[
188+
'getCurrentPath',
189+
'getName',
190+
'getSessionId',
191+
'getCookieLifetime',
192+
'getCookiePath',
193+
'getCookieDomain',
194+
]
195+
)
180196
->disableOriginalConstructor()
181197
->getMock();
182198
$this->backendUrlMock = $this->createMock(\Magento\Backend\Model\Url::class);
183199

184200
$this->coreFileStorageMock = $this->getMockBuilder(\Magento\MediaStorage\Helper\File\Storage\Database::class)
185201
->disableOriginalConstructor()
186202
->getMock();
203+
$allowedExtensions = [
204+
'allowed' => $this->allowedImageExtensions,
205+
'image_allowed' => $this->allowedImageExtensions,
206+
];
187207

188208
$this->objectManagerHelper = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
189209

@@ -205,8 +225,9 @@ protected function setUp()
205225
'resizeParameters' => $this->resizeParameters,
206226
'dirs' => [
207227
'exclude' => [],
208-
'include' => []
209-
]
228+
'include' => [],
229+
],
230+
'extensions' => $allowedExtensions,
210231
]
211232
);
212233
}
@@ -232,8 +253,8 @@ public function testGetResizeHeight()
232253
*/
233254
public function testDeleteDirectoryOverRoot()
234255
{
235-
$this->expectException(\Magento\Framework\Exception\LocalizedException::class);
236-
$this->expectExceptionMessage(
256+
$this->expectException(
257+
\Magento\Framework\Exception\LocalizedException::class,
237258
sprintf('Directory %s is not under storage root path.', self::INVALID_DIRECTORY_OVER_ROOT)
238259
);
239260
$this->driverMock->expects($this->atLeastOnce())->method('getRealPathSafety')->will($this->returnArgument(0));
@@ -245,8 +266,10 @@ public function testDeleteDirectoryOverRoot()
245266
*/
246267
public function testDeleteRootDirectory()
247268
{
248-
$this->expectException(\Magento\Framework\Exception\LocalizedException::class);
249-
$this->expectExceptionMessage(sprintf('We can\'t delete root directory %s right now.', self::STORAGE_ROOT_DIR));
269+
$this->expectException(
270+
\Magento\Framework\Exception\LocalizedException::class,
271+
sprintf('We can\'t delete root directory %s right now.', self::STORAGE_ROOT_DIR)
272+
);
250273
$this->driverMock->expects($this->atLeastOnce())->method('getRealPathSafety')->will($this->returnArgument(0));
251274
$this->imagesStorage->deleteDirectory(self::STORAGE_ROOT_DIR);
252275
}
@@ -302,8 +325,8 @@ public function testGetDirsCollection($exclude, $include, $fileNames, $expectedR
302325
'resizeParameters' => $this->resizeParameters,
303326
'dirs' => [
304327
'exclude' => $exclude,
305-
'include' => $include
306-
]
328+
'include' => $include,
329+
],
307330
]
308331
);
309332

@@ -328,48 +351,48 @@ public function dirsCollectionDataProvider()
328351
return [
329352
[
330353
'exclude' => [
331-
['name' => 'dress']
354+
['name' => 'dress'],
332355
],
333356
'include' => [],
334357
'filenames' => [],
335-
'expectRemoveKeys' => []
358+
'expectRemoveKeys' => [],
336359
],
337360
[
338361
'exclude' => [],
339362
'include' => [],
340363
'filenames' => [
341364
'/dress',
342365
],
343-
'expectRemoveKeys' => []
366+
'expectRemoveKeys' => [],
344367
],
345368
[
346369
'exclude' => [
347-
['name' => 'dress']
370+
['name' => 'dress'],
348371
],
349372
'include' => [],
350373
'filenames' => [
351374
'/collection',
352375
],
353-
'expectRemoveKeys' => []
376+
'expectRemoveKeys' => [],
354377
],
355378
[
356379
'exclude' => [
357380
['name' => 'gear', 'regexp' => 1],
358381
['name' => 'home', 'regexp' => 1],
359382
['name' => 'collection'],
360-
['name' => 'dress']
383+
['name' => 'dress'],
361384
],
362385
'include' => [
363386
['name' => 'home', 'regexp' => 1],
364-
['name' => 'collection']
387+
['name' => 'collection'],
365388
],
366389
'filenames' => [
367390
'/dress',
368391
'/collection',
369-
'/gear'
392+
'/gear',
370393
],
371-
'expectRemoveKeys' => [[0], [2]]
372-
]
394+
'expectRemoveKeys' => [[0], [2]],
395+
],
373396
];
374397
}
375398

@@ -411,4 +434,94 @@ protected function generalTestGetDirsCollection($path, $collectionArray = [], $e
411434

412435
$this->imagesStorage->getDirsCollection($path);
413436
}
437+
438+
public function testUploadFile()
439+
{
440+
$targetPath = '/target/path';
441+
$fileName = 'image.gif';
442+
$realPath = $targetPath . '/' . $fileName;
443+
$thumbnailTargetPath = self::STORAGE_ROOT_DIR . '/.thumbs';
444+
$thumbnailDestination = $thumbnailTargetPath . '/' . $fileName;
445+
$type = 'image';
446+
$result = [
447+
'result',
448+
'cookie' => [
449+
'name' => 'session_name',
450+
'value' => '1',
451+
'lifetime' => '50',
452+
'path' => 'cookie/path',
453+
'domain' => 'cookie_domain',
454+
],
455+
];
456+
$uploader = $this->getMockBuilder(\Magento\MediaStorage\Model\File\Uploader::class)
457+
->disableOriginalConstructor()
458+
->setMethods(
459+
[
460+
'setAllowedExtensions',
461+
'setAllowRenameFiles',
462+
'setFilesDispersion',
463+
'checkMimeType',
464+
'save',
465+
'getUploadedFileName',
466+
]
467+
)
468+
->getMock();
469+
$this->uploaderFactoryMock->expects($this->atLeastOnce())->method('create')->with(['fileId' => 'image'])
470+
->willReturn($uploader);
471+
$uploader->expects($this->atLeastOnce())->method('setAllowedExtensions')
472+
->with(array_keys($this->allowedImageExtensions))->willReturnSelf();
473+
$uploader->expects($this->atLeastOnce())->method('setAllowRenameFiles')->with(true)->willReturnSelf();
474+
$uploader->expects($this->atLeastOnce())->method('setFilesDispersion')->with(false)
475+
->willReturnSelf();
476+
$uploader->expects($this->atLeastOnce())->method('checkMimeType')
477+
->with(array_values($this->allowedImageExtensions))->willReturnSelf();
478+
$uploader->expects($this->atLeastOnce())->method('save')->with($targetPath)->willReturn($result);
479+
$uploader->expects($this->atLeastOnce())->method('getUploadedFileName')->willReturn($fileName);
480+
481+
$this->directoryMock->expects($this->atLeastOnce())->method('getRelativePath')->willReturnMap(
482+
[
483+
[$realPath, $realPath],
484+
[$thumbnailTargetPath, $thumbnailTargetPath],
485+
[$thumbnailDestination, $thumbnailDestination],
486+
]
487+
);
488+
$this->directoryMock->expects($this->atLeastOnce())->method('isFile')
489+
->willReturnMap(
490+
[
491+
[$realPath, true],
492+
[$thumbnailDestination, true],
493+
]
494+
);
495+
$this->directoryMock->expects($this->atLeastOnce())->method('isExist')
496+
->willReturnMap(
497+
[
498+
[$realPath, true],
499+
[$thumbnailTargetPath, true],
500+
]
501+
);
502+
503+
$image = $this->getMockBuilder(\Magento\Catalog\Model\Product\Image::class)
504+
->disableOriginalConstructor()
505+
->setMethods(['open', 'keepAspectRatio', 'resize', 'save'])
506+
->getMock();
507+
$image->expects($this->atLeastOnce())->method('open')->with($realPath);
508+
$image->expects($this->atLeastOnce())->method('keepAspectRatio')->with(true);
509+
$image->expects($this->atLeastOnce())->method('resize')->with(100, 50);
510+
$image->expects($this->atLeastOnce())->method('save')->with($thumbnailDestination);
511+
512+
$this->adapterFactoryMock->expects($this->atLeastOnce())->method('create')->willReturn($image);
513+
514+
$this->sessionMock->expects($this->atLeastOnce())->method('getName')
515+
->willReturn($result['cookie']['name']);
516+
$this->sessionMock->expects($this->atLeastOnce())->method('getSessionId')
517+
->willReturn($result['cookie']['value']);
518+
$this->sessionMock->expects($this->atLeastOnce())->method('getCookieLifetime')
519+
->willReturn($result['cookie']['lifetime']);
520+
$this->sessionMock->expects($this->atLeastOnce())->method('getCookiePath')
521+
->willReturn($result['cookie']['path']);
522+
$this->sessionMock->expects($this->atLeastOnce())->method('getCookieDomain')
523+
->willReturn($result['cookie']['domain']);
524+
525+
$this->assertEquals($result, $this->imagesStorage->uploadFile($targetPath, $type));
526+
}
414527
}

0 commit comments

Comments
 (0)