Skip to content

Commit 0fd6fd3

Browse files
BartlomiejszBartlomiejsz
committed
Fix #20309 - fix url rewrite check with query params, add integration test cases
1 parent 684b951 commit 0fd6fd3

File tree

5 files changed

+72
-6
lines changed

5 files changed

+72
-6
lines changed

‎app/code/Magento/UrlRewrite/Controller/Router.php

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,26 @@ public function match(RequestInterface $request)
9090
$this->storeManager->getStore()->getId()
9191
);
9292

93-
if ($rewrite === null || $rewrite->getRequestPath() === $rewrite->getTargetPath()) {
94-
// Either no rewrite rule matching current URl found or found one with request path equal to
95-
// target path, continuing with processing of this URL.
93+
if ($rewrite === null) {
94+
// No rewrite rule matching current URl found, continuing with
95+
// processing of this URL.
96+
return null;
97+
}
98+
99+
$requestStringTrimmed = ltrim($request->getRequestString(), '/');
100+
$rewriteRequestPath = $rewrite->getRequestPath();
101+
$rewriteTargetPath = $rewrite->getTargetPath();
102+
$rewriteTargetPathTrimmed = ltrim($rewriteTargetPath, '/');
103+
104+
if (preg_replace('/\?.*/', '', $rewriteRequestPath) === preg_replace('/\?.*/', '', $rewriteTargetPath) &&
105+
(
106+
!$requestStringTrimmed ||
107+
!$rewriteTargetPathTrimmed ||
108+
strpos($requestStringTrimmed, $rewriteTargetPathTrimmed) === 0
109+
)
110+
) {
111+
// Request and target paths of rewrite found without query params are equal and current request string
112+
// starts with request target path, continuing with processing of this URL.
96113
return null;
97114
}
98115

@@ -104,9 +121,9 @@ public function match(RequestInterface $request)
104121
// Rule provides actual URL that can be processed by a controller.
105122
$request->setAlias(
106123
UrlInterface::REWRITE_REQUEST_PATH_ALIAS,
107-
$rewrite->getRequestPath()
124+
$rewriteRequestPath
108125
);
109-
$request->setPathInfo('/' . $rewrite->getTargetPath());
126+
$request->setPathInfo('/' . $rewriteTargetPath);
110127
return $this->actionFactory->create(
111128
Forward::class
112129
);

‎app/code/Magento/UrlRewrite/Test/Unit/Controller/RouterTest.php

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,8 @@ public function testNoRewriteExist()
114114
{
115115
$this->request->method('getPathInfo')
116116
->willReturn('');
117+
$this->request->method('getRequestString')
118+
->willReturn('');
117119
$this->urlFinder->method('findOneByData')
118120
->willReturn(null);
119121
$this->storeManager->method('getStore')
@@ -142,6 +144,8 @@ public function testRewriteAfterStoreSwitcher()
142144
->willReturn($oldStoreAlias);
143145
$this->request->method('getPathInfo')
144146
->willReturn($initialRequestPath);
147+
$this->request->method('getRequestString')
148+
->willReturn($initialRequestPath);
145149
$oldStore = $this->getMockBuilder(Store::class)
146150
->disableOriginalConstructor()
147151
->getMock();
@@ -192,6 +196,7 @@ public function testRewriteAfterStoreSwitcher()
192196
public function testNoRewriteAfterStoreSwitcherWhenNoOldRewrite()
193197
{
194198
$this->request->method('getPathInfo')->willReturn('request-path');
199+
$this->request->method('getRequestString')->willReturn('request-path');
195200
$this->request->method('getParam')->with('___from_store')
196201
->willReturn('old-store');
197202
$oldStore = $this->getMockBuilder(Store::class)->disableOriginalConstructor()->getMock();
@@ -217,6 +222,7 @@ public function testNoRewriteAfterStoreSwitcherWhenNoOldRewrite()
217222
public function testNoRewriteAfterStoreSwitcherWhenOldRewriteEqualsToNewOne()
218223
{
219224
$this->request->method('getPathInfo')->willReturn('request-path');
225+
$this->request->method('getRequestString')->willReturn('request-path');
220226
$this->request->method('getParam')->with('___from_store')
221227
->willReturn('old-store');
222228
$oldStore = $this->getMockBuilder(Store::class)->disableOriginalConstructor()->getMock();
@@ -268,6 +274,8 @@ public function testMatchWithRedirect()
268274
->willReturn($this->store);
269275
$this->request->method('getPathInfo')
270276
->willReturn($requestPath);
277+
$this->request->method('getRequestString')
278+
->willReturn($requestPath);
271279
$urlRewrite = $this->getMockBuilder(UrlRewrite::class)
272280
->disableOriginalConstructor()
273281
->getMock();
@@ -312,6 +320,8 @@ public function testMatchWithCustomInternalRedirect($requestPath, $targetPath, $
312320
->willReturn($this->store);
313321
$this->request->method('getPathInfo')
314322
->willReturn($requestPath);
323+
$this->request->method('getRequestString')
324+
->willReturn($requestPath);
315325
$urlRewrite = $this->getMockBuilder(UrlRewrite::class)
316326
->disableOriginalConstructor()
317327
->getMock();
@@ -369,6 +379,8 @@ public function testMatchWithCustomExternalRedirect($targetPath)
369379
$this->storeManager->method('getStore')->willReturn($this->store);
370380
$this->request->method('getPathInfo')
371381
->willReturn($requestPath);
382+
$this->request->method('getRequestString')
383+
->willReturn($requestPath);
372384
$urlRewrite = $this->getMockBuilder(UrlRewrite::class)
373385
->disableOriginalConstructor()->getMock();
374386
$urlRewrite->method('getEntityType')->willReturn('custom');
@@ -408,6 +420,8 @@ public function testMatch()
408420
$this->storeManager->method('getStore')->willReturn($this->store);
409421
$this->request->method('getPathInfo')
410422
->willReturn($requestPath);
423+
$this->request->method('getRequestString')
424+
->willReturn($requestPath);
411425
$urlRewrite = $this->getMockBuilder(UrlRewrite::class)
412426
->disableOriginalConstructor()->getMock();
413427
$urlRewrite->method('getRedirectType')->willReturn(0);

‎dev/tests/integration/testsuite/Magento/UrlRewrite/Controller/UrlRewriteTest.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,21 @@ public function requestDataProvider(): array
133133
'request' => '/page-external4?param1=custom1&param2=custom2',
134134
'redirect' => 'https://example.com/external2/?param2=value2',
135135
],
136+
'Use Case #17: Rewrite: / --(301)--> /; No redirect' => [
137+
'request' => '/',
138+
'redirect' => '/',
139+
'expectedCode' => HttpResponse::STATUS_CODE_200,
140+
],
141+
'Use Case #18: Rewrite: contact/ --(301)--> contact?param1=1; '
142+
. 'Request: contact/ --(301)--> contact?param1=1' => [
143+
'request' => 'contact/',
144+
'redirect' => 'contact?param1=1',
145+
],
146+
'Use Case #19: Rewrite: contact/?param2=2 --(301)--> contact?param1=1&param2=2; '
147+
. 'Request: contact/?&param2=2 --(301)--> contact?param1=1&param2=2' => [
148+
'request' => 'contact/?&param2=2',
149+
'redirect' => 'contact?param1=1&param2=2',
150+
],
136151
];
137152
}
138153
}

‎dev/tests/integration/testsuite/Magento/UrlRewrite/_files/url_rewrite.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,24 @@
156156
->setDescription('From page-similar-query-param with trailing slash to page-e with query param');
157157
$rewriteResource->save($rewrite);
158158

159+
$rewrite = $objectManager->create(UrlRewrite::class);
160+
$rewrite->setEntityType('custom')
161+
->setRequestPath('/')
162+
->setTargetPath('/')
163+
->setRedirectType(OptionProvider::PERMANENT)
164+
->setStoreId($storeID)
165+
->setDescription('From / to /');
166+
$rewriteResource->save($rewrite);
167+
168+
$rewrite = $objectManager->create(UrlRewrite::class);
169+
$rewrite->setEntityType('custom')
170+
->setRequestPath('contact/')
171+
->setTargetPath('contact?param1=1')
172+
->setRedirectType(OptionProvider::PERMANENT)
173+
->setStoreId($storeID)
174+
->setDescription('From contact with trailing slash to contact with query param');
175+
$rewriteResource->save($rewrite);
176+
159177
$rewrite = $objectManager->create(UrlRewrite::class);
160178
$rewrite->setEntityType('custom')
161179
->setRequestPath('page-external1')

‎dev/tests/integration/testsuite/Magento/UrlRewrite/_files/url_rewrite_rollback.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,9 @@
4343
'http://example.com/external',
4444
'https://example.com/external2/',
4545
'http://example.com/external?param1=value1',
46-
'https://example.com/external2/?param2=value2'
46+
'https://example.com/external2/?param2=value2',
47+
'/',
48+
'contact?param1=1'
4749
]
4850
)
4951
->load()

0 commit comments

Comments
 (0)