Skip to content

Commit

Permalink
[Routing] Throw 405 instead of 404 when redirect is not possible
Browse files Browse the repository at this point in the history
  • Loading branch information
@nicolas-grekas
nicolas-grekas committed Feb 12, 2018
1 parent f7d9701 commit 9284281
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 24 deletions.
55 changes: 31 additions & 24 deletions src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren
$methods[] = 'HEAD';
}

$supportsTrailingSlash = $supportsRedirections && (!$methods || in_array('HEAD', $methods));
$supportsTrailingSlash = $supportsRedirections && (!$methods || in_array('GET', $methods));

if (!count($compiledRoute->getPathVariables()) && false !== preg_match('#^(.)\^(?P<url>.*?)\$\1#', $compiledRoute->getRegex(), $m)) {
if ($supportsTrailingSlash && '/' === substr($m['url'], -1)) {
Expand Down Expand Up @@ -258,34 +258,13 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren
EOF;

$gotoname = 'not_'.preg_replace('/[^A-Za-z0-9_]/', '', $name);
if ($methods) {
if (1 === count($methods)) {
$code .= <<<EOF
if (\$this->context->getMethod() != '$methods[0]') {
\$allow[] = '$methods[0]';
goto $gotoname;
}
EOF;
} else {
$methods = implode("', '", $methods);
$code .= <<<EOF
if (!in_array(\$this->context->getMethod(), array('$methods'))) {
\$allow = array_merge(\$allow, array('$methods'));
goto $gotoname;
}
EOF;
}
}

if ($hasTrailingSlash) {
$code .= <<<EOF
if ('/' === substr(\$pathinfo, -1)) {
// no-op
} elseif (!in_array(\$this->context->getMethod(), array('HEAD', 'GET'))) {
\$allow[] = 'GET';
goto $gotoname;
} else {
return \$this->redirect(\$rawPathinfo.'/', '$name');
Expand All @@ -303,13 +282,41 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren
$code .= <<<EOF
\$requiredSchemes = $schemes;
if (!isset(\$requiredSchemes[\$this->context->getScheme()])) {
if (!in_array(\$this->context->getMethod(), array('HEAD', 'GET'))) {
\$allow[] = 'GET';
goto $gotoname;
}
return \$this->redirect(\$rawPathinfo, '$name', key(\$requiredSchemes));
}
EOF;
}

if ($methods) {
if (1 === count($methods)) {
$code .= <<<EOF
if (\$this->context->getMethod() != '$methods[0]') {
\$allow[] = '$methods[0]';
goto $gotoname;
}
EOF;
} else {
$methods = implode("', '", $methods);
$code .= <<<EOF
if (!in_array(\$this->context->getMethod(), array('$methods'))) {
\$allow = array_merge(\$allow, array('$methods'));
goto $gotoname;
}
EOF;
}
}

// optimize parameters array
if ($matches || $hostMatches) {
$vars = array();
Expand All @@ -333,7 +340,7 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren
}
$code .= " }\n";

if ($methods || $hasTrailingSlash) {
if ($hasTrailingSlash || $schemes || $methods) {
$code .= " $gotoname:\n";
}

Expand Down
15 changes: 15 additions & 0 deletions src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher2.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ public function match($rawPathinfo)
if ('/' === substr($pathinfo, -1)) {
// no-op
} elseif (!in_array($this->context->getMethod(), array('HEAD', 'GET'))) {
$allow[] = 'GET';
goto not_baz3;
} else {
return $this->redirect($rawPathinfo.'/', 'baz3');
Expand All @@ -85,6 +86,7 @@ public function match($rawPathinfo)
if ('/' === substr($pathinfo, -1)) {
// no-op
} elseif (!in_array($this->context->getMethod(), array('HEAD', 'GET'))) {
$allow[] = 'GET';
goto not_baz4;
} else {
return $this->redirect($rawPathinfo.'/', 'baz4');
Expand Down Expand Up @@ -183,6 +185,7 @@ public function match($rawPathinfo)
if ('/' === substr($pathinfo, -1)) {
// no-op
} elseif (!in_array($this->context->getMethod(), array('HEAD', 'GET'))) {
$allow[] = 'GET';
goto not_hey;
} else {
return $this->redirect($rawPathinfo.'/', 'hey');
Expand Down Expand Up @@ -333,21 +336,33 @@ public function match($rawPathinfo)
if ('/secure' === $pathinfo) {
$requiredSchemes = array ( 'https' => 0,);
if (!isset($requiredSchemes[$this->context->getScheme()])) {
if (!in_array($this->context->getMethod(), array('HEAD', 'GET'))) {
$allow[] = 'GET';
goto not_secure;
}

return $this->redirect($rawPathinfo, 'secure', key($requiredSchemes));
}

return array('_route' => 'secure');
}
not_secure:

// nonsecure
if ('/nonsecure' === $pathinfo) {
$requiredSchemes = array ( 'http' => 0,);
if (!isset($requiredSchemes[$this->context->getScheme()])) {
if (!in_array($this->context->getMethod(), array('HEAD', 'GET'))) {
$allow[] = 'GET';
goto not_nonsecure;
}

return $this->redirect($rawPathinfo, 'nonsecure', key($requiredSchemes));
}

return array('_route' => 'nonsecure');
}
not_nonsecure:

throw 0 < count($allow) ? new MethodNotAllowedException(array_unique($allow)) : new ResourceNotFoundException();
}
Expand Down
8 changes: 8 additions & 0 deletions src/Symfony/Component/Routing/Tests/Matcher/DumpedRedirectableUrlMatcherTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@

class DumpedRedirectableUrlMatcherTest extends RedirectableUrlMatcherTest
{
/**
* @expectedException \Symfony\Component\Routing\Exception\MethodNotAllowedException
*/
public function testRedirectWhenNoSlashForNonSafeMethod()
{
parent::testRedirectWhenNoSlashForNonSafeMethod();
}

protected function getUrlMatcher(RouteCollection $routes, RequestContext $context = null)
{
static $i = 0;
Expand Down

0 comments on commit 9284281

Please sign in to comment.