8000 [Routing] fix greediness of trailing slash · symfony/symfony@eb6aee6 · GitHub
[go: up one dir, main page]
Skip to content

Commit eb6aee6

Browse files
nicolas-grekasnicolas-grekas
committed
[Routing] fix greediness of trailing slash
1 parent bce7748 commit eb6aee6

File tree

15 files changed

+241
-106
lines changed

15 files changed

+241
-106
lines changed

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

Lines changed: 41 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -550,16 +550,23 @@ private function compileStaticPrefixCollection(StaticPrefixCollection $tree, \st
550550
private function compileSwitchDefault(bool $hasVars, bool $matchHost): string
551551
{
552552
$code = sprintf("
553-
if ('/' !== \$pathinfo) {
554-
if (!\$hasTrailingSlash && '/' === \$pathinfo[-1]%s) {
555-
%s;
556-
}
557-
if (\$hasTrailingSlash && '/' !== \$pathinfo[-1]) {
558-
%2\$s;
553+
if ('/' !== \$pathinfo) {%s
554+
if (\$hasTrailingSlash !== ('/' === \$pathinfo[-1])) {%s
555+
break;
559556
}
560557
}\n",
561-
$hasVars ? ' && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n[\'MARK\']' : '',
562-
$this->supportsRedirections ? 'return null' : 'break'
558+
$hasVars ? "
559+
if ('/' === \$pathinfo[-1]) {
560+
if (preg_match(\$regex, substr(\$pathinfo, 0, -1), \$n) && \$m === (int) \$n['MARK']) {
561+
\$matches = \$n;
562+
} else {
563+
\$hasTrailingSlash = true;
564+
}
565+
}\n" : '',
566+
$this->supportsRedirections ? "
567+
if (!\$requiredMethods || isset(\$requiredMethods['GET'])) {
568+
return null;
569+
}" : ''
563570
);
564571

565572
if ($hasVars) {
@@ -616,26 +623,42 @@ private function compileSwitchDefault(bool $hasVars, bool $matchHost): string
616623
*/
617624
private function compileRoute(Route $route, string $name, bool $checkHost, bool $hasTrailingSlash): string
618625
{
626+
$compiledRoute = $route->compile();
627+
$conditions = array();
628+
$matches = (bool) $compiledRoute->getPathVariables();
629+
$hostMatches = (bool) $compiledRoute->getHostVariables();
630+
$methods = array_flip($route->getMethods());
619631
$code = " // $name";
620632

621-
if ('/' !== $route->getPath()) {
633+
if ('/' === $route->getPath()) {
634+
$code .= "\n";
635+
} elseif (!$matches) {
622636
$code .= sprintf("
623637
if ('/' !== \$pathinfo && '/' %s \$pathinfo[-1]) {
624638
%s;
625-
}\n",
639+
}\n\n",
626640
$hasTrailingSlash ? '!==' : '===',
627-
$this->supportsRedirections ? 'return null' : 'break'
641+
$this->supportsRedirections && (!$methods || isset($methods['GET'])) ? 'return null' : 'break'
642+
);
643+
} elseif ($hasTrailingSlash) {
644+
$code .= sprintf("
645+
if ('/' !== \$pathinfo[-1]) {
646+
%s;
647+
}
648+
if ('/' !== \$pathinfo && preg_match(\$regex, substr(\$pathinfo, 0, -1), \$n) && \$m === (int) \$n['MARK']) {
649+
\$matches = \$n;
650+
}\n\n",
651+
$this->supportsRedirections && (!$methods || isset($methods['GET'])) ? 'return null' : 'break'
628652
);
629653
} else {
630-
$code .= "\n";
654+
$code .= sprintf("
655+
if ('/' !== \$pathinfo && '/' === \$pathinfo[-1] && preg_match(\$regex, substr(\$pathinfo, 0, -1), \$n) && \$m === (int) \$n['MARK']) {
656+
%s;
657+
}\n\n",
658+
$this->supportsRedirections && (!$methods || isset($methods['GET'])) ? 'return null' : 'break'
659+
);
631660
}
632661

633-
$compiledRoute = $route->compile();
634-
$conditions = array();
635-
$matches = (bool) $compiledRoute->getPathVariables();
636-
$hostMatches = (bool) $compiledRoute->getHostVariables();
637-
$methods = array_flip($route->getMethods());
638-
639662
if ($route->getCondition()) {
640663
$expression = $this->getExpressionLanguage()->compile($route->getCondition(), array('context', 'request'));
641664

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

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -135,11 +135,12 @@ protected function matchCollection($pathinfo, RouteCollection $routes)
135135
foreach ($routes as $name => $route) {
136136
$compiledRoute = $route->compile();
137137
$staticPrefix = $compiledRoute->getStaticPrefix();
138+
$requiredMethods = $route->getMethods();
138139

139140
// check the static prefix of the URL first. Only use the more expensive preg_match when it matches
140141
if ('' === $staticPrefix || 0 === strpos($pathinfo, $staticPrefix)) {
141142
// no-op
142-
} elseif (!$supportsTrailingSlash) {
143+
} elseif (!$supportsTrailingSlash || ($requiredMethods && !\in_array('GET', $requiredMethods))) {
143144
continue;
144145
} elseif ('/' === $staticPrefix[-1] && substr($staticPrefix, 0, -1) === $pathinfo) {
145146
return;
@@ -161,11 +162,18 @@ protected function matchCollection($pathinfo, RouteCollection $routes)
161162
}
162163

163164
if ($supportsTrailingSlash) {
164-
if (!$hasTrailingSlash && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1))) {
165-
return;
165+
if ('/' === $pathinfo[-1]) {
166+
if (preg_match($regex, substr($pathinfo, 0, -1), $m)) {
167+
$matches = $m;
168+
} else {
169+
$hasTrailingSlash = true;
170+
}
166171
}
167-
if ($hasTrailingSlash && '/' !== $pathinfo[-1]) {
168-
return;
172+
if ($hasTrailingSlash !== ('/' === $pathinfo[-1])) {
173+
if (!$requiredMethods || \in_array('GET', $requiredMethods)) {
174+
return;
175+
}
176+
continue;
169177
}
170178
}
171179

@@ -181,7 +189,7 @@ protected function matchCollection($pathinfo, RouteCollection $routes)
181189
}
182190

183191
$hasRequiredScheme = !$route->getSchemes() || $route->hasScheme($this->context->getScheme());
184-
if ($requiredMethods = $route->getMethods()) {
192+
if ($requiredMethods) {
185193
// HEAD and GET are equivalent as per RFC
186194
if ('HEAD' === $method = $this->context->getMethod()) {
187195
$method = 'GET';

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

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,7 @@ public function match($rawPathinfo)
5555
list($ret, $requiredHost, $requiredMethods, $requiredSchemes, $hasTrailingSlash) = $routes[$trimmedPathinfo];
5656

5757
if ('/' !== $pathinfo) {
58-
if (!$hasTrailingSlash && '/' === $pathinfo[-1]) {
59-
break;
60-
}
61-
if ($hasTrailingSlash && '/' !== $pathinfo[-1]) {
58+
if ($hasTrailingSlash !== ('/' === $pathinfo[-1])) {
6259
break;
6360
}
6461
}
@@ -145,15 +142,23 @@ public function match($rawPathinfo)
145142
$matches = array('foo' => $matches[1] ?? null);
146143

147144
// baz4
148-
if ('/' !== $pathinfo && '/' !== $pathinfo[-1]) {
145+
if ('/' !== $pathinfo[-1]) {
149146
break;
150147
}
148+
if ('/' !== $pathinfo && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) {
149+
$matches = $n;
150+
}
151+
151152
return $this->mergeDefaults(array('_route' => 'baz4') + $matches, array());
152153

153154
// baz5
154-
if ('/' !== $pathinfo && '/' !== $pathinfo[-1]) {
155+
if ('/' !== $pathinfo[-1]) {
155156
break;
156157
}
158+
if ('/' !== $pathinfo && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) {
159+
$matches = $n;
160+
}
161+
157162
$ret = $this->mergeDefaults(array('_route' => 'baz5') + $matches, array());
158163
if (!isset(($a = array('POST' => 0))[$requestMethod])) {
159164
$allow += $a;
@@ -164,9 +169,13 @@ public function match($rawPathinfo)
164169
not_baz5:
165170

166171
// baz.baz6
167-
if ('/' !== $pathinfo && '/' !== $pathinfo[-1]) {
172+
if ('/' !== $pathinfo[-1]) {
168173
break;
169174
}
175+
if ('/' !== $pathinfo && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) {
176+
$matches = $n;
177+
}
178+
170179
$ret = $this->mergeDefaults(array('_route' => 'baz.baz6') + $matches, array());
171180
if (!isset(($a = array('PUT' => 0))[$requestMethod])) {
172181
$allow += $a;
@@ -181,9 +190,10 @@ public function match($rawPathinfo)
181190
$matches = array('foo' => $matches[1] ?? null);
182191

183192
// foo1
184-
if ('/' !== $pathinfo && '/' === $pathinfo[-1]) {
193+
if ('/' !== $pathinfo && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) {
185194
break;
186195
}
196+
187197
$ret = $this->mergeDefaults(array('_route' => 'foo1') + $matches, array());
188198
if (!isset(($a = array('PUT' => 0))[$requestMethod])) {
189199
$allow += $a;
@@ -198,19 +208,21 @@ public function match($rawPathinfo)
198208
$matches = array('foo1' => $matches[1] ?? null);
199209

200210
// foo2
201-
if ('/' !== $pathinfo && '/' === $pathinfo[-1]) {
211+
if ('/' !== $pathinfo && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) {
202212
break;
203213
}
214+
204215
return $this->mergeDefaults(array('_route' => 'foo2') + $matches, array());
205216

206217
break;
207218
case 279:
208219
$matches = array('_locale' => $matches[1] ?? null, 'foo' => $matches[2] ?? null);
209220

210221
// foo3
211-
if ('/' !== $pathinfo && '/' === $pathinfo[-1]) {
222+
if ('/' !== $pathinfo && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) {
212223
break;
213224
}
225+
214226
return $this->mergeDefaults(array('_route' => 'foo3') + $matches, array());
215227

216228
break;
@@ -238,10 +250,15 @@ public function match($rawPathinfo)
238250
list($ret, $vars, $requiredMethods, $requiredSchemes, $hasTrailingSlash) = $routes[$m];
239251

240252
if ('/' !== $pathinfo) {
241-
if (!$hasTrailingSlash && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) {
242-
break;
253+
if ('/' === $pathinfo[-1]) {
254+
if (preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) {
255+
$matches = $n;
256+
} else {
257+
$hasTrailingSlash = true;
258+
}
243259
}
244-
if ($hasTrailingSlash && '/' !== $pathinfo[-1]) {
260+
261+
if ($hasTrailingSlash !== ('/' === $pathinfo[-1])) {
245262
break;
246263
}
247264
}

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2794,10 +2794,15 @@ public function match($rawPathinfo)
27942794
list($ret, $vars, $requiredMethods, $requiredSchemes, $hasTrailingSlash) = $routes[$m];
27952795

27962796
if ('/' !== $pathinfo) {
2797-
if (!$hasTrailingSlash && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) {
2798-
break;
2797+
if ('/' === $pathinfo[-1]) {
2798+
if (preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) {
2799+
$matches = $n;
2800+
} else {
2801+
$hasTrailingSlash = true;
2802+
}
27992803
}
2800-
if ($hasTrailingSlash && '/' !== $pathinfo[-1]) {
2804+
2805+
if ($hasTrailingSlash !== ('/' === $pathinfo[-1])) {
28012806
break;
28022807
}
28032808
}

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

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -119,11 +119,19 @@ private function doMatch(string $rawPathinfo, array &$allow = array(), array &$a
119119
list($ret, $vars, $requiredMethods, $requiredSchemes, $hasTrailingSlash) = $routes[$m];
120120

121121
if ('/' !== $pathinfo) {
122-
if (!$hasTrailingSlash && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) {
123-
return null;
122+
if ('/' === $pathinfo[-1]) {
123+
if (preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) {
124+
$matches = $n;
125+
} else {
126+
$hasTrailingSlash = true;
127+
}
124128
}
125-
if ($hasTrailingSlash && '/' !== $pathinfo[-1]) {
126-
return null;
129+
130+
if ($hasTrailingSlash !== ('/' === $pathinfo[-1])) {
131+
if (!$requiredMethods || isset($requiredMethods['GET'])) {
132+
return null;
133+
}
134+
break;
127135
}
128136
}
129137

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,15 @@ public function match($rawPathinfo)
6464
list($ret, $vars, $requiredMethods, $requiredSchemes, $hasTrailingSlash) = $routes[$m];
6565

6666
if ('/' !== $pathinfo) {
67-
if (!$hasTrailingSlash && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) {
68-
break;
67+
if ('/' === $pathinfo[-1]) {
68+
if (preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) {
69+
$matches = $n;
70+
} else {
71+
$hasTrailingSlash = true;
72+
}
6973
}
70-
if ($hasTrailingSlash && '/' !== $pathinfo[-1]) {
74+
75+
if ($hasTrailingSlash !== ('/' === $pathinfo[-1])) {
7176
break;
7277
}
7378
}

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,15 +45,17 @@ public function match($rawPathinfo)
4545
$matches = array('foo' => $matches[1] ?? null, 'foo' => $matches[2] ?? null);
4646

4747
// r1
48-
if ('/' !== $pathinfo && '/' === $pathinfo[-1]) {
48+
if ('/' !== $pathinfo && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) {
4949
break;
5050
}
51+
5152
return $this->mergeDefaults(array('_route' => 'r1') + $matches, array());
5253

5354
3CA1 // r2
54-
if ('/' !== $pathinfo && '/' === $pathinfo[-1]) {
55+
if ('/' !== $pathinfo && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) {
5556
break;
5657
}
58+
5759
return $this->mergeDefaults(array('_route' => 'r2') + $matches, array());
5860

5961
break;

0 commit comments

Comments
 (0)
0