10000 bug #29298 [Routing] fix trailing slash redirection when using Redire… · symfony/symfony@95ebc9f · GitHub
[go: up one dir, main page]
Skip to content

Commit 95ebc9f

Browse files
nicolas-grekasnicolas-grekas
committed
bug #29298 [Routing] fix trailing slash redirection when using RedirectableUrlMatcher (nicolas-grekas)
This PR was merged into the 4.1 branch. Discussion ---------- [Routing] fix trailing slash redirection when using RedirectableUrlMatcher | Q | A | ------------- | --- | Branch? | 4.1 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #29297 | License | MIT | Doc PR | - This is #29297 for 4.1 Commits ------- 6968000 [Routing] fix trailing slash redirection when using RedirectableUrlMatcher
2 parents d788976 + 6968000 commit 95ebc9f

17 files changed

+1518
-1321
lines changed

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

Lines changed: 63 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ class PhpMatcherDumper extends MatcherDumper
2929
{
3030
private $expressionLanguage;
3131
private $signalingException;
32+
private $supportsRedirections;
3233

3334
/**
3435
* @var ExpressionFunctionProviderInterface[]
@@ -56,7 +57,7 @@ public function dump(array $options = array())
5657

5758
// trailing slash support is only enabled if we know how to redirect the user
5859
$interfaces = class_implements($options['base_class']);
59-
$supportsRedirections = isset($interfaces[RedirectableUrlMatcherInterface::class]);
60+
$this->supportsRedirections = isset($interfaces[RedirectableUrlMatcherInterface::class]);
6061

6162
return <<<EOF
6263
<?php
@@ -76,7 +77,7 @@ public function __construct(RequestContext \$context)
7677
\$this->context = \$context;
7778
}
7879
79-
{$this->generateMatchMethod($supportsRedirections)}
80+
{$this->generateMatchMethod()}
8081
}
8182
8283
EOF;
@@ -90,7 +91,7 @@ public function addExpressionLanguageProvider(ExpressionFunctionProviderInterfac
9091
/**
9192
* Generates the code for the match method implementing UrlMatcherInterface.
9293
*/
93-
private function generateMatchMethod(bool $supportsRedirections): string
94+
private function generateMatchMethod(): string
9495
{
9596
// Group hosts by same-suffix, re-order when possible
9697
$matchHost = false;
@@ -111,7 +112,7 @@ private function generateMatchMethod(bool $supportsRedirections): string
111112
$code = <<<EOF
112113
{
113114
\$allow = \$allowSchemes = array();
114-
\$pathinfo = rawurldecode(\$rawPathinfo);
115+
\$pathinfo = rawurldecode(\$rawPathinfo) ?: '/';
115116
\$context = \$this->context;
116117
\$requestMethod = \$canonicalMethod = \$context->getMethod();
117118
{$fetchHost}
@@ -123,7 +124,7 @@ private function generateMatchMethod(bool $supportsRedirections): string
123124
124125
EOF;
125126

126-
if ($supportsRedirections) {
127+
if ($this->supportsRedirections) {
127128
return <<<'EOF'
128129
public function match($pathinfo)
129130
{
@@ -213,9 +214,18 @@ private function groupStaticRoutes(RouteCollection $collection): array
213214
$compiledRoute = $route->compile();
214215
$hostRegex = $compiledRoute->getHostRegex();
215216
$regex = $compiledRoute->getRegex();
217+
if ($hasTrailingSlash = '/' !== $route->getPath()) {
218+
$pos = strrpos($regex, '$');
219+
$hasTrailingSlash = '/' === $regex[$pos - 1];
220+
$regex = substr_replace($regex, '/?$', $pos - $hasTrailingSlash, 1 + $hasTrailingSlash);
221+
}
222+
216223
if (!$compiledRoute->getPathVariables()) {
217224
$host = !$compiledRoute->getHostVariables() ? $route->getHost() : '';
218225
$url = $route->getPath();
226+
if ($hasTrailingSlash) {
227+
$url = substr($url, 0, -1);
228+
}
219229
foreach ($dynamicRegex as list($hostRx, $rx)) {
220230
if (preg_match($rx, $url) && (!$host || !$hostRx || preg_match($hostRx, $host))) {
221231
$dynamicRegex[] = array($hostRegex, $regex);
@@ -224,7 +234,7 @@ private function groupStaticRoutes(RouteCollection $collection): array
224234
}
225235
}
226236

227-
$staticRoutes[$url][$name] = $route;
237+
$staticRoutes[$url][$name] = array($route, $hasTrailingSlash);
228238
} else {
229239
$dynamicRegex[] = array($hostRegex, $regex);
230240
$dynamicRoutes->add($name, $route);
@@ -251,7 +261,7 @@ private function compileStaticRoutes(array $staticRoutes, bool $matchHost): stri
251261

252262
foreach ($staticRoutes as $url => $routes) {
253263
if (1 === \count($routes)) {
254-
foreach ($routes as $name => $route) {
264+
foreach ($routes as $name => list($route, $hasTrailingSlash)) {
255265
}
256266

257267
if (!$route->getCondition()) {
@@ -261,20 +271,21 @@ private function compileStaticRoutes(array $staticRoutes, bool $matchHost): stri
261271
unset($defaults['_canonical_route']);
262272
}
263273
$default .= sprintf(
264-
"%s => array(%s, %s, %s, %s),\n",
274+
"%s => array(%s, %s, %s, %s, %s),\n",
265275
self::export($url),
266276
self::export(array('_route' => $name) + $defaults),
267277
self::export(!$route->compile()->getHostVariables() ? $route->getHost() : $route->compile()->getHostRegex() ?: null),
268278
self::export(array_flip($route->getMethods()) ?: null),
269-
self::export(array_flip($route->getSchemes()) ?: null)
279+
self::export(array_flip($route->getSchemes()) ?: null),
280+
self::export($hasTrailingSlash)
270281
);
271282
continue;
272283
}
273284
}
274285

275286
$code .= sprintf(" case %s:\n", self::export($url));
276-
foreach ($routes as $name => $route) {
277-
$code .= $this->compileRoute($route, $name, true);
287+
foreach ($routes as $name => list($route, $hasTrailingSlash)) {
288+
$code .= $this->compileRoute($route, $name, true, $hasTrailingSlash);
278289
}
279290
$code .= " break;\n";
280291
}
@@ -285,15 +296,15 @@ private function compileStaticRoutes(array $staticRoutes, bool $matchHost): stri
285296
\$routes = array(
286297
{$this->indent($default, 4)} );
287298
288-
if (!isset(\$routes[\$pathinfo])) {
299+
if (!isset(\$routes[\$trimmedPathinfo])) {
289300
break;
290301
}
291-
list(\$ret, \$requiredHost, \$requiredMethods, \$requiredSchemes) = \$routes[\$pathinfo];
302+
list(\$ret, \$requiredHost, \$requiredMethods, \$requiredSchemes, \$hasTrailingSlash) = \$routes[\$trimmedPathinfo];
292303
{$this->compileSwitchDefault(false, $matchHost)}
293304
EOF;
294305
}
295306

296-
return sprintf(" switch (\$pathinfo) {\n%s }\n\n", $this->indent($code));
307+
return sprintf(" switch (\$trimmedPathinfo = '/' !== \$pathinfo && '/' === \$pathinfo[-1] ? substr(\$pathinfo, 0, -1) : \$pathinfo) {\n%s }\n\n", $this->indent($code));
297308
}
298309

299310
/**
@@ -394,7 +405,11 @@ private function compileDynamicRoutes(RouteCollection $collection, bool $matchHo
394405

395406
$state->vars = array();
396407
$regex = preg_replace_callback('#\?P<([^>]++)>#', $state->getVars, $rx[1]);
397-
$tree->addRoute($regex, array($name, $regex, $state->vars, $route));
408+
if ($hasTrailingSlash = '/' !== $regex && '/' === $regex[-1]) {
409+
$regex = substr($regex, 0, -1);
410+
}
411+
412+
$tree->addRoute($regex, array($name, $regex, $state->vars, $route, $hasTrailingSlash));
398413
}
399414

400415
$code .= $this->compileStaticPrefixCollection($tree, $state);
@@ -403,7 +418,7 @@ private function compileDynamicRoutes(RouteCollection $collection, bool $matchHo
403418
$code .= "\n .')'";
404419
$state->regex .= ')';
405420
}
406-
$rx = ")$}{$modifiers}";
421+
$rx = ")(?:/?)$}{$modifiers}";
407422
$code .= "\n .'{$rx}',";
408423
$state->regex .= $rx;
409424
$state->markTail = 0;
@@ -423,7 +438,7 @@ private function compileDynamicRoutes(RouteCollection $collection, bool $matchHo
423438
\$routes = array(
424439
{$this->indent($state->default, 4)} );
425440
426-
list(\$ret, \$vars, \$requiredMethods, \$requiredSchemes) = \$routes[\$m];
441+
list(\$ret, \$vars, \$requiredMethods, \$requiredSchemes, \$hasTrailingSlash) = \$routes[\$m];
427442
{$this->compileSwitchDefault(true, $matchHost)}
428443
EOF;
429444
}
@@ -478,11 +493,11 @@ private function compileStaticPrefixCollection(StaticPrefixCollection $tree, \st
478493
continue;
479494
}
480495

481-
list($name, $regex, $vars, $route) = $route;
496+
list($name, $regex, $vars, $route, $hasTrailingSlash) = $route;
482497
$compiledRoute = $route->compile();
483498

484499
if ($compiledRoute->getRegex() === $prevRegex) {
485-
$state->switch = 38BA substr_replace($state->switch, $this->compileRoute($route, $name, false)."\n", -19, 0);
500+
$state->switch = substr_replace($state->switch, $this->compileRoute($route, $name, false, $hasTrailingSlash)."\n", -19, 0);
486501
continue;
487502
}
488503

@@ -501,12 +516,13 @@ private function compileStaticPrefixCollection(StaticPrefixCollection $tree, \st
501516
unset($defaults['_canonical_route']);
502517
}
503518
$state->default .= sprintf(
504-
"%s => array(%s, %s, %s, %s),\n",
519+
"%s => array(%s, %s, %s, %s, %s),\n",
505520
$state->mark,
506521
self::export(array('_route' => $name) + $defaults),
507522
self::export($vars),
508523
self::export(array_flip($route->getMethods()) ?: null),
509-
self::export(array_flip($route->getSchemes()) ?: null)
524+
self::export(array_flip($route->getSchemes()) ?: null),
525+
self::export($hasTrailingSlash)
510526
);
511527
} else {
512528
$prevRegex = $compiledRoute->getRegex();
@@ -518,7 +534,7 @@ private function compileStaticPrefixCollection(StaticPrefixCollection $tree, \st
518534

519535
$state->switch .= <<<EOF
520536
case {$state->mark}:
521-
{$combine}{$this->compileRoute($route, $name, false)}
537+
{$combine}{$this->compileRoute($route, $name, false, $hasTrailingSlash)}
522538
break;
523539
524540
EOF;
@@ -533,8 +549,15 @@ private function compileStaticPrefixCollection(StaticPrefixCollection $tree, \st
533549
*/
534550
private function compileSwitchDefault(bool $hasVars, bool $matchHost): string
535551
{
552+
$code = sprintf("
553+
if ('/' !== \$pathinfo && \$hasTrailingSlash !== ('/' === \$pathinfo[-1])) {
554+
%s;
555+
}\n",
556+
$this->supportsRedirections ? 'return null' : 'break'
557+
);
558+
536559
if ($hasVars) {
537-
$code = <<<EOF
560+
$code .= <<<EOF
538561
539562
foreach (\$vars as \$i => \$v) {
540563
if (isset(\$matches[1 + \$i])) {
@@ -544,7 +567,7 @@ private function compileSwitchDefault(bool $hasVars, bool $matchHost): string
544567
545568
EOF;
546569
} elseif ($matchHost) {
547-
$code = <<<EOF
570+
$code .= <<<EOF
548571
549572
if (\$requiredHost) {
550573
if ('#' !== \$requiredHost[0] ? \$requiredHost !== \$host : !preg_match(\$requiredHost, \$host, \$hostMatches)) {
@@ -557,8 +580,6 @@ private function compileSwitchDefault(bool $hasVars, bool $matchHost): string
557580
}
558581
559582
EOF;
560-
} else {
561-
$code = '';
562583
}
563584

564585
$code .= <<<EOF
@@ -587,9 +608,22 @@ private function compileSwitchDefault(bool $hasVars, bool $matchHost): string
587608
*
588609
* @throws \LogicException
589610
*/
590-
private function compileRoute(Route $route, string $name, bool $checkHost): string
611+
private function compileRoute(Route $route, string $name, bool $checkHost, bool $hasTrailingSlash): string
591612
{
592-
$code = '';
613+
$code = " // $name";
614+
615+
if ('/' !== $route->getPath()) {
616+
$code .= sprintf("
617+
if ('/' !== \$pathinfo && '/' %s \$pathinfo[-1]) {
618+
%s;
619+
}\n",
620+
$hasTrailingSlash ? '!==' : '===',
621+
$this->supportsRedirections ? 'return null' : 'break'
622+
);
623+
} else {
624+
$code .= "\n";
625+
}
626+
593627
$compiledRoute = $route->compile();
594628
$conditions = array();
595629
$matches = (bool) $compiledRoute->getPathVariables();
@@ -617,12 +651,11 @@ private function compileRoute(Route $route, string $name, bool $checkHost): stri
617651

618652
if ($conditions) {
619653
$code .= <<<EOF
620-
// $name
621654
if ($conditions) {
622655
623656
EOF;
624657
} else {
625-
$code .= " // {$name}\n";
658+
$code = $this->indent($code);
626659
}
627660

628661
$gotoname = 'not_'.preg_replace('/[^A-Za-z0-9_]/', '', $name);

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

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,18 +130,40 @@ public function addExpressionLanguageProvider(ExpressionFunctionProviderInterfac
130130
*/
131131
protected function matchCollection($pathinfo, RouteCollection $routes)
132132
{
133+
$supportsTrailingSlash = '/' !== $pathinfo && '' !== $pathinfo && $this instanceof RedirectableUrlMatcherInterface;
134+
133135
foreach ($routes as $name => $route) {
134136
$compiledRoute = $route->compile();
137+
$staticPrefix = $compiledRoute->getStaticPrefix();
135138

136139
// check the static prefix of the URL first. Only use the more expensive preg_match when it matches
137-
if ('' !== $compiledRoute->getStaticPrefix() && 0 !== strpos($pathinfo, $compiledRoute->getStaticPrefix())) {
140+
if ('' === $staticPrefix || 0 === strpos($pathinfo, $staticPrefix)) {
141+
// no-op
142+
} elseif (!$supportsTrailingSlash) {
143+
continue;
144+
} elseif ('/' === $staticPrefix[-1] && substr($staticPrefix, 0, -1) === $pathinfo) {
145+
return;
146+
} elseif ('/' === $pathinfo[-1] && substr($pathinfo, 0, -1) === $staticPrefix) {
147+
return;
148+
} else {
138149
continue;
139150
}
151+
$regex = $compiledRoute->getRegex();
152+
153+
if ($supportsTrailingSlash) {
154+
$pos = strrpos($regex, '$');
155+
$hasTrailingSlash = '/' === $regex[$pos - 1];
156+
$regex = substr_replace($regex, '/?$', $pos - $hasTrailingSlash, 1 + $hasTrailingSlash);
157+
}
140158

141-
if (!preg_match($compiledRoute->getRegex(), $pathinfo, $matches)) {
159+
if (!preg_match($regex, $pathinfo, $matches)) {
142160
continue;
143161
}
144162

163+
if ($supportsTrailingSlash && $hasTrailingSlash !== ('/' === $pathinfo[-1])) {
164+
return;
165+
}
166+
145167
$hostMatches = array();
146168
if ($compiledRoute->getHostRegex() && !preg_match($compiledRoute->getHostRegex(), $this->context->getHost(), $hostMatches)) {
147169
continue;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ public function __construct(RequestContext $context)
1818
public function match($rawPathinfo)
1919
{
2020
$allow = $allowSchemes = array();
21-
$pathinfo = rawurldecode($rawPathinfo);
21+
$pathinfo = rawurldecode($rawPathinfo) ?: '/';
2222
$context = $this->context;
2323
$requestMethod = $canonicalMethod = $context->getMethod();
2424

0 commit comments

Comments
 (0)
0