diff --git a/src/Symfony/Bridge/Twig/Extension/RoutingExtension.php b/src/Symfony/Bridge/Twig/Extension/RoutingExtension.php index 54befc2cf9..8274abf321 100644 --- a/src/Symfony/Bridge/Twig/Extension/RoutingExtension.php +++ b/src/Symfony/Bridge/Twig/Extension/RoutingExtension.php @@ -35,8 +35,8 @@ class RoutingExtension extends \Twig_Extension public function getFunctions() { return array( - 'url' => new \Twig_Function_Method($this, 'getUrl'), - 'path' => new \Twig_Function_Method($this, 'getPath'), + 'url' => new \Twig_Function_Method($this, 'getUrl', array('is_safe_callback' => array($this, 'isUrlGenerationSafe'))), + 'path' => new \Twig_Function_Method($this, 'getPath', array('is_safe_callback' => array($this, 'isUrlGenerationSafe'))), ); } @@ -50,6 +50,44 @@ class RoutingExtension extends \Twig_Extension return $this->generator->generate($name, $parameters, $schemeRelative ? UrlGeneratorInterface::NETWORK_PATH : UrlGeneratorInterface::ABSOLUTE_URL); } + /** + * Determines at compile time whether the generated URL will be safe and thus + * saving the unneeded automatic escaping for performance reasons. + * + * The URL generation process percent encodes non-alphanumeric characters. So there is no risk + * that malicious/invalid characters are part of the URL. The only character within an URL that + * must be escaped in html is the ampersand ("&") which separates query params. So we cannot mark + * the URL generation as always safe, but only when we are sure there won't be multiple query + * params. This is the case when there are none or only one constant parameter given. + * E.g. we know beforehand this will be safe: + * - path('route') + * - path('route', {'param': 'value'}) + * But the following may not: + * - path('route', var) + * - path('route', {'param': ['val1', 'val2'] }) // a sub-array + * - path('route', {'param1': 'value1', 'param2': 'value2'}) + * If param1 and param2 reference placeholder in the route, it would still be safe. But we don't know. + * + * @param \Twig_Node $argsNode The arguments of the path/url function + * + * @return array An array with the contexts the URL is safe + */ + public function isUrlGenerationSafe(\Twig_Node $argsNode) + { + // support named arguments + $paramsNode = $argsNode->hasNode('parameters') ? $argsNode->getNode('parameters') : ( + $argsNode->hasNode(1) ? $argsNode->getNode(1) : null + ); + + if (null === $paramsNode || $paramsNode instanceof \Twig_Node_Expression_Array && count($paramsNode) <= 2 && + (!$paramsNode->hasNode(1) || $paramsNode->getNode(1) instanceof \Twig_Node_Expression_Constant) + ) { + return array('html'); + } + + return array(); + } + /** * Returns the name of the extension. * diff --git a/src/Symfony/Bridge/Twig/Tests/Extension/HttpKernelExtensionTest.php b/src/Symfony/Bridge/Twig/Tests/Extension/HttpKernelExtensionTest.php index 8e5e4d1d47..077927cd6d 100644 --- a/src/Symfony/Bridge/Twig/Tests/Extension/HttpKernelExtensionTest.php +++ b/src/Symfony/Bridge/Twig/Tests/Extension/HttpKernelExtensionTest.php @@ -20,6 +20,8 @@ class HttpKernelExtensionTest extends TestCase { protected function setUp() { + parent::setUp(); + if (!class_exists('Symfony\Component\HttpKernel\HttpKernel')) { $this->markTestSkipped('The "HttpKernel" component is not available'); } diff --git a/src/Symfony/Bridge/Twig/Tests/Extension/RoutingExtensionTest.php b/src/Symfony/Bridge/Twig/Tests/Extension/RoutingExtensionTest.php new file mode 100644 index 0000000000..3c5d762ca0 --- /dev/null +++ b/src/Symfony/Bridge/Twig/Tests/Extension/RoutingExtensionTest.php @@ -0,0 +1,60 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bridge\Twig\Tests\Extension; + +use Symfony\Bridge\Twig\Extension\RoutingExtension; +use Symfony\Bridge\Twig\Tests\TestCase; + +class RoutingExtensionTest extends TestCase +{ + protected function setUp() + { + parent::setUp(); + + if (!class_exists('Symfony\Component\Routing\Route')) { + $this->markTestSkipped('The "Routing" component is not available'); + } + } + + /** + * @dataProvider getEscapingTemplates + */ + public function testEscaping($template, $mustBeEscaped) + { + $twig = new \Twig_Environment(null, array('debug' => true, 'cache' => false, 'autoescape' => true, 'optimizations' => 0)); + $twig->addExtension(new RoutingExtension($this->getMock('Symfony\Component\Routing\Generator\UrlGeneratorInterface'))); + + $nodes = $twig->parse($twig->tokenize($template)); + + $this->assertSame($mustBeEscaped, $nodes->getNode('body')->getNode(0)->getNode('expr') instanceof \Twig_Node_Expression_Filter); + } + + public function getEscapingTemplates() + { + return array( + array('{{ path("foo") }}', false), + array('{{ path("foo", {}) }}', false), + array('{{ path("foo", { foo: "foo" }) }}', false), + array('{{ path("foo", foo) }}', true), + array('{{ path("foo", { foo: foo }) }}', true), + array('{{ path("foo", { foo: ["foo", "bar"] }) }}', true), + array('{{ path("foo", { foo: "foo", bar: "bar" }) }}', true), + + array('{{ path(name = "foo", parameters = {}) }}', false), + array('{{ path(name = "foo", parameters = { foo: "foo" }) }}', false), + array('{{ path(name = "foo", parameters = foo) }}', true), + array('{{ path(name = "foo", parameters = { foo: ["foo", "bar"] }) }}', true), + array('{{ path(name = "foo", parameters = { foo: foo }) }}', true), + array('{{ path(name = "foo", parameters = { foo: "foo", bar: "bar" }) }}', true), + ); + } +} diff --git a/src/Symfony/Bridge/Twig/Tests/Extension/TranslationExtensionTest.php b/src/Symfony/Bridge/Twig/Tests/Extension/TranslationExtensionTest.php index b5d866679f..2b9c553366 100644 --- a/src/Symfony/Bridge/Twig/Tests/Extension/TranslationExtensionTest.php +++ b/src/Symfony/Bridge/Twig/Tests/Extension/TranslationExtensionTest.php @@ -21,6 +21,8 @@ class TranslationExtensionTest extends TestCase { protected function setUp() { + parent::setUp(); + if (!class_exists('Symfony\Component\Translation\Translator')) { $this->markTestSkipped('The "Translation" component is not available'); }