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

Commit 8a2b788

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

File tree

6 files changed

+106
-26
lines changed

6 files changed

+106
-26
lines changed

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

Lines changed: 52 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,30 @@ 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+
private function generateMatchWithTrailingSlashManagement($supportsRedirections)
115+
{
116+
$code = rtrim($this->compileRoutes($this->getRoutes(), $supportsRedirections, true), "\n");
117+
118+
return <<<EOF
119+
public function matchWithTrailingSlashManagement(\$rawPathinfo)
100120
{
101121
\$allow = [];
102122
\$pathinfo = rawurldecode(\$rawPathinfo);
@@ -109,6 +129,30 @@ public function match(\$rawPathinfo)
109129
\$canonicalMethod = 'GET';
110130
}
111131
132+
$code
133+
134+
throw 0 < count(\$allow) ? new MethodNotAllowedException(array_unique(\$allow)) : new ResourceNotFoundException();
135+
}
136+
EOF;
137+
}
138+
139+
private function generateMatchWithoutTrailingSlashManagement($supportsRedirections)
140+
{
141+
$code = rtrim($this->compileRoutes($this->getRoutes(), $supportsRedirections, false), "\n");
142+
143+
return <<<EOF
144+
public function matchWithoutTrailingSlashManagement(\$rawPathinfo)
145+
{
146+
\$allow = [];
147+
\$pathinfo = rawurldecode(\$rawPathinfo);
148+
\$context = \$this->context;
149+
\$request = \$this->request ?: \$this->createRequest(\$pathinfo);
150+
\$requestMethod = \$canonicalMethod = \$context->getMethod();
151+
152+
if ('HEAD' === \$requestMethod) {
153+
\$canonicalMethod = 'GET';
154+
}
155+
112156
$code
113157
114158
throw 0 < count(\$allow) ? new MethodNotAllowedException(array_unique(\$allow)) : new ResourceNotFoundException();
@@ -124,7 +168,7 @@ public function match(\$rawPathinfo)
124168
*
125169
* @return string PHP code
126170
*/
127-
private function compileRoutes(RouteCollection $routes, $supportsRedirections)
171+
private function compileRoutes(RouteCollection $routes, $supportsRedirections, $trailingSlashManagement = true)
128172
{
129173
$fetchedHost = false;
130174
$groups = $this->groupRoutesByHostRegex($routes);
@@ -141,7 +185,7 @@ private function compileRoutes(RouteCollection $routes, $supportsRedirections)
141185
}
142186

143187
$tree = $this->buildStaticPrefixCollection($collection);
144-
$groupCode = $this->compileStaticPrefixRoutes($tree, $supportsRedirections);
188+
$groupCode = $this->compileStaticPrefixRoutes($tree, $supportsRedirections, $trailingSlashManagement);
145189

146190
if (null !== $regex) {
147191
// apply extra indention at each line (except empty ones)
@@ -184,7 +228,7 @@ private function buildStaticPrefixCollection(DumperCollection $collection)
184228
*
185229
* @return string PHP code
186230
*/
187-
private function compileStaticPrefixRoutes(StaticPrefixCollection $collection, $supportsRedirections, $ifOrElseIf = 'if')
231+
private function compileStaticPrefixRoutes(StaticPrefixCollection $collection, $supportsRedirections, $trailingSlashManagement, $ifOrElseIf = 'if')
188232
{
189233
$code = '';
190234
$prefix = $collection->getPrefix();
@@ -200,7 +244,7 @@ private function compileStaticPrefixRoutes(StaticPrefixCollection $collection, $
200244
$code .= $this->compileStaticPrefixRoutes($route, $supportsRedirections, $ifOrElseIf);
201245
$ifOrElseIf = 'elseif';
202246
} else {
203-
$code .= $this->compileRoute($route[1]->getRoute(), $route[1]->getName(), $supportsRedirections, $prefix)."\n";
247+
$code .= $this->compileRoute($route[1]->getRoute(), $route[1]->getName(), $supportsRedirections, $trailingSlashManagement, $prefix)."\n";
204248
$ifOrElseIf = 'if';
205249
}
206250
}
@@ -226,7 +270,7 @@ private function compileStaticPrefixRoutes(StaticPrefixCollection $collection, $
226270
*
227271
* @throws \LogicException
228272
*/
229-
private function compileRoute(Route $route, $name, $supportsRedirections, $parentPrefix = null)
273+
private function compileRoute(Route $route, $name, $supportsRedirections, $trailingSlashManagement, $parentPrefix = null)
230274
{
231275
$code = '';
232276
$compiledRoute = $route->compile();
@@ -236,7 +280,7 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren
236280
$hostMatches = false;
237281
$methods = $route->getMethods();
238282

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

242286
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 & 2 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,14 +27,22 @@ 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.'/');
37+
$parameters = $this->matchCollectionByTrailingSlashSupport($pathinfo.'/', true);
38+
39+
return array_replace($parameters, $this->redirect($pathinfo.'/', isset($parameters['_route']) ? $parameters['_route'] : null));
40+
} catch (ResourceNotFoundException $e2) {
41+
throw $e;
42+
}
43+
} catch (MethodNotAllowedException $e) {
44+
try {
45+
$parameters = $this->matchCollectionByTrailingSlashSupport($pathinfo.'/', true);
3646

3747
return array_replace($parameters, $this->redirect($pathinfo.'/', isset($parameters['_route']) ? $parameters['_route'] : null));
3848
} catch (ResourceNotFoundException $e2) {
@@ -62,4 +72,20 @@ protected function handleRouteRequirements($pathinfo, $name, Route $route)
6272

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

‎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
D306 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