8000 [Router] Fix DumpedUrlMatcher behaviour with trailing slash · symfony/symfony@60997a8 · GitHub
[go: up one dir, main page]
Skip to content

Commit 60997a8

Browse files
author
Xavier Leune
committed
[Router] Fix DumpedUrlMatcher behaviour with trailing slash
1 parent c08b42a commit 60997a8

File tree

6 files changed

+107
-27
lines changed

6 files changed

+107
-27
lines changed

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

Lines changed: 53 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,31 @@ public function addExpressionLanguageProvider(ExpressionFunctionProviderInterfac
9393
*/
9494
private function generateMatchMethod($supportsRedirections)
9595
{
96-
$code = rtrim($this->compileRoutes($this->getRoutes(), $supportsRedirections), "\n");
97-
9896
return <<<EOF
9997
public function match(\$rawPathinfo)
98+
{
99+
try {
100+
return \$this->matchWithoutTrailingSlashManagement(\$rawPathinfo);
101+
} catch (MethodNotAllowedException \$e) {
102+
throw \$e;
103+
} catch (ResourceNotFoundException \$e) {
104+
return \$this->matchWithTrailingSlashManagement(\$rawPathinfo);
105+
}
106+
}
107+
108+
{$this->generateMatchWithTrailingSlashManagement($supportsRedirections)}
109+
110+
{$this->generateMatchWithoutTrailingSlashManagement($supportsRedirections)}
111+
EOF;
112+
113+
}
114+
115+
private function generateMatchWithTrailingSlashManagement($supportsRedirections)
116+
{
117+
$code = rtrim($this->compileRoutes($this->getRoutes(), $supportsRedirections, true), "\n");
118+
119+
return <<<EOF
120+
public function matchWithTrailingSlashManagement(\$rawPathinfo)
100121
{
101122
\$allow = [];
102123
\$pathinfo = rawurldecode(\$rawPathinfo);
@@ -109,6 +130,30 @@ public function match(\$rawPathinfo)
109130
\$canonicalMethod = 'GET';
110131
}
111132
133+
$code
134+
135+
throw 0 < count(\$allow) ? new MethodNotAllowedException(array_unique(\$allow)) : new ResourceNotFoundException();
136+
}
137+
EOF;
138+
}
139+
140+
private function generateMatchWithoutTrailingSlashManagement($supportsRedirections)
141+
{
142+
$code = rtrim($this->compileRoutes($this->getRoutes(), $supportsRedirections, false), "\n");
143+
144+
return <<<EOF
145+
public function matchWithoutTrailingSlashManagement(\$rawPathinfo)
146+
{
147+
\$allow = [];
148+
\$pathinfo = rawurldecode(\$rawPathinfo);
149+
\$context = \$this->context;
150+
\$request = \$this->request ?: \$this->createRequest(\$pathinfo);
151+
\$requestMethod = \$canonicalMethod = \$context->getMethod();
152+
153+
if ('HEAD' === \$requestMethod) {
154+
\$canonicalMethod = 'GET';
155+
}
156+
112157
$code
113158
114159
throw 0 < count(\$allow) ? new MethodNotAllowedException(array_unique(\$allow)) : new ResourceNotFoundException();
@@ -124,7 +169,7 @@ public function match(\$rawPathinfo)
124169
*
125170
* @return string PHP code
126171
*/
127-
private function compileRoutes(RouteCollection $routes, $supportsRedirections)
172+
private function compileRoutes(RouteCollection $routes, $supportsRedirections, $trailingSlashManagement = true)
128173
{
129174
$fetchedHost = false;
130175
$groups = $this->groupRoutesByHostRegex($routes);
@@ -141,7 +186,7 @@ private function compileRoutes(RouteCollection $routes, $supportsRedirections)
141186
}
142187

143188
$tree = $this->buildStaticPrefixCollection($collection);
144-
$groupCode = $this->compileStaticPrefixRoutes($tree, $supportsRedirections);
189+
$groupCode = $this->compileStaticPrefixRoutes($tree, $supportsRedirections, $trailingSlashManagement);
145190

146191
if (null !== $regex) {
147192
// apply extra indention at each line (except empty ones)
@@ -184,7 +229,7 @@ private function buildStaticPrefixCollection(DumperCollection $collection)
184229
*
185230
* @return string PHP code
186231
*/
187-
private function compileStaticPrefixRoutes(StaticPrefixCollection $collection, $supportsRedirections, $ifOrElseIf = 'if')
232+
private function compileStaticPrefixRoutes(StaticPrefixCollection $collection, $supportsRedirections, $trailingSlashManagement, $ifOrElseIf = 'if')
188233
{
189234
$code = '';
190235
$prefix = $collection->getPrefix();
@@ -200,7 +245,7 @@ private function compileStaticPrefixRoutes(StaticPrefixCollection $collection, $
200245
$code .= $this->compileStaticPrefixRoutes($route, $supportsRedirections, $ifOrElseIf);
201246
$ifOrElseIf = 'elseif';
202247
} else {
203-
$code .= $this->compileRoute($route[1]->getRoute(), $route[1]->getName(), $supportsRedirections, $prefix)."\n";
248+
$code .= $this->compileRoute($route[1]->getRoute(), $route[1]->getName(), $supportsRedirections, $trailingSlashManagement, $prefix)."\n";
204249
$ifOrElseIf = 'if';
205250
}
206251
}
@@ -226,7 +271,7 @@ private function compileStaticPrefixRoutes(StaticPrefixCollection $collection, $
226271
*
227272
* @throws \LogicException
228273
*/
229-
private function compileRoute(Route $route, $name, $supportsRedirections, $parentPrefix = null)
274+
private function compileRoute(Route $route, $name, $supportsRedirections, $trailingSlashManagement, $parentPrefix = null)
230275
{
231276
$code = '';
232277
$compiledRoute = $route->compile();
@@ -236,7 +281,7 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren
236281
$hostMatches = false;
237282
$methods = $route->getMethods();
238283

239-
$supportsTrailingSlash = $supportsRedirections && (!$methods || \in_array('GET', $methods));
284+
$supportsTrailingSlash = $supportsRedirections && (!$methods || \in_array('GET', $methods)) && $trailingSlashManagement;
240285
$regex = $compiledRoute->getRegex();
241286

242287
if (!\count($compiledRoute->getPathVariables()) && false !== preg_match('#^(.)\^(?P<url>.*?)\$\1#'.('u' === substr($regex, -1) ? 'u' : ''), $regex, $m)) {

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

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

1212
namespace Symfony\Component\Routing\Matcher;
1313

14+
use Symfony\Component\Routing\Exception\MethodNotAllowedException;
15+
use Symfony\Component\Routing\Exception\NoConfigurationException;
1416
use Symfony\Component\Routing\Exception\ResourceNotFoundException;
1517
use Symfony\Component\Routing\Route;
1618

@@ -25,15 +27,21 @@ abstract class RedirectableUrlMatcher extends UrlMatcher implements Redirectable
2527
public function match($pathinfo)
2628
{
2729
try {
28-
$parameters = parent::match($pathinfo);
30+
$parameters = $this->matchCollectionByTrailingSlashSupport($pathinfo, false);
2931
} catch (ResourceNotFoundException $e) {
3032
if ('/' === substr($pathinfo, -1) || !\in_array($this->context->getMethod(), ['HEAD', 'GET'])) {
3133
throw $e;
3234
}
3335

3436
try {
35-
$parameters = parent::match($pathinfo.'/');
36-
37+
$parameters = $this->matchCollectionByTrailingSlashSupport($pathinfo.'/', true);
38+
return array_replace($parameters, $this->redirect($pathinfo.'/', isset($parameters['_route']) ? $parameters['_route'] : null));
39+
} catch (ResourceNotFoundException $e2) {
40+
throw $e;
41+
}
42+
} catch (MethodNotAllowedException $e) {
43+
try {
44+
$parameters = $this->matchCollectionByTrailingSlashSupport($pathinfo.'/', true);
3745
return array_replace($parameters, $this->redirect($pathinfo.'/', isset($parameters['_route']) ? $parameters['_route'] : null));
3846
} catch (ResourceNotFoundException $e2) {
3947
throw $e;
@@ -62,4 +70,21 @@ protected function handleRouteRequirements($pathinfo, $name, Route $route)
6270

6371
return [self::REQUIREMENT_MATCH, null];
6472
}
73+
74+
protected function matchCollectionByTrailingSlashSupport($pathinfo, $trailingSlashSupport)
75+
{
76+
$this->allow = [];
77+
if ($ret = $this->matchCollection(rawurldecode($pathinfo), $this->routes, $trailingSlashSupport)) {
78+
return $ret;
79+
}
80+
81+
if ('/' === $pathinfo && !$this->allow) {
82+
throw new NoConfigurationException();
83+
}
84+
85+
throw 0 < \count($this->allow)
86+
? new MethodNotAllowedException(array_unique($this->allow))
87+
: new ResourceNotFoundException(sprintf('No routes found for "%s".', $pathinfo));
88+
89+
}
6590
}

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,12 @@ public function getTracesForRequest(Request $request)
5050
return $traces;
5151
}
5252

53-
protected function matchCollection($pathinfo, RouteCollection $routes)
53+
protected function matchCollection($pathinfo, RouteCollection $routes, $supportsTrailingSlash)
5454
{
5555
// HEAD and GET are equivalent as per RFC
5656
if ('HEAD' === $method = $this->context->getMethod()) {
5757
$method = 'GET';
5858
}
59-
$supportsTrailingSlash = '/' !== $pathinfo && '' !== $pathinfo && $this instanceof RedirectableUrlMatcherInterface;
6059

6160
foreach ($routes as $name => $route) {
6261
$compiledRoute = $route->compile();

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,9 @@ public function getContext()
7171
public function match($pathinfo)
7272
{
7373
$this->allow = [];
74+
$supportsTrailingSlash = '/' !== $pathinfo && '' !== $pathinfo && $this instanceof RedirectableUrlMatcherInterface;
7475

75-
if ($ret = $this->matchCollection(rawurldecode($pathinfo), $this->routes)) {
76+
if ($ret = $this->matchCollection(rawurldecode($pathinfo), $this->routes, $supportsTrailingSlash)) {
7677
return $ret;
7778
}
7879

@@ -116,13 +117,12 @@ public function addExpressionLanguageProvider(ExpressionFunctionProviderInterfac
116117
* @throws ResourceNotFoundException If the resource could not be found
117118
* @throws MethodNotAllowedException If the resource was found but the request method is not allowed
118119
*/
119-
protected function matchCollection($pathinfo, RouteCollection $routes)
120+
protected function matchCollection($pathinfo, RouteCollection $routes, $supportsTrailingSlash)
120121
{
121122
// HEAD and GET are equivalent as per RFC
122123
if ('HEAD' === $method = $this->context->getMethod()) {
123124
$method = 'GET';
124125
}
125-
$supportsTrailingSlash = '/' !== $pathinfo && '' !== $pathinfo && $this instanceof RedirectableUrlMatcherInterface;
126126

127127
foreach ($routes as $name => $route) {
128128
$compiledRoute = $route->compile();

‎src/Symfony/Component/Routing/Tests/Matcher/RedirectableUrlMatcherTest.php

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -115,17 +115,6 @@ public function testSchemeRequirement()
115115
$this->assertSame(['_route' => 'foo'], $matcher->match('/foo'));
116116
}
117117

118-
public function testFallbackPage()
119-
{
120-
$coll = new RouteCollection();
121-
$coll->add('foo', new Route('/foo/'));
122-
$coll->add('bar', new Route('/{name}'));
123-
124-
$matcher = $this->getUrlMatcher($coll);
125-
$matcher->expects($this->once())->method('redirect')->with('/foo/')->willReturn(['_route' => 'foo']);
126-
$this->assertSame(['_route' => 'foo'], $matcher->match('/foo'));
127-
}
128-
129118
public function testSlashAndVerbPrecedenceWithRedirection()
130119
{
131120
$coll = new RouteCollection();

‎src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -609,6 +609,28 @@ public function testGreedyTrailingRequirement()
609609
$this->assertEquals(['_route' => 'a', 'a' => 'foo/'], $matcher->match('/foo/'));
610610
}
611611

612+
public function testRouteWithAndWithoutTrailingSlash()
613+
{
614+
$coll = new RouteCollection();
615+
$coll->add('hard_route_with_trailing_slash', new Route('/a/'));
616+
$coll->add('dynamic_route_without_trailing_slash', new Route('/{b}', [], ['b' => '.+']));
617+
618+
$matcher = $this->getUrlMatcher($coll);
619+
620+
$this->assertEquals(['_route' => 'hard_route_with_trailing_slash'], $matcher->match('/a/'), 'truc3');
621+
$this->assertEquals(['_route' => 'dynamic_route_without_trailing_slash', 'b' => 'a'], $matcher->match('/a'), 'truc4');
622+
623+
// Order of definitions matters
624+
$coll = new RouteCollection();
625+
$coll->add('dynamic_route_without_trailing_slash', new Route('/{b}', [], ['b' => '.+']));
626+
$coll->add('hard_route_with_trailing_slash', new Route('/a/'));
627+
628+
$matcher = $this->getUrlMatcher($coll);
629+
630+
$this->assertEquals(['_route' => 'dynamic_route_without_trailing_slash', 'b' => 'a/'], $matcher->match('/a/'), 'truc1');
631+
$this->assertEquals(['_route' => 'dynamic_route_without_trailing_slash', 'b' => 'a'], $matcher->match('/a'), 'truc2');
632+
}
633+
612634
protected function getUrlMatcher(RouteCollection $routes, RequestContext $context = null)
613635
{
614636
return new UrlMatcher($routes, $context ?: new RequestContext());

0 commit comments

Comments
 (0)
0