diff --git a/core/lib/Drupal/Component/Utility/SafeMarkup.php b/core/lib/Drupal/Component/Utility/SafeMarkup.php index b4e0989..35166ff 100644 --- a/core/lib/Drupal/Component/Utility/SafeMarkup.php +++ b/core/lib/Drupal/Component/Utility/SafeMarkup.php @@ -128,20 +128,6 @@ public static function setMultiple(array $safe_strings) { } /** - * Encodes special characters in a plain-text string for display as HTML. - * - * @param string $string - * A string. - * - * @return string - * The escaped string. If $string was already set as safe with - * self::set(), it won't be escaped again. - */ - public static function escape($string) { - return static::isSafe($string) ? $string : static::checkPlain($string); - } - - /** * Gets all strings currently marked as safe. * * This is useful for the batch and form APIs, where it is important to @@ -229,13 +215,18 @@ public static function format($string, array $args) { switch ($key[0]) { case '@': // Escaped only. - $args[$key] = static::escape($value); + if (!SafeMarkup::isSafe($value)) { + $args[$key] = Html::escape($value); + } break; case '%': default: // Escaped and placeholder. - $args[$key] = '' . static::escape($value) . ''; + if (!SafeMarkup::isSafe($value)) { + $value = Html::escape($value); + } + $args[$key] = '' . $value . ''; break; case '!': diff --git a/core/lib/Drupal/Core/Template/TwigExtension.php b/core/lib/Drupal/Core/Template/TwigExtension.php index 443cf73..533e98e 100644 --- a/core/lib/Drupal/Core/Template/TwigExtension.php +++ b/core/lib/Drupal/Core/Template/TwigExtension.php @@ -142,7 +142,7 @@ public function getFilters() { // Implements safe joining. // @todo Make that the default for |join? Upstream issue: // https://github.com/fabpot/Twig/issues/1420 - new \Twig_SimpleFilter('safe_join', 'twig_drupal_join_filter', array('is_safe' => array('html'))), + new \Twig_SimpleFilter('safe_join', [$this, 'safeJoin'], ['needs_environment' => true, 'is_safe' => ['html']]), // Array filters. new \Twig_SimpleFilter('without', 'twig_without'), @@ -514,4 +514,26 @@ public function renderVar($arg) { return $this->renderer->render($arg); } + /** + * Joins several strings together safely. + * + * @param \Twig_Environment $env + * A Twig_Environment instance. + * @param mixed[]|\Traversable $value + * The pieces to join. + * @param string $glue + * The delimiter with which to join the string. Defaults to an empty string. + * This value is expected to be safe for output and user provided data + * should never be used as a glue. + * + * @return string + * The strings joined together. + */ + public function safeJoin(\Twig_Environment $env, $value, $glue = '') { + return implode($glue, array_map(function($item) use ($env) { + // If $item is not marked safe then it will be escaped. + return $this->escapeFilter($env, $item, 'html', NULL, TRUE); + }, $value)); + } + } diff --git a/core/modules/content_translation/src/ContentTranslationHandler.php b/core/modules/content_translation/src/ContentTranslationHandler.php index 10cf5f3..3979fff 100644 --- a/core/modules/content_translation/src/ContentTranslationHandler.php +++ b/core/modules/content_translation/src/ContentTranslationHandler.php @@ -290,8 +290,8 @@ public function entityFormAlter(array &$form, FormStateInterface $form_state, En $title = $this->entityFormTitle($entity); // When editing the original values display just the entity label. if ($form_langcode != $entity_langcode) { - $t_args = array('%language' => $languages[$form_langcode]->getName(), '%title' => $entity->label(), '!title' => $title); - $title = empty($source_langcode) ? t('!title [%language translation]', $t_args) : t('Create %language translation of %title', $t_args); + $t_args = array('%language' => $languages[$form_langcode]->getName(), '%title' => $entity->label(), '@title' => $title); + $title = empty($source_langcode) ? t('@title [%language translation]', $t_args) : t('Create %language translation of %title', $t_args); } $form['#title'] = $title; } diff --git a/core/modules/system/src/Tests/Theme/TwigExtensionTest.php b/core/modules/system/src/Tests/Theme/TwigExtensionTest.php index 7e25ae5..7ca45fb 100644 --- a/core/modules/system/src/Tests/Theme/TwigExtensionTest.php +++ b/core/modules/system/src/Tests/Theme/TwigExtensionTest.php @@ -47,6 +47,8 @@ function testTwigExtensionFilter() { $this->drupalGet('twig-extension-test/filter'); $this->assertText('Every plant is not a mineral.', 'Success: String filtered.'); + // Test safe_join filter. + $this->assertRaw('<em>will be escaped</em>
will be markup
will be rendered'); } /** diff --git a/core/modules/system/tests/modules/batch_test/batch_test.callbacks.inc b/core/modules/system/tests/modules/batch_test/batch_test.callbacks.inc index 461219f..026c8a7 100644 --- a/core/modules/system/tests/modules/batch_test/batch_test.callbacks.inc +++ b/core/modules/system/tests/modules/batch_test/batch_test.callbacks.inc @@ -5,7 +5,7 @@ * Batch callbacks for the Batch API tests. */ -use Drupal\Component\Utility\SafeMarkup; +use Drupal\Component\Utility\Html; use Drupal\Core\Url; use Symfony\Component\HttpFoundation\RedirectResponse; @@ -90,7 +90,7 @@ function _batch_test_nested_batch_callback() { function _batch_test_finished_helper($batch_id, $success, $results, $operations) { if ($results) { foreach ($results as $op => $op_results) { - $messages[] = 'op '. SafeMarkup::escape($op) . ': processed ' . count($op_results) . ' elements'; + $messages[] = 'op '. Html::escape($op) . ': processed ' . count($op_results) . ' elements'; } } else { diff --git a/core/modules/system/tests/modules/twig_extension_test/src/TwigExtensionTestController.php b/core/modules/system/tests/modules/twig_extension_test/src/TwigExtensionTestController.php index 4ff849a..bbfe65e 100644 --- a/core/modules/system/tests/modules/twig_extension_test/src/TwigExtensionTestController.php +++ b/core/modules/system/tests/modules/twig_extension_test/src/TwigExtensionTestController.php @@ -7,10 +7,13 @@ namespace Drupal\twig_extension_test; +use Drupal\Core\StringTranslation\StringTranslationTrait; + /** * Controller routines for Twig extension test routes. */ class TwigExtensionTestController { + use StringTranslationTrait; /** * Menu callback for testing Twig filters in a Twig template. @@ -19,6 +22,11 @@ public function testFilterRender() { return array( '#theme' => 'twig_extension_test_filter', '#message' => 'Every animal is not a mineral.', + '#safe_join_items' => [ + 'will be escaped', + $this->t('will be markup'), + ['#markup' => 'will be rendered'] + ] ); } diff --git a/core/modules/system/tests/modules/twig_extension_test/templates/twig_extension_test.filter.html.twig b/core/modules/system/tests/modules/twig_extension_test/templates/twig_extension_test.filter.html.twig index 1e224d0..981874f 100644 --- a/core/modules/system/tests/modules/twig_extension_test/templates/twig_extension_test.filter.html.twig +++ b/core/modules/system/tests/modules/twig_extension_test/templates/twig_extension_test.filter.html.twig @@ -1,3 +1,6 @@
{{ message|testfilter }}
+
+ {{ safe_join_items|safe_join('
') }} +
diff --git a/core/modules/system/tests/modules/twig_extension_test/twig_extension_test.module b/core/modules/system/tests/modules/twig_extension_test/twig_extension_test.module index 7e89470..1c2729b 100644 --- a/core/modules/system/tests/modules/twig_extension_test/twig_extension_test.module +++ b/core/modules/system/tests/modules/twig_extension_test/twig_extension_test.module @@ -11,7 +11,7 @@ function twig_extension_test_theme($existing, $type, $theme, $path) { return array( 'twig_extension_test_filter' => array( - 'variables' => array('message' => NULL), + 'variables' => array('message' => NULL, 'safe_join_items' => NULL), 'template' => 'twig_extension_test.filter', ), 'twig_extension_test_function' => array( diff --git a/core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php b/core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php index 1278a6d..7ba686a 100644 --- a/core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php +++ b/core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php @@ -160,7 +160,7 @@ function providerCheckPlain() { * * @param string $string * The string to run through SafeMarkup::format(). - * @param string $args + * @param string[] $args * The arguments to pass into SafeMarkup::format(). * @param string $expected * The expected result from calling the function. @@ -169,10 +169,14 @@ function providerCheckPlain() { * @param bool $expected_is_safe * Whether the result is expected to be safe for HTML display. */ - function testFormat($string, $args, $expected, $message, $expected_is_safe) { + public function testFormat($string, array $args, $expected, $message, $expected_is_safe) { $result = SafeMarkup::format($string, $args); $this->assertEquals($expected, $result, $message); $this->assertEquals($expected_is_safe, SafeMarkup::isSafe($result), 'SafeMarkup::format correctly sets the result as safe or not safe.'); + + foreach ($args as $arg) { + $this->assertSame($arg instanceof SafeMarkupTestSafeString, SafeMarkup::isSafe($arg)); + } } /** @@ -192,25 +196,6 @@ function providerFormat() { return $tests; } - /** - * Tests the interaction between the safe list and XSS filtering. - * - * @covers ::escape - */ - public function testAdminXss() { - // Mark the string as safe. This is for test purposes only. - $text = 'text'; - SafeMarkup::set($text); - - // SafeMarkup::escape() will not escape the markup tag since the string was - // marked safe above. - $this->assertEquals('text', SafeMarkup::escape($text)); - - // SafeMarkup::checkPlain() will escape the markup tag even though the - // string was marked safe above. - $this->assertEquals('<marquee>text</marquee>', SafeMarkup::checkPlain($text)); - } - } class SafeMarkupTestString { diff --git a/core/tests/Drupal/Tests/Core/Template/TwigExtensionTest.php b/core/tests/Drupal/Tests/Core/Template/TwigExtensionTest.php index e8718d5..de02997 100644 --- a/core/tests/Drupal/Tests/Core/Template/TwigExtensionTest.php +++ b/core/tests/Drupal/Tests/Core/Template/TwigExtensionTest.php @@ -7,6 +7,9 @@ namespace Drupal\Tests\Core\Template; +use Drupal\Component\Utility\SafeMarkup; +use Drupal\Core\Render\RendererInterface; +use Drupal\Core\Template\TwigEnvironment; use Drupal\Core\Template\TwigExtension; use Drupal\Tests\UnitTestCase; @@ -124,6 +127,31 @@ public function testSafeStringEscaping() { $this->assertSame('<script>alert('here');</script>', $twig_extension->escapeFilter($twig, $string_object, 'html', 'UTF-8', TRUE)); } + /** + * @covers ::safeJoin + */ + public function testSafeJoin() { + $renderer = $this->prophesize(RendererInterface::class); + $renderer->render(['#markup' => 'will be rendered', '#printed' => FALSE])->willReturn('will be rendered'); + $renderer = $renderer->reveal(); + + $twig_extension = new TwigExtension($renderer); + $twig_environment = $this->prophesize(TwigEnvironment::class)->reveal(); + + + // Simulate t(). + $string = 'will be markup'; + SafeMarkup::setMultiple([$string => ['html' => TRUE]]); + + $items = [ + 'will be escaped', + $string, + ['#markup' => 'will be rendered'] + ]; + $result = $twig_extension->safeJoin($twig_environment, $items, '
'); + $this->assertEquals('<em>will be escaped</em>
will be markup
will be rendered', $result); + } + } class TwigExtensionTestString { diff --git a/core/themes/engines/twig/twig.engine b/core/themes/engines/twig/twig.engine index 96d0124..977f094 100644 --- a/core/themes/engines/twig/twig.engine +++ b/core/themes/engines/twig/twig.engine @@ -151,29 +151,3 @@ function twig_without($element) { } return $filtered_element; } - -/** - * Overrides twig_join_filter(). - * - * Safely joins several strings together. - * - * @param array|Traversable $value - * The pieces to join. - * @param string $glue - * The delimiter with which to join the string. Defaults to an empty string. - * This value is expected to be safe for output and user provided data should - * never be used as a glue. - * - * @return \Twig_Markup - * The imploded string, which is wrapped in \Twig_Markup because it is safe. - */ -function twig_drupal_join_filter($value, $glue = '') { - $separator = ''; - $output = ''; - foreach ($value as $item) { - $output .= $separator . SafeMarkup::escape($item); - $separator = $glue; - } - - return new \Twig_Markup($output, 'UTF-8'); -}