Skip to content

Commit 8073b96

Browse files
author
Oleksandr Gorkun
committed
Merge branch '2.3-develop' of https://github.com/magento/magento2ce into MAGETWO-92720
2 parents 2f4e77d + 5872c82 commit 8073b96

File tree

26 files changed

+1139
-181
lines changed

26 files changed

+1139
-181
lines changed

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

Lines changed: 15 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
*
@@ -227,7 +239,9 @@ public function saveFileToTmpDir($fileId)
227239
$uploader = $this->uploaderFactory->create(['fileId' => $fileId]);
228240
$uploader->setAllowedExtensions($this->getAllowedExtensions());
229241
$uploader->setAllowRenameFiles(true);
230-
242+
if (!$uploader->checkMimeType($this->allowedMimeTypes)) {
243+
throw new \Magento\Framework\Exception\LocalizedException(__('File validation failed.'));
244+
}
231245
$result = $uploader->save($this->mediaDirectory->getAbsolutePath($baseTmpPath));
232246
unset($result['path']);
233247

‎app/code/Magento/Catalog/Model/Product/Gallery/UpdateHandler.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ protected function processDeletedImages($product, array &$images)
3232
foreach ($images as &$image) {
3333
if (!empty($image['removed'])) {
3434
if (!empty($image['value_id']) && !isset($picturesInOtherStores[$image['file']])) {
35+
if (preg_match('/\.\.(\\\|\/)/', $image['file'])) {
36+
continue;
37+
}
3538
$recordsToDelete[] = $image['value_id'];
3639
$catalogPath = $this->mediaConfig->getBaseMediaPath();
3740
$isFile = $this->mediaDirectory->isFile($catalogPath . $image['file']);

‎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/CatalogImportExport/Model/Import/Uploader.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,7 @@ protected function _validateFile()
241241

242242
$fileExtension = pathinfo($filePath, PATHINFO_EXTENSION);
243243
if (!$this->checkAllowedExtension($fileExtension)) {
244+
$this->_directory->delete($filePath);
244245
throw new \Exception('Disallowed file type.');
245246
}
246247
//run validate callbacks

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

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -489,6 +489,9 @@ public function uploadFile($targetPath, $type = null)
489489
}
490490
$uploader->setAllowRenameFiles(true);
491491
$uploader->setFilesDispersion(false);
492+
if (!$uploader->checkMimeType($this->getAllowedMimeTypes($type))) {
493+
throw new \Magento\Framework\Exception\LocalizedException(__('File validation failed.'));
494+
}
492495
$result = $uploader->save($targetPath);
493496

494497
if (!$result) {
@@ -645,11 +648,7 @@ public function getSession()
645648
*/
646649
public function getAllowedExtensions($type = null)
647650
{
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-
}
651+
$allowed = $this->getExtensionsList($type);
653652

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

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

Lines changed: 131 additions & 22 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
}
@@ -229,24 +250,22 @@ public function testGetResizeHeight()
229250

230251
/**
231252
* @covers \Magento\Cms\Model\Wysiwyg\Images\Storage::deleteDirectory
253+
* @expectedException \Magento\Framework\Exception\LocalizedException
254+
* @expectedExceptionMessage Directory /storage/some/another/dir is not under storage root path.
232255
*/
233256
public function testDeleteDirectoryOverRoot()
234257
{
235-
$this->expectException(\Magento\Framework\Exception\LocalizedException::class);
236-
$this->expectExceptionMessage(
237-
sprintf('Directory %s is not under storage root path.', self::INVALID_DIRECTORY_OVER_ROOT)
238-
);
239258
$this->driverMock->expects($this->atLeastOnce())->method('getRealPathSafety')->will($this->returnArgument(0));
240259
$this->imagesStorage->deleteDirectory(self::INVALID_DIRECTORY_OVER_ROOT);
241260
}
242261

243262
/**
244263
* @covers \Magento\Cms\Model\Wysiwyg\Images\Storage::deleteDirectory
264+
* @expectedException \Magento\Framework\Exception\LocalizedException
265+
* @expectedExceptionMessage We can't delete root directory /storage/root/dir right now.
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));
250269
$this->driverMock->expects($this->atLeastOnce())->method('getRealPathSafety')->will($this->returnArgument(0));
251270
$this->imagesStorage->deleteDirectory(self::STORAGE_ROOT_DIR);
252271
}
@@ -302,8 +321,8 @@ public function testGetDirsCollection($exclude, $include, $fileNames, $expectedR
302321
'resizeParameters' => $this->resizeParameters,
303322
'dirs' => [
304323
'exclude' => $exclude,
305-
'include' => $include
306-
]
324+
'include' => $include,
325+
],
307326
]
308327
);
309328

@@ -328,48 +347,48 @@ public function dirsCollectionDataProvider()
328347
return [
329348
[
330349
'exclude' => [
331-
['name' => 'dress']
350+
['name' => 'dress'],
332351
],
333352
'include' => [],
334353
'filenames' => [],
335-
'expectRemoveKeys' => []
354+
'expectRemoveKeys' => [],
336355
],
337356
[
338357
'exclude' => [],
339358
'include' => [],
340359
'filenames' => [
341360
'/dress',
342361
],
343-
'expectRemoveKeys' => []
362+
'expectRemoveKeys' => [],
344363
],
345364
[
346365
'exclude' => [
347-
['name' => 'dress']
366+
['name' => 'dress'],
348367
],
349368
'include' => [],
350369
'filenames' => [
351370
'/collection',
352371
],
353-
'expectRemoveKeys' => []
372+
'expectRemoveKeys' => [],
354373
],
355374
[
356375
'exclude' => [
357376
['name' => 'gear', 'regexp' => 1],
358377
['name' => 'home', 'regexp' => 1],
359378
['name' => 'collection'],
360-
['name' => 'dress']
379+
['name' => 'dress'],
361380
],
362381
'include' => [
363382
['name' => 'home', 'regexp' => 1],
364-
['name' => 'collection']
383+
['name' => 'collection'],
365384
],
366385
'filenames' => [
367386
'/dress',
368387
'/collection',
369-
'/gear'
388+
'/gear',
370389
],
371-
'expectRemoveKeys' => [[0], [2]]
372-
]
390+
'expectRemoveKeys' => [[0], [2]],
391+
],
373392
];
374393
}
375394

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

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

0 commit comments

Comments
 (0)