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

Commit 8b1ff21

Browse files
author
Xavier Leune
committed
[Router] Fix TraceableUrlMatcher behaviour with trailing slash
1 parent ef0119a commit 8b1ff21

File tree

2 files changed

+64
-34
lines changed

2 files changed

+64
-34
lines changed

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

Lines changed: 58 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,40 @@ public function getTracesForRequest(Request $request)
5252

5353
protected function matchCollection($pathinfo, RouteCollection $routes)
5454
{
55+
// HEAD and GET are equivalent as per RFC
56+
if ('HEAD' === $method = $this->context->getMethod()) {
57+
$method = 'GET';
58+
}
59+
$supportsTrailingSlash = '/' !== $pathinfo && '' !== $pathinfo && $this instanceof RedirectableUrlMatcherInterface;
60+
5561
foreach ($routes as $name => $route) {
5662
$compiledRoute = $route->compile();
63+
$staticPrefix = $compiledRoute->getStaticPrefix();
64+
$requiredMethods = $route->getMethods();
65+
66+
// check the static prefix of the URL first. Only use the more expensive preg_match when it matches
67+
if ('' === $staticPrefix || 0 === strpos($pathinfo, $staticPrefix)) {
68+
// no-op
69+
} elseif (!$supportsTrailingSlash || ($requiredMethods && !\in_array('GET', $requiredMethods)) || 'GET' !== $method) {
70+
$this->addTrace(sprintf('Path "%s" does not match', $route->getPath()), self::ROUTE_DOES_NOT_MATCH, $name, $route);
71+
continue;
72+
} elseif ('/' === substr($staticPrefix, -1) && substr($staticPrefix, 0, -1) === $pathinfo) {
73+
$this->addTrace('Route matches!', self::ROUTE_MATCHES, $name, $route);
74+
return $this->allow = [];
75+
} else {
76+
$this->addTrace(sprintf('Path "%s" does not match', $route->getPath()), self::ROUTE_DOES_NOT_MATCH, $name, $route);
77+
continue;
78+
}
79+
$regex = $compiledRoute->getRegex();
80+
81+
if ($supportsTrailingSlash && $pos = strpos($regex, '/$')) {
82+
$regex = substr($regex, 0, $pos).'/?$'.substr($regex, $pos + 2);
83+
$hasTrailingSlash = true;
84+
} else {
85+
$hasTrailingSlash = false;
86+
}
5787

58-
if (!preg_match($compiledRoute->getRegex(), $pathinfo, $matches)) {
88+
if (!preg_match($regex, $pathinfo, $matches)) {
5989
// does it match without any requirements?
6090
$r = new Route($route->getPath(), $route->getDefaults(), [], $route->getOptions());
6191
$cr = $r->compile();
@@ -79,54 +109,50 @@ protected function matchCollection($pathinfo, RouteCollection $routes)
79109
continue;
80110
}
81111

82-
// check host requirement
112+
if ($hasTrailingSlash && '/' !== substr($pathinfo, -1)) {
113+
if ((!$requiredMethods || \in_array('GET', $requiredMethods)) && 'GET' === $method) {
114+
$this->addTrace('Route matches!', self::ROUTE_MATCHES, $name, $route);
115+
return $this->allow = [];
116+
}
117+
$this->addTrace(sprintf('Method "%s" does not match any of the required methods (%s)', $this->context->getMethod(), implode(', ', $requiredMethods)), self::ROUTE_ALMOST_MATCHES, $name, $route);
118+
continue;
119+
}
120+
83121
$hostMatches = [];
84122
if ($compiledRoute->getHostRegex() && !preg_match($compiledRoute->getHostRegex(), $this->context->getHost(), $hostMatches)) {
85123
$this->addTrace(sprintf('Host "%s" does not match the requirement ("%s")', $this->context->getHost(), $route->getHost()), self::ROUTE_ALMOST_MATCHES, $name, $route);
86-
87124
continue;
88125
}
89126

90-
// check HTTP method requirement
91-
if ($requiredMethods = $route->getMethods()) {
92-
// HEAD and GET are equivalent as per RFC
93-
if ('HEAD' === $method = $this->context->getMethod()) {
94-
$method = 'GET';
95-
}
96-
97-
if (!\in_array($method, $requiredMethods)) {
98-
$this->allow = array_merge($this->allow, $requiredMethods);
99-
100-
$this->addTrace(sprintf('Method "%s" does not match any of the required methods (%s)', $this->context->getMethod(), implode(', ', $requiredMethods)), self::ROUTE_ALMOST_MATCHES, $name, $route);
127+
$status = $this->handleRouteRequirements($pathinfo, $name, $route);
101128

102-
continue;
129+
if (self::REQUIREMENT_MISMATCH === $status[0]) {
130+
if ($route->getCondition()) {
131+
$this->addTrace(sprintf('Condition "%s" does not evaluate to "true"', $route->getCondition()), self::ROUTE_ALMOST_MATCHES, $name, $route);
132+
} else {
133+
$this->addTrace(sprintf('Scheme "%s" does not match any of the required schemes (%s); the user will be redirected to first required scheme', $this->getContext()->getScheme(), implode(', ', $route->getSchemes())), self::ROUTE_ALMOST_MATCHES, $name, $route);
103134
}
104-
}
105135

106-
// check condition
107-
if ($condition = $route->getCondition()) {
108-
if (!$this->getExpressionLanguage()->evaluate($condition, ['context' => $this->context, 'request' => $this->request ?: $this->createRequest($pathinfo)])) {
109-
$this->addTrace(sprintf('Condition "%s" does not evaluate to "true"', $condition), self::ROUTE_ALMOST_MATCHES, $name, $route);
110-
111-
continue;
112-
}
136+
continue;
113137
}
114138

115-
// check HTTP scheme requirement
116-
if ($requiredSchemes = $route->getSchemes()) {
117-
$scheme = $this->context->getScheme();
118-
119-
if (!$route->hasScheme($scheme)) {
120-
$this->addTrace(sprintf('Scheme "%s" does not match any of the required schemes (%s); the user will be redirected to first required scheme', $scheme, implode(', ', $requiredSchemes)), self::ROUTE_ALMOST_MATCHES, $name, $route);
139+
// check HTTP method requirement
140+
if ($requiredMethods) {
141+
if (!\in_array($method, $requiredMethods)) {
142+
if (self::REQUIREMENT_MATCH === $status[0]) {
143+
$this->allow = array_merge($this->allow, $requiredMethods);
144+
}
145+
$this->addTrace(sprintf('Method "%s" does not match any of the required methods (%s)', $this->context->getMethod(), implode(', ', $requiredMethods)), self::ROUTE_ALMOST_MATCHES, $name, $route);
121146

122-
return true;
147+
continue;
123148
}
124149
}
125150

126151
$this->addTrace('Route matches!', self::ROUTE_MATCHES, $name, $route);
127-
128-
return true;
152+
return $this->getAttributes($route, $name, array_replace($matches, $hostMatches, isset($status[1]) ? $status[1] : []));
129153
}
154+
155+
return [];
130156
}
131157

132158
private function addTrace($log, $level = self::ROUTE_DOES_NOT_MATCH, $name = null, $route = null)

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,13 @@
1111

1212
namespace Symfony\Component\Routing\Tests\Matcher;
1313

14-
use PHPUnit\Framework\TestCase;
1514
use Symfony\Component\HttpFoundation\Request;
1615
use Symfony\Component\Routing\Matcher\TraceableUrlMatcher;
1716
use Symfony\Component\Routing\RequestContext;
1817
use Symfony\Component\Routing\Route;
1918
use Symfony\Component\Routing\RouteCollection;
2019

21-
class TraceableUrlMatcherTest extends TestCase
20+
class TraceableUrlMatcherTest extends UrlMatcherTest
2221
{
2322
public function test()
2423
{
@@ -119,4 +118,9 @@ public function testRoutesWithConditions()
119118
$traces = $matcher->getTracesForRequest($matchingRequest);
120119
$this->assertEquals('Route matches!', $traces[0]['log']);
121120
}
121+
122+
protected function getUrlMatcher(RouteCollection $routes, RequestContext $context = null)
123+
{
124+
return new TraceableUrlMatcher($routes, $context ?: new RequestContext());
125+
}
122126
}

0 commit comments

Comments
 (0)
0