Commit 82db4839 authored by catch's avatar catch

Issue #2549393 by alexpott, dawehner, lauriii, andypost, xjm: Remove...

Issue #2549393 by alexpott, dawehner, lauriii, andypost, xjm: Remove SafeMarkup::escape() and rely more on Twig autoescaping
parent 71e54024
......@@ -127,20 +127,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.
*
......@@ -236,13 +222,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] = '<em class="placeholder">' . static::escape($value) . '</em>';
if (!SafeMarkup::isSafe($value)) {
$value = Html::escape($value);
}
$args[$key] = '<em class="placeholder">' . $value . '</em>';
break;
case '!':
......
......@@ -141,7 +141,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'),
......@@ -488,4 +488,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));
}
}
......@@ -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;
}
......
......@@ -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('&lt;em&gt;will be escaped&lt;/em&gt;<br/><em>will be markup</em><br/><strong>will be rendered</strong>');
}
/**
......
......@@ -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 {
......
......@@ -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' => [
'<em>will be escaped</em>',
$this->t('<em>will be markup</em>'),
['#markup' => '<strong>will be rendered</strong>']
]
);
}
......
<div class="testfilter">
{{ message|testfilter }}
</div>
<div>
{{ safe_join_items|safe_join('<br/>') }}
</div>
......@@ -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(
......
......@@ -161,7 +161,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.
......@@ -170,10 +170,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));
}
}
/**
......@@ -193,25 +197,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 = '<marquee>text</marquee>';
SafeMarkup::set($text);
// SafeMarkup::escape() will not escape the markup tag since the string was
// marked safe above.
$this->assertEquals('<marquee>text</marquee>', SafeMarkup::escape($text));
// SafeMarkup::checkPlain() will escape the markup tag even though the
// string was marked safe above.
$this->assertEquals('&lt;marquee&gt;text&lt;/marquee&gt;', SafeMarkup::checkPlain($text));
}
}
class SafeMarkupTestString {
......
......@@ -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('&lt;script&gt;alert(&#039;here&#039;);&lt;/script&gt;', $twig_extension->escapeFilter($twig, $string_object, 'html', 'UTF-8', TRUE));
}
/**
* @covers ::safeJoin
*/
public function testSafeJoin() {
$renderer = $this->prophesize(RendererInterface::class);
$renderer->render(['#markup' => '<strong>will be rendered</strong>', '#printed' => FALSE])->willReturn('<strong>will be rendered</strong>');
$renderer = $renderer->reveal();
$twig_extension = new TwigExtension($renderer);
$twig_environment = $this->prophesize(TwigEnvironment::class)->reveal();
// Simulate t().
$string = '<em>will be markup</em>';
SafeMarkup::setMultiple([$string => ['html' => TRUE]]);
$items = [
'<em>will be escaped</em>',
$string,
['#markup' => '<strong>will be rendered</strong>']
];
$result = $twig_extension->safeJoin($twig_environment, $items, '<br/>');
$this->assertEquals('&lt;em&gt;will be escaped&lt;/em&gt;<br/><em>will be markup</em><br/><strong>will be rendered</strong>', $result);
}
}
class TwigExtensionTestString {
......
......@@ -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');
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment