bug #31107 [Routing] fix trailing slash redirection with non-greedy trailing vars (nicolas-grekas)

This PR was merged into the 4.2 branch.

Discussion
----------

[Routing] fix trailing slash redirection with non-greedy trailing vars

| Q             | A
| ------------- | ---
| Branch?       | 4.2
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #30863, #31066
| License       | MIT
| Doc PR        | -

Fixes redirecting `/123/` to `/123` when the route is defined as `/{foo<\d+>}`

Commits
-------

d88833d27a [Routing] fix trailing slash redirection with non-greedy trailing vars
This commit is contained in:
Nicolas Grekas 2019-04-17 17:02:08 +02:00
commit 2d2ff38f1d
3 changed files with 28 additions and 8 deletions

View File

@ -131,6 +131,15 @@ trait PhpMatcherTrait
}
$hasTrailingVar = $trimmedPathinfo !== $pathinfo && $hasTrailingVar;
if ($hasTrailingVar && ($hasTrailingSlash || '/' !== substr($matches[\count($vars)], -1)) && preg_match($regex, $this->matchHost ? $host.'.'.$trimmedPathinfo : $trimmedPathinfo, $n) && $m === (int) $n['MARK']) {
if ($hasTrailingSlash) {
$matches = $n;
} else {
$hasTrailingVar = false;
}
}
if ('/' !== $pathinfo && !$hasTrailingVar && $hasTrailingSlash === ($trimmedPathinfo === $pathinfo)) {
if ($supportsRedirections && (!$requiredMethods || isset($requiredMethods['GET']))) {
return $allow = $allowSchemes = [];
@ -138,10 +147,6 @@ trait PhpMatcherTrait
continue;
}
if ($hasTrailingSlash && $hasTrailingVar && preg_match($regex, $this->matchHost ? $host.'.'.$trimmedPathinfo : $trimmedPathinfo, $n) && $m === (int) $n['MARK']) {
$matches = $n;
}
foreach ($vars as $i => $v) {
if (isset($matches[1 + $i])) {
$ret[$v] = $matches[1 + $i];

View File

@ -158,6 +158,14 @@ class UrlMatcher implements UrlMatcherInterface, RequestMatcherInterface
$hasTrailingVar = $trimmedPathinfo !== $pathinfo && preg_match('#\{\w+\}/?$#', $route->getPath());
if ($hasTrailingVar && ($hasTrailingSlash || '/' !== substr($matches[(\count($matches) - 1) >> 1], -1)) && preg_match($regex, $trimmedPathinfo, $m)) {
if ($hasTrailingSlash) {
$matches = $m;
} else {
$hasTrailingVar = false;
}
}
if ('/' !== $pathinfo && !$hasTrailingVar && $hasTrailingSlash === ($trimmedPathinfo === $pathinfo)) {
if ($supportsTrailingSlash && (!$requiredMethods || \in_array('GET', $requiredMethods))) {
return $this->allow = $this->allowSchemes = [];
@ -166,10 +174,6 @@ class UrlMatcher implements UrlMatcherInterface, RequestMatcherInterface
continue;
}
if ($hasTrailingSlash && $hasTrailingVar && preg_match($regex, $trimmedPathinfo, $m)) {
$matches = $m;
}
$hostMatches = [];
if ($compiledRoute->getHostRegex() && !preg_match($compiledRoute->getHostRegex(), $this->context->getHost(), $hostMatches)) {
continue;

View File

@ -187,6 +187,17 @@ class RedirectableUrlMatcherTest extends UrlMatcherTest
$this->assertEquals($expected, $matcher->match('/api/customers/123/contactpersons'));
}
public function testNonGreedyTrailingRequirement()
{
$coll = new RouteCollection();
$coll->add('a', new Route('/{a}', [], ['a' => '\d+']));
$matcher = $this->getUrlMatcher($coll);
$matcher->expects($this->once())->method('redirect')->with('/123')->willReturn([]);
$this->assertEquals(['_route' => 'a', 'a' => '123'], $matcher->match('/123/'));
}
protected function getUrlMatcher(RouteCollection $routes, RequestContext $context = null)
{
return $this->getMockForAbstractClass('Symfony\Component\Routing\Matcher\RedirectableUrlMatcher', [$routes, $context ?: new RequestContext()]);