Skip to content

Commit 3462faa

Browse files
zakdmazakdma
authored
Merge pull request #5634 from magento-tsg/2.3.6-develop-pr123
[TSG] Fixes for 2.3 (pr123) (2.3.6-develop)
2 parents f68f625 + b3d8beb commit 3462faa

File tree

7 files changed

+138
-34
lines changed

7 files changed

+138
-34
lines changed

‎app/code/Magento/ImportExport/Block/Adminhtml/Grid/Column/Renderer/Download.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@ class Download extends \Magento\Backend\Block\Widget\Grid\Column\Renderer\Text
2020
*/
2121
public function _getValue(\Magento\Framework\DataObject $row)
2222
{
23-
return '<p> ' . $row->getData('imported_file') . '</p><a href="'
24-
. $this->getUrl('*/*/download', ['filename' => $row->getData('imported_file')]) . '">'
25-
. __('Download')
23+
return '<p> ' . $this->escapeHtml($row->getData('imported_file')) . '</p><a href="'
24+
. $this->escapeUrl($this->getUrl('*/*/download', ['filename' => $row->getData('imported_file')])) . '">'
25+
. $this->escapeHtml(__('Download'))
2626
. '</a>';
2727
}
2828
}

‎app/code/Magento/ImportExport/Block/Adminhtml/Grid/Column/Renderer/Error.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,9 @@ public function _getValue(\Magento\Framework\DataObject $row)
2222
{
2323
$result = '';
2424
if ($row->getData('error_file') != '') {
25-
$result = '<p> ' . $row->getData('error_file') . '</p><a href="'
26-
. $this->getUrl('*/*/download', ['filename' => $row->getData('error_file')]) . '">'
27-
. __('Download')
25+
$result = '<p> ' . $this->escapeHtml($row->getData('error_file')) . '</p><a href="'
26+
. $this->escapeUrl($this->getUrl('*/*/download', ['filename' => $row->getData('error_file')])) . '">'
27+
. $this->escapeHtml(__('Download'))
2828
. '</a>';
2929
}
3030
return $result;

‎app/code/Magento/ImportExport/Test/Unit/Block/Adminhtml/Grid/Column/Renderer/DownloadTest.php

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,20 @@
55
*/
66
namespace Magento\ImportExport\Test\Unit\Block\Adminhtml\Grid\Column\Renderer;
77

8+
use Magento\Backend\Block\Context;
9+
use Magento\Backend\Model\Url;
10+
use Magento\Framework\DataObject;
11+
use Magento\Framework\Escaper;
812
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper;
13+
use Magento\ImportExport\Block\Adminhtml\Grid\Column\Renderer\Download;
914

15+
/**
16+
* Test for \Magento\ImportExport\Block\Adminhtml\Grid\Column\Renderer\Download class.
17+
*/
1018
class DownloadTest extends \PHPUnit\Framework\TestCase
1119
{
1220
/**
13-
* @var \Magento\Backend\Block\Context
21+
* @var Context
1422
*/
1523
protected $context;
1624

@@ -20,24 +28,31 @@ class DownloadTest extends \PHPUnit\Framework\TestCase
2028
protected $objectManagerHelper;
2129

2230
/**
23-
* @var \Magento\ImportExport\Block\Adminhtml\Grid\Column\Renderer\Download
31+
* @var Download
2432
*/
2533
protected $download;
2634

35+
/**
36+
* @var Escaper|\PHPUnit_Framework_MockObject_MockObjecti
37+
*/
38+
private $escaperMock;
39+
2740
/**
2841
* Set up
2942
*/
3043
protected function setUp()
3144
{
32-
$urlModel = $this->createPartialMock(\Magento\Backend\Model\Url::class, ['getUrl']);
45+
$this->escaperMock = $this->createMock(Escaper::class);
46+
$urlModel = $this->createPartialMock(Url::class, ['getUrl']);
3347
$urlModel->expects($this->any())->method('getUrl')->willReturn('url');
34-
$this->context = $this->createPartialMock(\Magento\Backend\Block\Context::class, ['getUrlBuilder']);
48+
$this->context = $this->createPartialMock(Context::class, ['getUrlBuilder', 'getEscaper']);
3549
$this->context->expects($this->any())->method('getUrlBuilder')->willReturn($urlModel);
50+
$this->context->expects($this->any())->method('getEscaper')->willReturn($this->escaperMock);
3651
$data = [];
3752

3853
$this->objectManagerHelper = new ObjectManagerHelper($this);
3954
$this->download = $this->objectManagerHelper->getObject(
40-
\Magento\ImportExport\Block\Adminhtml\Grid\Column\Renderer\Download::class,
55+
Download::class,
4156
[
4257
'context' => $this->context,
4358
'data' => $data
@@ -51,7 +66,16 @@ protected function setUp()
5166
public function testGetValue()
5267
{
5368
$data = ['imported_file' => 'file.csv'];
54-
$row = new \Magento\Framework\DataObject($data);
69+
$row = new DataObject($data);
70+
$this->escaperMock->expects($this->at(0))
71+
->method('escapeHtml')
72+
->with('file.csv')
73+
->willReturn('file.csv');
74+
$this->escaperMock->expects($this->once())->method('escapeUrl')->willReturn('url');
75+
$this->escaperMock->expects($this->at(2))
76+
->method('escapeHtml')
77+
->with('Download')
78+
->willReturn('Download');
5579
$this->assertEquals('<p> file.csv</p><a href="url">Download</a>', $this->download->_getValue($row));
5680
}
5781
}

‎app/code/Magento/Theme/Model/Design/Backend/File.php

Lines changed: 57 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
use Magento\MediaStorage\Model\File\UploaderFactory;
2323
use Magento\Theme\Model\Design\Config\FileUploader\FileProcessor;
2424
use Magento\MediaStorage\Helper\File\Storage\Database;
25+
use Magento\Framework\Filesystem\Io\File as IoFile;
26+
use Magento\Framework\Filesystem\Directory\ReadFactory;
2527

2628
/**
2729
* File Backend
@@ -45,6 +47,16 @@ class File extends BackendFile
4547
*/
4648
private $databaseHelper;
4749

50+
/**
51+
* @var IoFile|null
52+
*/
53+
private $ioFile;
54+
55+
/**
56+
* @var Read
57+
*/
58+
private $tmpDirectory;
59+
4860
/**
4961
* @param Context $context
5062
* @param Registry $registry
@@ -58,6 +70,8 @@ class File extends BackendFile
5870
* @param AbstractDb|null $resourceCollection
5971
* @param array $data
6072
* @param Database $databaseHelper
73+
* @param IoFile|null $ioFile
74+
* @param ReadFactory $tmpDirectory
6175
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
6276
*/
6377
public function __construct(
@@ -72,7 +86,9 @@ public function __construct(
7286
AbstractResource $resource = null,
7387
AbstractDb $resourceCollection = null,
7488
array $data = [],
75-
Database $databaseHelper = null
89+
Database $databaseHelper = null,
90+
IoFile $ioFile = null,
91+
ReadFactory $tmpDirectory = null
7692
) {
7793
parent::__construct(
7894
$context,
@@ -88,6 +104,12 @@ public function __construct(
88104
);
89105
$this->urlBuilder = $urlBuilder;
90106
$this->databaseHelper = $databaseHelper ?: ObjectManager::getInstance()->get(Database::class);
107+
$this->ioFile = $ioFile ?: ObjectManager::getInstance()->get(IoFile::class);
108+
/** @var ReadFactory $readFactory */
109+
$readFactory = ObjectManager::getInstance()->get(ReadFactory::class);
110+
$this->tmpDirectory = $tmpDirectory ?: $readFactory->create(
111+
$this->_mediaDirectory->getAbsolutePath() .'tmp/' . FileProcessor::FILE_DIR
112+
);
91113
}
92114

93115
/**
@@ -108,11 +130,18 @@ public function beforeSave()
108130
__('%1 does not contain field \'file\'', $this->getData('field_config/field'))
109131
);
110132
}
111-
if (isset($value['exists'])) {
133+
134+
$extension = $this->ioFile->getPathInfo($file);
135+
$fileExtension = is_array($extension) ? $extension['extension'] : '';
136+
if (!$this->isAllowedExtension($fileExtension)) {
137+
throw new LocalizedException(__("Invalid file provided."));
138+
}
139+
140+
if ($this->getOrigData('value') === $file) {
112141
$this->setValue($file);
113142
return $this;
114143
}
115-
144+
116145
//phpcs:ignore Magento2.Functions.DiscouragedFunction
117146
$this->updateMediaDirectory(basename($file), $value['url']);
118147

@@ -126,8 +155,10 @@ public function afterLoad()
126155
{
127156
$value = $this->getValue();
128157
if ($value && !is_array($value)) {
129-
//phpcs:ignore Magento2.Functions.DiscouragedFunction
130-
$fileName = $this->_getUploadDir() . '/' . basename($value);
158+
$fileName = $this->_mediaDirectory->getAbsolutePath(
159+
//phpcs:ignore Magento2.Functions.DiscouragedFunction
160+
$this->_getUploadDir() . DIRECTORY_SEPARATOR . basename($value)
161+
);
131162
$fileInfo = null;
132163
if ($this->_mediaDirectory->isExist($fileName)) {
133164
$stat = $this->_mediaDirectory->stat($fileName);
@@ -207,7 +238,7 @@ protected function getStoreMediaUrl($fileName)
207238
*/
208239
protected function getTmpMediaPath($filename)
209240
{
210-
return 'tmp/' . FileProcessor::FILE_DIR . '/' . $filename;
241+
return $this->tmpDirectory->getAbsolutePath($this->tmpDirectory->getRelativePath($filename));
211242
}
212243

213244
/**
@@ -259,10 +290,12 @@ private function getRelativeMediaPath(string $path): string
259290
*/
260291
private function updateMediaDirectory(string $filename, string $url)
261292
{
262-
$relativeMediaPath = $this->getRelativeMediaPath($url);
293+
$absoluteMediaPath = $this->_mediaDirectory->getAbsolutePath($this->getRelativeMediaPath($url));
263294
$tmpMediaPath = $this->getTmpMediaPath($filename);
264-
$mediaPath = $this->_mediaDirectory->isFile($relativeMediaPath) ? $relativeMediaPath : $tmpMediaPath;
265-
$destinationMediaPath = $this->_getUploadDir() . '/' . $filename;
295+
$mediaPath = $this->_mediaDirectory->isFile($absoluteMediaPath) ? $absoluteMediaPath : $tmpMediaPath;
296+
$destinationMediaPath = $this->_mediaDirectory->getAbsolutePath(
297+
$this->_getUploadDir() . DIRECTORY_SEPARATOR . $filename
298+
);
266299

267300
$result = $mediaPath === $destinationMediaPath;
268301
if (!$result) {
@@ -287,4 +320,19 @@ private function updateMediaDirectory(string $filename, string $url)
287320
$this->unsValue();
288321
}
289322
}
323+
324+
/**
325+
* Check if specified extension is allowed.
326+
*
327+
* @param string $extension
328+
* @return boolean
329+
*/
330+
public function isAllowedExtension(string $extension): bool
331+
{
332+
if (empty($this->getAllowedExtensions())) {
333+
return true;
334+
}
335+
336+
return in_array(strtolower($extension), $this->getAllowedExtensions());
337+
}
290338
}

‎app/code/Magento/Theme/Model/Design/Config/FileUploader/FileProcessor.php

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ public function __construct(
7878
}
7979

8080
/**
81-
* Save file to temp media directory
81+
* Save file to temp media directory.
8282
*
8383
* @param string $fileId
8484
*
@@ -92,40 +92,41 @@ public function saveToTmp($fileId)
9292
} catch (\Exception $e) {
9393
$result = ['error' => $e->getMessage(), 'errorcode' => $e->getCode()];
9494
}
95+
9596
return $result;
9697
}
9798

9899
/**
99-
* Retrieve temp media url
100+
* Retrieve temp media url.
100101
*
101102
* @param string $file
102103
* @return string
103104
*/
104105
protected function getTmpMediaUrl($file)
105106
{
106107
return $this->storeManager->getStore()->getBaseUrl(UrlInterface::URL_TYPE_MEDIA)
107-
. 'tmp/' . self::FILE_DIR . '/' . $this->prepareFile($file);
108+
. 'tmp' . DIRECTORY_SEPARATOR . self::FILE_DIR . DIRECTORY_SEPARATOR . $this->prepareFile($file);
108109
}
109110

110111
/**
111-
* Prepare file
112+
* Prepare file.
112113
*
113114
* @param string $file
114115
* @return string
115116
*/
116117
protected function prepareFile($file)
117118
{
118-
return ltrim(str_replace('\\', '/', $file), '/');
119+
return ltrim(str_replace('\\', DIRECTORY_SEPARATOR, $file), DIRECTORY_SEPARATOR);
119120
}
120121

121122
/**
122-
* Retrieve absolute temp media path
123+
* Retrieve absolute temp media path.
123124
*
124125
* @return string
125126
*/
126127
protected function getAbsoluteTmpMediaPath()
127128
{
128-
return $this->mediaDirectory->getAbsolutePath('tmp/' . self::FILE_DIR);
129+
return $this->mediaDirectory->getAbsolutePath('tmp' . DIRECTORY_SEPARATOR . self::FILE_DIR);
129130
}
130131

131132
/**

0 commit comments

Comments
 (0)