bug #29297 [Routing] fix trailing slash redirection when using RedirectableUrlMatcher (nicolas-grekas)

nicolas-grekas · nicolas-grekas · commit 0878006e222e · 2018-11-26T11:22:13.000+01:00
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
diff --git a/src/Symfony/Component/Routing/Matcher/UrlMatcher.php b/src/Symfony/Component/Routing/Matcher/UrlMatcher.php
@@ -118,18 +118,39 @@ public function addExpressionLanguageProvider(ExpressionFunctionProviderInterfac
      */
     protected function matchCollection($pathinfo, RouteCollection $routes)
     {
+        $supportsTrailingSlash = '/' !== $pathinfo && '' !== $pathinfo && $this instanceof RedirectableUrlMatcherInterface;
+
         foreach ($routes as $name => $route) {
             $compiledRoute = $route->compile();
+            $staticPrefix = $compiledRoute->getStaticPrefix();
 
             // check the static prefix of the URL first. Only use the more expensive preg_match when it matches
-            if ('' !== $compiledRoute->getStaticPrefix() && 0 !== strpos($pathinfo, $compiledRoute->getStaticPrefix())) {
+            if ('' === $staticPrefix || 0 === strpos($pathinfo, $staticPrefix)) {
+                // no-op
+            } elseif (!$supportsTrailingSlash) {
+                continue;
+            } elseif ('/' === substr($staticPrefix, -1) && substr($staticPrefix, 0, -1) === $pathinfo) {
+                return;
+            } else {
                 continue;
             }
+            $regex = $compiledRoute->getRegex();
+
+            if ($supportsTrailingSlash && $pos = strpos($regex, '/$')) {
+                $regex = substr($regex, 0, $pos).'/?$'.substr($regex, $pos + 2);
+                $hasTrailingSlash = true;
+            } else {
+                $hasTrailingSlash = false;
+            }
 
-            if (!preg_match($compiledRoute->getRegex(), $pathinfo, $matches)) {
+            if (!preg_match($regex, $pathinfo, $matches)) {
                 continue;
             }
 
+            if ($hasTrailingSlash && '/' !== substr($pathinfo, -1)) {
+                return;
+            }
+
             $hostMatches = array();
             if ($compiledRoute->getHostRegex() && !preg_match($compiledRoute->getHostRegex(), $this->context->getHost(), $hostMatches)) {
                 continue;
diff --git a/src/Symfony/Component/Routing/Tests/Matcher/RedirectableUrlMatcherTest.php b/src/Symfony/Component/Routing/Tests/Matcher/RedirectableUrlMatcherTest.php
@@ -117,6 +117,17 @@ public function testSchemeRequirement()
         $this->assertSame(array('_route' => 'foo'), $matcher->match('/foo'));
     }
 
+    public function testFallbackPage()
+    {
+        $coll = new RouteCollection();
+        $coll->add('foo', new Route('/foo/'));
+        $coll->add('bar', new Route('/{name}'));
+
+        $matcher = $this->getUrlMatcher($coll);
+        $matcher->expects($this->once())->method('redirect')->with('/foo/')->will($this->returnValue(array('_route' => 'foo')));
+        $this->assertSame(array('_route' => 'foo'), $matcher->match('/foo'));
+    }
+
     protected function getUrlMatcher(RouteCollection $routes, RequestContext $context = null)
     {
         return $this->getMockForAbstractClass('Symfony\Component\Routing\Matcher\RedirectableUrlMatcher', array($routes, $context ?: new RequestContext()));
