diff --git a/app/code/Magento/Store/Model/Config/Placeholder.php b/app/code/Magento/Store/Model/Config/Placeholder.php index 37c61ab418a6e..5f489fc070eb8 100644 --- a/app/code/Magento/Store/Model/Config/Placeholder.php +++ b/app/code/Magento/Store/Model/Config/Placeholder.php @@ -6,13 +6,16 @@ namespace Magento\Store\Model\Config; +use Magento\Framework\App\RequestInterface; +use Magento\Framework\Exception\LocalizedException; + /** * Placeholder configuration values processor. Replace placeholders in configuration with config values */ class Placeholder implements PlaceholderInterface { /** - * @var \Magento\Framework\App\RequestInterface + * @var RequestInterface */ protected $request; @@ -27,11 +30,11 @@ class Placeholder implements PlaceholderInterface protected $urlPlaceholder; /** - * @param \Magento\Framework\App\RequestInterface $request + * @param RequestInterface $request * @param string[] $urlPaths * @param string $urlPlaceholder */ - public function __construct(\Magento\Framework\App\RequestInterface $request, $urlPaths, $urlPlaceholder) + public function __construct(RequestInterface $request, $urlPaths, $urlPlaceholder) { $this->request = $request; $this->urlPaths = $urlPaths; @@ -43,6 +46,7 @@ public function __construct(\Magento\Framework\App\RequestInterface $request, $u * * @param array $data * @return array + * @throws LocalizedException * @SuppressWarnings(PHPMD.UnusedFormalParameter) */ public function process(array $data = []) @@ -71,6 +75,7 @@ function (&$value, $key, $data) { * @param array &$data * @param string $path * @return void + * @throws LocalizedException */ protected function _processData(&$data, $path) { @@ -90,30 +95,42 @@ protected function _processData(&$data, $path) * @param string $value * @param array $data * @return string + * @throws LocalizedException */ protected function _processPlaceholders($value, $data) { $placeholder = $this->_getPlaceholder($value); - if ($placeholder) { - $url = false; - if ($placeholder == 'unsecure_base_url') { - $url = $this->_getValue($this->urlPaths['unsecureBaseUrl'], $data); - } elseif ($placeholder == 'secure_base_url') { - $url = $this->_getValue($this->urlPaths['secureBaseUrl'], $data); - } + if (!$placeholder) { + return $value; + } - if ($url) { - $value = str_replace('{{' . $placeholder . '}}', $url, $value); - } elseif (strpos($value, (string)$this->urlPlaceholder) !== false) { - $distroBaseUrl = $this->request->getDistroBaseUrl(); + $url = null; + if ($placeholder === 'unsecure_base_url') { + $url = $this->_getValue($this->urlPaths['unsecureBaseUrl'], $data); + } elseif ($placeholder === 'secure_base_url') { + $url = $this->_getValue($this->urlPaths['secureBaseUrl'], $data); + } - $value = str_replace($this->urlPlaceholder, $distroBaseUrl, $value); - } + $originalValue = $value; - if (null !== $this->_getPlaceholder($value)) { - $value = $this->_processPlaceholders($value, $data); - } + if ($url) { + $value = str_replace('{{' . $placeholder . '}}', $url, $value); + } elseif (str_contains((string) $value, (string) $this->urlPlaceholder)) { + $value = str_replace($this->urlPlaceholder, $this->request->getDistroBaseUrl(), $value); + } else { + $configPath = $placeholder === 'secure_base_url' + ? $this->urlPaths['secureBaseUrl'] + : $this->urlPaths['unsecureBaseUrl']; + throw new LocalizedException( + __('Cannot resolve "{{%1}}" because "%2" is empty.', $placeholder, $configPath) + ); + } + + // Only recurse when the value changed; otherwise unresolved placeholders loop forever. + if ($value !== $originalValue && $this->_getPlaceholder($value) !== null) { + $value = $this->_processPlaceholders($value, $data); } + return $value; } @@ -125,11 +142,14 @@ protected function _processPlaceholders($value, $data) */ protected function _getPlaceholder($value) { - if (is_string($value) && preg_match('/{{(.*)}}.*/', $value, $matches)) { + if (!is_string($value) || $value === '') { + return null; + } + if (preg_match('/{{(.*)}}.*/', $value, $matches)) { $placeholder = $matches[1]; - if ($placeholder == 'unsecure_base_url' || - $placeholder == 'secure_base_url' || - strpos($value, (string)$this->urlPlaceholder) !== false + if ($placeholder === 'unsecure_base_url' || + $placeholder === 'secure_base_url' || + str_contains($value, (string) $this->urlPlaceholder) ) { return $placeholder; } @@ -142,7 +162,7 @@ protected function _getPlaceholder($value) * * @param string $path * @param array $data - * @return array|null + * @return array|string|null */ protected function _getValue($path, array $data) { diff --git a/app/code/Magento/Store/Test/Unit/Model/Config/PlaceholderTest.php b/app/code/Magento/Store/Test/Unit/Model/Config/PlaceholderTest.php index a157d89a77758..a62840e7c0547 100644 --- a/app/code/Magento/Store/Test/Unit/Model/Config/PlaceholderTest.php +++ b/app/code/Magento/Store/Test/Unit/Model/Config/PlaceholderTest.php @@ -8,15 +8,17 @@ namespace Magento\Store\Test\Unit\Model\Config; use Magento\Framework\App\Request\Http; -use Magento\Store\Model\Config\Processor\Placeholder as PlaceholderProcessor; +use Magento\Framework\Exception\LocalizedException; +use Magento\Store\Model\Config\Placeholder; use Magento\Store\Model\Store; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; class PlaceholderTest extends TestCase { /** - * @var PlaceholderProcessor + * @var Placeholder */ protected $_model; @@ -35,7 +37,7 @@ protected function setUp(): void )->willReturn( 'http://localhost/' ); - $this->_model = new \Magento\Store\Model\Config\Placeholder( + $this->_model = new Placeholder( $this->_requestMock, [ 'unsecureBaseUrl' => Store::XML_PATH_UNSECURE_BASE_URL, @@ -83,4 +85,93 @@ public function testProcessEmptyArray() $expectedResult = []; $this->assertEquals($expectedResult, $this->_model->process($data)); } + + /** + * @param mixed $secureBaseUrl + */ + #[DataProvider('emptySecureBaseUrlDataProvider')] + public function testProcessThrowsWhenSecureBaseUrlIsEmpty($secureBaseUrl): void + { + $this->expectException(LocalizedException::class); + $this->expectExceptionMessage( + 'Cannot resolve "{{secure_base_url}}" because "web/secure/base_url" is empty.' + ); + + $this->_model->process([ + 'web' => [ + 'unsecure' => [ + 'base_url' => 'http://localhost/', + ], + 'secure' => [ + 'base_url' => $secureBaseUrl, + 'base_link_url' => '{{secure_base_url}}website/de', + ], + ], + ]); + } + + /** + * @return array + */ + public static function emptySecureBaseUrlDataProvider(): array + { + return [ + 'null' => [null], + 'empty string' => [''], + ]; + } + + /** + * @param mixed $unsecureBaseUrl + */ + #[DataProvider('emptyUnsecureBaseUrlDataProvider')] + public function testProcessThrowsWhenUnsecureBaseUrlIsEmpty($unsecureBaseUrl): void + { + $this->expectException(LocalizedException::class); + $this->expectExceptionMessage( + 'Cannot resolve "{{unsecure_base_url}}" because "web/unsecure/base_url" is empty.' + ); + + $this->_model->process([ + 'web' => [ + 'unsecure' => [ + 'base_url' => $unsecureBaseUrl, + 'base_link_url' => '{{unsecure_base_url}}website/de', + ], + 'secure' => [ + 'base_url' => 'https://localhost/', + ], + ], + ]); + } + + /** + * @return array + */ + public static function emptyUnsecureBaseUrlDataProvider(): array + { + return [ + 'null' => [null], + 'empty string' => [''], + ]; + } + + public function testProcessThrowsWhenSecureBaseUrlPathIsMissing(): void + { + $this->expectException(LocalizedException::class); + $this->expectExceptionMessage( + 'Cannot resolve "{{secure_base_url}}" because "web/secure/base_url" is empty.' + ); + + $this->_model->process([ + 'web' => [ + 'unsecure' => [ + 'base_url' => 'http://localhost/', + ], + 'secure' => [ + 'base_link_url' => '{{secure_base_url}}website/de', + ], + ], + ]); + } } diff --git a/dev/tests/integration/testsuite/Magento/Store/Model/Config/PlaceholderTest.php b/dev/tests/integration/testsuite/Magento/Store/Model/Config/PlaceholderTest.php new file mode 100644 index 0000000000000..b67a0638a174a --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/Store/Model/Config/PlaceholderTest.php @@ -0,0 +1,197 @@ +placeholder = Bootstrap::getObjectManager()->create( + Placeholder::class, + [ + 'urlPaths' => [ + 'unsecureBaseUrl' => Store::XML_PATH_UNSECURE_BASE_URL, + 'secureBaseUrl' => Store::XML_PATH_SECURE_BASE_URL, + ], + 'urlPlaceholder' => Store::BASE_URL_PLACEHOLDER, + ] + ); + } + + /** + * Happy path: placeholders resolve using configured base URLs. + * + * @return void + */ + public function testProcessResolvesBaseUrlPlaceholders(): void + { + $data = [ + 'web' => [ + 'unsecure' => [ + 'base_url' => 'http://example.test/', + 'base_link_url' => '{{unsecure_base_url}}path/', + ], + 'secure' => [ + 'base_url' => 'https://example.test/', + 'base_link_url' => '{{secure_base_url}}path/', + ], + ], + ]; + + $result = $this->placeholder->process($data); + + $this->assertSame('http://example.test/path/', $result['web']['unsecure']['base_link_url']); + $this->assertSame('https://example.test/path/', $result['web']['secure']['base_link_url']); + } + + /** + * NULL secure base URL must not recurse infinitely; a clear exception is required. + * + * @return void + */ + public function testProcessThrowsWhenSecureBaseUrlIsNull(): void + { + $this->expectException(LocalizedException::class); + $this->expectExceptionMessage( + 'Cannot resolve "{{secure_base_url}}" because "web/secure/base_url" is empty.' + ); + + $this->placeholder->process([ + 'web' => [ + 'unsecure' => [ + 'base_url' => 'http://example.test/', + ], + 'secure' => [ + 'base_url' => null, + 'base_link_url' => '{{secure_base_url}}', + ], + ], + ]); + } + + /** + * Empty-string secure base URL must fail the same way as NULL. + * + * @return void + */ + public function testProcessThrowsWhenSecureBaseUrlIsEmptyString(): void + { + $this->expectException(LocalizedException::class); + $this->expectExceptionMessage( + 'Cannot resolve "{{secure_base_url}}" because "web/secure/base_url" is empty.' + ); + + $this->placeholder->process([ + 'web' => [ + 'unsecure' => [ + 'base_url' => 'http://example.test/', + ], + 'secure' => [ + 'base_url' => '', + 'base_link_url' => '{{secure_base_url}}checkout/', + ], + ], + ]); + } + + /** + * Missing secure base URL path must not recurse when a dependent placeholder is processed. + * + * @return void + */ + public function testProcessThrowsWhenSecureBaseUrlIsMissing(): void + { + $this->expectException(LocalizedException::class); + $this->expectExceptionMessage( + 'Cannot resolve "{{secure_base_url}}" because "web/secure/base_url" is empty.' + ); + + $this->placeholder->process([ + 'web' => [ + 'unsecure' => [ + 'base_url' => 'http://example.test/', + ], + 'secure' => [ + 'base_link_url' => '{{secure_base_url}}', + ], + ], + ]); + } + + /** + * NULL unsecure base URL must produce a localizable, path-specific error. + * + * @return void + */ + public function testProcessThrowsWhenUnsecureBaseUrlIsNull(): void + { + $this->expectException(LocalizedException::class); + $this->expectExceptionMessage( + 'Cannot resolve "{{unsecure_base_url}}" because "web/unsecure/base_url" is empty.' + ); + + $this->placeholder->process([ + 'web' => [ + 'unsecure' => [ + 'base_url' => null, + 'base_link_url' => '{{unsecure_base_url}}', + ], + 'secure' => [ + 'base_url' => 'https://example.test/', + ], + ], + ]); + } + + /** + * Scope-level processor used by config post-processing must surface the same failure. + * + * @return void + */ + public function testProcessorThrowsWhenSecureBaseUrlIsNull(): void + { + $this->expectException(LocalizedException::class); + $this->expectExceptionMessage( + 'Cannot resolve "{{secure_base_url}}" because "web/secure/base_url" is empty.' + ); + + /** @var Processor\Placeholder $processor */ + $processor = Bootstrap::getObjectManager()->get(Processor\Placeholder::class); + $processor->process([ + 'default' => [ + 'web' => [ + 'unsecure' => [ + 'base_url' => 'http://example.test/', + ], + 'secure' => [ + 'base_url' => null, + 'base_link_url' => '{{secure_base_url}}', + ], + ], + ], + ]); + } +}