Commit 8cf5b80c authored by catch's avatar catch

Issue #2550055 by geertvd, alexpott: Remove SafeMarkup::replace() and use...

Issue #2550055 by geertvd, alexpott: Remove SafeMarkup::replace() and use ViewsRenderPipelineSafeString instead
parent ba2268f0
......@@ -268,51 +268,4 @@ public static function placeholder($text) {
return $string;
}
/**
* Replaces all occurrences of the search string with the replacement string.
*
* Functions identically to str_replace(), but marks the returned output as
* safe if all the inputs and the subject have also been marked as safe.
*
* @param string|array $search
* The value being searched for. An array may be used to designate multiple
* values to search for.
* @param string|array $replace
* The replacement value that replaces found search values. An array may be
* used to designate multiple replacements.
* @param string $subject
* The string or array being searched and replaced on.
*
* @return string
* The passed subject with replaced values.
*/
public static function replace($search, $replace, $subject) {
$output = str_replace($search, $replace, $subject);
// If any replacement is unsafe, then the output is also unsafe, so just
// return the output.
if (!is_array($replace)) {
if (!SafeMarkup::isSafe($replace)) {
return $output;
}
}
else {
foreach ($replace as $replacement) {
if (!SafeMarkup::isSafe($replacement)) {
return $output;
}
}
}
// If the subject is unsafe, then the output is as well, so return it.
if (!SafeMarkup::isSafe($subject)) {
return $output;
}
else {
// If we have reached this point, then all replacements were safe. If the
// subject was also safe, then mark the entire output as safe.
return SafeMarkup::set($output);
}
}
}
......@@ -11,14 +11,12 @@
* Implements hook_views_form_substitutions().
*/
function action_views_form_substitutions() {
// Views SafeMarkup::checkPlain()s the column label, so we need to match that.
$select_all_placeholder = SafeMarkup::checkPlain('<!--action-bulk-form-select-all-->');
$select_all = array(
'#type' => 'checkbox',
'#default_value' => FALSE,
'#attributes' => array('class' => array('action-table-select-all')),
);
return array(
$select_all_placeholder => drupal_render($select_all),
'<!--action-bulk-form-select-all-->' => drupal_render($select_all),
);
}
......@@ -7,6 +7,8 @@
namespace Drupal\views\Tests;
use Drupal\Core\Render\Element\Markup;
use Drupal\Core\Render\RenderContext;
use Drupal\views\Views;
/**
......@@ -97,4 +99,26 @@ public function testHooks() {
}
}
/**
* Tests how hook_views_form_substitutions() makes substitutions.
*
* @see views_test_data_views_form_substitutions()
* @see views_pre_render_views_form_views_form()
*/
public function testViewsPreRenderViewsFormViewsForm() {
$element = [
'output' => [
'#markup' => '<!--will-be-escaped--><!--will-be-not-escaped-->',
'#safe_strategy' => Markup::SAFE_STRATEGY_ESCAPE,
],
'#substitutions' => ['#value' => []],
];
$element = \Drupal::service('renderer')->executeInRenderContext(new RenderContext(), function() use ($element) {
return views_pre_render_views_form_views_form($element);
});
$this->setRawContent((string) $element['output']['#markup']);
$this->assertEscaped('<em>escaped</em>');
$this->assertRaw('<em>unescaped</em>');
}
}
......@@ -19,8 +19,13 @@ function views_test_data_views_query_substitutions(ViewExecutable $view) {
/**
* Implements hook_views_form_substitutions().
*/
function views_test_data_views_form_substitutions(ViewExecutable $view) {
function views_test_data_views_form_substitutions() {
\Drupal::state()->set('views_hook_test_views_form_substitutions', TRUE);
$render = ['#markup' => '<em>unescaped</em>'];
return array(
'<!--will-be-escaped-->' => '<em>escaped</em>',
'<!--will-be-not-escaped-->' => \Drupal::service('renderer')->renderPlain($render),
);
}
/**
......
......@@ -625,7 +625,8 @@ function hook_views_query_substitutions(ViewExecutable $view) {
*
* @return array
* An associative array where each key is a string to be replaced, and the
* corresponding value is its replacement.
* corresponding value is its replacement. The value will be escaped unless it
* is already marked safe.
*/
function hook_views_form_substitutions() {
return array(
......
......@@ -23,6 +23,7 @@
use Drupal\views\ViewExecutable;
use Drupal\Component\Plugin\Exception\PluginException;
use Drupal\views\Entity\View;
use Drupal\views\Render\ViewsRenderPipelineSafeString;
use Drupal\views\Views;
use Drupal\field\FieldConfigInterface;
......@@ -663,12 +664,17 @@ function views_pre_render_views_form_views_form($element) {
// Add in substitutions from hook_views_form_substitutions().
$substitutions = \Drupal::moduleHandler()->invokeAll('views_form_substitutions');
foreach ($substitutions as $placeholder => $substitution) {
$search[] = $placeholder;
$search[] = Html::escape($placeholder);
// Ensure that any replacements made are safe to make.
if (!SafeMarkup::isSafe($substitution)) {
$substitution = Html::escape($substitution);
}
$replace[] = $substitution;
}
// Apply substitutions to the rendered output.
$element['output'] = ['#markup' => SafeMarkup::replace($search, $replace, drupal_render($element['output']))];
$output = str_replace($search, $replace, drupal_render($element['output']));
$element['output'] = ['#markup' => ViewsRenderPipelineSafeString::create($output)];
// Sort, render and add remaining form fields.
$children = Element::children($element, TRUE);
......
......@@ -201,18 +201,6 @@ function testPlaceholder() {
$this->assertEquals('<em class="placeholder">Some text</em>', SafeMarkup::placeholder('Some text'));
}
/**
* Tests SafeMarkup::replace().
*
* @dataProvider providerReplace
* @covers ::replace
*/
public function testReplace($search, $replace, $subject, $expected, $is_safe) {
$result = SafeMarkup::replace($search, $replace, $subject);
$this->assertEquals($expected, $result);
$this->assertEquals($is_safe, SafeMarkup::isSafe($result));
}
/**
* Tests the interaction between the safe list and XSS filtering.
*
......@@ -232,63 +220,6 @@ public function testAdminXss() {
$this->assertEquals('&lt;marquee&gt;text&lt;/marquee&gt;', SafeMarkup::checkPlain($text));
}
/**
* Data provider for testReplace().
*
* @see testReplace()
*/
public function providerReplace() {
$tests = [];
// Subject unsafe.
$tests[] = [
'<placeholder>',
SafeMarkupTestSafeString::create('foo'),
'<placeholder>bazqux',
'foobazqux',
FALSE,
];
// All safe.
$tests[] = [
'<placeholder>',
SafeMarkupTestSafeString::create('foo'),
SafeMarkupTestSafeString::create('<placeholder>barbaz'),
'foobarbaz',
TRUE,
];
// Safe subject, but should result in unsafe string because replacement is
// unsafe.
$tests[] = [
'<placeholder>',
'fubar',
SafeMarkupTestSafeString::create('<placeholder>barbaz'),
'fubarbarbaz',
FALSE,
];
// Array with all safe.
$tests[] = [
['<placeholder1>', '<placeholder2>', '<placeholder3>'],
[SafeMarkupTestSafeString::create('foo'), SafeMarkupTestSafeString::create('bar'), SafeMarkupTestSafeString::create('baz')],
SafeMarkupTestSafeString::create('<placeholder1><placeholder2><placeholder3>'),
'foobarbaz',
TRUE,
];
// Array with unsafe replacement.
$tests[] = [
['<placeholder1>', '<placeholder2>', '<placeholder3>',],
[SafeMarkupTestSafeString::create('bar'), SafeMarkupTestSafeString::create('baz'), 'qux'],
SafeMarkupTestSafeString::create('<placeholder1><placeholder2><placeholder3>'),
'barbazqux',
FALSE,
];
return $tests;
}
}
class SafeMarkupTestString {
......
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