diff --git a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md index 4b4388b54f..6c48b34d87 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md +++ b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md @@ -38,3 +38,5 @@ CHANGELOG create the class cache. * [BC BREAK] TemplateNameParser::parseFromFilename() has been moved to a dedicated parser: TemplateFilenameParser::parse(). + * [BC BREAK] Kernel parameters are replaced by their value whereever they appear + in Route patterns, requirements and defaults. Use '%%' as the escaped value for '%'. diff --git a/src/Symfony/Bundle/FrameworkBundle/Routing/Router.php b/src/Symfony/Bundle/FrameworkBundle/Routing/Router.php index 10133346e3..3f9b5cf75e 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Routing/Router.php +++ b/src/Symfony/Bundle/FrameworkBundle/Routing/Router.php @@ -16,6 +16,8 @@ use Symfony\Component\Routing\RequestContext; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\Routing\RouteCollection; use Symfony\Component\HttpKernel\CacheWarmer\WarmableInterface; +use Symfony\Component\DependencyInjection\Exception\ParameterNotFoundException; +use Symfony\Component\DependencyInjection\Exception\RuntimeException; /** * This Router only creates the Loader only when the cache is empty. @@ -72,9 +74,12 @@ class Router extends BaseRouter implements WarmableInterface } /** - * Replaces placeholders with service container parameter values in route defaults and requirements. + * Replaces placeholders with service container parameter values in: + * - the route defaults, + * - the route requirements, + * - the route pattern. * - * @param $collection + * @param RouteCollection $collection */ private function resolveParameters(RouteCollection $collection) { @@ -83,27 +88,60 @@ class Router extends BaseRouter implements WarmableInterface $this->resolveParameters($route); } else { foreach ($route->getDefaults() as $name => $value) { - if (!$value || '%' !== $value[0] || '%' !== substr($value, -1)) { - continue; - } - - $key = substr($value, 1, -1); - if ($this->container->hasParameter($key)) { - $route->setDefault($name, $this->container->getParameter($key)); - } + $route->setDefault($name, $this->resolveString($value)); } foreach ($route->getRequirements() as $name => $value) { - if (!$value || '%' !== $value[0] || '%' !== substr($value, -1)) { - continue; - } - - $key = substr($value, 1, -1); - if ($this->container->hasParameter($key)) { - $route->setRequirement($name, $this->container->getParameter($key)); - } + $route->setRequirement($name, $this->resolveString($value)); } + + $route->setPattern($this->resolveString($route->getPattern())); } } } + + /** + * Replaces placeholders with the service container parameters in the given string. + * + * @param string $value The source string which might contain %placeholders% + * + * @return string A string where the placeholders have been replaced. + * + * @throws ParameterNotFoundException When a placeholder does not exist as a container parameter + * @throws RuntimeException When a container value is not a string or a numeric value + */ + private function resolveString($value) + { + $container = $this->container; + + $escapedValue = preg_replace_callback('/%%|%([^%\s]+)%/', function ($match) use ($container, $value) { + // skip %% + if (!isset($match[1])) { + return '%%'; + } + + $key = strtolower($match[1]); + + if (!$container->hasParameter($key)) { + throw new ParameterNotFoundException($key); + } + + $resolved = $container->getParameter($key); + + if (is_string($resolved) || is_numeric($resolved)) { + return (string) $resolved; + } + + throw new RuntimeException(sprintf( + 'A string value must be composed of strings and/or numbers,' . + 'but found parameter "%s" of type %s inside string value "%s".', + $key, + gettype($resolved), + $value) + ); + + }, $value); + + return str_replace('%%', '%', $escapedValue); + } } diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Routing/RouterTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Routing/RouterTest.php index 6c9f072ab1..1d50d7a25f 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/Routing/RouterTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Routing/RouterTest.php @@ -17,72 +17,151 @@ use Symfony\Component\Routing\RouteCollection; class RoutingTest extends \PHPUnit_Framework_TestCase { - public function testPlaceholders() + public function testDefaultsPlaceholders() { $routes = new RouteCollection(); - $routes->add('foo', new Route('/foo', array( - 'foo' => '%foo%', - 'bar' => '%bar%', - 'foobar' => 'foobar', - 'foo1' => '%foo', - 'foo2' => 'foo%', - 'foo3' => 'f%o%o', - ), array( - 'foo' => '%foo%', - 'bar' => '%bar%', - 'foobar' => 'foobar', - 'foo1' => '%foo', - 'foo2' => 'foo%', - 'foo3' => 'f%o%o', - ))); + + $routes->add('foo', new Route( + '/foo', + array( + 'foo' => 'before_%parameter.foo%', + 'bar' => '%parameter.bar%_after', + 'baz' => '%%unescaped%%', + ), + array( + ) + )); $sc = $this->getServiceContainer($routes); - $sc->expects($this->at(1))->method('hasParameter')->will($this->returnValue(false)); - $sc->expects($this->at(2))->method('hasParameter')->will($this->returnValue(true)); - $sc->expects($this->at(3))->method('getParameter')->will($this->returnValue('bar')); - $sc->expects($this->at(4))->method('hasParameter')->will($this->returnValue(false)); - $sc->expects($this->at(5))->method('hasParameter')->will($this->returnValue(true)); - $sc->expects($this->at(6))->method('getParameter')->will($this->returnValue('bar')); + + $sc->expects($this->at(1))->method('hasParameter')->will($this->returnValue(true)); + $sc->expects($this->at(2))->method('getParameter')->will($this->returnValue('foo')); + $sc->expects($this->at(3))->method('hasParameter')->will($this->returnValue(true)); + $sc->expects($this->at(4))->method('getParameter')->will($this->returnValue('bar')); $router = new Router($sc, 'foo'); $route = $router->getRouteCollection()->get('foo'); - $this->assertEquals('%foo%', $route->getDefault('foo')); - $this->assertEquals('bar', $route->getDefault('bar')); - $this->assertEquals('foobar', $route->getDefault('foobar')); - $this->assertEquals('%foo', $route->getDefault('foo1')); - $this->assertEquals('foo%', $route->getDefault('foo2')); - $this->assertEquals('f%o%o', $route->getDefault('foo3')); + $this->assertEquals( + array( + 'foo' => 'before_foo', + 'bar' => 'bar_after', + 'baz' => '%unescaped%', + ), + $route->getDefaults() + ); + } - $this->assertEquals('%foo%', $route->getRequirement('foo')); - $this->assertEquals('bar', $route->getRequirement('bar')); - $this->assertEquals('foobar', $route->getRequirement('foobar')); - $this->assertEquals('%foo', $route->getRequirement('foo1')); - $this->assertEquals('foo%', $route->getRequirement('foo2')); - $this->assertEquals('f%o%o', $route->getRequirement('foo3')); + public function testRequirementsPlaceholders() + { + $routes = new RouteCollection(); + + $routes->add('foo', new Route( + '/foo', + array( + ), + array( + 'foo' => 'before_%parameter.foo%', + 'bar' => '%parameter.bar%_after', + 'baz' => '%%unescaped%%', + ) + )); + + $sc = $this->getServiceContainer($routes); + + $sc->expects($this->at(1))->method('hasParameter')->with('parameter.foo')->will($this->returnValue(true)); + $sc->expects($this->at(2))->method('getParameter')->with('parameter.foo')->will($this->returnValue('foo')); + $sc->expects($this->at(3))->method('hasParameter')->with('parameter.bar')->will($this->returnValue(true)); + $sc->expects($this->at(4))->method('getParameter')->with('parameter.bar')->will($this->returnValue('bar')); + + $router = new Router($sc, 'foo'); + $route = $router->getRouteCollection()->get('foo'); + + $this->assertEquals( + array( + 'foo' => 'before_foo', + 'bar' => 'bar_after', + 'baz' => '%unescaped%', + ), + $route->getRequirements() + ); + } + + public function testPatternPlaceholders() + { + $routes = new RouteCollection(); + + $routes->add('foo', new Route('/before/%parameter.foo%/after/%%unescaped%%')); + + $sc = $this->getServiceContainer($routes); + + $sc->expects($this->at(1))->method('hasParameter')->with('parameter.foo')->will($this->returnValue(true)); + $sc->expects($this->at(2))->method('getParameter')->with('parameter.foo')->will($this->returnValue('foo')); + + $router = new Router($sc, 'foo'); + $route = $router->getRouteCollection()->get('foo'); + + $this->assertEquals( + '/before/foo/after/%unescaped%', + $route->getPattern() + ); + } + + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\ParameterNotFoundException + * @expectedExceptionMessage You have requested a non-existent parameter "nope". + */ + public function testExceptionOnNonExistentParameter() + { + $routes = new RouteCollection(); + + $routes->add('foo', new Route('/%nope%')); + + $sc = $this->getServiceContainer($routes); + + $sc->expects($this->at(1))->method('hasParameter')->with('nope')->will($this->returnValue(false)); + + $router = new Router($sc, 'foo'); + $router->getRouteCollection()->get('foo'); + } + + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException + * @expectedExceptionMessage A string value must be composed of strings and/or numbers,but found parameter "object" of type object inside string value "/%object%". + */ + public function testExceptionOnNonStringParameter() + { + $routes = new RouteCollection(); + + $routes->add('foo', new Route('/%object%')); + + $sc = $this->getServiceContainer($routes); + + $sc->expects($this->at(1))->method('hasParameter')->with('object')->will($this->returnValue(true)); + $sc->expects($this->at(2))->method('getParameter')->with('object')->will($this->returnValue(new \stdClass())); + + $router = new Router($sc, 'foo'); + $router->getRouteCollection()->get('foo'); } private function getServiceContainer(RouteCollection $routes) - { - $sc = $this->getMock('Symfony\Component\DependencyInjection\ContainerInterface'); - $sc - ->expects($this->once()) - ->method('get') - ->will($this->returnValue($this->getLoader($routes))) - ; - - return $sc; - } - - private function getLoader(RouteCollection $routes) { $loader = $this->getMock('Symfony\Component\Config\Loader\LoaderInterface'); + $loader ->expects($this->any()) ->method('load') ->will($this->returnValue($routes)) ; - return $loader; + $sc = $this->getMock('Symfony\\Component\\DependencyInjection\\ContainerInterface'); + + $sc + ->expects($this->once()) + ->method('get') + ->will($this->returnValue($loader)) + ; + + return $sc; } }