diogo/symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

[Routing] default requirement of variable should not disallow previous char, but the slash '/' (closes #5469)

Browse Source
This commit is contained in:
Tobias Schultze 2012-09-08 12:51:33 +02:00 committed by Fabien Potencier
parent 4868bec46c
commit a3147e9f13
4 changed files with 76 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
src/Symfony/Component/Routing/RouteCompiler.php
View File

@ -42,7 +42,6 @@ class RouteCompiler implements RouteCompilerInterface
$variables = array();
$matches = array();
$pos = 0;
$lastSeparator = '/';
// Match all variables enclosed in "{}" and iterate over them. But we only want to match the innermost variable
// in case of nested "{}", e.g. {foo{bar}}. This in ensured because \w does not match "{" or "}" itself.
@ -68,17 +67,18 @@ class RouteCompiler implements RouteCompilerInterface
$tokens[] = array('text', $precedingText);
}
// use the character preceding the variable as a separator
// save it for later use as default separator for variables that follow directly without having a preceding char e.g. "/{x}{y}"
if ($isSeparator) {
$lastSeparator = $precedingChar;
}
$regexp = $route->getRequirement($varName);
if (null === $regexp) {
// use the character following the variable (ignoring other placeholders) as a separator when it's not the same as the preceding separator
$nextSeparator = $this->findNextSeparator(substr($pattern, $pos));
$regexp = sprintf('[^%s]+', preg_quote($lastSeparator !== $nextSeparator ? $lastSeparator.$nextSeparator : $lastSeparator, self::REGEX_DELIMITER));
$followingPattern = (string) substr($pattern, $pos);
// Find the next static character after the variable that functions as a separator. By default, this separator and '/'
// are disallowed for the variable. This default requirement makes sure that optional variables can be matched at all
// and that the generating-matching-combination of URLs unambiguous, i.e. the params used for generating the URL are
// the same that will be matched. Example: new Route('/{page}.{_format}', array('_format' => 'html'))
// If {page} would also match the separating dot, {_format} would never match as {page} will eagerly consume everything.
// Also even if {_format} was not optional the requirement prevents that {page} matches something that was originally
// part of {_format} when generating the URL, e.g. _format = 'mobile.html'.
$nextSeparator = $this->findNextSeparator($followingPattern);
$regexp = sprintf('[^/%s]+', '/' !== $nextSeparator && '' !== $nextSeparator ? preg_quote($nextSeparator, self::REGEX_DELIMITER) : '');
}
$tokens[] = array('variable', $isSeparator ? $precedingChar : '', $regexp, $varName);

27
src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php
View File

@ -285,6 +285,33 @@ class UrlGeneratorTest extends \PHPUnit_Framework_TestCase
$this->assertSame('/app.php/getSitesSuffix', $generator->generate('test', array('what' => 'Sites')));
}
public function testDefaultRequirementOfVariable()
{
$routes = $this->getRoutes('test', new Route('/{page}.{_format}'));
$generator = $this->getGenerator($routes);
$this->assertSame('/app.php/index.mobile.html', $generator->generate('test', array('page' => 'index', '_format' => 'mobile.html')));
}
/**
* @expectedException Symfony\Component\Routing\Exception\InvalidParameterException
*/
public function testDefaultRequirementOfVariableDisallowsSlash()
{
$routes = $this->getRoutes('test', new Route('/{page}.{_format}'));
$this->getGenerator($routes)->generate('test', array('page' => 'index', '_format' => 'sl/ash'));
}
/**
* @expectedException Symfony\Component\Routing\Exception\InvalidParameterException
*/
public function testDefaultRequirementOfVariableDisallowsNextSeparator()
{
$routes = $this->getRoutes('test', new Route('/{page}.{_format}'));
$this->getGenerator($routes)->generate('test', array('page' => 'do.t', '_format' => 'html'));
}
protected function getGenerator(RouteCollection $routes, array $parameters = array(), $logger = null)
{
$context = new RequestContext('/app.php');

33
src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php
View File

@ -276,6 +276,39 @@ class UrlMatcherTest extends \PHPUnit_Framework_TestCase
$this->assertEquals(array('what' => 'Sites', '_route' => 'test'), $matcher->match('/getSitesSuffix'));
}
public function testDefaultRequirementOfVariable()
{
$coll = new RouteCollection();
$coll->add('test', new Route('/{page}.{_format}'));
$matcher = new UrlMatcher($coll, new RequestContext());
$this->assertEquals(array('page' => 'index', '_format' => 'mobile.html', '_route' => 'test'), $matcher->match('/index.mobile.html'));
}
/**
* @expectedException Symfony\Component\Routing\Exception\ResourceNotFoundException
*/
public function testDefaultRequirementOfVariableDisallowsSlash()
{
$coll = new RouteCollection();
$coll->add('test', new Route('/{page}.{_format}'));
$matcher = new UrlMatcher($coll, new RequestContext());
$matcher->match('/index.sl/ash');
}
/**
* @expectedException Symfony\Component\Routing\Exception\ResourceNotFoundException
*/
public function testDefaultRequirementOfVariableDisallowsNextSeparator()
{
$coll = new RouteCollection();
$coll->add('test', new Route('/{page}.{_format}', array(), array('_format' => 'html|xml')));
$matcher = new UrlMatcher($coll, new RequestContext());
$matcher->match('/do.t.html');
}
/**
* @expectedException Symfony\Component\Routing\Exception\ResourceNotFoundException
*/

12
src/Symfony/Component/Routing/Tests/RouteCompilerTest.php
View File

@ -115,8 +115,8 @@ class RouteCompilerTest extends \PHPUnit_Framework_TestCase
array(
'Route with a variable in last position',
array('/foo-{bar}'),
'/foo', '#^/foo\-(?<bar>[^\-]+)$#s', array('bar'), array(
array('variable', '-', '[^\-]+', 'bar'),
'/foo', '#^/foo\-(?<bar>[^/]+)$#s', array('bar'), array(
array('variable', '-', '[^/]+', 'bar'),
array('text', '/foo'),
)),
@ -132,8 +132,8 @@ class RouteCompilerTest extends \PHPUnit_Framework_TestCase
array(
'Route without separator between variables',
array('/{w}{x}{y}{z}.{_format}', array('z' => 'default-z', '_format' => 'html'), array('y' => '(y|Y)')),
'', '#^/(?<w>[^/\.]+)(?<x>[^/\.]+)(?<y>(y|Y))(?:(?<z>[^/\.]+)(?:\.(?<_format>[^\.]+))?)?$#s', array('w', 'x', 'y', 'z', '_format'), array(
array('variable', '.', '[^\.]+', '_format'),
'', '#^/(?<w>[^/\.]+)(?<x>[^/\.]+)(?<y>(y|Y))(?:(?<z>[^/\.]+)(?:\.(?<_format>[^/]+))?)?$#s', array('w', 'x', 'y', 'z', '_format'), array(
array('variable', '.', '[^/]+', '_format'),
array('variable', '', '[^/\.]+', 'z'),
array('variable', '', '(y|Y)', 'y'),
array('variable', '', '[^/\.]+', 'x'),
@ -143,8 +143,8 @@ class RouteCompilerTest extends \PHPUnit_Framework_TestCase
array(
'Route with a format',
array('/foo/{bar}.{_format}'),
'/foo', '#^/foo/(?<bar>[^/\.]+)\.(?<_format>[^\.]+)$#s', array('bar', '_format'), array(
array('variable', '.', '[^\.]+', '_format'),
'/foo', '#^/foo/(?<bar>[^/\.]+)\.(?<_format>[^/]+)$#s', array('bar', '_format'), array(
array('variable', '.', '[^/]+', '_format'),
array('variable', '/', '[^/\.]+', 'bar'),
array('text', '/foo'),
)),