diogo/symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

bug #31182 [Routing] fix trailing slash matching with empty-matching trailing vars (nicolas-grekas)

Browse Source 
This PR was merged into the 4.2 branch.

Discussion
----------

[Routing] fix trailing slash matching with empty-matching trailing vars

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

Reported by @bmack in https://github.com/symfony/symfony/pull/31107#issuecomment-484681404

This highlights a small inconsistency that exists for a long time (checked on 2.7 at least):
`new Route('/en-en/{b}', ['b' => 'bbb'], ['b' => '.*'])` matches `/en-en/`
`new Route('/en-en/{b}', ['b' => 'bbb'], ['b' => '.+'])` doesn't match it
(while both match `/en-en` and `/en-en/foo`)

This PR ensures the former behavior is preserved, while #31167 redirects the later to `/en-en`.

Commits
-------

d6da21ac19 [Routing] fix trailing slash matching with empty-matching trailing vars
This commit is contained in:
Nicolas Grekas 2019-04-19 16:27:49 +02:00
commit bff2db71e9
4 changed files with 41 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherTrait.php
View File

@ -132,7 +132,7 @@ trait PhpMatcherTrait
$hasTrailingVar = $trimmedPathinfo !== $pathinfo && $hasTrailingVar;
if ($hasTrailingVar && ($hasTrailingSlash || !($n = $matches[\count($vars)] ?? '') || '/' !== substr($n, -1)) && preg_match($regex, $this->matchHost ? $host.'.'.$trimmedPathinfo : $trimmedPathinfo, $n) && $m === (int) $n['MARK']) {
if ($hasTrailingVar && ($hasTrailingSlash || (null === $n = $matches[\count($vars)] ?? null) || '/' !== ($n[-1] ?? '/')) && preg_match($regex, $this->matchHost ? $host.'.'.$trimmedPathinfo : $trimmedPathinfo, $n) && $m === (int) $n['MARK']) {
if ($hasTrailingSlash) {
$matches = $n;
} else {

2
src/Symfony/Component/Routing/Matcher/UrlMatcher.php
View File

@ -158,7 +158,7 @@ class UrlMatcher implements UrlMatcherInterface, RequestMatcherInterface
$hasTrailingVar = $trimmedPathinfo !== $pathinfo && preg_match('#\{\w+\}/?$#', $route->getPath());
if ($hasTrailingVar && ($hasTrailingSlash || !($m = $matches[\count($compiledRoute->getPathVariables())] ?? '') || '/' !== substr($m, -1)) && preg_match($regex, $trimmedPathinfo, $m)) {
if ($hasTrailingVar && ($hasTrailingSlash || (null === $m = $matches[\count($compiledRoute->getPathVariables())] ?? null) || '/' !== ($m[-1] ?? '/')) && preg_match($regex, $trimmedPathinfo, $m)) {
if ($hasTrailingSlash) {
$matches = $m;
} else {

8
src/Symfony/Component/Routing/Tests/Matcher/RedirectableUrlMatcherTest.php
View File

@ -198,15 +198,15 @@ class RedirectableUrlMatcherTest extends UrlMatcherTest
$this->assertEquals(['_route' => 'a', 'a' => '123'], $matcher->match('/123/'));
}
public function testTrailingRequirementWithDefault()
public function testTrailingRequirementWithDefault_A()
{
$coll = new RouteCollection();
$coll->add('a', new Route('/foo/{a}', ['a' => 'bar'], ['a' => '.+']));
$coll->add('a', new Route('/fr-fr/{a}', ['a' => 'aaa'], ['a' => '.+']));
$matcher = $this->getUrlMatcher($coll);
$matcher->expects($this->once())->method('redirect')->with('/foo')->willReturn([]);
$matcher->expects($this->once())->method('redirect')->with('/fr-fr')->willReturn([]);
$this->assertEquals(['_route' => 'a', 'a' => 'bar'], $matcher->match('/foo/'));
$this->assertEquals(['_route' => 'a', 'a' => 'aaa'], $matcher->match('/fr-fr/'));
}
protected function getUrlMatcher(RouteCollection $routes, RequestContext $context = null)

35
src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php
View File

@ -757,6 +757,41 @@ class UrlMatcherTest extends TestCase
$this->assertEquals(['_route' => 'a', 'a' => 'foo/'], $matcher->match('/foo/'));
}
public function testTrailingRequirementWithDefault()
{
$coll = new RouteCollection();
$coll->add('a', new Route('/fr-fr/{a}', ['a' => 'aaa'], ['a' => '.+']));
$coll->add('b', new Route('/en-en/{b}', ['b' => 'bbb'], ['b' => '.*']));
$matcher = $this->getUrlMatcher($coll);
$this->assertEquals(['_route' => 'a', 'a' => 'aaa'], $matcher->match('/fr-fr'));
$this->assertEquals(['_route' => 'a', 'a' => 'AAA'], $matcher->match('/fr-fr/AAA'));
$this->assertEquals(['_route' => 'b', 'b' => 'bbb'], $matcher->match('/en-en'));
$this->assertEquals(['_route' => 'b', 'b' => 'BBB'], $matcher->match('/en-en/BBB'));
}
public function testTrailingRequirementWithDefault_A()
{
$coll = new RouteCollection();
$coll->add('a', new Route('/fr-fr/{a}', ['a' => 'aaa'], ['a' => '.+']));
$matcher = $this->getUrlMatcher($coll);
$this->expectException(ResourceNotFoundException::class);
$matcher->match('/fr-fr/');
}
public function testTrailingRequirementWithDefault_B()
{
$coll = new RouteCollection();
$coll->add('b', new Route('/en-en/{b}', ['b' => 'bbb'], ['b' => '.*']));
$matcher = $this->getUrlMatcher($coll);
$this->assertEquals(['_route' => 'b', 'b' => ''], $matcher->match('/en-en/'));
}
protected function getUrlMatcher(RouteCollection $routes, RequestContext $context = null)
{
return new UrlMatcher($routes, $context ?: new RequestContext());