Skip to content

Commit 30e3863

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '3.4' into 4.3
* 3.4: Fix inconsistent return points. [Security/Core] UserInterface::getPassword() can return null [Router] Fix TraceableUrlMatcher behaviour with trailing slash
2 parents d662423 + 9458f1f commit 30e3863

File tree

3 files changed

+61
-32
lines changed

3 files changed

+61
-32
lines changed

‎Generator/UrlGenerator.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,8 @@ public function generate($name, $parameters = [], $referenceType = self::ABSOLUT
154154
* @throws MissingMandatoryParametersException When some parameters are missing that are mandatory for the route
155155
* @throws InvalidParameterException When a parameter value for a placeholder is not correct because
156156
* it does not match the requirement
157+
*
158+
* @return string
157159
*/
158160
protected function doGenerate($variables, $defaults, $requirements, $tokens, $parameters, $name, $referenceType, $hostTokens, array $requiredSchemes = [])
159161
{

‎Matcher/TraceableUrlMatcher.php

Lines changed: 53 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,30 @@ 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 = 'GET' === $method && $this instanceof RedirectableUrlMatcherInterface;
60+
$trimmedPathinfo = rtrim($pathinfo, '/') ?: '/';
61+
5562
foreach ($routes as $name => $route) {
5663
$compiledRoute = $route->compile();
64+
$staticPrefix = rtrim($compiledRoute->getStaticPrefix(), '/');
65+
$requiredMethods = $route->getMethods();
5766

58-
if (!preg_match($compiledRoute->getRegex(), $pathinfo, $matches)) {
67+
// check the static prefix of the URL first. Only use the more expensive preg_match when it matches
68+
if ('' !== $staticPrefix && 0 !== strpos($trimmedPathinfo, $staticPrefix)) {
69+
$this->addTrace(sprintf('Path "%s" does not match', $route->getPath()), self::ROUTE_DOES_NOT_MATCH, $name, $route);
70+
continue;
71+
}
72+
$regex = $compiledRoute->getRegex();
73+
74+
$pos = strrpos($regex, '$');
75+
$hasTrailingSlash = '/' === $regex[$pos - 1];
76+
$regex = substr_replace($regex, '/?$', $pos - $hasTrailingSlash, 1 + $hasTrailingSlash);
77+
78+
if (!preg_match($regex, $pathinfo, $matches)) {
5979
// does it match without any requirements?
6080
$r = new Route($route->getPath(), $route->getDefaults(), [], $route->getOptions());
6181
$cr = $r->compile();
@@ -79,54 +99,57 @@ protected function matchCollection($pathinfo, RouteCollection $routes)
7999
continue;
80100
}
81101

82-
// check host requirement
102+
$hasTrailingVar = $trimmedPathinfo !== $pathinfo && preg_match('#\{\w+\}/?$#', $route->getPath());
103+
104+
if ($hasTrailingVar && ($hasTrailingSlash || (null === $m = $matches[\count($compiledRoute->getPathVariables())] ?? null) || '/' !== ($m[-1] ?? '/')) && preg_match($regex, $trimmedPathinfo, $m)) {
105+
if ($hasTrailingSlash) {
106+
$matches = $m;
107+
} else {
108+
$hasTrailingVar = false;
109+
}
110+
}
111+
83112
$hostMatches = [];
84113
if ($compiledRoute->getHostRegex() && !preg_match($compiledRoute->getHostRegex(), $this->context->getHost(), $hostMatches)) {
85114
$this->addTrace(sprintf('Host "%s" does not match the requirement ("%s")', $this->context->getHost(), $route->getHost()), self::ROUTE_ALMOST_MATCHES, $name, $route);
86-
87115
continue;
88116
}
89117

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);
118+
$status = $this->handleRouteRequirements($pathinfo, $name, $route);
99119

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);
101-
102-
continue;
103-
}
120+
if (self::REQUIREMENT_MISMATCH === $status[0]) {
121+
$this->addTrace(sprintf('Condition "%s" does not evaluate to "true"', $route->getCondition()), self::ROUTE_ALMOST_MATCHES, $name, $route);
122+
continue;
104123
}
105124

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);
125+
if ('/' !== $pathinfo && !$hasTrailingVar && $hasTrailingSlash === ($trimmedPathinfo === $pathinfo)) {
126+
if ($supportsTrailingSlash && (!$requiredMethods || \in_array('GET', $requiredMethods))) {
127+
$this->addTrace('Route matches!', self::ROUTE_MATCHES, $name, $route);
110128

111-
continue;
129+
return $this->allow = $this->allowSchemes = [];
112130
}
131+
$this->addTrace(sprintf('Path "%s" does not match', $route->getPath()), self::ROUTE_DOES_NOT_MATCH, $name, $route);
132+
continue;
113133
}
114134

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);
135+
if ($route->getSchemes() && !$route->hasScheme($this->context->getScheme())) {
136+
$this->allowSchemes = array_merge($this->allowSchemes, $route->getSchemes());
137+
$this->addTrace(sprintf('Scheme "%s" does not match any of the required schemes (%s)', $this->context->getScheme(), implode(', ', $route->getSchemes())), self::ROUTE_ALMOST_MATCHES, $name, $route);
138+
continue;
139+
}
121140

122-
return true;
123-
}
141+
if ($requiredMethods && !\in_array($method, $requiredMethods)) {
142+
$this->allow = array_merge($this->allow, $requiredMethods);
143+
$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);
144+
continue;
124145
}
125146

126147
$this->addTrace('Route matches!', self::ROUTE_MATCHES, $name, $route);
127148

128-
return true;
149+
return $this->getAttributes($route, $name, array_replace($matches, $hostMatches, isset($status[1]) ? $status[1] : []));
129150
}
151+
152+
return [];
130153
}
131154

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

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