Skip to content

Commit

Permalink
[Routing] added route compile check to identify a default value of a …
Browse files Browse the repository at this point in the history
…required variable that does not match the requirement
  • Loading branch information
Tobion committed Aug 31, 2012
1 parent 2cf50b7 commit cb7e3f5
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 8 deletions.
18 changes: 14 additions & 4 deletions src/Symfony/Component/Routing/RouteCompiler.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ class RouteCompiler implements RouteCompilerInterface
/**
* {@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)
{
Expand Down Expand Up @@ -67,14 +68,23 @@ public function compile(Route $route)
$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;
for ($i = count($tokens) - 1; $i >= 0; $i--) {
$token = $tokens[$i];
if ('variable' === $token[0] && $route->hasDefault($token[3])) {
$firstOptional = $i;
if ($optional) {
$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 {
break;
$optional = false;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,13 @@ public function testRelativeUrlWithNullParameter()
}

/**
* @expectedException Symfony\Component\Routing\Exception\InvalidParameterException
* @expectedException \LogicException
*/
public function testRelativeUrlWithNullParameterButNotOptional()
{
$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.
// Generating path "/testing//bar" would be wrong as matching this route would fail.
// It should raise an exception when compiling this route because the given default value is absolutely
// irrelevant for both matching and generating URLs.
$this->getGenerator($routes)->generate('test', array(), false);
}

Expand Down
13 changes: 12 additions & 1 deletion src/Symfony/Component/Routing/Tests/RouteCompilerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,17 @@ public function testRouteWithSameVariableTwice()
{
$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();
}
}

0 comments on commit cb7e3f5

Please sign in to comment.