Skip to content

Commit

Permalink
bug #29441 [Routing] ignore trailing slash for non-GET requests (nico…
Browse files Browse the repository at this point in the history
…las-grekas)

This PR was merged into the 3.4 branch.

Discussion
----------

[Routing] ignore trailing slash for non-GET requests

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #29410
| License       | MIT
| Doc PR        | -

Another test case provided by @peterrehm in the linked issue - the dumped matcher already passes this test - but the non-dumped one doesn't (neither does the dumped one in 4.1 - I'll fix while merging up)

Commits
-------

7521af7 [Routing] ignore trailing slash for non-GET requests
  • Loading branch information
nicolas-grekas committed Dec 3, 2018
2 parents 0ca1614 + 7521af7 commit 317a2f9
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 7 deletions.
13 changes: 6 additions & 7 deletions src/Symfony/Component/Routing/Matcher/UrlMatcher.php
Expand Up @@ -118,6 +118,10 @@ public function addExpressionLanguageProvider(ExpressionFunctionProviderInterfac
*/
protected function matchCollection($pathinfo, RouteCollection $routes)
{
// HEAD and GET are equivalent as per RFC
if ('HEAD' === $method = $this->context->getMethod()) {
$method = 'GET';
}
$supportsTrailingSlash = '/' !== $pathinfo && '' !== $pathinfo && $this instanceof RedirectableUrlMatcherInterface;

foreach ($routes as $name => $route) {
Expand All @@ -128,7 +132,7 @@ protected function matchCollection($pathinfo, RouteCollection $routes)
// 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 || ($requiredMethods && !\in_array('GET', $requiredMethods))) {
} elseif (!$supportsTrailingSlash || ($requiredMethods && !\in_array('GET', $requiredMethods)) || 'GET' !== $method) {
continue;
} elseif ('/' === substr($staticPrefix, -1) && substr($staticPrefix, 0, -1) === $pathinfo) {
return $this->allow = array();
Expand All @@ -149,7 +153,7 @@ protected function matchCollection($pathinfo, RouteCollection $routes)
}

if ($hasTrailingSlash && '/' !== substr($pathinfo, -1)) {
if (!$requiredMethods || \in_array('GET', $requiredMethods)) {
if ((!$requiredMethods || \in_array('GET', $requiredMethods)) && 'GET' === $method) {
return $this->allow = array();
}
continue;
Expand All @@ -168,11 +172,6 @@ protected function matchCollection($pathinfo, RouteCollection $routes)

// check HTTP method requirement
if ($requiredMethods) {
// HEAD and GET are equivalent as per RFC
if ('HEAD' === $method = $this->context->getMethod()) {
$method = 'GET';
}

if (!\in_array($method, $requiredMethods)) {
if (self::REQUIREMENT_MATCH === $status[0]) {
$this->allow = array_merge($this->allow, $requiredMethods);
Expand Down
11 changes: 11 additions & 0 deletions src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php
Expand Up @@ -514,6 +514,17 @@ public function testSlashAndVerbPrecedence()
'customerId' => '123',
);
$this->assertEquals($expected, $matcher->match('/api/customers/123/contactpersons'));

$coll = new RouteCollection();
$coll->add('a', new Route('/api/customers/{customerId}/contactpersons/', array(), array(), array(), '', array(), array('get')));
$coll->add('b', new Route('/api/customers/{customerId}/contactpersons', array(), array(), array(), '', array(), array('post')));

$matcher = $this->getUrlMatcher($coll, new RequestContext('', 'POST'));
$expected = array(
'_route' => 'b',
'customerId' => '123',
);
$this->assertEquals($expected, $matcher->match('/api/customers/123/contactpersons'));
}

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

0 comments on commit 317a2f9

Please sign in to comment.