8000 bug #31182 [Routing] fix trailing slash matching with empty-matching … · symfony/symfony@bff2db7 · GitHub
[go: up one dir, main page]
Skip to content

Commit bff2db7

Browse files
nicolas-grekasnicolas-grekas
committed
bug #31182 [Routing] fix trailing slash matching with empty-matching trailing vars (nicolas-grekas)
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 #31107 (comment) 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 ------- d6da21a [Routing] fix trailing slash matching with empty-matching trailing vars
2 parents 3b05c07 + d6da21a commit bff2db7

File tree

4 files changed

+41
-6
lines changed

4 files changed

+41
-6
lines changed

‎src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherTrait.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ private function doMatch(string $pathinfo, array &$allow = [], array &$allowSche
132132

133133
$hasTrailingVar = $trimmedPathinfo !== $pathinfo && $hasTrailingVar;
134134

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

‎src/Symfony/Component/Routing/Matcher/UrlMatcher.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ protected function matchCollection($pathinfo, RouteCollection $routes)
158158

159159
$hasTrailingVar = $trimmedPathinfo !== $pathinfo && preg_match('#\{\w+\}/?$#', $route->getPath());
160160

161-
if ($hasTrailingVar && ($hasTrailingSlash || !($m = $matches[\count($compiledRoute->getPathVariables())] ?? '') || '/' !== substr($m, -1)) && preg_match($regex, $trimmedPathinfo, $m)) {
161+
if ($hasTrailingVar && ($hasTrailingSlash || (null === $m = $matches[\count($compiledRoute->getPathVariables())] ?? null) || '/' !== ($m[-1] ?? '/')) && preg_match($regex, $trimmedPathinfo, $m)) {
162162
if ($hasTrailingSlash) {
163163
$matches = $m;
164164
} else {

‎src/Symfony/Component/Routing/Tests/Matcher/RedirectableUrlMatcherTest.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -198,15 +198,15 @@ public function testNonGreedyTrailingRequirement()
198198
$this->assertEquals(['_route' => 'a', 'a' => '123'], $matcher->match('/123/'));
199199
}
200200

201-
public function testTrailingRequirementWithDefault()
201+
public function testTrailingRequirementWithDefault_A()
202202
{
203203
$coll = new RouteCollection();
204-
$coll->add('a', new Route('/foo/{a}', ['a' => 'bar'], ['a' => '.+']));
204+
$coll->add('a', new Route('/fr-fr/{a}', ['a' => 'aaa'], ['a' => '.+']));
205205

206206
$matcher = $this->getUrlMatcher($coll);
207-
$matcher->expects($this->once())->method('redirect')->with('/foo')->willReturn([]);
207+
$matcher->expects($this->once())->method('redirect')->with('/fr-fr')->willReturn([]);
208208

209-
$this->assertEquals(['_route' => 'a', 'a' => 'bar'], $matcher->match('/foo/'));
209+
$this->assertEquals(['_route' => 'a', 'a' => 'aaa'], $matcher->match('/fr-fr/'));
210210
}
211211

212212
protected function getUrlMatcher(RouteCollection $routes, RequestContext $context = null)

‎src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -757,6 +757,41 @@ public function testGreedyTrailingRequirement()
757757
$this->assertEquals(['_route' => 'a', 'a' => 'foo/'], $matcher->match('/foo/'));
758758
}
759759

760+
public function testTrailingRequirementWithDefault()
761+
{
762+
$coll = new RouteCollection();
763+
$coll->add('a', new Route('/fr-fr/{a}', ['a' => 'aaa'], ['a' => '.+']));
764+
$coll->add('b', new Route('/en-en/{b}', ['b' => 'bbb'], ['b' => '.*']));
765+
766+
$matcher = $this->getUrlMatcher($coll);
767+
768+
$this->assertEquals(['_route' => 'a', 'a' => 'aaa'], $matcher->match('/fr-fr'));
769+
$this->assertEquals(['_route' => 'a', 'a' => 'AAA'], $matcher->match('/fr-fr/AAA'));
770+
$this->assertEquals(['_route' => 'b', 'b' => 'bbb'], $matcher->match('/en-en'));
771+
$this->assertEquals(['_route' => 'b', 'b' => 'BBB'], $matcher->match('/en-en/BBB'));
772+
}
773+
774+
public function testTrailingRequirementWithDefault_A()
775+
{
776+
$coll = new RouteCollection();
777+
$coll->add('a', new Route('/fr-fr/{a}', ['a' => 'aaa'], ['a' => '.+']));
778+
779+
$matcher = $this->getUrlMatcher($coll);
780+
781+
$this->expectException(ResourceNotFoundException::class);
782+
$matcher->match('/fr-fr/');
783+
}
784+
785+
public function testTrailingRequirementWithDefault_B()
786+
{
787+
$coll = new RouteCollection();
788+
$coll->add('b', new Route('/en-en/{b}', ['b' => 'bbb'], ['b' => '.*']));
789+
790+
$matcher = $this->getUrlMatcher($coll);
791+
792+
$this->assertEquals(['_route' => 'b', 'b' => ''], $matcher->match('/en-en/'));
793+
}
794+
760795
protected function getUrlMatcher(RouteCollection $routes, RequestContext $context = null)
761796
{
762797
return new UrlMatcher($routes, $context ?: new RequestContext());

0 commit comments

Comments
 (0)
0