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
Appearance settings

Commit 95ebc9f

Browse filesBrowse 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
Copy full SHA for 95ebc9f

17 files changed

+1518
-1321
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php
+63-30Lines 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 = 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Routing/Matcher/UrlMatcher.php
+24-2Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher0.php
+1-1Lines 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)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.