diff --git a/core/modules/views/src/Plugin/views/PluginBase.php b/core/modules/views/src/Plugin/views/PluginBase.php index 8c1e31f..69c4f19 100644 --- a/core/modules/views/src/Plugin/views/PluginBase.php +++ b/core/modules/views/src/Plugin/views/PluginBase.php @@ -9,6 +9,7 @@ use Drupal\Component\Plugin\DependentPluginInterface; use Drupal\Component\Utility\SafeMarkup; +use Drupal\Component\Utility\Xss; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Language\LanguageInterface; use Drupal\Core\Plugin\ContainerFactoryPluginInterface; @@ -333,23 +334,27 @@ public function globalTokenReplace($string = '', array $options = array()) { } /** - * Replaces Views' tokens in a given string. It is the responsibility of the - * calling function to ensure $text and $token replacements are sanitized. + * Replaces Views' tokens in a given string. The resulting string will be + * sanitized with Xss::filterAdmin. * * This used to be a simple strtr() scattered throughout the code. Some Views * tokens, such as arguments (e.g.: %1 or !1), still use the old format so we * handle those as well as the new Twig-based tokens (e.g.: {{ field_name }}) * * @param $text - * String with possible tokens. + * Unsanitized string with possible tokens. * @param $tokens * Array of token => replacement_value items. * * @return String */ protected function viewsTokenReplace($text, $tokens) { + if (!strlen($text)) { + // No need to run filterAdmin on an empty string. + return ''; + } if (empty($tokens)) { - return $text; + return Xss::filterAdmin($text); } // Separate Twig tokens from other tokens (e.g.: contextual filter tokens in @@ -370,11 +375,19 @@ protected function viewsTokenReplace($text, $tokens) { // Non-Twig tokens are a straight string replacement, Twig tokens get run // through an inline template for rendering and replacement. $text = strtr($text, $other_tokens); - if ($twig_tokens && !empty($text)) { + if ($twig_tokens) { + // Use the unfiltered text for the Twig template, then filter the output. + // Otherwise, Xss::filterAdmin could remove valid Twig syntax before the + // template is parsed. $build = array( '#type' => 'inline_template', '#template' => $text, '#context' => $twig_tokens, + '#post_render' => [ + function ($children, $elements) { + return Xss::filterAdmin($children); + } + ], ); return $this->getRenderer()->render($build); diff --git a/core/modules/views/src/Plugin/views/field/FieldPluginBase.php b/core/modules/views/src/Plugin/views/field/FieldPluginBase.php index f17df18..44ffe57 100644 --- a/core/modules/views/src/Plugin/views/field/FieldPluginBase.php +++ b/core/modules/views/src/Plugin/views/field/FieldPluginBase.php @@ -1284,9 +1284,7 @@ public function renderText($alter) { * Render this field as user-defined altered text. */ protected function renderAltered($alter, $tokens) { - // Filter this right away as our substitutions are already sanitized. - $template = Xss::filterAdmin($alter['text']); - return $this->viewsTokenReplace($template, $tokens); + return $this->viewsTokenReplace($alter['text'], $tokens); } /** diff --git a/core/modules/views/src/Plugin/views/style/StylePluginBase.php b/core/modules/views/src/Plugin/views/style/StylePluginBase.php index af10867..a29c86c 100644 --- a/core/modules/views/src/Plugin/views/style/StylePluginBase.php +++ b/core/modules/views/src/Plugin/views/style/StylePluginBase.php @@ -239,6 +239,11 @@ public function tokenizeValue($value, $row_index) { $value = $this->viewsTokenReplace($value, $tokens); } + else { + // ::viewsTokenReplace() will run Xss::filterAdmin on the + // resulting string. We do the same here for consistency. + $value = Xss::filterAdmin($value); + } return $value; } diff --git a/core/modules/views/src/Tests/Handler/FieldUnitTest.php b/core/modules/views/src/Tests/Handler/FieldUnitTest.php index 5a5d73f..373730b 100644 --- a/core/modules/views/src/Tests/Handler/FieldUnitTest.php +++ b/core/modules/views/src/Tests/Handler/FieldUnitTest.php @@ -192,7 +192,7 @@ public function testFieldTokens() { $name_field_1->options['alter']['text'] = '{{ name_1 }} {{ name }}'; $name_field_2->options['alter']['alter_text'] = TRUE; - $name_field_2->options['alter']['text'] = '{{ name_2 }} {{ name_1 }}'; + $name_field_2->options['alter']['text'] = '{% if name_2|length > 3 %}{{ name_2 }} {{ name_1 }}{% endif %}'; foreach ($view->result as $row) { $expected_output_0 = $row->views_test_data_name; @@ -247,11 +247,37 @@ public function testFieldTokens() { $output = $renderer->executeInRenderContext(new RenderContext(), function () use ($job_field, $row) { return $job_field->advancedRender($row); }); - $this->assertSubString($output, $old_token, format_string('Make sure the old token style (!token => !value) is not changed in the output (!output)', [ + $this->assertEqual($output, $old_token, format_string('Make sure the old token style (!token => !value) is not changed in the output (!output)', [ '!value' => $random_text, '!output' => $output, '!token' => $job_field->options['alter']['text'], ])); + + // Verify HTML tags are allowed in rewrite templates while token + // replacements are escaped. + $job_field->options['alter']['text'] = '

{{ job }}

'; + $random_text = $this->randomMachineName(); + $job_field->setTestValue('' . $random_text . ''); + $output = $job_field->advancedRender($row); + $this->assertEqual($output, '

<span>' . $random_text . '</span>

', 'Valid tags are allowed in rewrite templates and token replacements.'); + + // Verify '; + $job_field->options['alter']['text'] = $rewrite_template; + $random_text = $this->randomMachineName(); + $job_field->setTestValue($random_text); + $output = $job_field->advancedRender($row); + $this->assertNotSubString($output, ''; + $job_field->options['alter']['text'] = $rewrite_template; + $random_text = $this->randomMachineName(); + $job_field->setTestValue($random_text); + $output = $job_field->advancedRender($row); + $this->assertEqual($output, $random_text, format_string('Make sure a script tag in the template (!template) is removed, leaving only the replaced token in the output (!output)', [ + '!output' => $output, + '!template' => $rewrite_template, + ])); } /** diff --git a/core/modules/views/src/Tests/ViewExecutableTest.php b/core/modules/views/src/Tests/ViewExecutableTest.php index 3771c0d..26e381d 100644 --- a/core/modules/views/src/Tests/ViewExecutableTest.php +++ b/core/modules/views/src/Tests/ViewExecutableTest.php @@ -8,6 +8,7 @@ namespace Drupal\views\Tests; use Drupal\comment\Tests\CommentTestTrait; +use Drupal\Component\Utility\Xss; use Drupal\views\Entity\View; use Drupal\views\Views; use Drupal\views\ViewExecutable; @@ -326,7 +327,7 @@ public function testPropertyMethods() { // Test the title methods. $title = $this->randomString(); $view->setTitle($title); - $this->assertEqual($view->getTitle(), $title); + $this->assertEqual($view->getTitle(), Xss::filterAdmin($title)); } /** diff --git a/core/modules/views/tests/src/Unit/Plugin/field/FieldPluginBaseTest.php b/core/modules/views/tests/src/Unit/Plugin/field/FieldPluginBaseTest.php index 11b6eb8..ec3b0d0 100644 --- a/core/modules/views/tests/src/Unit/Plugin/field/FieldPluginBaseTest.php +++ b/core/modules/views/tests/src/Unit/Plugin/field/FieldPluginBaseTest.php @@ -453,6 +453,7 @@ public function testRenderAsLinkWithPathAndTokens($path, $tokens, $link_html) { '#type' => 'inline_template', '#template' => 'base:test-path/' . explode('/', $path)[1], '#context' => ['foo' => 123], + '#post_render' => [function() {}], ]; $this->renderer->expects($this->once())