Skip to content

Commit

Permalink
feature #26283 [Routing] Redirect from trailing slash to no-slash whe…
Browse files Browse the repository at this point in the history 
…n possible (nicolas-grekas)

This PR was merged into the 4.1-dev branch.

Discussion
----------

[Routing] Redirect from trailing slash to no-slash when possible

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #26207
| License       | MIT
| Doc PR        | -

Implemented as suggest by @Tobion in #26059 (comment)

When a route for `/foo` exists but the request is for `/foo/`, we now redirect.
(this complements the flipped side redirection, which already exists.)

Commits
-------

69a4e94 [Routing] Redirect from trailing slash to no-slash when possible
  • Loading branch information
@Tobion
Tobion committed Feb 24, 2018
2 parents dc56a83 + 69a4e94 commit be1a3b4
Show file tree
Hide file tree
Showing 20 changed files with 387 additions and 428 deletions.
172 changes: 72 additions & 100 deletions src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php
View file

Large diffs are not rendered by default.

6 changes: 0 additions & 6 deletions src/Symfony/Component/Routing/Matcher/Dumper/StaticPrefixCollection.php
View file
Expand Up @@ -180,12 +180,6 @@ private function getCommonPrefix(string $prefix, string $anotherPrefix): array
break;
}
}
if (1 < $i && '/' === $prefix[$i - 1]) {
--$i;
}
if (null !== $staticLength && 1 < $staticLength && '/' === $prefix[$staticLength - 1]) {
--$staticLength;
}

return array(substr($prefix, 0, $i), substr($prefix, 0, $staticLength ?? $i));
}
Expand Down
11 changes: 5 additions & 6 deletions src/Symfony/Component/Routing/Matcher/RedirectableUrlMatcher.php
View file
Expand Up @@ -25,22 +25,21 @@ abstract class RedirectableUrlMatcher extends UrlMatcher implements Redirectable
public function match($pathinfo)
{
try {
$parameters = parent::match($pathinfo);
return parent::match($pathinfo);
} catch (ResourceNotFoundException $e) {
if ('/' === substr($pathinfo, -1) || !in_array($this->context->getMethod(), array('HEAD', 'GET'))) {
if ('/' === $pathinfo || !\in_array($this->context->getMethod(), array('HEAD', 'GET'), true)) {
throw $e;
}

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

return array_replace($parameters, $this->redirect($pathinfo.'/', isset($parameters['_route']) ? $parameters['_route'] : null));
return $this->redirect($pathinfo, $ret['_route'] ?? null) + $ret;
} catch (ResourceNotFoundException $e2) {
throw $e;
}
}

return $parameters;
}

/**
Expand Down
1 change: 0 additions & 1 deletion src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher0.php
View file
Expand Up @@ -19,7 +19,6 @@ public function match($rawPathinfo)
{
$allow = array();
$pathinfo = rawurldecode($rawPathinfo);
$trimmedPathinfo = rtrim($pathinfo, '/');
$context = $this->context;
$requestMethod = $canonicalMethod = $context->getMethod();

Expand Down
85 changes: 42 additions & 43 deletions src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher1.php
View file
Expand Up @@ -19,7 +19,6 @@ public function match($rawPathinfo)
{
$allow = array();
$pathinfo = rawurldecode($rawPathinfo);
$trimmedPathinfo = rtrim($pathinfo, '/');
$context = $this->context;
$requestMethod = $canonicalMethod = $context->getMethod();
$host = strtolower($context->getHost());
Expand Down Expand Up @@ -82,41 +81,41 @@ public function match($rawPathinfo)
.'|/([^/]++)(*:57)'
.'|head/([^/]++)(*:77)'
.')'
.'|/test/([^/]++)(?'
.'|/(*:103)'
.'|/test/([^/]++)/(?'
.'|(*:103)'
.')'
.'|/([\']+)(*:119)'
.'|/a(?'
.'|/b\'b/([^/]++)(?'
.'|/a/(?'
.'|b\'b/([^/]++)(?'
.'|(*:148)'
.'|(*:156)'
.')'
.'|/(.*)(*:170)'
.'|/b\'b/([^/]++)(?'
.'|(*:194)'
.'|(*:202)'
.'|(.*)(*:169)'
.'|b\'b/([^/]++)(?'
.'|(*:192)'
.'|(*:200)'
.')'
.')'
.'|/multi/hello(?:/([^/]++))?(*:238)'
.'|/multi/hello(?:/([^/]++))?(*:236)'
.'|/([^/]++)/b/([^/]++)(?'
.'|(*:269)'
.'|(*:277)'
.'|(*:267)'
.'|(*:275)'
.')'
.'|/aba/([^/]++)(*:299)'
.'|/aba/([^/]++)(*:297)'
.')|(?i:([^\\.]++)\\.example\\.com)(?'
.'|/route1(?'
.'|3/([^/]++)(*:359)'
.'|4/([^/]++)(*:377)'
.'|3/([^/]++)(*:357)'
.'|4/([^/]++)(*:375)'
.')'
.')|(?i:c\\.example\\.com)(?'
.'|/route15/([^/]++)(*:427)'
.'|/route15/([^/]++)(*:425)'
.')|[^/]*+(?'
.'|/route16/([^/]++)(*:462)'
.'|/a(?'
.'|/a\\.\\.\\.(*:483)'
.'|/b(?'
.'|/([^/]++)(*:505)'
.'|/c/([^/]++)(*:524)'
.'|/route16/([^/]++)(*:460)'
.'|/a/(?'
.'|a\\.\\.\\.(*:481)'
.'|b/(?'
.'|([^/]++)(*:502)'
.'|c/([^/]++)(*:520)'
.')'
.')'
.')'
Expand All @@ -130,10 +129,10 @@ public function match($rawPathinfo)
$matches = array('foo' => $matches[1] ?? null);

// baz4
return $this->mergeDefaults(array_replace($matches, array('_route' => 'baz4')), array());
return $this->mergeDefaults(array('_route' => 'baz4') + $matches, array());

// baz5
$ret = $this->mergeDefaults(array_replace($matches, array('_route' => 'baz5')), array());
$ret = $this->mergeDefaults(array('_route' => 'baz5') + $matches, array());
if (!isset(($a = array('POST' => 0))[$requestMethod])) {
$allow += $a;
goto not_baz5;
Expand All @@ -143,7 +142,7 @@ public function match($rawPathinfo)
not_baz5:

// baz.baz6
$ret = $this->mergeDefaults(array_replace($matches, array('_route' => 'baz.baz6')), array());
$ret = $this->mergeDefaults(array('_route' => 'baz.baz6') + $matches, array());
if (!isset(($a = array('PUT' => 0))[$requestMethod])) {
$allow += $a;
goto not_bazbaz6;
Expand All @@ -157,7 +156,7 @@ public function match($rawPathinfo)
$matches = array('foo' => $matches[1] ?? null);

// foo1
$ret = $this->mergeDefaults(array_replace($matches, array('_route' => 'foo1')), array());
$ret = $this->mergeDefaults(array('_route' => 'foo1') + $matches, array());
if (!isset(($a = array('PUT' => 0))[$requestMethod])) {
$allow += $a;
goto not_foo1;
Expand All @@ -167,18 +166,18 @@ public function match($rawPathinfo)
not_foo1:

break;
case 194:
case 192:
$matches = array('foo1' => $matches[1] ?? null);

// foo2
return $this->mergeDefaults(array_replace($matches, array('_route' => 'foo2')), array());
return $this->mergeDefaults(array('_route' => 'foo2') + $matches, array());

break;
case 269:
case 267:
$matches = array('_locale' => $matches[1] ?? null, 'foo' => $matches[2] ?? null);

// foo3
return $this->mergeDefaults(array_replace($matches, array('_route' => 'foo3')), array());
return $this->mergeDefaults(array('_route' => 'foo3') + $matches, array());

break;
default:
Expand All @@ -188,18 +187,18 @@ public function match($rawPathinfo)
77 => array(array('_route' => 'barhead'), array('foo'), array('GET' => 0), null),
119 => array(array('_route' => 'quoter'), array('quoter'), null, null),
156 => array(array('_route' => 'bar1'), array('bar'), null, null),
170 => array(array('_route' => 'overridden'), array('var'), null, null),
202 => array(array('_route' => 'bar2'), array('bar1'), null, null),
238 => array(array('_route' => 'helloWorld', 'who' => 'World!'), array('who'), null, null),
277 => array(array('_route' => 'bar3'), array('_locale', 'bar'), null, null),
299 => array(array('_route' => 'foo4'), array('foo'), null, null),
359 => array(array('_route' => 'route13'), array('var1', 'name'), null, null),
377 => array(array('_route' => 'route14', 'var1' => 'val'), array('var1', 'name'), null, null),
427 => array(array('_route' => 'route15'), array('name'), null, null),
462 => array(array('_route' => 'route16', 'var1' => 'val'), array('name'), null, null),
483 => array(array('_route' => 'a'), array(), null, null),
505 => array(array('_route' => 'b'), array('var'), null, null),
524 => array(array('_route' => 'c'), array('var'), null, null),
169 => array(array('_route' => 'overridden'), array('var'), null, null),
200 => array(array('_route' => 'bar2'), array('bar1'), null, null),
236 => array(array('_route' => 'helloWorld', 'who' => 'World!'), array('who'), null, null),
275 => array(array('_route' => 'bar3'), array('_locale', 'bar'), null, null),
297 => array(array('_route' => 'foo4'), array('foo'), null, null),
357 => array(array('_route' => 'route13'), array('var1', 'name'), null, null),
375 => array(array('_route' => 'route14', 'var1' => 'val'), array('var1', 'name'), null, null),
425 => array(array('_route' => 'route15'), array('name'), null, null),
460 => array(array('_route' => 'route16', 'var1' => 'val'), array('name'), null, null),
481 => array(array('_route' => 'a'), array(), null, null),
502 => array(array('_route' => 'b'), array('var'), null, null),
520 => array(array('_route' => 'c'), array('var'), null, null),
);

list($ret, $vars, $requiredMethods, $requiredSchemes) = $routes[$m];
Expand All @@ -218,7 +217,7 @@ public function match($rawPathinfo)
return $ret;
}

if (524 === $m) {
if (520 === $m) {
break;
}
$regex = substr_replace($regex, 'F', $m - $offset, 1 + strlen($m));
Expand Down
1 change: 0 additions & 1 deletion src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher10.php
View file
Expand Up @@ -19,7 +19,6 @@ public function match($rawPathinfo)
{
$allow = array();
$pathinfo = rawurldecode($rawPathinfo);
$trimmedPathinfo = rtrim($pathinfo, '/');
$context = $this->context;
$requestMethod = $canonicalMethod = $context->getMethod();

Expand Down
102 changes: 55 additions & 47 deletions src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher11.php
View file
Expand Up @@ -15,11 +15,26 @@ public function __construct(RequestContext $context)
$this->context = $context;
}

public function match($rawPathinfo)
public function match($pathinfo)
{
$allow = array();
if ($ret = $this->doMatch($pathinfo, $allow)) {
return $ret;
}
if ('/' !== $pathinfo && in_array($this->context->getMethod(), array('HEAD', 'GET'), true)) {
$pathinfo = '/' !== $pathinfo[-1] ? $pathinfo.'/' : substr($pathinfo, 0, -1);
if ($ret = $this->doMatch($pathinfo)) {
return $this->redirect($pathinfo, $ret['_route']) + $ret;
}
}

throw $allow ? new MethodNotAllowedException(array_keys($allow)) : new ResourceNotFoundException();
}

private function doMatch(string $rawPathinfo, array &$allow = array()): ?array
{
$allow = array();
$pathinfo = rawurldecode($rawPathinfo);
$trimmedPathinfo = rtrim($pathinfo, '/');
$context = $this->context;
$requestMethod = $canonicalMethod = $context->getMethod();

Expand All @@ -30,32 +45,34 @@ public function match($rawPathinfo)
$matchedPathinfo = $pathinfo;
$regexList = array(
0 => '{^(?'
.'|/(en|fr)(?'
.'|/admin/post(?'
.'|/?(*:34)'
.'|/new(*:45)'
.'|/(\\d+)(?'
.'|(*:61)'
.'|/edit(*:73)'
.'|/delete(*:87)'
.'|/(en|fr)/(?'
.'|admin/post/(?'
.'|(*:33)'
.'|new(*:43)'
.'|(\\d+)(?'
.'|(*:58)'
.'|/(?'
.'|edit(*:73)'
.'|delete(*:86)'
.')'
.')'
.')'
.'|/blog(?'
.'|/?(*:106)'
.'|/rss\\.xml(*:123)'
.'|/p(?'
.'|age/([^/]++)(*:148)'
.'|osts/([^/]++)(*:169)'
.'|blog/(?'
.'|(*:104)'
.'|rss\\.xml(*:120)'
.'|p(?'
.'|age/([^/]++)(*:144)'
.'|osts/([^/]++)(*:165)'
.')'
.'|/comments/(\\d+)/new(*:197)'
.'|/search(*:212)'
.'|comments/(\\d+)/new(*:192)'
.'|search(*:206)'
.')'
.'|/log(?'
.'|in(*:230)'
.'|out(*:241)'
.'|log(?'
.'|in(*:223)'
.'|out(*:234)'
.')'
.')'
.'|/(en|fr)?(*:260)'
.'|/(en|fr)?(*:253)'
.')$}sD',
);

Expand All @@ -64,20 +81,20 @@ public function match($rawPathinfo)
switch ($m = (int) $matches['MARK']) {
default:
$routes = array(
34 => array(array('_route' => 'a', '_locale' => 'en'), array('_locale'), null, null, true),
45 => array(array('_route' => 'b', '_locale' => 'en'), array('_locale'), null, null),
61 => array(array('_route' => 'c', '_locale' => 'en'), array('_locale', 'id'), null, null),
33 => array(array('_route' => 'a', '_locale' => 'en'), array('_locale'), null, null),
43 => array(array('_route' => 'b', '_locale' => 'en'), array('_locale'), null, null),
58 => array(array('_route' => 'c', '_locale' => 'en'), array('_locale', 'id'), null, null),
73 => array(array('_route' => 'd', '_locale' => 'en'), array('_locale', 'id'), null, null),
87 => array(array('_route' => 'e', '_locale' => 'en'), array('_locale', 'id'), null, null),
106 => array(array('_route' => 'f', '_locale' => 'en'), array('_locale'), null, null, true),
123 => array(array('_route' => 'g', '_locale' => 'en'), array('_locale'), null, null),
148 => array(array('_route' => 'h', '_locale' => 'en'), array('_locale', 'page'), null, null),
169 => array(array('_route' => 'i', '_locale' => 'en'), array('_locale', 'page'), null, null),
197 => array(array('_route' => 'j', '_locale' => 'en'), array('_locale', 'id'), null, null),
212 => array(array('_route' => 'k', '_locale' => 'en'), array('_locale'), null, null),
230 => array(array('_route' => 'l', '_locale' => 'en'), array('_locale'), null, null),
241 => array(array('_route' => 'm', '_locale' => 'en'), array('_locale'), null, null),
260 => array(array('_route' => 'n', '_locale' => 'en'), array('_locale'), null, null),
86 => array(array('_route' => 'e', '_locale' => 'en'), array('_locale', 'id'), null, null),
104 => array(array('_route' => 'f', '_locale' => 'en'), array('_locale'), null, null),
120 => array(array('_route' => 'g', '_locale' => 'en'), array('_locale'), null, null),
144 => array(array('_route' => 'h', '_locale' => 'en'), array('_locale', 'page'), null, null),
165 => array(array('_route' => 'i', '_locale' => 'en'), array('_locale', 'page'), null, null),
192 => array(array('_route' => 'j', '_locale' => 'en'), array('_locale', 'id'), null, null),
206 => array(array('_route' => 'k', '_locale' => 'en'), array('_locale'), null, null),
223 => array(array('_route' => 'l', '_locale' => 'en'), array('_locale'), null, null),
234 => array(array('_route' => 'm', '_locale' => 'en'), array('_locale'), null, null),
253 => array(array('_route' => 'n', '_locale' => 'en'), array('_locale'), null, null),
);

list($ret, $vars, $requiredMethods, $requiredSchemes) = $routes[$m];
Expand All @@ -88,22 +105,13 @@ public function match($rawPathinfo)
}
}

if (empty($routes[$m][4]) || '/' === $pathinfo[-1]) {
// no-op
} elseif ('GET' !== $canonicalMethod) {
$allow['GET'] = 'GET';
break;
} else {
return array_replace($ret, $this->redirect($rawPathinfo.'/', $ret['_route']));
}

if ($requiredSchemes && !isset($requiredSchemes[$context->getScheme()])) {
if ('GET' !== $canonicalMethod) {
$allow['GET'] = 'GET';
break;
}

return array_replace($ret, $this->redirect($rawPathinfo, $ret['_route'], key($requiredSchemes)));
return $this->redirect($rawPathinfo, $ret['_route'], key($requiredSchemes)) + $ret;
}

if ($requiredMethods && !isset($requiredMethods[$canonicalMethod]) && !isset($requiredMethods[$requestMethod])) {
Expand All @@ -114,14 +122,14 @@ public function match($rawPathinfo)
return $ret;
}

if (260 === $m) {
if (253 === $m) {
break;
}
$regex = substr_replace($regex, 'F', $m - $offset, 1 + strlen($m));
$offset += strlen($m);
}
}

throw $allow ? new MethodNotAllowedException(array_keys($allow)) : new ResourceNotFoundException();
return null;
}
}

0 comments on commit be1a3b4

Please sign in to comment.