[Routing] fix taking verb into account when redirecting

Author: nicolas-grekas

Matcher/UrlMatcher.php

@@ -123,14 +123,15 @@ protected function matchCollection($pathinfo, RouteCollection $routes)
         foreach ($routes as $name => $route) {
             $compiledRoute = $route->compile();
             $staticPrefix = $compiledRoute->getStaticPrefix();
+            $requiredMethods = $route->getMethods();
 
             // check the static prefix of the URL first. Only use the more expensive preg_match when it matches
             if ('' === $staticPrefix || 0 === strpos($pathinfo, $staticPrefix)) {
                 // no-op
-            } elseif (!$supportsTrailingSlash) {
+            } elseif (!$supportsTrailingSlash || ($requiredMethods && !\in_array('GET', $requiredMethods))) {
                 continue;
             } elseif ('/' === substr($staticPrefix, -1) && substr($staticPrefix, 0, -1) === $pathinfo) {
-                return;
+                return $this->allow = array();
             } else {
                 continue;
             }
@@ -148,7 +149,10 @@ protected function matchCollection($pathinfo, RouteCollection $routes)
             }
 
             if ($hasTrailingSlash && '/' !== substr($pathinfo, -1)) {
-                return;
+                if (!$requiredMethods || \in_array('GET', $requiredMethods)) {
+                    return $this->allow = array();
+                }
+                continue;
             }
 
             $hostMatches = array();
@@ -163,7 +167,7 @@ protected function matchCollection($pathinfo, RouteCollection $routes)
             }
 
             // check HTTP method requirement
-            if ($requiredMethods = $route->getMethods()) {
+            if ($requiredMethods) {
                 // HEAD and GET are equivalent as per RFC
                 if ('HEAD' === $method = $this->context->getMethod()) {
                     $method = 'GET';
@@ -180,6 +184,8 @@ protected function matchCollection($pathinfo, RouteCollection $routes)
 
             return $this->getAttributes($route, $name, array_replace($matches, $hostMatches, isset($status[1]) ? $status[1] : array()));
         }
+
+        return array();
     }
 
     /**

Tests/Matcher/RedirectableUrlMatcherTest.php

@@ -128,6 +128,23 @@ public function testFallbackPage()
         $this->assertSame(array('_route' => 'foo'), $matcher->match('/foo'));
     }
 
+    public function testSlashAndVerbPrecedenceWithRedirection()
+    {
+        $coll = new RouteCollection();
+        $coll->add('a', new Route('/api/customers/{customerId}/contactpersons', array(), array(), array(), '', array(), array('post')));
+        $coll->add('b', new Route('/api/customers/{customerId}/contactpersons/', array(), array(), array(), '', array(), array('get')));
+
+        $matcher = $this->getUrlMatcher($coll);
+        $expected = array(
+            '_route' => 'b',
+            'customerId' => '123',
+        );
+        $this->assertEquals($expected, $matcher->match('/api/customers/123/contactpersons/'));
+
+        $matcher->expects($this->once())->method('redirect')->with('/api/customers/123/contactpersons/')->willReturn(array());
+        $this->assertEquals($expected, $matcher->match('/api/customers/123/contactpersons'));
+    }
+
     protected function getUrlMatcher(RouteCollection $routes, RequestContext $context = null)
     {
         return $this->getMockForAbstractClass('Symfony\Component\Routing\Matcher\RedirectableUrlMatcher', array($routes, $context ?: new RequestContext()));

Tests/Matcher/UrlMatcherTest.php

@@ -502,6 +502,20 @@ public function testSchemeAndMethodMismatch()
         $matcher->match('/');
     }
 
+    public function testSlashAndVerbPrecedence()
+    {
+        $coll = new RouteCollection();
+        $coll->add('a', new Route('/api/customers/{customerId}/contactpersons/', array(), array(), array(), '', array(), array('post')));
+        $coll->add('b', new Route('/api/customers/{customerId}/contactpersons', array(), array(), array(), '', array(), array('get')));
+
+        $matcher = $this->getUrlMatcher($coll);
+        $expected = array(
+            '_route' => 'b',
+            'customerId' => '123',
+        );
+        $this->assertEquals($expected, $matcher->match('/api/customers/123/contactpersons'));
+    }
+
     protected function getUrlMatcher(RouteCollection $routes, RequestContext $context = null)
     {
         return new UrlMatcher($routes, $context ?: new RequestContext());