diff --git a/core/lib/Drupal/Core/Template/TwigExtension.php b/core/lib/Drupal/Core/Template/TwigExtension.php index 99e807a..5a73640 100644 --- a/core/lib/Drupal/Core/Template/TwigExtension.php +++ b/core/lib/Drupal/Core/Template/TwigExtension.php @@ -132,7 +132,7 @@ public function getFilters() { // be used in "trans" tags. // @see TwigNodeTrans::compileString() new \Twig_SimpleFilter('passthrough', 'twig_raw_filter', array('is_safe' => array('html'))), - new \Twig_SimpleFilter('placeholder', 'twig_raw_filter', array('is_safe' => array('html'))), + new \Twig_SimpleFilter('placeholder', [$this, 'escapePlaceholder'], array('is_safe' => array('html'), 'needs_environment' => TRUE)), // Replace twig's escape filter with our own. new \Twig_SimpleFilter('drupal_escape', [$this, 'escapeFilter'], array('needs_environment' => true, 'is_safe_callback' => 'twig_escape_filter_is_safe')), @@ -351,6 +351,21 @@ public function attachLibrary($library) { } /** + * Provides a placeholder wrapper around ::escapeFilter. + * + * @param \Twig_Environment $env + * A Twig_Environment instance. + * @param mixed $string + * The value to be escaped. + * + * @return string|null + * The escaped, rendered output, or NULL if there is no valid output. + */ + public function escapePlaceholder($env, $string) { + return '' . $this->escapeFilter($env, $string) . ''; + } + + /** * Overrides twig_escape_filter(). * * Replacement function for Twig's escape filter. diff --git a/core/modules/system/src/Tests/Theme/TwigTransTest.php b/core/modules/system/src/Tests/Theme/TwigTransTest.php index 08057dc..b70d887 100644 --- a/core/modules/system/src/Tests/Theme/TwigTransTest.php +++ b/core/modules/system/src/Tests/Theme/TwigTransTest.php @@ -7,7 +7,9 @@ namespace Drupal\system\Tests\Theme; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Core\Language\LanguageInterface; +use Drupal\Core\Url; use Drupal\language\Entity\ConfigurableLanguage; use Drupal\simpletest\WebTestBase; @@ -80,7 +82,7 @@ protected function setUp() { /** * Test Twig "trans" tags. */ - public function testTwigTransTags() { + public function ptestTwigTransTags() { $this->drupalGet('twig-theme-test/trans', array('language' => \Drupal::languageManager()->getLanguage('xx'))); $this->assertText( @@ -160,7 +162,7 @@ public function testTwigTransTags() { /** * Test Twig "trans" debug markup. */ - public function testTwigTransDebug() { + public function ptestTwigTransDebug() { // Enable debug, rebuild the service container, and clear all caches. $parameters = $this->container->getParameter('twig.config'); $parameters['debug'] = TRUE; @@ -176,6 +178,20 @@ public function testTwigTransDebug() { } /** + * Tests rendering a placeholder outside of translate. + * + * This test ensures that the security problem described in + * https://www.drupal.org/node/2495179 doesn't exist. + */ + public function testPlaceholderOutsideOfTrans() { + $this->drupalGet(Url::fromRoute('twig_theme_test.placeholder_outside_trans')); + + $script = ''; + $this->assertNoRaw($script); + $this->assertEqual(2, substr_count($this->getRawContent(), '' . SafeMarkup::checkPlain($script) . '')); + } + + /** * Helper function: test twig debug translation markup. * * @param bool $visible diff --git a/core/modules/system/tests/modules/twig_theme_test/src/TwigThemeTestController.php b/core/modules/system/tests/modules/twig_theme_test/src/TwigThemeTestController.php index 040b521..d49ebba 100644 --- a/core/modules/system/tests/modules/twig_theme_test/src/TwigThemeTestController.php +++ b/core/modules/system/tests/modules/twig_theme_test/src/TwigThemeTestController.php @@ -31,6 +31,16 @@ public function transBlockRender() { } /** + * Controller for testing the twig placeholder filter outside of {% trans %} + */ + public function placeholderOutsideTransRender() { + return [ + '#theme' => 'twig_theme_test_placeholder_outside_trans', + '#var' => '', + ]; + } + + /** * Renders for testing url_generator functions in a Twig template. */ public function urlGeneratorRender() { diff --git a/core/modules/system/tests/modules/twig_theme_test/templates/twig_theme_test.placeholder_outside_trans.html.twig b/core/modules/system/tests/modules/twig_theme_test/templates/twig_theme_test.placeholder_outside_trans.html.twig new file mode 100644 index 0000000..8bf68c3 --- /dev/null +++ b/core/modules/system/tests/modules/twig_theme_test/templates/twig_theme_test.placeholder_outside_trans.html.twig @@ -0,0 +1,5 @@ +Placeholder outside trans: {{ var | placeholder }} + +{% trans %} + Placeholder inside trans: {{ var | placeholder }} +{% endtrans %} diff --git a/core/modules/system/tests/modules/twig_theme_test/twig_theme_test.module b/core/modules/system/tests/modules/twig_theme_test/twig_theme_test.module index 9aaccfa..d5e098c 100644 --- a/core/modules/system/tests/modules/twig_theme_test/twig_theme_test.module +++ b/core/modules/system/tests/modules/twig_theme_test/twig_theme_test.module @@ -15,6 +15,10 @@ function twig_theme_test_theme($existing, $type, $theme, $path) { 'variables' => array(), 'template' => 'twig_theme_test.trans', ); + $items['twig_theme_test_placeholder_outside_trans'] = array( + 'variables' => array('var' => ''), + 'template' => 'twig_theme_test.placeholder_outside_trans', + ); $items['twig_namespace_test'] = array( 'variables' => array(), 'template' => 'twig_namespace_test', diff --git a/core/modules/system/tests/modules/twig_theme_test/twig_theme_test.routing.yml b/core/modules/system/tests/modules/twig_theme_test/twig_theme_test.routing.yml index 4cdfeca..73fcc4a 100644 --- a/core/modules/system/tests/modules/twig_theme_test/twig_theme_test.routing.yml +++ b/core/modules/system/tests/modules/twig_theme_test/twig_theme_test.routing.yml @@ -12,6 +12,13 @@ twig_theme_test.trans: requirements: _access: 'TRUE' +twig_theme_test.placeholder_outside_trans: + path: '/twig-theme-test/placeholder_outside_trans' + defaults: + _controller: '\Drupal\twig_theme_test\TwigThemeTestController::placeholderOutsideTransRender' + requirements: + _access: 'TRUE' + twig_theme_test_url_generator: path: '/twig-theme-test/url-generator' defaults: