[Routing] added route compile check to identify a default value of a required variable that does not match the requirement
This commit is contained in:
parent
2cf50b7801
commit
cb7e3f5edb
|
@ -23,7 +23,8 @@ class RouteCompiler implements RouteCompilerInterface
|
||||||
/**
|
/**
|
||||||
* {@inheritDoc}
|
* {@inheritDoc}
|
||||||
*
|
*
|
||||||
* @throws \LogicException If a variable is referenced more than once
|
* @throws \LogicException If a variable is referenced more than once or if a required variable
|
||||||
|
* has a default value that doesn't meet its own requirement.
|
||||||
*/
|
*/
|
||||||
public function compile(Route $route)
|
public function compile(Route $route)
|
||||||
{
|
{
|
||||||
|
@ -67,14 +68,23 @@ class RouteCompiler implements RouteCompilerInterface
|
||||||
$tokens[] = array('text', substr($pattern, $pos));
|
$tokens[] = array('text', substr($pattern, $pos));
|
||||||
}
|
}
|
||||||
|
|
||||||
// find the first optional token
|
// find the first optional token and validate the default values for non-optional variables
|
||||||
|
$optional = true;
|
||||||
$firstOptional = INF;
|
$firstOptional = INF;
|
||||||
for ($i = count($tokens) - 1; $i >= 0; $i--) {
|
for ($i = count($tokens) - 1; $i >= 0; $i--) {
|
||||||
$token = $tokens[$i];
|
$token = $tokens[$i];
|
||||||
if ('variable' === $token[0] && $route->hasDefault($token[3])) {
|
if ('variable' === $token[0] && $route->hasDefault($token[3])) {
|
||||||
|
if ($optional) {
|
||||||
$firstOptional = $i;
|
$firstOptional = $i;
|
||||||
|
} elseif (!preg_match('#^'.$token[2].'$#', $route->getDefault($token[3]))) {
|
||||||
|
throw new \LogicException(sprintf('The default value "%s" of the required variable "%s" in pattern "%s" does not match the requirement "%s". ' .
|
||||||
|
'This route definition makes no sense because this default can neither be used as default for generating URLs nor can it ever be returned by the matching process. ' .
|
||||||
|
'You should change the default to something that meets the requirement or remove it.',
|
||||||
|
$route->getDefault($token[3]), $token[3], $route->getPattern(), $token[2]
|
||||||
|
));
|
||||||
|
}
|
||||||
} else {
|
} else {
|
||||||
break;
|
$optional = false;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -75,13 +75,13 @@ class UrlGeneratorTest extends \PHPUnit_Framework_TestCase
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @expectedException Symfony\Component\Routing\Exception\InvalidParameterException
|
* @expectedException \LogicException
|
||||||
*/
|
*/
|
||||||
public function testRelativeUrlWithNullParameterButNotOptional()
|
public function testRelativeUrlWithNullParameterButNotOptional()
|
||||||
{
|
{
|
||||||
$routes = $this->getRoutes('test', new Route('/testing/{foo}/bar', array('foo' => null)));
|
$routes = $this->getRoutes('test', new Route('/testing/{foo}/bar', array('foo' => null)));
|
||||||
// This must raise an exception because the default requirement for "foo" is "[^/]+" which is not met with these params.
|
// It should raise an exception when compiling this route because the given default value is absolutely
|
||||||
// Generating path "/testing//bar" would be wrong as matching this route would fail.
|
// irrelevant for both matching and generating URLs.
|
||||||
$this->getGenerator($routes)->generate('test', array(), false);
|
$this->getGenerator($routes)->generate('test', array(), false);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -131,6 +131,17 @@ class RouteCompilerTest extends \PHPUnit_Framework_TestCase
|
||||||
{
|
{
|
||||||
$route = new Route('/{name}/{name}');
|
$route = new Route('/{name}/{name}');
|
||||||
|
|
||||||
$compiled = $route->compile();
|
$route->compile();
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @expectedException \LogicException
|
||||||
|
*/
|
||||||
|
public function testRouteWithRequiredVariableAndBadDefault()
|
||||||
|
{
|
||||||
|
$route = new Route('/{foo}/', array('foo' => null));
|
||||||
|
// It should raise an exception when compiling this route because the given default value is absolutely
|
||||||
|
// irrelevant for both matching and generating URLs.
|
||||||
|
$route->compile();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
Reference in New Issue