10000 [Routing] dont redirect routes with greedy trailing vars with no expl… · symfony/symfony@19c9cb6 · GitHub
[go: up one dir, main page]
Skip to content

Commit 19c9cb6

Browse files
nicolas-grekasnicolas-grekas
committed
[Routing] dont redirect routes with greedy trailing vars with no explicit slash
1 parent 78c23c7 commit 19c9cb6

File tree

13 files changed

+81
-231
lines changed

13 files changed

+81
-231
lines changed

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

Lines changed: 9 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -558,35 +558,29 @@ 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'
589579
580+
if ($hasTrailingSlash && $hasTrailingVar && preg_match($regex, rtrim($matchedPathinfo, '/') ?: '/', $n) && $m === (int) $n['MARK']) {
581+
$matches = $n;
582+
}
583+
590584
foreach ($vars as $i => $v) {
591585
if (isset($matches[1 + $i])) {
592586
$ret[$v] = $matches[1 + $i];
@@ -666,33 +660,8 @@ private function compileRoute(Route $route, string $name, bool $checkHost, bool
666660
$matches = $n;
667661
}
668662
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;
676663
} else {
677-
$hasTrailingSlash = true;
678-
}
679-
680-
if ('/' !== $pathinfo && $hasTrailingSlash === ($trimmedPathinfo === $pathinfo)) {%s
681-
if ($trimmedPathinfo === $pathinfo) {
682-
goto %s;
683-
}
684-
}
685-
EOF;
686-
} else {
687-
$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) {
693-
goto %2$s;
694-
}
695-
EOF;
664+
$code = rtrim($code);
696665
}
697666

698667
if ($this->supportsRedirections && (!$methods || isset($methods['GET']))) {

‎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: 6 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -187,13 +187,6 @@ public function match($pathinfo)
187187
break;
188188
case 160:
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) {
195-
goto not_foo1;
196-
}
197190

198191
$matches = ['foo' => $matches[1] ?? null];
199192

@@ -209,13 +202,6 @@ public function match($pathinfo)
209202
break;
210203
case 204:
211204
// 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) {
217-
goto not_foo2;
218-
}
219205

220206
$matches = ['foo1' => $matches[1] ?? null];
221207

@@ -225,13 +211,6 @@ public function match($pathinfo)
225211
break;
226212
case 279:
227213
// 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) {
233-
goto not_foo3;
234-
}
235214

236215
$matches = ['_locale' => $matches[1] ?? null, 'foo' => $matches[2] ?? null];
237216

@@ -262,17 +241,13 @@ public function match($pathinfo)
262241

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

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;
244+
$hasTrailingVar = $trimmedPathinfo !== $pathinfo && $hasTrailingVar;
245+
if ('/' !== $pathinfo && !$hasTrailingVar && $hasTrailingSlash === ($trimmedPathinfo === $pathinfo)) {
246+
break;
271247
}
272-
if ('/' !== $pathinfo && $hasTrail F438 ingSlash === ($trimmedPathinfo === $pathinfo)) {
273-
if ($trimmedPathinfo === $pathinfo || !$hasTrailingVar) {
274-
break;
275-
}
248+
249+
if ($hasTrailingSlash && $hasTrailingVar && preg_match($regex, rtrim($matchedPathinfo, '/') ?: '/', $n) && $m === (int) $n['MARK']) {
250+
$matches = $n;
276251
}
277252

278253
foreach ($vars as $i => $v) {

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

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2794,17 +2794,13 @@ 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+
2802+
if ($hasTrailingSlash && $hasTrailingVar && preg_match($regex, rtrim($matchedPathinfo, '/') ?: '/', $n) && $m === (int) $n['MARK']) {
2803+
$matches = $n;
28082804
}
28092805

28102806
foreach ($vars as $i => $v) {

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

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -119,20 +119,16 @@ 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+
130+
if ($hasTrailingSlash && $hasTrailingVar && preg_match($regex, rtrim($matchedPathinfo, '/') ?: '/', $n) && $m === (int) $n['MARK']) {
131+
$matches = $n;
136132
}
137133

138134
foreach ($vars as $i => $v) {

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

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -64,17 +64,13 @@ 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+
72+
if ($hasTrailingSlash && $hasTrailingVar && preg_match($regex, rtrim($matchedPathinfo, '/') ?: '/', $n) && $m === (int) $n['MARK']) {
73+
$matches = $n;
7874
}
7975

8076
foreach ($vars as $i => $v) {

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

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -44,27 +44,13 @@ public function match($pathinfo)
4444
switch ($m = (int) $matches['MARK']) {
4545
case 56:
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) {
52-
goto not_r1;
53-
}
5447

5548
$matches = ['foo' => $matches[1] ?? null, 'foo' => $matches[2] ?? null];
5649

5750
return $this->mergeDefaults(['_route' => 'r1'] + $matches, []);
5851
not_r1:
5952

6053
// 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) {
66-
goto not_r2;
67-
}
6854

6955
return $this->mergeDefaults(['_route' => 'r2'] + $matches, []);
7056
not_r2:

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

Lines changed: 7 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -230,13 +230,6 @@ private function doMatch(string $pathinfo, array &$allow = [], array &$allowSche
230230
break;
231231
case 160:
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) {
238-
goto not_foo1;
239-
}
240233

241234
$matches = ['foo' => $matches[1] ?? null];
242235

@@ -252,23 +245,6 @@ private function doMatch(string $pathinfo, array &$allow = [], array &$allowSche
252245
break;
253246
case 204:
254247
// 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-
}
271-
}
272248

273249
$matches = ['foo1' => $matches[1] ?? null];
274250

@@ -278,23 +254,6 @@ private function doMatch(string $pathinfo, array &$allow = [], array &$allowSche
278254
break;
279255
case 279:
280256
// 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-
}
297-
}
298257

299258
$matches = ['_locale' => $matches[1] ?? null, 'foo' => $matches[2] ?? null];
300259

@@ -325,20 +284,16 @@ private function doMatch(string $pathinfo, array &$allow = [], array &$allowSche
325284

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

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)) {
287+
$hasTrailingVar = $trimmedPathinfo !== $pathinfo && $hasTrailingVar;
288+
if ('/' !== $pathinfo && !$hasTrailingVar && $hasTrailingSlash === ($trimmedPathinfo === $pathinfo)) {
336289
if ('GET' === $canonicalMethod && (!$requiredMethods || isset($requiredMethods['GET']))) {
337290
return $allow = $allowSchemes = [];
338291
}
339-
if ($trimmedPathinfo === $pathinfo || !$hasTrailingVar) {
340-
break;
341-
}
292+
break;
293+
}
294+
295+
if ($hasTrailingSlash && $hasTrailingVar && preg_match($regex, rtrim($matchedPathinfo, '/') ?: '/', $n) && $m === (int) $n['MARK']) {
296+
$matches = $n;
342297
}
343298

344299
foreach ($vars as $i => $v) {

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

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -84,17 +84,13 @@ 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+
92+
if ($hasTrailingSlash && $hasTrailingVar && preg_match($regex, rtrim($matchedPathinfo, '/') ?: '/', $n) && $m === (int) $n['MARK']) {
93+
$matches = $n;
9894
}
9995

10096
foreach ($vars as $i => $v) {

0 commit comments

Comments
 (0)
0