[Routing] fix taking verb into account when redirecting

This commit is contained in:
Nicolas Grekas 2018-12-02 12:14:59 +01:00
parent 6b38491fc1
commit 6b65fac2cf
3 changed files with 41 additions and 4 deletions

View File

@ -123,14 +123,15 @@ class UrlMatcher implements UrlMatcherInterface, RequestMatcherInterface
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 @@ class UrlMatcher implements UrlMatcherInterface, RequestMatcherInterface
}
if ($hasTrailingSlash && '/' !== substr($pathinfo, -1)) {
return;
if (!$requiredMethods || \in_array('GET', $requiredMethods)) {
return $this->allow = array();
}
continue;
}
$hostMatches = array();
@ -163,7 +167,7 @@ class UrlMatcher implements UrlMatcherInterface, RequestMatcherInterface
}
// 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 @@ class UrlMatcher implements UrlMatcherInterface, RequestMatcherInterface
return $this->getAttributes($route, $name, array_replace($matches, $hostMatches, isset($status[1]) ? $status[1] : array()));
}
return array();
}
/**

View File

@ -128,6 +128,23 @@ class RedirectableUrlMatcherTest extends UrlMatcherTest
$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()));

View File

@ -502,6 +502,20 @@ class UrlMatcherTest extends TestCase
$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());