Skip to content

Commit

Permalink
bug #33363 [Routing] fix static route reordering when a previous dyna…
Browse files Browse the repository at this point in the history
…mic route conflicts (nicolas-grekas)

This PR was merged into the 4.3 branch.

Discussion
----------

[Routing] fix static route reordering when a previous dynamic route conflicts

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

Spotted while playing with the code.
I confirm the test case is green on 3.4 too (but route reordering didn't exist then.)

Commits
-------

cba3b62 [Routing] fix static route reordering when a previous dynamic route conflicts
  • Loading branch information
nicolas-grekas committed Aug 30, 2019
2 parents 92ac848 + cba3b62 commit 4b66bee
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 1 deletion.
Expand Up @@ -187,7 +187,7 @@ private function groupStaticRoutes(RouteCollection $collection): array
$url = substr($url, 0, -1);
}
foreach ($dynamicRegex as list($hostRx, $rx, $prefix)) {
if (('' === $prefix || 0 === strpos($url, $prefix)) && preg_match($rx, $url) && (!$host || !$hostRx || preg_match($hostRx, $host))) {
if (('' === $prefix || 0 === strpos($url, $prefix)) && (preg_match($rx, $url) || preg_match($rx, $url.'/')) && (!$host || !$hostRx || preg_match($hostRx, $host))) {
$dynamicRegex[] = [$hostRegex, $regex, $staticPrefix];
$dynamicRoutes->add($name, $route);
continue 2;
Expand Down
11 changes: 11 additions & 0 deletions src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php
Expand Up @@ -929,6 +929,17 @@ public function testTrailingRequirementWithDefault_B()
$this->assertEquals(['_route' => 'b', 'b' => ''], $matcher->match('/en-en/'));
}

public function testRestrictiveTrailingRequirementWithStaticRouteAfter()
{
$coll = new RouteCollection();
$coll->add('a', new Route('/hello{_}', [], ['_' => '/(?!/)']));
$coll->add('b', new Route('/hello'));

$matcher = $this->getUrlMatcher($coll);

$this->assertEquals(['_route' => 'a', '_' => '/'], $matcher->match('/hello/'));
}

protected function getUrlMatcher(RouteCollection $routes, RequestContext $context = null)
{
return new UrlMatcher($routes, $context ?: new RequestContext());
Expand Down

0 comments on commit 4b66bee

Please sign in to comment.