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

merged branch Tobion/path-slash (PR #5683)

Browse Source 
This PR was merged into the 2.1 branch.

Commits
-------

1566f9f [Routing] fix handling of whitespace and synch between collection prefix and route pattern
90145d2 [Routing] fix handling of two starting slashes in the pattern

Discussion
----------

[Routing] fix handling of slashes and whitespace in pattern/prefix

BC break: no
feature addition: no

The first commit fixes the handling of two starting slashes in the pattern. It would be confused with a network path e.g. `//domain/path` when generating a path, so should be prevented.

The second commit fixes the handling of whitespace in RouteCollection::addPrefix. It wasn't trimmed there but it is trimmed in Route::setPattern. So it can be out-of-synch between RouteCollection::getPrefix <-> Route::getPattern.
This commit is contained in:
Fabien Potencier 2012-10-09 09:19:55 +02:00
commit 1202d9a738
5 changed files with 36 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
src/Symfony/Component/Routing/Route.php
View File

@ -95,13 +95,9 @@ class Route implements \Serializable
*/
public function setPattern($pattern)
{
$this->pattern = trim($pattern);
// a route must start with a slash
if ('' === $this->pattern || '/' !== $this->pattern[0]) {
$this->pattern = '/'.$this->pattern;
}
// A pattern must start with a slash and must not have multiple slashes at the beginning because the
// generated path for this route would be confused with a network path, e.g. '//domain.com/path'.
$this->pattern = '/' . ltrim(trim($pattern), '/');
$this->compiled = null;
return $this;

18
src/Symfony/Component/Routing/RouteCollection.php
View File

@ -223,25 +223,25 @@ class RouteCollection implements \IteratorAggregate, \Countable
*/
public function addPrefix($prefix, $defaults = array(), $requirements = array(), $options = array())
{
// a prefix must not end with a slash
$prefix = rtrim($prefix, '/');
$prefix = trim(trim($prefix), '/');
if ('' === $prefix && empty($defaults) && empty($requirements) && empty($options)) {
return;
}
// a prefix must start with a slash
if ('' !== $prefix && '/' !== $prefix[0]) {
$prefix = '/'.$prefix;
// a prefix must start with a single slash and must not end with a slash
if ('' !== $prefix) {
$this->prefix = '/' . $prefix . $this->prefix;
}
$this->prefix = $prefix.$this->prefix;
foreach ($this->routes as $route) {
if ($route instanceof RouteCollection) {
$route->addPrefix($prefix, $defaults, $requirements, $options);
// we add the slashes so the prefix is not lost by trimming in the sub-collection
$route->addPrefix('/' . $prefix . '/', $defaults, $requirements, $options);
} else {
$route->setPattern($prefix.$route->getPattern());
if ('' !== $prefix) {
$route->setPattern('/' . $prefix . $route->getPattern());
}
$route->addDefaults($defaults);
$route->addRequirements($requirements);
$route->addOptions($options);

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

@ -214,6 +214,14 @@ class UrlGeneratorTest extends \PHPUnit_Framework_TestCase
$routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'http')));
$this->assertEquals('http://localhost/app.php/', $this->getGenerator($routes, array('scheme' => 'https'))->generate('test'));
}
public function testPathWithTwoStartingSlashes()
{
$routes = $this->getRoutes('test', new Route('//path-and-not-domain'));
// this must not generate '//path-and-not-domain' because that would be a network path
$this->assertSame('/path-and-not-domain', $this->getGenerator($routes, array('BaseUrl' => ''))->generate('test'));
}
public function testNoTrailingSlashForMultipleOptionalParameters()
{

19
src/Symfony/Component/Routing/Tests/RouteCollectionTest.php
View File

@ -140,14 +140,19 @@ class RouteCollectionTest extends \PHPUnit_Framework_TestCase
{
$collection = new RouteCollection();
$collection->add('foo', $foo = new Route('/foo'));
$collection->add('bar', $bar = new Route('/bar'));
$collection2 = new RouteCollection();
$collection2->add('bar', $bar = new Route('/bar'));
$collection->addCollection($collection2);
$this->assertSame('', $collection->getPrefix(), '->getPrefix() is initialized with ""');
$collection->addPrefix(' / ');
$this->assertSame('', $collection->getPrefix(), '->addPrefix() trims the prefix and a single slash has no effect');
$collection->addPrefix('/{admin}', array('admin' => 'admin'), array('admin' => '\d+'), array('foo' => 'bar'));
$this->assertEquals('/{admin}/foo', $collection->get('foo')->getPattern(), '->addPrefix() adds a prefix to all routes');
$this->assertEquals('/{admin}/bar', $collection->get('bar')->getPattern(), '->addPrefix() adds a prefix to all routes');
$this->assertEquals(array('admin' => 'admin'), $collection->get('foo')->getDefaults(), '->addPrefix() adds a prefix to all routes');
$this->assertEquals(array('admin' => 'admin'), $collection->get('bar')->getDefaults(), '->addPrefix() adds a prefix to all routes');
$this->assertEquals(array('admin' => '\d+'), $collection->get('foo')->getRequirements(), '->addPrefix() adds a prefix to all routes');
$this->assertEquals(array('admin' => '\d+'), $collection->get('bar')->getRequirements(), '->addPrefix() adds a prefix to all routes');
$this->assertEquals(array('admin' => 'admin'), $collection->get('foo')->getDefaults(), '->addPrefix() adds defaults to all routes');
$this->assertEquals(array('admin' => 'admin'), $collection->get('bar')->getDefaults(), '->addPrefix() adds defaults to all routes');
$this->assertEquals(array('admin' => '\d+'), $collection->get('foo')->getRequirements(), '->addPrefix() adds requirements to all routes');
$this->assertEquals(array('admin' => '\d+'), $collection->get('bar')->getRequirements(), '->addPrefix() adds requirements to all routes');
$this->assertEquals(
array('foo' => 'bar', 'compiler_class' => 'Symfony\\Component\\Routing\\RouteCompiler'),
$collection->get('foo')->getOptions(), '->addPrefix() adds an option to all routes'
@ -158,6 +163,10 @@ class RouteCollectionTest extends \PHPUnit_Framework_TestCase
);
$collection->addPrefix('0');
$this->assertEquals('/0/{admin}', $collection->getPrefix(), '->addPrefix() ensures a prefix must start with a slash and must not end with a slash');
$collection->addPrefix('/ /');
$this->assertSame('/ /0/{admin}', $collection->getPrefix(), '->addPrefix() can handle spaces if desired');
$this->assertSame('/ /0/{admin}/foo', $collection->get('foo')->getPattern(), 'the route pattern is in synch with the collection prefix');
$this->assertSame('/ /0/{admin}/bar', $collection->get('bar')->getPattern(), 'the route pattern in a sub-collection is in synch with the collection prefix');
}
public function testAddPrefixOverridesDefaultsAndRequirements()

2
src/Symfony/Component/Routing/Tests/RouteTest.php
View File

@ -34,6 +34,8 @@ class RouteTest extends \PHPUnit_Framework_TestCase
$route->setPattern('bar');
$this->assertEquals('/bar', $route->getPattern(), '->setPattern() adds a / at the beginning of the pattern if needed');
$this->assertEquals($route, $route->setPattern(''), '->setPattern() implements a fluent interface');
$route->setPattern('//path');
$this->assertEquals('/path', $route->getPattern(), '->setPattern() does not allow two slahes "//" at the beginning of the pattern as it would be confused with a network path when generating the path from the route');
}
public function testOptions()