diff --git a/core/includes/batch.inc b/core/includes/batch.inc index 0958c3a..9a1c7a3 100644 --- a/core/includes/batch.inc +++ b/core/includes/batch.inc @@ -14,6 +14,7 @@ * @see batch_get() */ +use Drupal\Component\Utility\Html; use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\Timer; use Drupal\Component\Utility\UrlHelper; @@ -102,6 +103,14 @@ function _batch_do() { // Perform actual processing. list($percentage, $message, $label) = _batch_process(); + // Escape unsafe $message or $label variables before they are sent + // to JavaScript. + if (!SafeMarkup::isSafe($message)) { + $message = Html::escape($message); + } + if (!SafeMarkup::isSafe($label)) { + $label = Html::escape($label); + } return new JsonResponse(array('status' => TRUE, 'percentage' => $percentage, 'message' => $message, 'label' => $label)); } @@ -167,10 +176,21 @@ function _batch_progress_page() { $url = $batch['url']->toString(TRUE)->getGeneratedUrl(); + // Ensure messages sent to markup are safe. + if (!SafeMarkup::isSafe($current_set['init_message'])) { + $current_set['init_message'] = Html::escape($current_set['init_message']); + } + + if (!SafeMarkup::isSafe($current_set['error_message'])) { + $current_set['error_message'] = Html::escape($current_set['error_message']); + } + if (!SafeMarkup::isSafe($batch['error_message'])) { + $batch['error_message'] = Html::escape($batch['error_message']); + } $build = array( '#theme' => 'progress_bar', '#percent' => $percentage, - '#message' => array('#markup' => $message), + '#message' => $message, '#label' => $label, '#attached' => array( 'html_head' => array( @@ -315,6 +335,11 @@ function _batch_process() { $progress_message = $old_set['progress_message']; } + // Ensure the message safeness or escape for HTML usage. + if (!SafeMarkup::isSafe($progress_message)) { + $progress_message = Html::escape($progress_message); + } + // Total progress is the number of operations that have fully run plus the // completion level of the current operation. $current = $total - $remaining + $finished; diff --git a/core/includes/form.inc b/core/includes/form.inc index 16353d8..b895b4e 100644 --- a/core/includes/form.inc +++ b/core/includes/form.inc @@ -571,14 +571,6 @@ function template_preprocess_form_element_label(&$variables) { * batch_process('node/1'); * @endcode * - * Note: if the batch 'title', 'init_message', 'progress_message', or - * 'error_message' could contain any user input, it is the responsibility of - * the code calling batch_set() to sanitize them first with a function like - * \Drupal\Component\Utility\SafeMarkup::checkPlain() or - * \Drupal\Component\Utility\Xss::filter(). Furthermore, if the batch operation - * returns any user input in the 'results' or 'message' keys of $context, it - * must also sanitize them first. - * * Sample callback_batch_operation(): * @code * // Simple and artificial: load a node of a given type for a given user @@ -601,8 +593,8 @@ function template_preprocess_form_element_label(&$variables) { * * $nodes = entity_load_multiple_by_properties('node', array('uid' => $uid, 'type' => $type)); * $node = reset($nodes); - * $context['results'][] = $node->id() . ' : ' . SafeMarkup::checkPlain($node->label()); - * $context['message'] = SafeMarkup::checkPlain($node->label()); + * $context['results'][] = $node->id() . ' : ' . $node->label(); + * $context['message'] = $node->label(); * } * * // A more advanced example is a multi-step operation that loads all rows, @@ -621,10 +613,12 @@ function template_preprocess_form_element_label(&$variables) { * ->range(0, $limit) * ->execute(); * foreach ($result as $row) { - * $context['results'][] = $row->id . ' : ' . SafeMarkup::checkPlain($row->title); + * // This can be used by the batch finished callback. If this is displayed + * // to the user via the item list theme then it will be auto-escaped. + * $context['results'][] = $row->id . ' : ' . $row->title; * $context['sandbox']['progress']++; * $context['sandbox']['current_id'] = $row->id; - * $context['message'] = SafeMarkup::checkPlain($row->title); + * $context['message'] = $row->title; * } * if ($context['sandbox']['progress'] != $context['sandbox']['max']) { * $context['finished'] = $context['sandbox']['progress'] / $context['sandbox']['max']; @@ -749,7 +743,7 @@ function batch_set($batch_definition) { // Tweak init_message to avoid the bottom of the page flickering down after // init phase. - $batch_set['init_message'] .= '
 '; + $batch_set['init_message'] = SafeMarkup::format('@message
 ', ['@message' => $batch_set['init_message']]);; // The non-concurrent workflow of batch execution allows us to save // numberOfItems() queries by handling our own counter. diff --git a/core/lib/Drupal/Component/Utility/SafeMarkup.php b/core/lib/Drupal/Component/Utility/SafeMarkup.php index c1c86f5..1e164ce 100644 --- a/core/lib/Drupal/Component/Utility/SafeMarkup.php +++ b/core/lib/Drupal/Component/Utility/SafeMarkup.php @@ -15,9 +15,13 @@ * provides a store for known safe strings and methods to manage them * throughout the page request. * - * Strings sanitized by self::checkPlain() and self::escape() are automatically - * marked safe, as are markup strings created from @link theme_render render - * arrays @endlink via drupal_render(). + * Strings that are considered safe include: + * - Strings sanitized by self::format() unless a !placeholder argument is not + * already marked safe. + * - The output of TranslationManager::translate() and + * TranslationManager::formatPlural(). + * - Markup strings created from @link theme_render render arrays @endlink via + * Renderer::render(). * * This class should be limited to internal use only. Module developers should * instead use the appropriate @@ -27,6 +31,9 @@ * * @see TwigExtension::escapeFilter() * @see twig_render_template() + * @see \Drupal\Core\Render\Renderer::render() + * @see \Drupal\Core\StringTranslation\TranslationManager::translate() + * @see \Drupal\Core\StringTranslation\TranslationManager::formatPlural() * @see sanitization * @see theme_render */ @@ -130,36 +137,6 @@ public static function getAll() { } /** - * Encodes special characters in a plain-text string for display as HTML. - * - * Also validates strings as UTF-8. All processed strings are also - * automatically flagged as safe markup strings for rendering. - * - * @param string $text - * The text to be checked or processed. - * - * @return string - * An HTML safe version of $text, or an empty string if $text is not valid - * UTF-8. - * - * @ingroup sanitization - * - * @deprecated Will be removed before Drupal 8.0.0. Rely on Twig's - * auto-escaping feature, or use the @link theme_render #plain_text @endlink - * key when constructing a render array that contains plain text in order to - * use the renderer's auto-escaping feature. If neither of these are - * possible, \Drupal\Component\Utility\Html::escape() can be used in places - * where explicit escaping is needed. - * - * @see drupal_validate_utf8() - */ - public static function checkPlain($text) { - $string = Html::escape($text); - static::$safeStrings[$string]['html'] = TRUE; - return $string; - } - - /** * Formats a string for HTML display by replacing variable placeholders. * * This method replaces variable placeholders in a string with the requested @@ -213,7 +190,7 @@ public static function checkPlain($text) { * - Non-HTML usage, such as a plain-text email. * - Non-direct HTML output, such as a plain-text variable that will be * printed as an HTML attribute value and therefore formatted with - * self::checkPlain() as part of that. + * \Drupal\Component\Utility\Html::escape() as part of that. * - Some other special reason for suppressing sanitization. * * @return string diff --git a/core/modules/system/src/Tests/Batch/PageTest.php b/core/modules/system/src/Tests/Batch/PageTest.php index 02ce098..d446749 100644 --- a/core/modules/system/src/Tests/Batch/PageTest.php +++ b/core/modules/system/src/Tests/Batch/PageTest.php @@ -68,7 +68,7 @@ function testBatchProgressMessages() { // Go to the initial step only. $this->maximumMetaRefreshCount = 0; $this->drupalGet('batch-test/test-title'); - $this->assertRaw('
Initializing.
 
', 'Initial progress message appears correctly.'); + $this->assertRaw('
Initializing.
 
', 'Initial progress message appears correctly.'); $this->assertNoRaw(' ', 'Initial progress message is not double escaped.'); // Now also go to the next step. $this->maximumMetaRefreshCount = 1; diff --git a/core/modules/views/src/Plugin/views/field/FieldPluginBase.php b/core/modules/views/src/Plugin/views/field/FieldPluginBase.php index 23eccf2..6bfef9b 100644 --- a/core/modules/views/src/Plugin/views/field/FieldPluginBase.php +++ b/core/modules/views/src/Plugin/views/field/FieldPluginBase.php @@ -1565,7 +1565,7 @@ public function getRenderTokens($item) { // Use strip tags as there should never be HTML in the path. // However, we need to preserve special characters like " that - // were removed by SafeMarkup::checkPlain(). + // were removed by \Drupal\Component\Utility\Html::escape(). $tokens["{{ raw_arguments.$arg }}"] = isset($this->view->args[$count]) ? strip_tags(Html::decodeEntities($this->view->args[$count])) : ''; $count++; } diff --git a/core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php b/core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php index 12f2012..26c0f5b 100644 --- a/core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php +++ b/core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php @@ -154,44 +154,6 @@ public function testInvalidSetMultiple() { } /** - * Tests SafeMarkup::checkPlain(). - * - * @dataProvider providerCheckPlain - * @covers ::checkPlain - * - * @param string $text - * The text to provide to SafeMarkup::checkPlain(). - * @param string $expected - * The expected output from the function. - * @param string $message - * The message to provide as output for the test. - * @param bool $ignorewarnings - * Whether or not to ignore PHP 5.3+ invalid multibyte sequence warnings. - */ - function testCheckPlain($text, $expected, $message, $ignorewarnings = FALSE) { - $result = $ignorewarnings ? @SafeMarkup::checkPlain($text) : SafeMarkup::checkPlain($text); - $this->assertEquals($expected, $result, $message); - } - - /** - * Data provider for testCheckPlain(). - * - * @see testCheckPlain() - */ - function providerCheckPlain() { - // Checks that invalid multi-byte sequences are escaped. - $tests[] = array("Foo\xC0barbaz", 'Foo�barbaz', 'SafeMarkup::checkPlain() escapes invalid sequence "Foo\xC0barbaz"', TRUE); - $tests[] = array("\xc2\"", '�"', 'SafeMarkup::checkPlain() escapes invalid sequence "\xc2\""', TRUE); - $tests[] = array("Fooÿñ", "Fooÿñ", 'SafeMarkup::checkPlain() does not escape valid sequence "Fooÿñ"'); - - // Checks that special characters are escaped. - $tests[] = array("