From c1b046bbbfe02c0e57998e3e770a09da4a48605b Mon Sep 17 00:00:00 2001 From: Stas Kozar Date: Fri, 30 Apr 2021 16:53:13 +0300 Subject: [PATCH 1/3] MC-41013: Prevent errors from incorrect configurations --- app/etc/di.xml | 3 + .../Interpreter/ConfigurableObject.php | 55 +++++++++++++------ .../Interpreter/ConfigurableObjectTest.php | 25 +++++++++ 3 files changed, 66 insertions(+), 17 deletions(-) diff --git a/app/etc/di.xml b/app/etc/di.xml index e934b201142af..64456e62ce069 100644 --- a/app/etc/di.xml +++ b/app/etc/di.xml @@ -698,6 +698,9 @@ Magento\Framework\Data\OptionSourceInterface Magento\Framework\View\Element\UiComponent\DataProvider\DataProviderInterface + + Magento\Framework\Model\ResourceModel\AbstractResource + diff --git a/lib/internal/Magento/Framework/View/Element/UiComponent/Argument/Interpreter/ConfigurableObject.php b/lib/internal/Magento/Framework/View/Element/UiComponent/Argument/Interpreter/ConfigurableObject.php index 2691bc21357b0..de8bddeac30e9 100644 --- a/lib/internal/Magento/Framework/View/Element/UiComponent/Argument/Interpreter/ConfigurableObject.php +++ b/lib/internal/Magento/Framework/View/Element/UiComponent/Argument/Interpreter/ConfigurableObject.php @@ -24,6 +24,11 @@ class ConfigurableObject implements InterpreterInterface */ private $classWhitelist = []; + /** + * @var array + */ + private $deniedClassList = []; + /** * @var ObjectManagerInterface */ @@ -52,17 +57,20 @@ class ConfigurableObject implements InterpreterInterface * @param array $classWhitelist * @param ClassReader|null $classReader * @param ConfigInterface|null $objectManagerConfig + * @param array $deniedClassList */ public function __construct( ObjectManagerInterface $objectManager, InterpreterInterface $argumentInterpreter, array $classWhitelist = [], ClassReader $classReader = null, - ConfigInterface $objectManagerConfig = null + ConfigInterface $objectManagerConfig = null, + array $deniedClassList = [] ) { $this->objectManager = $objectManager; $this->argumentInterpreter = $argumentInterpreter; $this->classWhitelist = $classWhitelist; + $this->deniedClassList = $deniedClassList; $this->classReader = $classReader ?? $objectManager->get(ClassReader::class); $this->objectManagerConfig = $objectManagerConfig ?? $objectManager->get(ConfigInterface::class); } @@ -85,25 +93,12 @@ public function evaluate(array $data) if (!isset($arguments['class'])) { throw new \InvalidArgumentException('Node "argument" with name "class" is required for this type.'); } - $className = $arguments['class']; unset($arguments['class']); - - $type = $this->objectManagerConfig->getInstanceType( - $this->objectManagerConfig->getPreference($className) - ); - - $classParents = $this->getParents($type); - - $whitelistIntersection = array_intersect($classParents, $this->classWhitelist); - - if (empty($whitelistIntersection)) { - throw new \InvalidArgumentException( - sprintf('Class argument is invalid: %s', $className) - ); - } } + $this->isValid($className); + return $this->objectManager->create($className, $arguments); } @@ -115,7 +110,7 @@ public function evaluate(array $data) */ private function getParents(string $type) { - $classParents = $this->classReader->getParents($type); + $classParents = $this->classReader->getParents($type) ?? []; foreach ($classParents as $parent) { if (empty($parent)) { continue; @@ -125,4 +120,30 @@ private function getParents(string $type) return $classParents; } + + /** + * Check that provided class could be evaluated like an argument. + * + * @param string $className + * @throws \InvalidArgumentException + */ + private function isValid(string $className): void + { + $type = $this->objectManagerConfig->getInstanceType( + $this->objectManagerConfig->getPreference($className) + ); + + $classParents = $this->getParents($type); + + if (!empty($classParents)) { + $whitelistIntersection = array_intersect($classParents, $this->classWhitelist); + $deniedIntersection = array_intersect($classParents, $this->deniedClassList); + + if (empty($whitelistIntersection) || !empty($deniedIntersection)) { + throw new \InvalidArgumentException( + sprintf('Class argument is invalid: %s', $className) + ); + } + } + } } diff --git a/lib/internal/Magento/Framework/View/Test/Unit/UiComponent/Argument/Interpreter/ConfigurableObjectTest.php b/lib/internal/Magento/Framework/View/Test/Unit/UiComponent/Argument/Interpreter/ConfigurableObjectTest.php index 4f1e8be9021cd..e2cb559edae88 100644 --- a/lib/internal/Magento/Framework/View/Test/Unit/UiComponent/Argument/Interpreter/ConfigurableObjectTest.php +++ b/lib/internal/Magento/Framework/View/Test/Unit/UiComponent/Argument/Interpreter/ConfigurableObjectTest.php @@ -65,6 +65,10 @@ protected function setUp(): void ], 'classReader' => $this->classReader, 'objectManagerConfig' => $this->objectManagerConfig, + 'deniedClassList' => [ + \Foo\Bar\ClassC::class, + \Foo\Bar\InterfaceC::class, + ], ] ); } @@ -268,6 +272,27 @@ public function invalidDataProvider() \InvalidArgumentException::class, 'Class argument is invalid: MyFooClass' ], + [ + [ + 'argument' => [ + 'class' => ['value' => 'MyFooClass'], + 'myarg' => ['value' => 'bar'], + ], + ], + 'MyFooClass', + [ + ['MyFooClass', ['Something', 'skipme']], + ['Something', ['dontcare', 'SomethingElse']], + ['SomethingElse', [\Foo\Bar\ClassC::class, 'unrelated']], + ['skipme', []], + ['dontcare', []], + ['unrelated', [\Foo\Bar\InterfaceC::class]], + [\Foo\Bar\ClassC::class, []], + [\Foo\Bar\InterfaceC::class, []], + ], + \InvalidArgumentException::class, + 'Class argument is invalid: MyFooClass', + ], ]; } } From a0dccc09c8ef456c44107de732d95eecd8a1a945 Mon Sep 17 00:00:00 2001 From: Stas Kozar Date: Wed, 5 May 2021 20:40:04 +0300 Subject: [PATCH 2/3] MC-41013: Prevent errors from incorrect configurations --- .../Interpreter/ConfigurableObject.php | 54 +++++++++---------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/lib/internal/Magento/Framework/View/Element/UiComponent/Argument/Interpreter/ConfigurableObject.php b/lib/internal/Magento/Framework/View/Element/UiComponent/Argument/Interpreter/ConfigurableObject.php index de8bddeac30e9..c94159d5a76b8 100644 --- a/lib/internal/Magento/Framework/View/Element/UiComponent/Argument/Interpreter/ConfigurableObject.php +++ b/lib/internal/Magento/Framework/View/Element/UiComponent/Argument/Interpreter/ConfigurableObject.php @@ -93,11 +93,37 @@ public function evaluate(array $data) if (!isset($arguments['class'])) { throw new \InvalidArgumentException('Node "argument" with name "class" is required for this type.'); } + $className = $arguments['class']; unset($arguments['class']); + + $type = $this->objectManagerConfig->getInstanceType( + $this->objectManagerConfig->getPreference($className) + ); + + $classParents = $this->getParents($type); + + $whitelistIntersection = array_intersect($classParents, $this->classWhitelist); + + if (empty($whitelistIntersection)) { + throw new \InvalidArgumentException( + sprintf('Class argument is invalid: %s', $className) + ); + } } - $this->isValid($className); + $type = $this->objectManagerConfig->getInstanceType( + $this->objectManagerConfig->getPreference($className) + ); + $classParents = $this->getParents($type); + + $deniedIntersection = array_intersect($classParents, $this->deniedClassList); + + if (!empty($deniedIntersection)) { + throw new \InvalidArgumentException( + sprintf('Class argument is invalid: %s', $className) + ); + } return $this->objectManager->create($className, $arguments); } @@ -120,30 +146,4 @@ private function getParents(string $type) return $classParents; } - - /** - * Check that provided class could be evaluated like an argument. - * - * @param string $className - * @throws \InvalidArgumentException - */ - private function isValid(string $className): void - { - $type = $this->objectManagerConfig->getInstanceType( - $this->objectManagerConfig->getPreference($className) - ); - - $classParents = $this->getParents($type); - - if (!empty($classParents)) { - $whitelistIntersection = array_intersect($classParents, $this->classWhitelist); - $deniedIntersection = array_intersect($classParents, $this->deniedClassList); - - if (empty($whitelistIntersection) || !empty($deniedIntersection)) { - throw new \InvalidArgumentException( - sprintf('Class argument is invalid: %s', $className) - ); - } - } - } } From 98e17ef0c1905e580d2d635eb0e278eaaa76e520 Mon Sep 17 00:00:00 2001 From: Stas Kozar Date: Fri, 7 May 2021 19:03:53 +0300 Subject: [PATCH 3/3] MC-41013: Prevent errors from incorrect configurations --- .../Argument/Interpreter/ConfigurableObject.php | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/internal/Magento/Framework/View/Element/UiComponent/Argument/Interpreter/ConfigurableObject.php b/lib/internal/Magento/Framework/View/Element/UiComponent/Argument/Interpreter/ConfigurableObject.php index c94159d5a76b8..32950d7d0e4fd 100644 --- a/lib/internal/Magento/Framework/View/Element/UiComponent/Argument/Interpreter/ConfigurableObject.php +++ b/lib/internal/Magento/Framework/View/Element/UiComponent/Argument/Interpreter/ConfigurableObject.php @@ -80,6 +80,7 @@ public function __construct( */ public function evaluate(array $data) { + $type = null; if (isset($data['value'])) { $className = $data['value']; $arguments = []; @@ -112,10 +113,12 @@ public function evaluate(array $data) } } - $type = $this->objectManagerConfig->getInstanceType( - $this->objectManagerConfig->getPreference($className) - ); - $classParents = $this->getParents($type); + if ($type === null) { + $type = $this->objectManagerConfig->getInstanceType( + $this->objectManagerConfig->getPreference($className) + ); + $classParents = array_merge([$type], $this->getParents($type)); + } $deniedIntersection = array_intersect($classParents, $this->deniedClassList);