Skip to content

Commit 858d356

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '4.1'
* 4.1: (23 commits) [Routing] fix trailing slash redirection when using RedirectableUrlMatcher [PropertyAccessor] fix encoding of cache keys [WebProfiler] Detect empty file paths in file viewer fixed CS Changes for upcoming Travis' infra migration Doc fix: clarify isMethodCacheable() returns true only for GET & HEAD [MonologBridge] Return empty list for unknonwn requests [DomCrawler] exclude fields inside "template" tags Use XLIFF source rather than resname when there's no target [DoctrineBridge] catch errors while converting to db values in data collector [DoctrineBridge] fix case sensitivity issue in RememberMe\DoctrineTokenProvider [EventDispatcher] Unwrap wrapped listeners internally [Routing] fix trailing slash redirection when using RedirectableUrlMatcher Removed the return type phpdoc fix authorization checker variable name [Routing] Remove duplicate schemes and methods for invokable controllers Indentation error [HttpFoundation] Fix trailing space for mime-type with parameters [HttpFoundation] Fixed absolute Request URI with default port [Bridge/PhpUnit] fix the fix ...
2 parents 7af6518 + eebc037 commit 858d356

20 files changed

+1365
-1285
lines changed

‎Loader/AnnotationClassLoader.php

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -120,12 +120,9 @@ public function load($class, $type = null)
120120
}
121121

122122
if (0 === $collection->count() && $class->hasMethod('__invoke')) {
123+
$globals = $this->resetGlobals();
123124
foreach ($this->reader->getClassAnnotations($class) as $annot) {
124125
if ($annot instanceof $this->routeAnnotationClass) {
125-
$globals['path'] = '';
126-
$globals['name'] = '';
127-
$globals['localized_paths'] = array();
128-
129126
$this->addRoute($collection, $annot, $globals, $class, $class->getMethod('__invoke'));
130127
}
131128
}
@@ -262,18 +259,7 @@ protected function getDefaultRouteName(\ReflectionClass $class, \ReflectionMetho
262259

263260
protected function getGlobals(\ReflectionClass $class)
264261
{
265-
$globals = array(
266-
'path' => null,
267-
'localized_paths' => array(),
268-
'requirements' => array(),
269-
'options' => array(),
270-
'defaults' => array(),
271-
'schemes' => array(),
272-
'methods' => array(),
273-
'host' => '',
274-
'condition' => '',
275-
'name' => '',
276-
);
262+
$globals = $this->resetGlobals();
277263

278264
if ($annot = $this->reader->getClassAnnotation($class, $this->routeAnnotationClass)) {
279265
if (null !== $annot->getName()) {
@@ -324,6 +310,22 @@ protected function getGlobals(\ReflectionClass $class)
324310
return $globals;
325311
}
326312

313+
private function resetGlobals()
314+
{
315+
return array(
316+
'path' => null,
317+
'localized_paths' => array(),
318+
'requirements' => array(),
319+
'options' => array(),
320+
'defaults' => array(),
321+
'schemes' => array(),
322+
'methods' => array(),
323+
'host' => '',
324+
'condition' => '',
325+
'name' => '',
326+
);
327+
}
328+
327329
protected function createRoute($path, $defaults, $requirements, $options, $host, $schemes, $methods, $condition)
328330
{
329331
return new Route($path, $defaults, $requirements, $options, $host, $schemes, $methods, $condition);

‎Matcher/Dumper/PhpMatcherDumper.php

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -161,9 +161,18 @@ private function groupStaticRoutes(RouteCollection $collection): array
161161
$compiledRoute = $route->compile();
162162
$hostRegex = $compiledRoute->getHostRegex();
163163
$regex = $compiledRoute->getRegex();
164+
if ($hasTrailingSlash = '/' !== $route->getPath()) {
165+
$pos = strrpos($regex, '$');
166+
$hasTrailingSlash = '/' === $regex[$pos - 1];
167+
$regex = substr_replace($regex, '/?$', $pos - $hasTrailingSlash, 1 + $hasTrailingSlash);
168+
}
169+
164170
if (!$compiledRoute->getPathVariables()) {
165171
$host = !$compiledRoute->getHostVariables() ? $route->getHost() : '';
166172
$url = $route->getPath();
173+
if ($hasTrailingSlash) {
174+
$url = substr($url, 0, -1);
175+
}
167176
foreach ($dynamicRegex as list($hostRx, $rx)) {
168177
if (preg_match($rx, $url) && (!$host || !$hostRx || preg_match($hostRx, $host))) {
169178
$dynamicRegex[] = array($hostRegex, $regex);
@@ -172,7 +181,7 @@ private function groupStaticRoutes(RouteCollection $collection): array
172181
}
173182
}
174183

175-
$staticRoutes[$url][$name] = $route;
184+
$staticRoutes[$url][$name] = array($route, $hasTrailingSlash);
176185
} else {
177186
$dynamicRegex[] = array($hostRegex, $regex);
178187
$dynamicRoutes->add($name, $route);
@@ -199,8 +208,8 @@ private function compileStaticRoutes(array $staticRoutes, array &$conditions): s
199208

200209
foreach ($staticRoutes as $url => $routes) {
201210
$code .= self::export($url)." => array(\n";
202-
foreach ($routes as $name => $route) {
203-
$code .= $this->compileRoute($route, $name, !$route->compile()->getHostVariables() ? $route->getHost() : $route->compile()->getHostRegex() ?: null, $conditions);
211+
foreach ($routes as $name => list($route, $hasTrailingSlash)) {
212+
$code .= $this->compileRoute($route, $name, !$route->compile()->getHostVariables() ? $route->getHost() : $route->compile()->getHostRegex() ?: null, $hasTrailingSlash, $conditions);
204213
}
205214
$code .= "),\n";
206215
}
@@ -309,7 +318,11 @@ private function compileDynamicRoutes(RouteCollection $collection, bool $matchHo
309318

310319
$state->vars = array();
311320
$regex = preg_replace_callback('#\?P<([^>]++)>#', $state->getVars, $rx[1]);
312-
$tree->addRoute($regex, array($name, $regex, $state->vars, $route));
321+
if ($hasTrailingSlash = '/' !== $regex && '/' === $regex[-1]) {
322+
$regex = substr($regex, 0, -1);
323+
}
324+
325+
$tree->addRoute($regex, array($name, $regex, $state->vars, $route, $hasTrailingSlash));
313326
}
314327

315328
$code .= $this->compileStaticPrefixCollection($tree, $state, 0, $conditions);
@@ -318,7 +331,7 @@ private function compileDynamicRoutes(RouteCollection $collection, bool $matchHo
318331
$code .= "\n .')'";
319332
$state->regex .= ')';
320333
}
321-
$rx = ")$}{$modifiers}";
334+
$rx = ")(?:/?)$}{$modifiers}";
322335
$code .= "\n .'{$rx}',";
323336
$state->regex .= $rx;
324337
$state->markTail = 0;
@@ -364,12 +377,12 @@ private function compileStaticPrefixCollection(StaticPrefixCollection $tree, \st
364377
continue;
365378
}
366379

367-
list($name, $regex, $vars, $route) = $route;
380+
list($name, $regex, $vars, $route, $hasTrailingSlash) = $route;
368381
$compiledRoute = $route->compile();
369382
$vars = array_merge($state->hostVars, $vars);
370383

371384
if ($compiledRoute->getRegex() === $prevRegex) {
372-
$state->routes = substr_replace($state->routes, $this->compileRoute($route, $name, $vars, $conditions), -3, 0);
385+
$state->routes = substr_replace($state->routes, $this->compileRoute($route, $name, $vars, $hasTrailingSlash, $conditions), -3, 0);
373386
continue;
374387
}
375388

@@ -380,7 +393,7 @@ private function compileStaticPrefixCollection(StaticPrefixCollection $tree, \st
380393
$state->regex .= $rx;
381394

382395
$prevRegex = $compiledRoute->getRegex();
383-
$state->routes .= sprintf("%s => array(\n%s),\n", $state->mark, $this->compileRoute($route, $name, $vars, $conditions));
396+
$state->routes .= sprintf("%s => array(\n%s),\n", $state->mark, $this->compileRoute($route, $name, $vars, $hasTrailingSlash, $conditions));
384397
}
385398

386399
return $code;
@@ -389,7 +402,7 @@ private function compileStaticPrefixCollection(StaticPrefixCollection $tree, \st
389402
/**
390403
* Compiles a single Route to PHP code used to match it against the path info.
391404
*/
392-
private function compileRoute(Route $route, string $name, $vars, array &$conditions): string
405+
private function compileRoute(Route $route, string $name, $vars, bool $hasTrailingSlash, array &$conditions): string
393406
{
394407
$defaults = $route->getDefaults();
395408

@@ -406,11 +419,12 @@ private function compileRoute(Route $route, string $name, $vars, array &$conditi
406419
}
407420

408421
return sprintf(
409-
" array(%s, %s, %s, %s, %s),\n",
422+
" array(%s, %s, %s, %s, %s, %s),\n",
410423
self::export(array('_route' => $name) + $defaults),
411424
self::export($vars),
412425
self::export(array_flip($route->getMethods()) ?: null),
413426
self::export(array_flip($route->getSchemes()) ?: null),
427+
self::export($hasTrailingSlash),
414428
$condition
415429
);
416430
}

‎Matcher/Dumper/PhpMatcherTrait.php

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,10 @@ public function match($pathinfo)
7070
private function doMatch(string $rawPathinfo, array &$allow = array(), array &$allowSchemes = array()): ?array
7171
{
7272
$allow = $allowSchemes = array();
73-
$pathinfo = rawurldecode($rawPathinfo);
73+
$pathinfo = rawurldecode($rawPathinfo) ?: '/';
7474
$context = $this->context;
7575
$requestMethod = $canonicalMethod = $context->getMethod();
76+
$trimmedPathinfo = '/' !== $pathinfo && '/' === $pathinfo[-1] ? substr($pathinfo, 0, -1) : $pathinfo;
7677

7778
if ($this->matchHost) {
7879
$host = strtolower($context->getHost());
@@ -82,11 +83,19 @@ private function doMatch(string $rawPathinfo, array &$allow = array(), array &$a
8283
$canonicalMethod = 'GET';
8384
}
8485

85-
foreach ($this->staticRoutes[$pathinfo] ?? array() as list($ret, $requiredHost, $requiredMethods, $requiredSchemes, $condition)) {
86+
foreach ($this->staticRoutes[$trimmedPathinfo] ?? array() as list($ret, $requiredHost, $requiredMethods, $requiredSchemes, $hasTrailingSlash, $condition)) {
8687
if ($condition && !($this->checkCondition)($condition, $context, 0 < $condition ? $request ?? $request = $this->request ?: $this->createRequest($pathinfo) : null)) {
8788
continue;
8889
}
8990

91+
if ('/' === $pathinfo || $hasTrailingSlash === ('/' === $pathinfo[-1])) {
92+
// no-op
93+
} elseif ($this instanceof RedirectableUrlMatcherInterface) {
94+
return null;
95+
} else {
96+
continue;
97+
}
98+
9099
if ($requiredHost) {
91100
if ('#' !== $requiredHost[0] ? $requiredHost !== $host : !preg_match($requiredHost, $host, $hostMatches)) {
92101
continue;
@@ -116,11 +125,19 @@ private function doMatch(string $rawPathinfo, array &$allow = array(), array &$a
116125

117126
foreach ($this->regexpList as $offset => $regex) {
118127
while (preg_match($regex, $matchedPathinfo, $matches)) {
119-
foreach ($this->dynamicRoutes[$m = (int) $matches['MARK']] as list($ret, $vars, $requiredMethods, $requiredSchemes, $condition)) {
128+
foreach ($this->dynamicRoutes[$m = (int) $matches['MARK']] as list($ret, $vars, $requiredMethods, $requiredSchemes, $hasTrailingSlash, $condition)) {
120129
if ($condition && !($this->checkCondition)($condition, $context, 0 < $condition ? $request ?? $request = $this->request ?: $this->createRequest($pathinfo) : null)) {
121130
continue;
122131
}
123132

133+
if ('/' === $pathinfo || $hasTrailingSlash === ('/' === $pathinfo[-1])) {
134+
// no-op
135+
} elseif ($this instanceof RedirectableUrlMatcherInterface) {
136+
return null;
137+
} else {
138+
continue;
139+
}
140+
124141
foreach ($vars as $i => $v) {
125142
if (isset($matches[1 + $i])) {
126143
$ret[$v] = $matches[1 + $i];

‎Matcher/UrlMatcher.php

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,18 +130,40 @@ public function addExpressionLanguageProvider(ExpressionFunctionProviderInterfac
130130
*/
131131
protected function matchCollection($pathinfo, RouteCollection $routes)
132132
{
133+
$supportsTrailingSlash = '/' !== $pathinfo && '' !== $pathinfo && $this instanceof RedirectableUrlMatcherInterface;
134+
133135
foreach ($routes as $name => $route) {
134136
$compiledRoute = $route->compile();
137+
$staticPrefix = $compiledRoute->getStaticPrefix();
135138

136139
// check the static prefix of the URL first. Only use the more expensive preg_match when it matches
137-
if ('' !== $compiledRoute->getStaticPrefix() && 0 !== strpos($pathinfo, $compiledRoute->getStaticPrefix())) {
140+
if ('' === $staticPrefix || 0 === strpos($pathinfo, $staticPrefix)) {
141+
// no-op
142+
} elseif (!$supportsTrailingSlash) {
143+
continue;
144+
} elseif ('/' === $staticPrefix[-1] && substr($staticPrefix, 0, -1) === $pathinfo) {
145+
return;
146+
} elseif ('/' === $pathinfo[-1] && substr($pathinfo, 0, -1) === $staticPrefix) {
147+
return;
148+
} else {
138149
continue;
139150
}
151+
$regex = $compiledRoute->getRegex();
152+
153+
if ($supportsTrailingSlash) {
154+
$pos = strrpos($regex, '$');
155+
$hasTrailingSlash = '/' === $regex[$pos - 1];
156+
$regex = substr_replace($regex, '/?$', $pos - $hasTrailingSlash, 1 + $hasTrailingSlash);
157+
}
140158

141-
if (!preg_match($compiledRoute->getRegex(), $pathinfo, $matches)) {
159+
if (!preg_match($regex, $pathinfo, $matches)) {
142160
continue;
143161
}
144162

163+
if ($supportsTrailingSlash && $hasTrailingSlash !== ('/' === $pathinfo[-1])) {
164+
return;
165+
}
166+
145167
$hostMatches = array();
146168
if ($compiledRoute->getHostRegex() && !preg_match($compiledRoute->getHostRegex(), $this->context->getHost(), $hostMatches)) {
147169
continue;

‎Tests/Fixtures/AnnotationFixtures/InvokableController.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
use Symfony\Component\Routing\Annotation\Route;
66

77
/**
8-
* @Route("/here", name="lol")
8+
* @Route("/here", name="lol", methods={"GET", "POST"}, schemes={"https"})
99
*/
1010
class InvokableController
1111
{

0 commit comments

Comments
 (0)