10BC0 bug #29297 [Routing] fix trailing slash redirection when using Redire… · symfony/symfony@0878006 · GitHub
[go: up one dir, main page]
Skip to content

Commit 0878006

Browse files
nicolas-grekasnicolas-grekas
committed
bug #29297 [Routing] fix trailing slash redirection when using RedirectableUrlMatcher (nicolas-grekas)
This PR was merged into the 3.4 branch. Discussion ---------- [Routing] fix trailing slash redirection when using RedirectableUrlMatcher | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #29287 | License | MIT | Doc PR | - This fixes #29287 by considering that the behavior of the dumped matcher is the correct one: lower priority routes never impact previous ones. I think it's what makes the most sense because that's what requires the most local knowledge to understand what's going on (ie that's the less surprising behavior). Commits ------- dc4c3f6 [Routing] fix trailing slash redirection when using RedirectableUrlMatcher
2 parents 11053c5 + dc4c3f6 commit 0878006

File tree

2 files changed

+34
-2
lines changed

2 files changed

+34
-2
lines changed

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

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,18 +118,39 @@ public function addExpressionLanguageProvider(ExpressionFunctionProviderInterfac
118118
*/
119119
protected function matchCollection($pathinfo, RouteCollection $routes)
120120
{
121+
$supportsTrailingSlash = '/' !== $pathinfo && '' !== $pathinfo && $this instanceof RedirectableUrlMatcherInterface;
122+
121123
foreach ($routes as $name => $route) {
122124
$compiledRoute = $route->compile();
125+
$staticPrefix = $compiledRoute->getStaticPrefix();
123126

124127
// check the static prefix of the URL first. Only use the more expensive preg_match when it matches
125-
if ('' !== $compiledRoute->getStaticPrefix() && 0 !== strpos($pathinfo, $compiledRoute->getStaticPrefix())) {
128+
if ('' === $staticPrefix || 0 === strpos($pathinfo, $staticPrefix)) {
129+
// no-op
130+
} elseif (!$supportsTrailingSlash) {
131+
continue;
132+
} elseif ('/' === substr($staticPrefix, -1) && substr($staticPrefix, 0, -1) === $pathinfo) {
133+
return;
134+
} else {
126135
continue;
127136
}
137+
$regex = $compiledRoute->getRegex();
138+
139+
if ($supportsTrailingSlash && $pos = strpos($regex, '/$')) {
140+
$regex = substr($regex, 0, $pos).'/?$'.substr($regex, $pos + 2);
141+
$hasTrailingSlash = true;
142+
} else {
143+
$hasTrailingSlash = false;
144+
}
128145

129-
if (!preg_match($compiledRoute->getRegex(), $pathinfo, $matches)) {
146+
if (!preg_match($regex, $pathinfo, $matches)) {
130147
continue;
131148
}
132149

150+
if ($hasTrailingSlash && '/' !== substr($pathinfo, -1)) {
151+
return;
152+
}
153+
133154
$hostMatches = array();
134155
if ($compiledRoute->getHostRegex() && !preg_match($compiledRoute->getHostRegex(), $this->context->getHost(), $hostMatches)) {
135156
continue;

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,17 @@ public function testSchemeRequirement()
117117
$this->assertSame(array('_route' => 'foo'), $matcher->match('/foo'));
118118
}
119119

120+
public function testFallbackPage()
121+
{
122+
$coll = new RouteCollection();
123+
$coll->add('foo', new Route('/foo/'));
124+
$coll->add('bar', new Route('/{name}'));
125+
126+
$matcher = $this->getUrlMatcher($coll);
127+
$matcher->expects($this->once())->method('redirect')->with('/foo/')->will($this->returnValue(array('_route' => 'foo')));
128+
$this->assertSame(array('_route' => 'foo'), $matcher->match('/foo'));
129+
}
130+
120131
protected function getUrlMatcher(RouteCollection $routes, RequestContext $context = null)
121132
{
122133
return $this->getMockForAbstractClass('Symfony\Component\Routing\Matcher\RedirectableUrlMatcher', array($routes, $context ?: new RequestContext()));

0 commit comments

Comments
 (0)
0