Skip to content

Commit 3a15828

Browse files
zakdmazakdma
authored
Merge pull request #6646 from magento-tsg/2.4.3-develop-pr124
[Condor] Fixes for 2.4 (pr124) (2.4.3-develop)
2 parents ff591a3 + da9a7a4 commit 3a15828

File tree

5 files changed

+173
-28
lines changed

5 files changed

+173
-28
lines changed

‎app/code/Magento/Customer/Model/Metadata/Form/File.php

Lines changed: 13 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use Magento\Framework\Exception\LocalizedException;
1515
use Magento\Framework\File\UploaderFactory;
1616
use Magento\Framework\Filesystem;
17+
use Magento\Framework\Filesystem\Io\File as IoFile;
1718

1819
/**
1920
* Processes files that are save for customer.
@@ -62,6 +63,11 @@ class File extends AbstractData
6263
*/
6364
protected $fileProcessorFactory;
6465

66+
/**
67+
* @var IoFile|null
68+
*/
69+
private $ioFile;
70+
6571
/**
6672
* Constructor
6773
*
@@ -77,6 +83,7 @@ class File extends AbstractData
7783
* @param Filesystem $fileSystem
7884
* @param UploaderFactory $uploaderFactory
7985
* @param \Magento\Customer\Model\FileProcessorFactory|null $fileProcessorFactory
86+
* @param IoFile|null $ioFile
8087
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
8188
*/
8289
public function __construct(
@@ -91,7 +98,8 @@ public function __construct(
9198
\Magento\MediaStorage\Model\File\Validator\NotProtectedExtension $fileValidator,
9299
Filesystem $fileSystem,
93100
UploaderFactory $uploaderFactory,
94-
\Magento\Customer\Model\FileProcessorFactory $fileProcessorFactory = null
101+
\Magento\Customer\Model\FileProcessorFactory $fileProcessorFactory = null,
102+
IoFile $ioFile = null
95103
) {
96104
$value = $this->prepareFileValue($value);
97105
parent::__construct($localeDate, $logger, $attribute, $localeResolver, $value, $entityTypeCode, $isAjax);
@@ -102,6 +110,8 @@ public function __construct(
102110
$this->fileProcessorFactory = $fileProcessorFactory ?: ObjectManager::getInstance()
103111
->get(FileProcessorFactory::class);
104112
$this->fileProcessor = $this->fileProcessorFactory->create(['entityTypeCode' => $this->_entityTypeCode]);
113+
$this->ioFile = $ioFile ?: ObjectManager::getInstance()
114+
->get(IoFile::class);
105115
}
106116

107117
/**
@@ -183,7 +193,7 @@ protected function _validateByRules($value)
183193
{
184194
$label = $value['name'];
185195
$rules = $this->getAttribute()->getValidationRules();
186-
$extension = $this->getFileExtension($value['name']);
196+
$extension = $this->ioFile->getPathInfo($value['name'])['extension'];
187197
$fileExtensions = ArrayObjectSearch::getArrayElementByName(
188198
$rules,
189199
'file_extensions'
@@ -221,28 +231,6 @@ protected function _validateByRules($value)
221231
return [];
222232
}
223233

224-
/**
225-
* Get file extension from the file if it exists, otherwise, get from filename.
226-
*
227-
* @param string $fileName
228-
* @return string
229-
*/
230-
private function getFileExtension(string $fileName): string
231-
{
232-
return pathinfo($fileName, PATHINFO_EXTENSION);
233-
}
234-
235-
/**
236-
* Get file basename from the file if it exists, otherwise, get from filename.
237-
*
238-
* @param string $fileName
239-
* @return string
240-
*/
241-
private function getFileBasename(string $fileName): string
242-
{
243-
return pathinfo($fileName, PATHINFO_BASENAME);
244-
}
245-
246234
/**
247235
* Helper function that checks if the file was uploaded.
248236
*
@@ -259,7 +247,7 @@ protected function _isUploadedFile($filename)
259247
}
260248

261249
// This case is required for file uploader UI component
262-
$temporaryFile = FileProcessor::TMP_DIR . '/' . $this->getFileBasename($filename);
250+
$temporaryFile = FileProcessor::TMP_DIR . '/' . $this->ioFile->getPathInfo($filename)['basename'];
263251
if ($this->fileProcessor->isExist($temporaryFile)) {
264252
return true;
265253
}

‎app/code/Magento/Security/view/base/web/js/escaper.js

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,8 @@ define([], function () {
157157
attribute = treeWalker.currentNode.attributes[i];
158158
nodeName = treeWalker.currentNode.nodeName.toLowerCase();
159159

160-
if (this.generallyAllowedAttributes.indexOf(attribute.name) === -1 || // eslint-disable-line max-depth,max-len
160+
if (this.generallyAllowedAttributes.indexOf(attribute.name) === -1 || // eslint-disable-line max-depth,max-len
161+
this._checkHrefValue(attribute) ||
161162
this.forbiddenAttributesByElement[nodeName] &&
162163
this.forbiddenAttributesByElement[nodeName].indexOf(attribute.name) !== -1
163164
) {
@@ -169,6 +170,16 @@ define([], function () {
169170
attributesToRemove.forEach(function (attributeToRemove) {
170171
attributeToRemove.ownerElement.removeAttribute(attributeToRemove.name);
171172
});
173+
},
174+
175+
/**
176+
* Check that attribute contains script content
177+
*
178+
* @param {Object} attribute
179+
* @private
180+
*/
181+
_checkHrefValue: function (attribute) {
182+
return attribute.nodeName === 'href' && attribute.nodeValue.startsWith('javascript');
172183
}
173184
};
174185
});

‎dev/tests/js/jasmine/tests/app/code/Magento/Security/view/base/web/js/escaper.test.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,11 @@ define([
131131
data: '<spa>n id="id1">Some string</span>',
132132
expected: 'n id="id1"&gt;Some string',
133133
allowedTags: ['span']
134+
},
135+
'link with script content': {
136+
data: '<a href="javascript:void">Click</a>',
137+
expected: '<a>Click</a>',
138+
allowedTags: ['a']
134139
}
135140
};
136141
}

‎lib/internal/Magento/Framework/App/StaticResource.php

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
use Magento\Framework\ObjectManager\ConfigLoaderInterface;
1010
use Magento\Framework\Filesystem;
1111
use Magento\Framework\Config\ConfigOptionsListConstants;
12+
use Magento\Framework\Validator\Locale;
13+
use Magento\Framework\View\Design\Theme\ThemePackageList;
1214
use Psr\Log\LoggerInterface;
1315
use Magento\Framework\Debug;
1416
use Magento\Framework\Filesystem\Driver\File;
@@ -80,6 +82,16 @@ class StaticResource implements \Magento\Framework\AppInterface
8082
*/
8183
private $driver;
8284

85+
/**
86+
* @var ThemePackageList
87+
*/
88+
private $themePackageList;
89+
90+
/**
91+
* @var Locale
92+
*/
93+
private $localeValidator;
94+
8395
/**
8496
* @param State $state
8597
* @param Response\FileInterface $response
@@ -91,6 +103,8 @@ class StaticResource implements \Magento\Framework\AppInterface
91103
* @param ConfigLoaderInterface $configLoader
92104
* @param DeploymentConfig|null $deploymentConfig
93105
* @param File|null $driver
106+
* @param ThemePackageList|null $themePackageList
107+
* @param Locale|null $localeValidator
94108
*
95109
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
96110
*/
@@ -104,7 +118,9 @@ public function __construct(
104118
\Magento\Framework\ObjectManagerInterface $objectManager,
105119
ConfigLoaderInterface $configLoader,
106120
DeploymentConfig $deploymentConfig = null,
107-
File $driver = null
121+
File $driver = null,
122+
ThemePackageList $themePackageList = null,
123+
Locale $localeValidator = null
108124
) {
109125
$this->state = $state;
110126
$this->response = $response;
@@ -116,6 +132,8 @@ public function __construct(
116132
$this->configLoader = $configLoader;
117133
$this->deploymentConfig = $deploymentConfig ?: ObjectManager::getInstance()->get(DeploymentConfig::class);
118134
$this->driver = $driver ?: ObjectManager::getInstance()->get(File::class);
135+
$this->themePackageList = $themePackageList ?? ObjectManager::getInstance()->get(ThemePackageList::class);
136+
$this->localeValidator = $localeValidator ?? ObjectManager::getInstance()->get(Locale::class);
119137
}
120138

121139
/**
@@ -149,6 +167,16 @@ public function launch()
149167
throw $e;
150168
}
151169

170+
if (!($this->isThemeAllowed($params['area'] . DIRECTORY_SEPARATOR . $params['theme'])
171+
&& $this->localeValidator->isValid($params['locale']))
172+
) {
173+
if ($appMode == \Magento\Framework\App\State::MODE_PRODUCTION) {
174+
$this->response->setHttpResponseCode(404);
175+
return $this->response;
176+
}
177+
throw new \InvalidArgumentException('Requested path ' . $path . ' is wrong.');
178+
}
179+
152180
$this->state->setAreaCode($params['area']);
153181
$this->objectManager->configure($this->configLoader->load($params['area']));
154182
$file = $params['file'];
@@ -247,4 +275,9 @@ private function getLogger()
247275

248276
return $this->logger;
249277
}
278+
279+
private function isThemeAllowed(string $theme): bool
280+
{
281+
return in_array($theme, array_keys($this->themePackageList->getThemes()));
282+
}
250283
}

‎lib/internal/Magento/Framework/App/Test/Unit/StaticResourceTest.php

Lines changed: 109 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,10 @@
1919
use Magento\Framework\Filesystem\Driver\File;
2020
use Magento\Framework\Module\ModuleList;
2121
use Magento\Framework\ObjectManagerInterface;
22+
use Magento\Framework\Validator\Locale;
2223
use Magento\Framework\View\Asset\LocalInterface;
2324
use Magento\Framework\View\Asset\Repository;
25+
use Magento\Framework\View\Design\Theme\ThemePackageList;
2426
use PHPUnit\Framework\MockObject\MockObject;
2527
use PHPUnit\Framework\TestCase;
2628
use Psr\Log\LoggerInterface;
@@ -85,6 +87,16 @@ class StaticResourceTest extends TestCase
8587
*/
8688
private $driverMock;
8789

90+
/**
91+
* @var ThemePackageList|MockObject
92+
*/
93+
private $themePackageListMock;
94+
95+
/**
96+
* @var Locale|MockObject
97+
*/
98+
private $localeValidatorMock;
99+
88100
/**
89101
* @var StaticResource
90102
*/
@@ -106,6 +118,8 @@ protected function setUp(): void
106118
$this->configLoaderMock = $this->createMock(ConfigLoader::class);
107119
$this->deploymentConfigMock = $this->createMock(DeploymentConfig::class);
108120
$this->driverMock = $this->createMock(File::class);
121+
$this->themePackageListMock = $this->createMock(ThemePackageList::class);
122+
$this->localeValidatorMock = $this->createMock(Locale::class);
109123
$this->object = new StaticResource(
110124
$this->stateMock,
111125
$this->responseMock,
@@ -116,7 +130,9 @@ protected function setUp(): void
116130
$this->objectManagerMock,
117131
$this->configLoaderMock,
118132
$this->deploymentConfigMock,
119-
$this->driverMock
133+
$this->driverMock,
134+
$this->themePackageListMock,
135+
$this->localeValidatorMock
120136
);
121137
}
122138

@@ -210,6 +226,16 @@ public function testLaunch(
210226
$this->driverMock->expects($this->once())
211227
->method('getRealPathSafety')
212228
->willReturnArgument(0);
229+
$this->themePackageListMock->expects($this->atLeastOnce())->method('getThemes')->willReturn(
230+
[
231+
'area/Magento/theme' => [
232+
'area' => 'area',
233+
'vendor' => 'Magento',
234+
'name' => 'theme',
235+
],
236+
],
237+
);
238+
$this->localeValidatorMock->expects($this->once())->method('isValid')->willReturn(true);
213239
$this->object->launch();
214240
}
215241

@@ -353,4 +379,86 @@ public function testLaunchPathAbove()
353379

354380
$this->object->launch();
355381
}
382+
383+
/**
384+
* @param array $themes
385+
* @dataProvider themesDataProvider
386+
*/
387+
public function testLaunchWithInvalidTheme(array $themes): void
388+
{
389+
$this->expectException('InvalidArgumentException');
390+
$path = 'frontend/Test/luma/en_US/calendar.css';
391+
392+
$this->stateMock->expects($this->once())
393+
->method('getMode')
394+
->willReturn(State::MODE_DEVELOPER);
395+
$this->requestMock->expects($this->once())
396+
->method('get')
397+
->with('resource')
398+
->willReturn($path);
399+
$this->driverMock->expects($this->once())
400+
->method('getRealPathSafety')
401+
->with($path)
402+
->willReturn($path);
403+
$this->themePackageListMock->expects($this->once())->method('getThemes')->willReturn($themes);
404+
$this->localeValidatorMock->expects($this->never())->method('isValid');
405+
$this->expectExceptionMessage('Requested path ' . $path . ' is wrong.');
406+
407+
$this->object->launch();
408+
}
409+
410+
/**
411+
* @param array $themes
412+
* @dataProvider themesDataProvider
413+
*/
414+
public function testLaunchWithInvalidLocale(array $themes): void
415+
{
416+
$this->expectException('InvalidArgumentException');
417+
$path = 'frontend/Magento/luma/test/calendar.css';
418+
419+
$this->stateMock->expects($this->once())
420+
->method('getMode')
421+
->willReturn(State::MODE_DEVELOPER);
422+
$this->requestMock->expects($this->once())
423+
->method('get')
424+
->with('resource')
425+
->willReturn($path);
426+
$this->driverMock->expects($this->once())
427+
->method('getRealPathSafety')
428+
->with($path)
429+
->willReturn($path);
430+
$this->themePackageListMock->expects($this->once())->method('getThemes')->willReturn($themes);
431+
$this->localeValidatorMock->expects($this->once())->method('isValid')->willReturn(false);
432+
$this->expectExceptionMessage('Requested path ' . $path . ' is wrong.');
433+
434+
$this->object->launch();
435+
}
436+
437+
/**
438+
* @return array
439+
*/
440+
public function themesDataProvider(): array
441+
{
442+
return [
443+
[
444+
[
445+
'adminhtml/Magento/backend' => [
446+
'area' => 'adminhtml',
447+
'vendor' => 'Magento',
448+
'name' => 'backend',
449+
],
450+
'frontend/Magento/blank' => [
451+
'area' => 'frontend',
452+
'vendor' => 'Magento',
453+
'name' => 'blank',
454+
],
455+
'frontend/Magento/luma' => [
456+
'area' => 'frontend',
457+
'vendor' => 'Magento',
458+
'name' => 'luma',
459+
],
460+
],
461+
],
462+
];
463+
}
356464
}

0 commit comments

Comments
 (0)