8000 [Routing] Redirect from trailing slash to no-slash when possible · symfony/symfony@5665fff · GitHub
[go: up one dir, main page]
Skip to content

Commit 5665fff

Browse files
nicolas-grekasnicolas-grekas
committed
[Routing] Redirect from trailing slash to no-slash when possible
1 parent a38cbd0 commit 5665fff

18 files changed

+271
-286
lines changed

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

Lines changed: 52 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -107,12 +107,10 @@ private function generateMatchMethod(bool $supportsRedirections): string
107107
$code = rtrim($this->compileRoutes($routes, $supportsRedirections, $matchHost), "\n");
108108
$fetchHost = $matchHost ? " \$host = strtolower(\$context->getHost());\n" : '';
109109

110-
return <<<EOF
111-
public function match(\$rawPathinfo)
110+
$code = <<<EOF
112111
{
113112
\$allow = array();
114113
\$pathinfo = rawurldecode(\$rawPathinfo);
115-
\$trimmedPathinfo = rtrim(\$pathinfo, '/');
116114
\$context = \$this->context;
117115
\$requestMethod = \$canonicalMethod = \$context->getMethod();
118116
{$fetchHost}
@@ -125,6 +123,36 @@ public function match(\$rawPathinfo)
125123
throw \$allow ? new MethodNotAllowedException(array_keys(\$allow)) : new ResourceNotFoundException();
126124
}
127125
EOF;
126+
127+
if ($supportsRedirections) {
128+
return <<<'EOF'
129+
public function match($pathinfo)
130+
{
131+
try {
132+
return $this->doMatch($pathinfo);
133+
} catch (ResourceNotFoundException $e) {
134+
if (!in_array($this->context->getMethod(), array('HEAD', 'GET'), true)) {
135+
throw $e;
136+
}
137+
138+
try {
139+
$pathinfo = '/' !== $pathinfo[-1] ? $pathinfo.'/' : substr($pathinfo, 0, -1);
140+
$ret = $this->doMatch($pathinfo);
141+
142+
return $this->redirect($pathinfo, $ret['_route']) + $ret;
143+
} catch (ResourceNotFoundException $e2) {
144+
throw $e;
145+
}
146+
}
147+
}
148+
149+
private function doMatch($rawPathinfo)
150+
151+
EOF
152+
.$code;
153+
}
154+
155+
return " public function match(\$rawPathinfo)\n".$code;
128156
}
129157

130158
/**
@@ -172,15 +200,9 @@ private function groupStaticRoutes(RouteCollection $collection, bool $supportsRe
172200
$compiledRoute = $route->compile();
173201
$hostRegex = $compiledRoute->getHostRegex();
174202
$regex = $compiledRoute->getRegex();
175-
if ($hasTrailingSlash = $supportsRedirections && $pos = strpos($regex, '/$')) {
176-
$regex = substr_replace($regex, '/?$', $pos, 2);
177-
}
178203
if (!$compiledRoute->getPathVariables()) {
179204
$host = !$compiledRoute->getHostVariables() ? $route->getHost() : '';
180205
$url = $route->getPath();
181-
if ($hasTrailingSlash) {
182-
$url = rtrim($url, '/');
183-
}
184206
foreach ($dynamicRegex as list($hostRx, $rx)) {
185207
if (preg_match($rx, $url) && (!$host || !$hostRx || preg_match($hostRx, $host))) {
186208
$dynamicRegex[] = array($hostRegex, $regex);
@@ -189,7 +211,7 @@ private function groupStaticRoutes(RouteCollection $collection, bool $supportsRe
189211
}
190212
}
191213

192-
$staticRoutes[$url][$name] = array($hasTrailingSlash, $route);
214+
$staticRoutes[$url][$name] = $route;
193215
} else {
194216
$dynamicRegex[] = array($hostRegex, $regex);
195217
$dynamicRoutes->add($name, $route);
@@ -213,54 +235,50 @@ private function compileStaticRoutes(array $staticRoutes, bool $supportsRedirect
213235
return '';
214236
}
215237
$code = $default = '';
216-
$checkTrailingSlash = false;
217238

218239
foreach ($staticRoutes as $url => $routes) {
219240
if (1 === count($routes)) {
220-
foreach ($routes as $name => list($hasTrailingSlash, $route)) {
241+
foreach ($routes as $name => $route) {
221242
}
222243

223244
if (!$route->getCondition()) {
224245
if (!$supportsRedirections && $route->getSchemes()) {
225246
throw new \LogicException('The "schemes" requirement is only supported for URL matchers that implement RedirectableUrlMatcherInterface.');
226247
}
227-
$checkTrailingSlash = $checkTrailingSlash || $hasTrailingSlash;
228248
$default .= sprintf(
229249
"%s => array(%s, %s, %s, %s),\n",
230250
self::export($url),
231251
self::export(array('_route' => $name) + $route->getDefaults()),
232252
self::export(!$route->compile()->getHostVariables() ? $route->getHost() : $route->compile()->getHostRegex() ?: null),
233253
self::export(array_flip($route->getMethods()) ?: null),
234-
self::export(array_flip($route->getSchemes()) ?: null).($hasTrailingSlash ? ', true' : '')
254+
self::export(array_flip($route->getSchemes()) ?: null)
235255
);
236256
continue;
237257
}
238258
}
239259

240260
$code .= sprintf(" case %s:\n", self::export($url));
241-
foreach ($routes as $name => list($hasTrailingSlash, $route)) {
242-
$code .= $this->compileRoute($route, $name, $supportsRedirections, $hasTrailingSlash, true);
261+
foreach ($routes as $name => $route) {
262+
$code .= $this->compileRoute($route, $name, $supportsRedirections, true);
243263
}
244264
$code .= " break;\n";
245265
}
246266

247-
$matchedPathinfo = $supportsRedirections ? '$trimmedPathinfo' : '$pathinfo';
248-
249267
if ($default) {
250268
$code .= <<<EOF
251269
default:
252270
\$routes = array(
253271
{$this->indent($default, 4)} );
254272
255-
if (!isset(\$routes[{$matchedPathinfo}])) {
273+
if (!isset(\$routes[\$pathinfo])) {
256274
break;
257275
}
258-
list(\$ret, \$requiredHost, \$requiredMethods, \$requiredSchemes) = \$routes[{$matchedPathinfo}];
259-
{$this->compileSwitchDefault(false, $matchedPathinfo, $matchHost, $supportsRedirections, $checkTrailingSlash)}
276+
list(\$ret, \$requiredHost, \$requiredMethods, \$requiredSchemes) = \$routes[\$pathinfo];
277+
{$this->compileSwitchDefault(false, $matchHost, $supportsRedirections)}
260278
EOF;
261279
}
262280

263-
return sprintf(" switch (%s) {\n%s }\n\n", $matchedPathinfo, $this->indent($code));
281+
return sprintf(" switch (\$pathinfo) {\n%s }\n\n", $this->indent($code));
264282
}
265283

266284
/**
@@ -294,7 +312,6 @@ private function compileDynamicRoutes(RouteCollection $collection, bool $support
294312
'mark' => 0,
295313
'markTail' => 0,
296314
'supportsRedirections' => $supportsRedirections,
297-
'checkTrailingSlash' => false,
298315
'hostVars' => array(),
299316
'vars' => array(),
300317
);
@@ -392,7 +409,7 @@ private function compileDynamicRoutes(RouteCollection $collection, bool $support
392409
{$this->indent($state->default, 4)} );
393410
394411
list(\$ret, \$vars, \$requiredMethods, \$requiredSchemes) = \$routes[\$m];
395-
{$this->compileSwitchDefault(true, '$m', $matchHost, $supportsRedirections, $state->checkTrailingSlash)}
412+
{$this->compileSwitchDefault(true, $matchHost, $supportsRedirections)}
396413
EOF;
397414
}
398415

@@ -448,32 +465,28 @@ private function compileStaticPrefixCollection(StaticPrefixCollection $tree, \st
448465

449466
list($name, $regex, $vars, $route) = $route;
450467
$compiledRoute = $route->compile();
451-
$hasTrailingSlash = $state->supportsRedirections && '' !== $regex && '/' === $regex[-1];
452468

453469
if ($compiledRoute->getRegex() === $prevRegex) {
454-
$state->switch = substr_replace($state->switch, $this->compileRoute($route, $name, $state->supportsRedirections, $hasTrailingSlash, false)."\n", -19, 0);
470+
$state->switch = substr_replace($state->switch, $this->compileRoute($route, $name, $state->supportsRedirections, false)."\n", -19, 0);
455471
continue;
456472
}
457473

458-
$methods = array_flip($route->getMethods());
459-
$hasTrailingSlash = $hasTrailingSlash && (!$methods || isset($methods['GET']));
460-
$state->mark += 3 + $state->markTail + $hasTrailingSlash + strlen($regex) - $prefixLen;
474+
$state->mark += 3 + $state->markTail + strlen($regex) - $prefixLen;
461475
$state->markTail = 2 + strlen($state->mark);
462-
$rx = sprintf('|%s(*:%s)', substr($regex, $prefixLen).($hasTrailingSlash ? '?' : ''), $state->mark);
476+
$rx = sprintf('|%s(*:%s)', substr($regex, $prefixLen), $state->mark);
463477
$code .= "\n .".self::export($rx);
464478
$state->regex .= $rx;
465479
$vars = array_merge($state->hostVars, $vars);
466480

467481
if (!$route->getCondition() && (!is_array($next = $routes[1 + $i] ?? null) || $regex !== $next[1])) {
468482
$prevRegex = null;
469-
$state->checkTrailingSlash = $state->checkTrailingSlash || $hasTrailingSlash;
470483
$state->default .= sprintf(
471484
"%s => array(%s, %s, %s, %s),\n",
472485
$state->mark,
473486
self::export(array('_route' => $name) + $route->getDefaults()),
474487
self::export($vars),
475-
self::export($methods ?: null),
476-
self::export(array_flip($route->getSchemes()) ?: null).($hasTrailingSlash ? ', true' : '')
488+
self::export(array_flip($route->getMethods()) ?: null),
489+
self::export(array_flip($route->getSchemes()) ?: null)
477490
);
478491
} else {
479492
$prevRegex = $compiledRoute->getRegex();
@@ -485,7 +498,7 @@ private function compileStaticPrefixCollection(StaticPrefixCollection $tree, \st
485498

486499
$state->switch .= <<<EOF
487500
case {$state->mark}:
488-
{$combine}{$this->compileRoute($route, $name, $state->supportsRedirections, $hasTrailingSlash, false)}
501+
{$combine}{$this->compileRoute($route, $name, $state->supportsRedirections, false)}
489502
break;
490503
491504
EOF;
@@ -498,7 +511,7 @@ private function compileStaticPrefixCollection(StaticPrefixCollection $tree, \st
498511
/**
499512
* A simple helper to compiles the switch's "default" for both static and dynamic routes.
500513
*/
501-
private function compileSwitchDefault(bool $hasVars, string $routesKey, bool $matchHost, bool $supportsRedirections, bool $checkTrailingSlash): string
514+
private function compileSwitchDefault(bool $hasVars, bool $matchHost, bool $supportsRedirections): string
502515
{
503516
if ($hasVars) {
504517
$code = <<<EOF
@@ -527,20 +540,6 @@ private function compileSwitchDefault(bool $hasVars, string $routesKey, bool $ma
527540
} else {
528541
$code = '';
529542
}
530-
if ($supportsRedirections && $checkTrailingSlash) {
531-
$code .= <<<EOF
532-
533-
if (empty(\$routes[{$routesKey}][4]) || '/' === \$pathinfo[-1]) {
534-
// no-op
535-
} elseif ('GET' !== \$canonicalMethod) {
536-
\$allow['GET'] = 'GET';
537-
break;
538-
} else {
539-
return array_replace(\$ret, \$this->redirect(\$rawPathinfo.'/', \$ret['_route']));
540-
}
541-
542-
EOF;
543-
}
544543
if ($supportsRedirections) {
545544
$code .= <<<EOF
546545
@@ -574,20 +573,14 @@ private function compileSwitchDefault(bool $hasVars, string $routesKey, bool $ma
574573
*
575574
* @throws \LogicException
576575
*/
577-
private function compileRoute(Route $route, string $name, bool $supportsRedirections, bool $hasTrailingSlash, bool $checkHost): string
576+
private function compileRoute(Route $route, string $name, bool $supportsRedirections, bool $checkHost): string
578577
{
579578
$code = '';
580579
$compiledRoute = $route->compile();
581580
$conditions = array();
582581
$matches = (bool) $compiledRoute->getPathVariables();
583582
$hostMatches = (bool) $compiledRoute->getHostVariables();
584583
$methods = array_flip($route->getMethods());
585-
$supportsTrailingSlash = $supportsRedirections && (!$methods || isset($methods['GET']));
586-
587-
if ($hasTrailingSlash && !$supportsTrailingSlash) {
588-
$hasTrailingSlash = false;
589-
$conditions[] = "'/' === \$pathinfo[-1]";
590-
}
591584

592585
if ($route->getCondition()) {
593586
$expression = $this->getExpressionLanguage()->compile($route->getCondition(), array('context', 'request'));
@@ -645,21 +638,6 @@ private function compileRoute(Route $route, string $name, bool $supportsRedirect
645638
$code .= sprintf(" \$ret = array('_route' => '%s');\n", $name);
646639
}
647640

648-
if ($hasTrailingSlash) {
649-
$code .= <<<EOF
650-
if ('/' === \$pathinfo[-1]) {
651-
// no-op
652-
} elseif ('GET' !== \$canonicalMethod) {
653-
\$allow['GET'] = 'GET';
654-
goto $gotoname;
655-
} else {
656-
return array_replace(\$ret, \$this->redirect(\$rawPathinfo.'/', '$name'));
657-
}
658-
659-
660-
EOF;
661-
}
662-
663641
if ($schemes = $route->getSchemes()) {
664642
if (!$supportsRedirections) {
665643
throw new \LogicException('The "schemes" requirement is only supported for URL matchers that implement RedirectableUrlMatcherInterface.');
@@ -694,18 +672,18 @@ private function compileRoute(Route $route, string $name, bool $supportsRedirect
694672
EOF;
695673
}
696674

697-
if ($hasTrailingSlash || $schemes || $methods) {
675+
if ($schemes || $methods) {
698676
$code .= " return \$ret;\n";
699677
} else {
700678
$code = substr_replace($code, 'return', $retOffset, 6);
701679
}
702680
if ($conditions) {
703681
$code .= " }\n";
704-
} elseif ($hasTrailingSlash || $schemes || $methods) {
682+
} elseif ($schemes || $methods) {
705683
$code .= ' ';
706684
}
707685

708-
if ($hasTrailingSlash || $schemes || $methods) {
686+
if ($schemes || $methods) {
709687
$code .= " $gotoname:\n";
710688
}
711689

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

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,22 +25,21 @@ abstract class RedirectableUrlMatcher extends UrlMatcher implements Redirectable
2525
public function match($pathinfo)
2626
{
2727
try {
28-
$parameters = parent::match($pathinfo);
28+
return parent::match($pathinfo);
2929
} catch (ResourceNotFoundException $e) {
30-
if ('/' === substr($pathinfo, -1) || !in_array($this->context->getMethod(), array('HEAD', 'GET'))) {
30+
if (!\in_array($this->context->getMethod(), array('HEAD', 'GET'), true)) {
3131
throw $e;
3232
}
3333

3434
try {
35-
$parameters = parent::match($pathinfo.'/');
35+
$pathinfo = '/' !== $pathinfo[-1] ? $pathinfo.'/' : substr($pathinfo, 0, -1);
36+
$ret = parent::match($pathinfo);
3637

37-
return array_replace($parameters, $this->redirect($pathinfo.'/', isset($parameters['_route']) ? $parameters['_route'] : null));
38+
return $this->redirect($pathinfo, $ret['_route'] ?? null) + $ret;
3839
} catch (ResourceNotFoundException $e2) {
3940
throw $e;
4041
}
4142
}
42-
43-
return $parameters;
4443
}
4544

4645
/**

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ public function match($rawPathinfo)
1919
{
2020
$allow = array();
2121
$pathinfo = rawurldecode($rawPathinfo);
22-
$trimmedPathinfo = rtrim($pathinfo, '/');
2322
$context = $this->context;
2423
$requestMethod = $canonicalMethod = $context->getMethod();
2524

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ public function match($rawPathinfo)
1919
{
2020
$allow = array();
2121
$pathinfo = rawurldecode($rawPathinfo);
22-
$trimmedPathinfo = rtrim($pathinfo, '/');
2322
$context = $this->context;
2423
$requestMethod = $canonicalMethod = $context->getMethod();
2524
$host = strtolower($context->getHost());

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ public function match($rawPathinfo)
1919
{
2020
$allow = array();
2121
$pathinfo = rawurldecode($rawPathinfo);
22-
$trimmedPathinfo = rtrim($pathinfo, '/');
2322
$context = $this->context;
2423
$requestMethod = $canonicalMethod = $context->getMethod();
2524

0 commit comments

Comments
 (0)
0