Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit05071a4

Browse files
bug#30013 [Routing] dont redirect routes with greedy trailing vars with no explicit slash (nicolas-grekas)
This PR was merged into the 4.1 branch.Discussion----------[Routing] dont redirect routes with greedy trailing vars with no explicit slash| Q | A| ------------- | ---| Branch? | 4.1| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#29673#29734#29575| License | MIT| Doc PR | -From the linked issue:> The current logic is the following:> - when a route is declared with a trailing slash, the trimmed-slash url is redirected to the one with the slash> - when a route is declared with *no* trailing slash, the slashed url is redirected to the trimmed-slash one>> That includes routes with slash-greedy requirements: when the same greedy requirement matches both the slashed and the trimmed-slash URLs, only one of them is considered the canonical one and a redirection happens.>> We could fine tune this logic and make an exception when a trailing slash-greedy requirement is declared with no explicit trailing slash after it. (ie disable any redirections for `/foo/{.*}` but keep it for `/foo/{.*}/`. That would mean `/foo/bar` and `/foo/bar/` wouldn't trigger the redirection for route `/foo/{.*}`, breaking the "not-semantics" property of trailing slashes for catch-all routes. Which might be legit afterall.This PR implements this fine tuning, as that's the most BC behavior (and thus the correct one).See#30012 for `testGreedyTrailingRequirement` in action on 3.4 as a proof.Commits-------2bb8890 [Routing] dont redirect routes with greedy trailing vars with no explicit slash
2 parents78c23c7 +2bb8890 commit05071a4

File tree

13 files changed

+80
-213
lines changed

13 files changed

+80
-213
lines changed

‎src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php‎

Lines changed: 8 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -558,34 +558,27 @@ private function compileSwitchDefault(bool $hasVars, bool $matchHost): string
558558
$code ='';
559559
}
560560

561-
$code .=$hasVars ?'
562-
if ($trimmedPathinfo === $pathinfo || !$hasTrailingVar) {
563-
break;
564-
}' :'
565-
break;';
566-
567561
$code =sprintf(<<<'EOF'
568-
if ('/' !== $pathinfo && $hasTrailingSlash === ($trimmedPathinfo === $pathinfo)) {%s
562+
if ('/' !== $pathinfo && %s$hasTrailingSlash === ($trimmedPathinfo === $pathinfo)) {%s
563+
break;
569564
}
570565

571566
EOF
572567
,
568+
$hasVars ?'!$hasTrailingVar &&' :'',
573569
$code
574570
);
575571

576572
if ($hasVars) {
577573
$code = <<<'EOF'
578574
579-
if ($trimmedPathinfo === $pathinfo || !$hasTrailingVar) {
580-
// no-op
581-
} elseif (preg_match($regex, rtrim($matchedPathinfo, '/') ?: '/', $n) && $m === (int) $n['MARK']) {
582-
$matches = $n;
583-
} else {
584-
$hasTrailingSlash = true;
585-
}
575+
$hasTrailingVar = $trimmedPathinfo !== $pathinfo && $hasTrailingVar;
586576

587577
EOF
588578
.$code.<<<'EOF'
579+
if ($hasTrailingSlash && $hasTrailingVar && preg_match($regex, rtrim($matchedPathinfo, '/') ?: '/', $n) && $m === (int) $n['MARK']) {
580+
$matches = $n;
581+
}
589582
590583
foreach ($vars as $i => $v) {
591584
if (isset($matches[1 + $i])) {
@@ -665,31 +658,10 @@ private function compileRoute(Route $route, string $name, bool $checkHost, bool
665658
if ('/' !== $pathinfo && preg_match($regex, rtrim($matchedPathinfo, '/') ?: '/', $n) && $m === (int) $n['MARK']) {
666659
$matches = $n;
667660
}
668-
EOF;
669-
}elseif ($this->supportsRedirections && (!$methods ||isset($methods['GET']))) {
670-
$code .= <<<'EOF'
671-
$hasTrailingSlash = false;
672-
if ($trimmedPathinfo === $pathinfo) {
673-
// no-op
674-
} elseif (preg_match($regex, rtrim($matchedPathinfo, '/') ?: '/', $n) && $m === (int) $n['MARK']) {
675-
$matches = $n;
676-
} else {
677-
$hasTrailingSlash = true;
678-
}
679-
680-
if ('/' !== $pathinfo && $hasTrailingSlash === ($trimmedPathinfo === $pathinfo)) {%s
681-
if ($trimmedPathinfo === $pathinfo) {
682-
goto %s;
683-
}
684-
}
685661
EOF;
686662
}else {
687663
$code .= <<<'EOF'
688-
if ($trimmedPathinfo === $pathinfo) {
689-
// no-op
690-
} elseif (preg_match($regex, rtrim($matchedPathinfo, '/') ?: '/', $n) && $m === (int) $n['MARK']) {
691-
$matches = $n;
692-
} elseif ('/' !== $pathinfo) {
664+
if ($trimmedPathinfo !== $pathinfo) {
693665
goto %2$s;
694666
}
695667
EOF;

‎src/Symfony/Component/Routing/Matcher/UrlMatcher.php‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -156,21 +156,18 @@ protected function matchCollection($pathinfo, RouteCollection $routes)
156156
continue;
157157
}
158158

159-
if ($trimmedPathinfo ===$pathinfo || !$hasTrailingVar =preg_match('#\{\w+\}/?$#',$route->getPath())) {
160-
// no-op
161-
}elseif (preg_match($regex,$trimmedPathinfo,$m)) {
162-
$matches =$m;
163-
}else {
164-
$hasTrailingSlash =true;
165-
}
159+
$hasTrailingVar =$trimmedPathinfo !==$pathinfo &&preg_match('#\{\w+\}/?$#',$route->getPath());
166160

167-
if ('/' !==$pathinfo &&$hasTrailingSlash === ($trimmedPathinfo ===$pathinfo)) {
161+
if ('/' !==$pathinfo &&!$hasTrailingVar &&$hasTrailingSlash === ($trimmedPathinfo ===$pathinfo)) {
168162
if ($supportsTrailingSlash && (!$requiredMethods ||\in_array('GET',$requiredMethods))) {
169163
return$this->allow =$this->allowSchemes = [];
170164
}
171-
if ($trimmedPathinfo ===$pathinfo || !$hasTrailingVar) {
172-
continue;
173-
}
165+
166+
continue;
167+
}
168+
169+
if ($hasTrailingSlash &&$hasTrailingVar &&preg_match($regex,$trimmedPathinfo,$m)) {
170+
$matches =$m;
174171
}
175172

176173
$hostMatches = [];

‎src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher1.php‎

Lines changed: 8 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -187,11 +187,7 @@ public function match($pathinfo)
187187
break;
188188
case160:
189189
// foo1
190-
if ($trimmedPathinfo ===$pathinfo) {
191-
// no-op
192-
}elseif (preg_match($regex,rtrim($matchedPathinfo,'/') ?:'/',$n) &&$m === (int)$n['MARK']) {
193-
$matches =$n;
194-
}elseif ('/' !==$pathinfo) {
190+
if ($trimmedPathinfo !==$pathinfo) {
195191
goto not_foo1;
196192
}
197193

@@ -209,11 +205,7 @@ public function match($pathinfo)
209205
break;
210206
case204:
211207
// foo2
212-
if ($trimmedPathinfo ===$pathinfo) {
213-
// no-op
214-
}elseif (preg_match($regex,rtrim($matchedPathinfo,'/') ?:'/',$n) &&$m === (int)$n['MARK']) {
215-
$matches =$n;
216-
}elseif ('/' !==$pathinfo) {
208+
if ($trimmedPathinfo !==$pathinfo) {
217209
goto not_foo2;
218210
}
219211

@@ -225,11 +217,7 @@ public function match($pathinfo)
225217
break;
226218
case279:
227219
// foo3
228-
if ($trimmedPathinfo ===$pathinfo) {
229-
// no-op
230-
}elseif (preg_match($regex,rtrim($matchedPathinfo,'/') ?:'/',$n) &&$m === (int)$n['MARK']) {
231-
$matches =$n;
232-
}elseif ('/' !==$pathinfo) {
220+
if ($trimmedPathinfo !==$pathinfo) {
233221
goto not_foo3;
234222
}
235223

@@ -262,17 +250,12 @@ public function match($pathinfo)
262250

263251
list($ret,$vars,$requiredMethods,$requiredSchemes,$hasTrailingSlash,$hasTrailingVar) =$routes[$m];
264252

265-
if ($trimmedPathinfo ===$pathinfo || !$hasTrailingVar) {
266-
// no-op
267-
}elseif (preg_match($regex,rtrim($matchedPathinfo,'/') ?:'/',$n) &&$m === (int)$n['MARK']) {
268-
$matches =$n;
269-
}else {
270-
$hasTrailingSlash =true;
253+
$hasTrailingVar =$trimmedPathinfo !==$pathinfo &&$hasTrailingVar;
254+
if ('/' !==$pathinfo && !$hasTrailingVar &&$hasTrailingSlash === ($trimmedPathinfo ===$pathinfo)) {
255+
break;
271256
}
272-
if ('/' !==$pathinfo &&$hasTrailingSlash === ($trimmedPathinfo ===$pathinfo)) {
273-
if ($trimmedPathinfo ===$pathinfo || !$hasTrailingVar) {
274-
break;
275-
}
257+
if ($hasTrailingSlash &&$hasTrailingVar &&preg_match($regex,rtrim($matchedPathinfo,'/') ?:'/',$n) &&$m === (int)$n['MARK']) {
258+
$matches =$n;
276259
}
277260

278261
foreach ($varsas$i =>$v) {

‎src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher10.php‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2794,17 +2794,12 @@ public function match($pathinfo)
27942794

27952795
list($ret,$vars,$requiredMethods,$requiredSchemes,$hasTrailingSlash,$hasTrailingVar) =$routes[$m];
27962796

2797-
if ($trimmedPathinfo ===$pathinfo || !$hasTrailingVar) {
2798-
// no-op
2799-
}elseif (preg_match($regex,rtrim($matchedPathinfo,'/') ?:'/',$n) &&$m === (int)$n['MARK']) {
2800-
$matches =$n;
2801-
}else {
2802-
$hasTrailingSlash =true;
2797+
$hasTrailingVar =$trimmedPathinfo !==$pathinfo &&$hasTrailingVar;
2798+
if ('/' !==$pathinfo && !$hasTrailingVar &&$hasTrailingSlash === ($trimmedPathinfo ===$pathinfo)) {
2799+
break;
28032800
}
2804-
if ('/' !==$pathinfo &&$hasTrailingSlash === ($trimmedPathinfo ===$pathinfo)) {
2805-
if ($trimmedPathinfo ===$pathinfo || !$hasTrailingVar) {
2806-
break;
2807-
}
2801+
if ($hasTrailingSlash &&$hasTrailingVar &&preg_match($regex,rtrim($matchedPathinfo,'/') ?:'/',$n) &&$m === (int)$n['MARK']) {
2802+
$matches =$n;
28082803
}
28092804

28102805
foreach ($varsas$i =>$v) {

‎src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher11.php‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -119,20 +119,15 @@ private function doMatch(string $pathinfo, array &$allow = [], array &$allowSche
119119

120120
list($ret,$vars,$requiredMethods,$requiredSchemes,$hasTrailingSlash,$hasTrailingVar) =$routes[$m];
121121

122-
if ($trimmedPathinfo ===$pathinfo || !$hasTrailingVar) {
123-
// no-op
124-
}elseif (preg_match($regex,rtrim($matchedPathinfo,'/') ?:'/',$n) &&$m === (int)$n['MARK']) {
125-
$matches =$n;
126-
}else {
127-
$hasTrailingSlash =true;
128-
}
129-
if ('/' !==$pathinfo &&$hasTrailingSlash === ($trimmedPathinfo ===$pathinfo)) {
122+
$hasTrailingVar =$trimmedPathinfo !==$pathinfo &&$hasTrailingVar;
123+
if ('/' !==$pathinfo && !$hasTrailingVar &&$hasTrailingSlash === ($trimmedPathinfo ===$pathinfo)) {
130124
if ('GET' ===$canonicalMethod && (!$requiredMethods ||isset($requiredMethods['GET']))) {
131125
return$allow =$allowSchemes = [];
132126
}
133-
if ($trimmedPathinfo ===$pathinfo || !$hasTrailingVar) {
134-
break;
135-
}
127+
break;
128+
}
129+
if ($hasTrailingSlash &&$hasTrailingVar &&preg_match($regex,rtrim($matchedPathinfo,'/') ?:'/',$n) &&$m === (int)$n['MARK']) {
130+
$matches =$n;
136131
}
137132

138133
foreach ($varsas$i =>$v) {

‎src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher12.php‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -64,17 +64,12 @@ public function match($pathinfo)
6464

6565
list($ret,$vars,$requiredMethods,$requiredSchemes,$hasTrailingSlash,$hasTrailingVar) =$routes[$m];
6666

67-
if ($trimmedPathinfo ===$pathinfo || !$hasTrailingVar) {
68-
// no-op
69-
}elseif (preg_match($regex,rtrim($matchedPathinfo,'/') ?:'/',$n) &&$m === (int)$n['MARK']) {
70-
$matches =$n;
71-
}else {
72-
$hasTrailingSlash =true;
67+
$hasTrailingVar =$trimmedPathinfo !==$pathinfo &&$hasTrailingVar;
68+
if ('/' !==$pathinfo && !$hasTrailingVar &&$hasTrailingSlash === ($trimmedPathinfo ===$pathinfo)) {
69+
break;
7370
}
74-
if ('/' !==$pathinfo &&$hasTrailingSlash === ($trimmedPathinfo ===$pathinfo)) {
75-
if ($trimmedPathinfo ===$pathinfo || !$hasTrailingVar) {
76-
break;
77-
}
71+
if ($hasTrailingSlash &&$hasTrailingVar &&preg_match($regex,rtrim($matchedPathinfo,'/') ?:'/',$n) &&$m === (int)$n['MARK']) {
72+
$matches =$n;
7873
}
7974

8075
foreach ($varsas$i =>$v) {

‎src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher13.php‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,7 @@ public function match($pathinfo)
4444
switch ($m = (int)$matches['MARK']) {
4545
case56:
4646
// r1
47-
if ($trimmedPathinfo ===$pathinfo) {
48-
// no-op
49-
}elseif (preg_match($regex,rtrim($matchedPathinfo,'/') ?:'/',$n) &&$m === (int)$n['MARK']) {
50-
$matches =$n;
51-
}elseif ('/' !==$pathinfo) {
47+
if ($trimmedPathinfo !==$pathinfo) {
5248
goto not_r1;
5349
}
5450

@@ -58,11 +54,7 @@ public function match($pathinfo)
5854
not_r1:
5955

6056
// r2
61-
if ($trimmedPathinfo ===$pathinfo) {
62-
// no-op
63-
}elseif (preg_match($regex,rtrim($matchedPathinfo,'/') ?:'/',$n) &&$m === (int)$n['MARK']) {
64-
$matches =$n;
65-
}elseif ('/' !==$pathinfo) {
57+
if ($trimmedPathinfo !==$pathinfo) {
6658
goto not_r2;
6759
}
6860

‎src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher2.php‎

Lines changed: 11 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -230,11 +230,7 @@ private function doMatch(string $pathinfo, array &$allow = [], array &$allowSche
230230
break;
231231
case160:
232232
// foo1
233-
if ($trimmedPathinfo ===$pathinfo) {
234-
// no-op
235-
}elseif (preg_match($regex,rtrim($matchedPathinfo,'/') ?:'/',$n) &&$m === (int)$n['MARK']) {
236-
$matches =$n;
237-
}elseif ('/' !==$pathinfo) {
233+
if ($trimmedPathinfo !==$pathinfo) {
238234
goto not_foo1;
239235
}
240236

@@ -252,22 +248,8 @@ private function doMatch(string $pathinfo, array &$allow = [], array &$allowSche
252248
break;
253249
case204:
254250
// foo2
255-
$hasTrailingSlash =false;
256-
if ($trimmedPathinfo ===$pathinfo) {
257-
// no-op
258-
}elseif (preg_match($regex,rtrim($matchedPathinfo,'/') ?:'/',$n) &&$m === (int)$n['MARK']) {
259-
$matches =$n;
260-
}else {
261-
$hasTrailingSlash =true;
262-
}
263-
264-
if ('/' !==$pathinfo &&$hasTrailingSlash === ($trimmedPathinfo ===$pathinfo)) {
265-
if ('GET' ===$canonicalMethod) {
266-
return$allow =$allowSchemes = [];
267-
}
268-
if ($trimmedPathinfo ===$pathinfo) {
269-
goto not_foo2;
270-
}
251+
if ($trimmedPathinfo !==$pathinfo) {
252+
goto not_foo2;
271253
}
272254

273255
$matches = ['foo1' =>$matches[1] ??null];
@@ -278,22 +260,8 @@ private function doMatch(string $pathinfo, array &$allow = [], array &$allowSche
278260
break;
279261
case279:
280262
// foo3
281-
$hasTrailingSlash =false;
282-
if ($trimmedPathinfo ===$pathinfo) {
283-
// no-op
284-
}elseif (preg_match($regex,rtrim($matchedPathinfo,'/') ?:'/',$n) &&$m === (int)$n['MARK']) {
285-
$matches =$n;
286-
}else {
287-
$hasTrailingSlash =true;
288-
}
289-
290-
if ('/' !==$pathinfo &&$hasTrailingSlash === ($trimmedPathinfo ===$pathinfo)) {
291-
if ('GET' ===$canonicalMethod) {
292-
return$allow =$allowSchemes = [];
293-
}
294-
if ($trimmedPathinfo ===$pathinfo) {
295-
goto not_foo3;
296-
}
263+
if ($trimmedPathinfo !==$pathinfo) {
264+
goto not_foo3;
297265
}
298266

299267
$matches = ['_locale' =>$matches[1] ??null,'foo' =>$matches[2] ??null];
@@ -325,20 +293,15 @@ private function doMatch(string $pathinfo, array &$allow = [], array &$allowSche
325293

326294
list($ret,$vars,$requiredMethods,$requiredSchemes,$hasTrailingSlash,$hasTrailingVar) =$routes[$m];
327295

328-
if ($trimmedPathinfo ===$pathinfo || !$hasTrailingVar) {
329-
// no-op
330-
}elseif (preg_match($regex,rtrim($matchedPathinfo,'/') ?:'/',$n) &&$m === (int)$n['MARK']) {
331-
$matches =$n;
332-
}else {
333-
$hasTrailingSlash =true;
334-
}
335-
if ('/' !==$pathinfo &&$hasTrailingSlash === ($trimmedPathinfo ===$pathinfo)) {
296+
$hasTrailingVar =$trimmedPathinfo !==$pathinfo &&$hasTrailingVar;
297+
if ('/' !==$pathinfo && !$hasTrailingVar &&$hasTrailingSlash === ($trimmedPathinfo ===$pathinfo)) {
336298
if ('GET' ===$canonicalMethod && (!$requiredMethods ||isset($requiredMethods['GET']))) {
337299
return$allow =$allowSchemes = [];
338300
}
339-
if ($trimmedPathinfo ===$pathinfo || !$hasTrailingVar) {
340-
break;
341-
}
301+
break;
302+
}
303+
if ($hasTrailingSlash &&$hasTrailingVar &&preg_match($regex,rtrim($matchedPathinfo,'/') ?:'/',$n) &&$m === (int)$n['MARK']) {
304+
$matches =$n;
342305
}
343306

344307
foreach ($varsas$i =>$v) {

‎src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher3.php‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -84,17 +84,12 @@ public function match($pathinfo)
8484

8585
list($ret,$vars,$requiredMethods,$requiredSchemes,$hasTrailingSlash,$hasTrailingVar) =$routes[$m];
8686

87-
if ($trimmedPathinfo ===$pathinfo || !$hasTrailingVar) {
88-
// no-op
89-
}elseif (preg_match($regex,rtrim($matchedPathinfo,'/') ?:'/',$n) &&$m === (int)$n['MARK']) {
90-
$matches =$n;
91-
}else {
92-
$hasTrailingSlash =true;
87+
$hasTrailingVar =$trimmedPathinfo !==$pathinfo &&$hasTrailingVar;
88+
if ('/' !==$pathinfo && !$hasTrailingVar &&$hasTrailingSlash === ($trimmedPathinfo ===$pathinfo)) {
89+
break;
9390
}
94-
if ('/' !==$pathinfo &&$hasTrailingSlash === ($trimmedPathinfo ===$pathinfo)) {
95-
if ($trimmedPathinfo ===$pathinfo || !$hasTrailingVar) {
96-
break;
97-
}
91+
if ($hasTrailingSlash &&$hasTrailingVar &&preg_match($regex,rtrim($matchedPathinfo,'/') ?:'/',$n) &&$m === (int)$n['MARK']) {
92+
$matches =$n;
9893
}
9994

10095
foreach ($varsas$i =>$v) {

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp