From c6a8219e4e92f6d058250fe68a39b8920c2f1db0 Mon Sep 17 00:00:00 2001 From: MeenakshiSundaram Date: Mon, 15 Nov 2021 21:54:03 +0530 Subject: [PATCH 1/5] Fix 'magento' adding in the URL when Use Web Server Rewrites set to NO using Console Command --- app/code/Magento/Store/Model/Store.php | 9 ++-- .../Store/Test/Unit/Model/StoreTest.php | 41 +++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/app/code/Magento/Store/Model/Store.php b/app/code/Magento/Store/Model/Store.php index 05c762cce5164..c1da90ffec1c3 100644 --- a/app/code/Magento/Store/Model/Store.php +++ b/app/code/Magento/Store/Model/Store.php @@ -704,9 +704,12 @@ protected function _updatePathUseRewrites($url) if ($this->_isCustomEntryPoint()) { $indexFileName = 'index.php'; } else { - $scriptFilename = $this->_request->getServer('SCRIPT_FILENAME'); - // phpcs:ignore Magento2.Functions.DiscouragedFunction - $indexFileName = is_string($scriptFilename) ? basename($scriptFilename) : ''; + $indexFileName = ''; + if ($this->_request->getOriginalPathInfo()) { + $scriptFilename = $this->_request->getServer('SCRIPT_FILENAME'); + // phpcs:ignore Magento2.Functions.DiscouragedFunction + $indexFileName = basename($scriptFilename); + } } $url .= $indexFileName . '/'; } diff --git a/app/code/Magento/Store/Test/Unit/Model/StoreTest.php b/app/code/Magento/Store/Test/Unit/Model/StoreTest.php index b453b90b4c582..86d79d1422f18 100644 --- a/app/code/Magento/Store/Test/Unit/Model/StoreTest.php +++ b/app/code/Magento/Store/Test/Unit/Model/StoreTest.php @@ -98,6 +98,7 @@ protected function setUp(): void 'getDistroBaseUrl', 'isSecure', 'getServer', + 'getOriginalPathInfo' ]); $this->filesystemMock = $this->createMock(Filesystem::class); @@ -409,6 +410,9 @@ public function testGetBaseUrlEntryPoint() ->willReturnCallback(function ($path, $scope, $scopeCode) use ($expectedPath) { return $expectedPath == $path ? 'http://domain.com/' . $path . '/' : null; }); + $this->requestMock->expects($this->once()) + ->method('getOriginalPathInfo') + ->willReturn('web/unsecure/base_link_url/test_script.php/'); $this->requestMock->expects($this->once()) ->method('getServer') ->with('SCRIPT_FILENAME') @@ -433,6 +437,43 @@ public function testGetBaseUrlEntryPoint() ); } + /** + * @return void + */ + public function testGetBaseUrlFromCli() + { + $expectedPath = 'web/unsecure/base_link_url'; + $expectedBaseUrl = 'http://domain.com/web/unsecure/base_link_url/'; + /** @var \Magento\Framework\App\Config\ReinitableConfigInterface $configMock */ + $configMock = $this->getMockForAbstractClass(ReinitableConfigInterface::class); + $configMock->expects($this->atLeastOnce()) + ->method('getValue') + ->willReturnCallback(function ($path, $scope, $scopeCode) use ($expectedPath) { + return $expectedPath == $path ? 'http://domain.com/' . $path . '/' : null; + }); + $this->requestMock->expects($this->once()) + ->method('getOriginalPathInfo') + ->willReturn(''); + + /** @var Store $model */ + $model = $this->objectManagerHelper->getObject( + Store::class, + [ + 'config' => $configMock, + 'isCustomEntryPoint' => false, + 'request' => $this->requestMock + ] + ); + $model->setCode('scopeCode'); + + $this->setUrlModifier($model); + + $this->assertEquals( + $expectedBaseUrl, + $model->getBaseUrl(UrlInterface::URL_TYPE_LINK, false) + ); + } + public function testGetBaseUrlWrongType() { $this->expectException(\InvalidArgumentException::class); From 5d33f58432fa6e3858940bb325617a9da14921c5 Mon Sep 17 00:00:00 2001 From: engcom-Bravo Date: Fri, 31 Dec 2021 17:57:29 +0530 Subject: [PATCH 2/5] ISSUE-25976 Fixed Unit tests --- app/code/Magento/Store/Model/Store.php | 11 ++- .../Store/Test/Unit/Model/StoreTest.php | 67 ++++++++++++++++--- 2 files changed, 61 insertions(+), 17 deletions(-) diff --git a/app/code/Magento/Store/Model/Store.php b/app/code/Magento/Store/Model/Store.php index c1da90ffec1c3..5cbfb26c6191e 100644 --- a/app/code/Magento/Store/Model/Store.php +++ b/app/code/Magento/Store/Model/Store.php @@ -704,12 +704,11 @@ protected function _updatePathUseRewrites($url) if ($this->_isCustomEntryPoint()) { $indexFileName = 'index.php'; } else { - $indexFileName = ''; - if ($this->_request->getOriginalPathInfo()) { - $scriptFilename = $this->_request->getServer('SCRIPT_FILENAME'); - // phpcs:ignore Magento2.Functions.DiscouragedFunction - $indexFileName = basename($scriptFilename); - } + $scriptFilename = $this->_request->getOriginalPathInfo() ? + $this->_request->getServer('SCRIPT_FILENAME') : + '/server.php'; + // phpcs:ignore Magento2.Functions.DiscouragedFunction + $indexFileName = is_string($scriptFilename) ? basename($scriptFilename) : ''; } $url .= $indexFileName . '/'; } diff --git a/app/code/Magento/Store/Test/Unit/Model/StoreTest.php b/app/code/Magento/Store/Test/Unit/Model/StoreTest.php index 86d79d1422f18..d2e18de8012f1 100644 --- a/app/code/Magento/Store/Test/Unit/Model/StoreTest.php +++ b/app/code/Magento/Store/Test/Unit/Model/StoreTest.php @@ -412,7 +412,7 @@ public function testGetBaseUrlEntryPoint() }); $this->requestMock->expects($this->once()) ->method('getOriginalPathInfo') - ->willReturn('web/unsecure/base_link_url/test_script.php/'); + ->willReturn('test.html'); $this->requestMock->expects($this->once()) ->method('getServer') ->with('SCRIPT_FILENAME') @@ -438,20 +438,35 @@ public function testGetBaseUrlEntryPoint() } /** - * @return void + * @dataProvider getCliBaseUrlDataProvider + * + * @covers \Magento\Store\Model\Store::getBaseUrl + * @covers \Magento\Store\Model\Store::getCode + * @covers \Magento\Store\Model\Store::_updatePathUseRewrites + * @covers \Magento\Store\Model\Store::getConfig + * + * @param string $type + * @param boolean $secure + * @param boolean $isCustomEntryPoint + * @param string $expectedPath + * @param string $expectedBaseUrl */ - public function testGetBaseUrlFromCli() - { - $expectedPath = 'web/unsecure/base_link_url'; - $expectedBaseUrl = 'http://domain.com/web/unsecure/base_link_url/'; + public function testGetBaseUrlFromCli( + $type, + $secure, + $isCustomEntryPoint, + $expectedPath, + $expectedBaseUrl + ) { /** @var \Magento\Framework\App\Config\ReinitableConfigInterface $configMock */ $configMock = $this->getMockForAbstractClass(ReinitableConfigInterface::class); $configMock->expects($this->atLeastOnce()) ->method('getValue') - ->willReturnCallback(function ($path, $scope, $scopeCode) use ($expectedPath) { - return $expectedPath == $path ? 'http://domain.com/' . $path . '/' : null; + ->willReturnCallback(function ($path, $scope, $scopeCode) use ($secure, $expectedPath) { + $url = $secure ? '{{base_url}}' : 'http://domain.com/'; + return $expectedPath == $path ? $url . $path . '/' : null; }); - $this->requestMock->expects($this->once()) + $this->requestMock->expects($this->any()) ->method('getOriginalPathInfo') ->willReturn(''); @@ -460,7 +475,7 @@ public function testGetBaseUrlFromCli() Store::class, [ 'config' => $configMock, - 'isCustomEntryPoint' => false, + 'isCustomEntryPoint' => $isCustomEntryPoint, 'request' => $this->requestMock ] ); @@ -470,10 +485,40 @@ public function testGetBaseUrlFromCli() $this->assertEquals( $expectedBaseUrl, - $model->getBaseUrl(UrlInterface::URL_TYPE_LINK, false) + $model->getBaseUrl($type, $secure) ); } + /** + * @return array + */ + public function getCliBaseUrlDataProvider() + { + return [ + [ + UrlInterface::URL_TYPE_LINK, + false, + false, + 'web/unsecure/base_link_url', + 'http://domain.com/web/unsecure/base_link_url/server.php/' + ], + [ + UrlInterface::URL_TYPE_DIRECT_LINK, + false, + false, + 'web/unsecure/base_link_url', + 'http://domain.com/web/unsecure/base_link_url/server.php/' + ], + [ + UrlInterface::URL_TYPE_LINK, + true, + false, + 'web/secure/base_link_url', + 'web/secure/base_link_url/server.php/' + ], + ]; + } + public function testGetBaseUrlWrongType() { $this->expectException(\InvalidArgumentException::class); From b8872d65fcf371da7d94c9c2ac81a8df51818b89 Mon Sep 17 00:00:00 2001 From: engcom-Bravo Date: Fri, 31 Dec 2021 20:08:40 +0530 Subject: [PATCH 3/5] ISSUE-25976 Updated code as integration tests failing --- app/code/Magento/Store/Model/Store.php | 4 ++-- .../Magento/Store/Test/Unit/Model/StoreTest.php | 17 ++++++++--------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/app/code/Magento/Store/Model/Store.php b/app/code/Magento/Store/Model/Store.php index 5cbfb26c6191e..38836f50b298e 100644 --- a/app/code/Magento/Store/Model/Store.php +++ b/app/code/Magento/Store/Model/Store.php @@ -704,9 +704,9 @@ protected function _updatePathUseRewrites($url) if ($this->_isCustomEntryPoint()) { $indexFileName = 'index.php'; } else { - $scriptFilename = $this->_request->getOriginalPathInfo() ? + $scriptFilename = $this->_request->getServer('SCRIPT_FILENAME') !== 'bin/magento' ? $this->_request->getServer('SCRIPT_FILENAME') : - '/server.php'; + ''; // phpcs:ignore Magento2.Functions.DiscouragedFunction $indexFileName = is_string($scriptFilename) ? basename($scriptFilename) : ''; } diff --git a/app/code/Magento/Store/Test/Unit/Model/StoreTest.php b/app/code/Magento/Store/Test/Unit/Model/StoreTest.php index d2e18de8012f1..071e8ec7fbb19 100644 --- a/app/code/Magento/Store/Test/Unit/Model/StoreTest.php +++ b/app/code/Magento/Store/Test/Unit/Model/StoreTest.php @@ -410,10 +410,8 @@ public function testGetBaseUrlEntryPoint() ->willReturnCallback(function ($path, $scope, $scopeCode) use ($expectedPath) { return $expectedPath == $path ? 'http://domain.com/' . $path . '/' : null; }); - $this->requestMock->expects($this->once()) - ->method('getOriginalPathInfo') - ->willReturn('test.html'); - $this->requestMock->expects($this->once()) + + $this->requestMock->expects($this->exactly(2)) ->method('getServer') ->with('SCRIPT_FILENAME') ->willReturn('test_script.php'); @@ -467,8 +465,9 @@ public function testGetBaseUrlFromCli( return $expectedPath == $path ? $url . $path . '/' : null; }); $this->requestMock->expects($this->any()) - ->method('getOriginalPathInfo') - ->willReturn(''); + ->method('getServer') + ->with('SCRIPT_FILENAME') + ->willReturn('bin/magento'); /** @var Store $model */ $model = $this->objectManagerHelper->getObject( @@ -500,21 +499,21 @@ public function getCliBaseUrlDataProvider() false, false, 'web/unsecure/base_link_url', - 'http://domain.com/web/unsecure/base_link_url/server.php/' + 'http://domain.com/web/unsecure/base_link_url/' ], [ UrlInterface::URL_TYPE_DIRECT_LINK, false, false, 'web/unsecure/base_link_url', - 'http://domain.com/web/unsecure/base_link_url/server.php/' + 'http://domain.com/web/unsecure/base_link_url/' ], [ UrlInterface::URL_TYPE_LINK, true, false, 'web/secure/base_link_url', - 'web/secure/base_link_url/server.php/' + 'web/secure/base_link_url/' ], ]; } From 1aeeb5c1b4e1555a7f7e424ae0d5140e6645d2ab Mon Sep 17 00:00:00 2001 From: engcom-Bravo Date: Sun, 2 Jan 2022 10:54:29 +0530 Subject: [PATCH 4/5] ISSUE-25976 Fixed unit test failure --- app/code/Magento/Store/Test/Unit/Model/StoreTest.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/code/Magento/Store/Test/Unit/Model/StoreTest.php b/app/code/Magento/Store/Test/Unit/Model/StoreTest.php index 071e8ec7fbb19..b1a09a2d6fd8b 100644 --- a/app/code/Magento/Store/Test/Unit/Model/StoreTest.php +++ b/app/code/Magento/Store/Test/Unit/Model/StoreTest.php @@ -464,6 +464,10 @@ public function testGetBaseUrlFromCli( $url = $secure ? '{{base_url}}' : 'http://domain.com/'; return $expectedPath == $path ? $url . $path . '/' : null; }); + $this->requestMock->expects($this->any()) + ->method('getDistroBaseUrl') + ->willReturn('http://distro.com/'); + $this->requestMock->expects($this->any()) ->method('getServer') ->with('SCRIPT_FILENAME') @@ -513,7 +517,7 @@ public function getCliBaseUrlDataProvider() true, false, 'web/secure/base_link_url', - 'web/secure/base_link_url/' + 'http://distro.com/web/secure/base_link_url/' ], ]; } From 2f8e49858622aeec6c81d872810fe28eed16c883 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Bajsarowicz?= Date: Sun, 30 Aug 2026 13:20:17 +0200 Subject: [PATCH 5/5] Use the default front controller for URLs generated outside a web request (#25976) With "Use Web Server Rewrites" disabled, Store::_updatePathUseRewrites() appended basename(SCRIPT_FILENAME) to the base URL, which under CLI is the console binary - producing links such as .../magento/catalog/... . Fall back to index.php whenever the running script is not a .php entry point, which also covers an empty or missing SCRIPT_FILENAME. --- app/code/Magento/Store/Model/Store.php | 8 +- .../Store/Test/Unit/Model/StoreTest.php | 79 ++++++++++--------- 2 files changed, 45 insertions(+), 42 deletions(-) diff --git a/app/code/Magento/Store/Model/Store.php b/app/code/Magento/Store/Model/Store.php index 38836f50b298e..80d0812c4808b 100644 --- a/app/code/Magento/Store/Model/Store.php +++ b/app/code/Magento/Store/Model/Store.php @@ -704,11 +704,13 @@ protected function _updatePathUseRewrites($url) if ($this->_isCustomEntryPoint()) { $indexFileName = 'index.php'; } else { - $scriptFilename = $this->_request->getServer('SCRIPT_FILENAME') !== 'bin/magento' ? - $this->_request->getServer('SCRIPT_FILENAME') : - ''; + $scriptFilename = $this->_request->getServer('SCRIPT_FILENAME'); // phpcs:ignore Magento2.Functions.DiscouragedFunction $indexFileName = is_string($scriptFilename) ? basename($scriptFilename) : ''; + if (!str_ends_with(strtolower($indexFileName), '.php')) { + // The running script is not a web entry point (CLI, cron, unknown) - use the default one + $indexFileName = 'index.php'; + } } $url .= $indexFileName . '/'; } diff --git a/app/code/Magento/Store/Test/Unit/Model/StoreTest.php b/app/code/Magento/Store/Test/Unit/Model/StoreTest.php index b1a09a2d6fd8b..b8519324695e0 100644 --- a/app/code/Magento/Store/Test/Unit/Model/StoreTest.php +++ b/app/code/Magento/Store/Test/Unit/Model/StoreTest.php @@ -98,7 +98,6 @@ protected function setUp(): void 'getDistroBaseUrl', 'isSecure', 'getServer', - 'getOriginalPathInfo' ]); $this->filesystemMock = $this->createMock(Filesystem::class); @@ -410,8 +409,7 @@ public function testGetBaseUrlEntryPoint() ->willReturnCallback(function ($path, $scope, $scopeCode) use ($expectedPath) { return $expectedPath == $path ? 'http://domain.com/' . $path . '/' : null; }); - - $this->requestMock->expects($this->exactly(2)) + $this->requestMock->expects($this->once()) ->method('getServer') ->with('SCRIPT_FILENAME') ->willReturn('test_script.php'); @@ -436,42 +434,34 @@ public function testGetBaseUrlEntryPoint() } /** - * @dataProvider getCliBaseUrlDataProvider + * Entry point appended to the base URL when web server rewrites are disabled * * @covers \Magento\Store\Model\Store::getBaseUrl - * @covers \Magento\Store\Model\Store::getCode * @covers \Magento\Store\Model\Store::_updatePathUseRewrites - * @covers \Magento\Store\Model\Store::getConfig * - * @param string $type - * @param boolean $secure - * @param boolean $isCustomEntryPoint - * @param string $expectedPath + * @param string|null $scriptFilename + * @param bool $isCustomEntryPoint * @param string $expectedBaseUrl + * @return void */ - public function testGetBaseUrlFromCli( - $type, - $secure, + #[DataProvider('getBaseUrlEntryPointDataProvider')] + public function testGetBaseUrlEntryPointResolution( + $scriptFilename, $isCustomEntryPoint, - $expectedPath, $expectedBaseUrl ) { - /** @var \Magento\Framework\App\Config\ReinitableConfigInterface $configMock */ - $configMock = $this->getMockForAbstractClass(ReinitableConfigInterface::class); + $expectedPath = 'web/unsecure/base_link_url'; + /** @var ReinitableConfigInterface $configMock */ + $configMock = $this->createMock(ReinitableConfigInterface::class); $configMock->expects($this->atLeastOnce()) ->method('getValue') - ->willReturnCallback(function ($path, $scope, $scopeCode) use ($secure, $expectedPath) { - $url = $secure ? '{{base_url}}' : 'http://domain.com/'; - return $expectedPath == $path ? $url . $path . '/' : null; + ->willReturnCallback(function ($path, $scope, $scopeCode) use ($expectedPath) { + return $expectedPath == $path ? 'http://domain.com/' . $path . '/' : null; }); - $this->requestMock->expects($this->any()) - ->method('getDistroBaseUrl') - ->willReturn('http://distro.com/'); - $this->requestMock->expects($this->any()) ->method('getServer') ->with('SCRIPT_FILENAME') - ->willReturn('bin/magento'); + ->willReturn($scriptFilename); /** @var Store $model */ $model = $this->objectManagerHelper->getObject( @@ -488,36 +478,47 @@ public function testGetBaseUrlFromCli( $this->assertEquals( $expectedBaseUrl, - $model->getBaseUrl($type, $secure) + $model->getBaseUrl(UrlInterface::URL_TYPE_LINK, false) ); } /** * @return array */ - public function getCliBaseUrlDataProvider() + public static function getBaseUrlEntryPointDataProvider() { + $baseUrl = 'http://domain.com/web/unsecure/base_link_url/'; + return [ - [ - UrlInterface::URL_TYPE_LINK, + 'web request through the default entry point' => [ + '/var/www/html/pub/index.php', false, - false, - 'web/unsecure/base_link_url', - 'http://domain.com/web/unsecure/base_link_url/' + $baseUrl . 'index.php/', ], - [ - UrlInterface::URL_TYPE_DIRECT_LINK, + 'web request through a custom entry point script' => [ + '/var/www/html/pub/custom_entry.php', false, + $baseUrl . 'custom_entry.php/', + ], + 'console command executed via bin/magento' => [ + '/var/www/html/bin/magento', false, - 'web/unsecure/base_link_url', - 'http://domain.com/web/unsecure/base_link_url/' + $baseUrl . 'index.php/', ], - [ - UrlInterface::URL_TYPE_LINK, + 'entry point forced by the custom_entry_point parameter' => [ + '/var/www/html/bin/magento', true, + $baseUrl . 'index.php/', + ], + 'empty SCRIPT_FILENAME' => [ + '', + false, + $baseUrl . 'index.php/', + ], + 'missing SCRIPT_FILENAME' => [ + null, false, - 'web/secure/base_link_url', - 'http://distro.com/web/secure/base_link_url/' + $baseUrl . 'index.php/', ], ]; }