diff --git a/core/includes/batch.inc b/core/includes/batch.inc index 5eff1dc..8bd33b7 100644 --- a/core/includes/batch.inc +++ b/core/includes/batch.inc @@ -14,6 +14,7 @@ * @see batch_get() */ +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\Timer; use Drupal\Core\Batch\Percentage; use Drupal\Core\Page\DefaultHtmlPageRenderer; @@ -44,7 +45,12 @@ function _batch_page(Request $request) { return new RedirectResponse(url('', array('absolute' => TRUE))); } } - + // Restore safe strings from previous batches. + // @todo Ensure we are not storing an excessively large string list in: + // https://www.drupal.org/node/2295823 + if (!empty($batch['safe_strings'])) { + SafeMarkup::setMultiple($batch['safe_strings']); + } // Register database update for the end of processing. drupal_register_shutdown_function('_batch_shutdown'); @@ -481,6 +487,10 @@ function _batch_finished() { */ function _batch_shutdown() { if ($batch = batch_get()) { + // Update safe strings. + // @todo Ensure we are not storing an excessively large string list in: + // https://www.drupal.org/node/2295823 + $batch['safe_strings'] = SafeMarkup::getAll(); \Drupal::service('batch.storage')->update($batch); } } diff --git a/core/includes/bootstrap.inc b/core/includes/bootstrap.inc index 635ddb8..aa92f9c 100644 --- a/core/includes/bootstrap.inc +++ b/core/includes/bootstrap.inc @@ -7,6 +7,7 @@ use Drupal\Component\Datetime\DateTimePlus; use Drupal\Component\Utility\Crypt; use Drupal\Component\Utility\Environment; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\String; use Drupal\Component\Utility\Unicode; use Drupal\Core\DrupalKernel; @@ -888,8 +889,27 @@ function watchdog($type, $message, array $variables = array(), $severity = WATCH * * @return array|null * A multidimensional array with keys corresponding to the set message types. - * The indexed array values of each contain the set messages for that type. - * Or, if there are no messages set, the function returns NULL. + * The indexed array values of each contain the set messages for that type, + * and each message is an associative array with the following format: + * - safe: Boolean indicating whether the message string has been marked as + * safe. Non-safe strings will be escaped automatically. + * - message: The message string. + * So, the following is an example of the full return array structure: + * @code + * array( + * 'status' => array( + * array( + * 'safe' => TRUE, + * 'message' => 'A safe markup string.', + * ), + * array( + * 'safe' => FALSE, + * 'message' => "$arbitrary_user_input to escape.", + * ), + * ), + * ); + * @endcode + * If there are no messages set, the function returns NULL. * * @see drupal_get_messages() * @see theme_status_messages() @@ -901,7 +921,10 @@ function drupal_set_message($message = NULL, $type = 'status', $repeat = FALSE) } if ($repeat || !in_array($message, $_SESSION['messages'][$type])) { - $_SESSION['messages'][$type][] = $message; + $_SESSION['messages'][$type][] = array( + 'safe' => SafeMarkup::isSafe($message), + 'message' => $message, + ); } // Mark this page as being uncacheable. @@ -928,17 +951,25 @@ function drupal_set_message($message = NULL, $type = 'status', $repeat = FALSE) * intact. Defaults to TRUE. * * @return array - * A multidimensional array with keys corresponding to the set message types. - * The indexed array values of each contain the set messages for that type. - * The messages returned are limited to the type specified in the $type - * parameter. If there are no messages of the specified type, an empty array - * is returned. + * An associative, nested array of messages grouped by message type, with + * the top-level keys as the message type. The messages returned are + * limited to the type specified in the $type parameter, if any. If there + * are no messages of the specified type, an empty array is returned. See + * drupal_set_message() for the array structure of indivdual messages. * * @see drupal_set_message() * @see theme_status_messages() */ function drupal_get_messages($type = NULL, $clear_queue = TRUE) { if ($messages = drupal_set_message()) { + foreach ($messages as $message_type => $message_typed_messages) { + foreach ($message_typed_messages as $key => $message) { + if ($message['safe']) { + $message['message'] = SafeMarkup::set($message['message']); + } + $messages[$message_type][$key] = $message['message']; + } + } if ($type) { if ($clear_queue) { unset($_SESSION['messages'][$type]); diff --git a/core/includes/common.inc b/core/includes/common.inc index 1c301f4..6976f9d 100644 --- a/core/includes/common.inc +++ b/core/includes/common.inc @@ -14,6 +14,7 @@ use Drupal\Component\Utility\Bytes; use Drupal\Component\Utility\Crypt; use Drupal\Component\Utility\Number; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\SortArray; use Drupal\Component\Utility\String; use Drupal\Component\Utility\Tags; @@ -442,11 +443,15 @@ function format_rss_item($title, $link, $description, $args = array()) { /** * Formats XML elements. * + * Note: It is the caller's responsibility to sanitize any input parameters. + * This function does not perform sanitization. + * * @param $array * An array where each item represents an element and is either a: * - (key => value) pair (value) * - Associative array with fields: - * - 'key': element name + * - 'key': The element name. Element names are not sanitized, so do not + * pass user input. * - 'value': element contents * - 'attributes': associative array of element attributes * @@ -475,7 +480,11 @@ function format_xml_elements($array) { $output .= ' <' . $key . '>' . (is_array($value) ? format_xml_elements($value) : String::checkPlain($value)) . "\n"; } } - return $output; + // @todo This is marking the output string as safe HTML, but we have only + // sanitized the attributes and tag values, not the tag names, and we + // cannot guarantee the assembled markup is safe. Consider a fix in: + // https://www.drupal.org/node/2296885 + return SafeMarkup::set($output); } /** @@ -859,8 +868,7 @@ function l($text, $path, array $options = array()) { // Sanitize the link text if necessary. $text = $variables['options']['html'] ? $variables['text'] : String::checkPlain($variables['text']); - - return '' . $text . ''; + return SafeMarkup::set('' . $text . ''); } /** @@ -2640,12 +2648,17 @@ function drupal_pre_render_conditional_comments($elements) { /** * Pre-render callback: Renders a generic HTML tag with attributes into #markup. * + * Note: It is the caller's responsibility to sanitize any input parameters. + * This callback does not perform sanitization. + * * @param array $element * An associative array containing: * - #tag: The tag name to output. Typical tags added to the HTML HEAD: * - meta: To provide meta information, such as a page refresh. * - link: To refer to stylesheets and other contextual information. * - script: To load JavaScript. + * The value of #tag is not escaped or sanitized, so do not pass in user + * input. * - #attributes: (optional) An array of HTML attributes to apply to the * tag. * - #value: (optional) A string containing tag content, such as inline @@ -2658,7 +2671,11 @@ function drupal_pre_render_conditional_comments($elements) { function drupal_pre_render_html_tag($element) { $attributes = isset($element['#attributes']) ? new Attribute($element['#attributes']) : ''; if (!isset($element['#value'])) { - $markup = '<' . $element['#tag'] . $attributes . " />\n"; + // This function is intended for internal use, so we assume that no unsafe + // values are passed in #tag. The attributes are already safe because + // Attribute output is already automatically sanitized. + // @todo Escape this properly instead? https://www.drupal.org/node/2296101 + $markup = SafeMarkup::set('<' . $element['#tag'] . $attributes . " />\n"); } else { $markup = '<' . $element['#tag'] . $attributes . '>'; @@ -2670,6 +2687,9 @@ function drupal_pre_render_html_tag($element) { $markup .= $element['#value_suffix']; } $markup .= '\n"; + // @todo We cannot actually guarantee this markup is safe. Consider a fix + // in: https://www.drupal.org/node/2296101 + $markup = SafeMarkup::set($markup); } if (!empty($element['#noscript'])) { $element['#markup'] = ''; @@ -3109,6 +3129,7 @@ function drupal_render(&$elements, $is_recursive_call = FALSE) { if (!$is_recursive_call) { _drupal_render_process_post_render_cache($elements); } + $elements['#markup'] = SafeMarkup::set($elements['#markup']); return $elements['#markup']; } } @@ -3161,6 +3182,11 @@ function drupal_render(&$elements, $is_recursive_call = FALSE) { $elements['#children'] = ''; } + // @todo Simplify after https://drupal.org/node/2273925 + if (isset($elements['#markup'])) { + $elements['#markup'] = SafeMarkup::set($elements['#markup']); + } + // Assume that if #theme is set it represents an implemented hook. $theme_is_implemented = isset($elements['#theme']); @@ -3184,6 +3210,7 @@ function drupal_render(&$elements, $is_recursive_call = FALSE) { foreach ($children as $key) { $elements['#children'] .= drupal_render($elements[$key], TRUE); } + $elements['#children'] = SafeMarkup::set($elements['#children']); } // If #theme is not implemented and the element has raw #markup as a @@ -3194,7 +3221,7 @@ function drupal_render(&$elements, $is_recursive_call = FALSE) { // required. Eventually #theme_wrappers will expect both #markup and // #children to be a single string as #children. if (!$theme_is_implemented && isset($elements['#markup'])) { - $elements['#children'] = $elements['#markup'] . $elements['#children']; + $elements['#children'] = SafeMarkup::set($elements['#markup'] . $elements['#children']); } // Let the theme functions in #theme_wrappers add markup around the rendered @@ -3284,6 +3311,7 @@ function drupal_render(&$elements, $is_recursive_call = FALSE) { } $elements['#printed'] = TRUE; + $elements['#markup'] = SafeMarkup::set($elements['#markup']); return $elements['#markup']; } @@ -3311,7 +3339,7 @@ function drupal_render_children(&$element, $children_keys = NULL) { $output .= drupal_render($element[$key]); } } - return $output; + return SafeMarkup::set($output); } /** diff --git a/core/includes/errors.inc b/core/includes/errors.inc index fd949be..1da90ee 100644 --- a/core/includes/errors.inc +++ b/core/includes/errors.inc @@ -5,10 +5,11 @@ * Functions for error handling. */ +use Drupal\Component\Utility\SafeMarkup; +use Drupal\Component\Utility\String; use Drupal\Component\Utility\Xss; use Drupal\Core\Page\DefaultHtmlPageRenderer; use Drupal\Core\Utility\Error; -use Drupal\Component\Utility\String; use Symfony\Component\HttpFoundation\Response; /** @@ -212,7 +213,7 @@ function _drupal_log_error($error, $fatal = FALSE) { // Generate a backtrace containing only scalar argument values. $message .= '
' . Error::formatBacktrace($backtrace) . '
'; } - drupal_set_message($message, $class, TRUE); + drupal_set_message(SafeMarkup::set($message), $class, TRUE); } if ($fatal) { diff --git a/core/includes/form.inc b/core/includes/form.inc index acb81e8..131c4c2 100644 --- a/core/includes/form.inc +++ b/core/includes/form.inc @@ -7,6 +7,7 @@ use Drupal\Component\Utility\NestedArray; use Drupal\Component\Utility\Number; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\String; use Drupal\Component\Utility\UrlHelper; use Drupal\Component\Utility\Xss; @@ -907,7 +908,7 @@ function form_select_options($element, $choices = NULL) { $options .= ''; } } - return $options; + return SafeMarkup::set($options); } /** @@ -1575,7 +1576,7 @@ function theme_tableselect($variables) { // A header can span over multiple cells and in this case the cells // are passed in an array. The order of this array determines the // order in which they are added. - if (!isset($element['#options'][$key][$fieldname]['data']) && is_array($element['#options'][$key][$fieldname])) { + if (is_array($element['#options'][$key][$fieldname]) && !isset($element['#options'][$key][$fieldname]['data'])) { foreach ($element['#options'][$key][$fieldname] as $cell) { $row['data'][] = $cell; } @@ -1921,13 +1922,13 @@ function form_process_machine_name($element, &$form_state) { $element['#machine_name']['suffix'] = '#' . $suffix_id; if ($element['#machine_name']['standalone']) { - $element['#suffix'] .= '  '; + $element['#suffix'] = SafeMarkup::set($element['#suffix'] . '  '); } else { // Append a field suffix to the source form element, which will contain // the live preview of the machine name. $source += array('#field_suffix' => ''); - $source['#field_suffix'] .= '  '; + $source['#field_suffix'] = SafeMarkup::set($source['#field_suffix'] . '  '); $parents = array_merge($element['#machine_name']['source'], array('#field_suffix')); NestedArray::setValue($form_state['complete_form'], $parents, $source['#field_suffix']); @@ -3104,6 +3105,8 @@ function _form_set_attributes(&$element, $class = array()) { * - css: Array of paths to CSS files to be used on the progress page. * - url_options: options passed to url() when constructing redirect URLs for * the batch. + * - safe_strings: Internal use only. Used to store and retrieve strings + * marked as safe between requests. */ function batch_set($batch_definition) { if ($batch_definition) { @@ -3227,6 +3230,11 @@ function batch_process($redirect = NULL, $url = 'batch', $redirect_callback = NU $request->query->remove('destination'); } + // Store safe strings. + // @todo Ensure we are not storing an excessively large string list in: + // https://www.drupal.org/node/2295823 + $batch['safe_strings'] = SafeMarkup::getAll(); + // Store the batch. \Drupal::service('batch.storage')->create($batch); diff --git a/core/includes/install.core.inc b/core/includes/install.core.inc index 4774a54..214ce28 100644 --- a/core/includes/install.core.inc +++ b/core/includes/install.core.inc @@ -1,5 +1,6 @@ Drupal installation complete'; - - drupal_set_message(t('Congratulations, you installed @drupal!', array( + $success_message = t('Congratulations, you installed @drupal!', array( '@drupal' => drupal_install_profile_distribution_name(), - )) . $pifr_assertion); + )); + drupal_set_message(SafeMarkup::set($success_message . $pifr_assertion)); } /** diff --git a/core/includes/theme.inc b/core/includes/theme.inc index ccebe5d..248c795 100644 --- a/core/includes/theme.inc +++ b/core/includes/theme.inc @@ -8,6 +8,7 @@ * customized by user themes. */ +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\String; use Drupal\Component\Utility\UrlHelper; use Drupal\Component\Utility\Xss; @@ -584,7 +585,7 @@ function _theme($hook, $variables = array()) { $output = ''; if (isset($info['function'])) { if (function_exists($info['function'])) { - $output = $info['function']($variables); + $output = SafeMarkup::set($info['function']($variables)); } } else { @@ -2003,7 +2004,7 @@ function template_preprocess_html(&$variables) { // Construct page title. if ($page->hasTitle()) { $head_title = array( - 'title' => trim(strip_tags($page->getTitle())), + 'title' => SafeMarkup::set(trim(strip_tags($page->getTitle()))), 'name' => String::checkPlain($site_config->get('name')), ); } @@ -2023,7 +2024,13 @@ function template_preprocess_html(&$variables) { } $variables['head_title_array'] = $head_title; - $variables['head_title'] = implode(' | ', $head_title); + $output = ''; + $separator = ''; + foreach ($head_title as $item) { + $output .= $separator . SafeMarkup::escape($item); + $separator = ' | '; + } + $variables['head_title'] = SafeMarkup::set($output); // @todo Remove drupal_*_html_head() and refactor accordingly. $html_heads = drupal_get_html_head(FALSE); diff --git a/core/includes/theme.maintenance.inc b/core/includes/theme.maintenance.inc index 8a19a49..71daaee 100644 --- a/core/includes/theme.maintenance.inc +++ b/core/includes/theme.maintenance.inc @@ -5,6 +5,7 @@ * Theming for maintenance pages. */ +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\Unicode; use Drupal\Core\Site\Settings; @@ -110,6 +111,8 @@ function _drupal_maintenance_theme() { * @param $variables * An associative array containing: * - items: An associative array of maintenance tasks. + * It's the caller's responsibility to ensure this array's items contain no + * dangerous HTML such as SCRIPT tags. * - active: The key for the currently active maintenance task. * * @ingroup themeable @@ -187,6 +190,8 @@ function theme_authorize_report($variables) { * @param $variables * An associative array containing: * - message: The log message. + * It's the caller's responsibility to ensure this string contains no + * dangerous HTML such as SCRIPT tags. * - success: A boolean indicating failure or success. * * @ingroup themeable diff --git a/core/lib/Drupal/Component/Diff/Engine/HWLDFWordAccumulator.php b/core/lib/Drupal/Component/Diff/Engine/HWLDFWordAccumulator.php index 384593b..8c4ebea 100644 --- a/core/lib/Drupal/Component/Diff/Engine/HWLDFWordAccumulator.php +++ b/core/lib/Drupal/Component/Diff/Engine/HWLDFWordAccumulator.php @@ -4,6 +4,7 @@ use Drupal\Component\Utility\String; use Drupal\Component\Utility\Unicode; +use Drupal\Component\Utility\SafeMarkup; /** * Additions by Axel Boldt follow, partly taken from diff.php, phpwiki-1.3.3 @@ -46,7 +47,9 @@ protected function _flushGroup($new_tag) { protected function _flushLine($new_tag) { $this->_flushGroup($new_tag); if ($this->line != '') { - array_push($this->lines, $this->line); + // @todo This is probably not the right place to do this. To be + // addressed in https://drupal.org/node/2280963 + array_push($this->lines, SafeMarkup::set($this->line)); } else { // make empty lines visible by inserting an NBSP diff --git a/core/lib/Drupal/Component/Utility/SafeMarkup.php b/core/lib/Drupal/Component/Utility/SafeMarkup.php new file mode 100644 index 0000000..dc0a6a1 --- /dev/null +++ b/core/lib/Drupal/Component/Utility/SafeMarkup.php @@ -0,0 +1,152 @@ +'); + * @endcode + * + * @param string $string + * The content to be marked as secure. + * @param string $strategy + * The escaping strategy used for this string. Two values are supported + * by default: + * - 'html': (default) The string is safe for use in HTML code. + * - 'all': The string is safe for all use cases. + * See the + * @link http://twig.sensiolabs.org/doc/filters/escape.html Twig escape documentation @endlink + * for more information on escaping strategies in Twig. + * + * @return string + * The input string that was marked as safe. + */ + public static function set($string, $strategy = 'html') { + $string = (string) $string; + static::$safeStrings[$string][$strategy] = TRUE; + return $string; + } + + /** + * Checks if a string is safe to output. + * + * @param string $string + * The content to be checked. + * @param string $strategy + * The escaping strategy. See SafeMarkup::set(). Defaults to 'html'. + * + * @return bool + * TRUE if the string has been marked secure, FALSE otherwise. + */ + public static function isSafe($string, $strategy = 'html') { + return isset(static::$safeStrings[(string) $string][$strategy]) || + isset(static::$safeStrings[(string) $string]['all']); + } + + /** + * Adds previously retrieved known safe strings to the safe string list. + * + * This is useful for the batch and form APIs, where it is important to + * preserve the safe markup state across page requests. The strings will be + * added to any safe strings already marked for the current request. + * + * @param array $safe_strings + * A list of safe strings as previously retrieved by SafeMarkup::getAll(). + * + * @throws \UnexpectedValueException + */ + public static function setMultiple(array $safe_strings) { + foreach ($safe_strings as $string => $strategies) { + foreach ($strategies as $strategy => $value) { + $string = (string) $string; + if ($value === TRUE) { + static::$safeStrings[$string][$strategy] = TRUE; + } + else { + // Danger - something is very wrong. + throw new \UnexpectedValueException('Only the value TRUE is accepted for safe strings'); + } + } + } + } + + /** + * Encodes special characters in a plain-text string for display as HTML. + * + * @param $string + * A string. + * + * @return string + * The escaped string. If $string was already set as safe with + * SafeString::set, it won't be escaped again. + */ + public static function escape($string) { + return static::isSafe($string) ? $string : String::checkPlain($string); + } + + /** + * Retrieves all strings currently marked as safe. + * + * This is useful for the batch and form APIs, where it is important to + * preserve the safe markup state across page requests. + * + * @return array + * Returns all strings currently marked safe. + */ + public static function getAll() { + return static::$safeStrings; + } + +} diff --git a/core/lib/Drupal/Component/Utility/String.php b/core/lib/Drupal/Component/Utility/String.php index 2a3ae58..e48934b 100644 --- a/core/lib/Drupal/Component/Utility/String.php +++ b/core/lib/Drupal/Component/Utility/String.php @@ -7,6 +7,8 @@ namespace Drupal\Component\Utility; +use Drupal\Component\Utility\SafeMarkup; + /** * Provides helpers to operate on strings. * @@ -17,7 +19,8 @@ class String { /** * Encodes special characters in a plain-text string for display as HTML. * - * Also validates strings as UTF-8. + * 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. @@ -29,9 +32,10 @@ class String { * @ingroup sanitization * * @see drupal_validate_utf8() + * @see \Drupal\Component\Utility\SafeMarkup */ public static function checkPlain($text) { - return htmlspecialchars($text, ENT_QUOTES, 'UTF-8'); + return SafeMarkup::set(htmlspecialchars($text, ENT_QUOTES, 'UTF-8')); } /** @@ -65,7 +69,8 @@ public static function decodeEntities($text) { * addition to formatting it. * * @param $string - * A string containing placeholders. + * A string containing placeholders. The string itself is not escaped, any + * unsafe content must be in $args and inserted via placeholders. * @param $args * An associative array of replacements to make. Occurrences in $string of * any key in $args are replaced with the corresponding value, after @@ -111,7 +116,7 @@ public static function format($string, array $args = array()) { // Pass-through. } } - return strtr($string, $args); + return SafeMarkup::set(strtr($string, $args)); } /** @@ -126,7 +131,8 @@ public static function format($string, array $args = array()) { * The formatted text (html). */ public static function placeholder($text) { - return '' . static::checkPlain($text) . ''; + return SafeMarkup::set('' . static::checkPlain($text) . ''); } + } diff --git a/core/lib/Drupal/Component/Utility/Xss.php b/core/lib/Drupal/Component/Utility/Xss.php index dc49913..9189c08 100644 --- a/core/lib/Drupal/Component/Utility/Xss.php +++ b/core/lib/Drupal/Component/Utility/Xss.php @@ -7,6 +7,8 @@ namespace Drupal\Component\Utility; +use Drupal\Component\Utility\SafeMarkup; + /** * Provides helper to filter for cross-site scripting. * @@ -41,12 +43,14 @@ class Xss { * Based on kses by Ulf Harnhammar, see http://sourceforge.net/projects/kses. * For examples of various XSS attacks, see: http://ha.ckers.org/xss.html. * - * This code does four things: + * This code does five things: * - Removes characters and constructs that can trick browsers. * - Makes sure all HTML entities are well-formed. * - Makes sure all HTML tags and attributes are well-formed. * - Makes sure no HTML tags contain URLs with a disallowed protocol (e.g. * javascript:). + * - Marks the sanitized, XSS-safe version of $string as safe markup for + * rendering. * * @param $string * The string with raw HTML in it. It will be stripped of everything that @@ -63,6 +67,7 @@ class Xss { * valid UTF-8. * * @see \Drupal\Component\Utility\Unicode::validateUtf8() + * @see \Drupal\Component\Utility\SafeMarkup * * @ingroup sanitization */ @@ -90,7 +95,7 @@ public static function filter($string, $html_tags = array('a', 'em', 'strong', ' $splitter = function ($matches) use ($html_tags, $mode) { return static::split($matches[1], $html_tags, $mode); }; - return preg_replace_callback('% + return SafeMarkup::set(preg_replace_callback('% ( <(?=[^a-zA-Z!/]) # a lone < | # or @@ -99,7 +104,7 @@ public static function filter($string, $html_tags = array('a', 'em', 'strong', ' <[^>]*(>|$) # a string that starts with a <, up until the > or the end of the string | # or > # just a > - )%x', $splitter, $string); + )%x', $splitter, $string)); } /** diff --git a/core/lib/Drupal/Core/Controller/ExceptionController.php b/core/lib/Drupal/Core/Controller/ExceptionController.php index 4067074..dbd227f 100644 --- a/core/lib/Drupal/Core/Controller/ExceptionController.php +++ b/core/lib/Drupal/Core/Controller/ExceptionController.php @@ -17,6 +17,7 @@ use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpKernel\HttpKernelInterface; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\String; use Symfony\Component\Debug\Exception\FlattenException; use Drupal\Core\ContentNegotiation; @@ -316,7 +317,7 @@ public function on500Html(FlattenException $exception, Request $request) { // Generate a backtrace containing only scalar argument values. $message .= '
' . Error::formatFlattenedBacktrace($backtrace) . '
'; } - drupal_set_message($message, $class, TRUE); + drupal_set_message(SafeMarkup::set($message), $class, TRUE); } $content = $this->t('The website has encountered an error. Please try again later.'); diff --git a/core/lib/Drupal/Core/CoreServiceProvider.php b/core/lib/Drupal/Core/CoreServiceProvider.php index 02a5625..a7f5ef4 100644 --- a/core/lib/Drupal/Core/CoreServiceProvider.php +++ b/core/lib/Drupal/Core/CoreServiceProvider.php @@ -90,9 +90,7 @@ public static function registerTwig(ContainerBuilder $container) { // When in the installer, twig_cache must be FALSE until we know the // files folder is writable. 'cache' => drupal_installation_attempted() ? FALSE : Settings::get('twig_cache', TRUE), - // @todo Remove in followup issue - // @see http://drupal.org/node/1712444. - 'autoescape' => FALSE, + 'autoescape' => TRUE, 'debug' => Settings::get('twig_debug', FALSE), 'auto_reload' => Settings::get('twig_auto_reload', NULL), )) diff --git a/core/lib/Drupal/Core/Form/FormBuilder.php b/core/lib/Drupal/Core/Form/FormBuilder.php index 53ddcb3..db2900c 100644 --- a/core/lib/Drupal/Core/Form/FormBuilder.php +++ b/core/lib/Drupal/Core/Form/FormBuilder.php @@ -9,6 +9,7 @@ use Drupal\Component\Utility\Crypt; use Drupal\Component\Utility\NestedArray; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\String; use Drupal\Component\Utility\UrlHelper; use Drupal\Core\Access\CsrfTokenGenerator; @@ -361,6 +362,13 @@ public function getCache($form_build_id, &$form_state) { require_once DRUPAL_ROOT . '/' . $file; } } + // Retrieve the list of previously known safe strings and store it + // for this request. + // @todo Ensure we are not storing an excessively large string list + // in: https://www.drupal.org/node/2295823 + $form_state['build_info'] += array('safe_strings' => array()); + SafeMarkup::setMultiple($form_state['build_info']['safe_strings']); + unset($form_state['build_info']['safe_strings']); } return $form; } @@ -383,6 +391,12 @@ public function setCache($form_build_id, $form, $form_state) { } // Cache form state. + + // Store the known list of safe strings for form re-use. + // @todo Ensure we are not storing an excessively large string list in: + // https://www.drupal.org/node/2295823 + $form_state['build_info']['safe_strings'] = SafeMarkup::getAll(); + if ($data = array_diff_key($form_state, array_flip($this->getUncacheableKeys()))) { $this->keyValueExpirableFactory->get('form_state')->setWithExpire($form_build_id, $data, $expire); } diff --git a/core/lib/Drupal/Core/Page/HeadElement.php b/core/lib/Drupal/Core/Page/HeadElement.php index 49c1f2d..49c421d 100644 --- a/core/lib/Drupal/Core/Page/HeadElement.php +++ b/core/lib/Drupal/Core/Page/HeadElement.php @@ -7,10 +7,13 @@ namespace Drupal\Core\Page; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Core\Template\Attribute; /** * This class represents an HTML element that appears in the HEAD tag. + * + * @see template_preprocess_html() */ class HeadElement { @@ -52,7 +55,7 @@ public function __toString() { if ($this->noScript) { $string = ""; } - return $string; + return SafeMarkup::set($string); } /** diff --git a/core/lib/Drupal/Core/Page/HtmlPage.php b/core/lib/Drupal/Core/Page/HtmlPage.php index eb4c63d..e6c75f2 100644 --- a/core/lib/Drupal/Core/Page/HtmlPage.php +++ b/core/lib/Drupal/Core/Page/HtmlPage.php @@ -7,6 +7,7 @@ namespace Drupal\Core\Page; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Core\Template\Attribute; /** @@ -84,7 +85,10 @@ public function getHtmlAttributes() { * A string of meta and link tags. */ public function getHead() { - return implode("\n", $this->getMetaElements()) . implode("\n", $this->getLinkElements()); + // Each MetaElement or LinkElement is a subclass of + // \Drupal\Core\Page\HeadElement and generates safe output when __toString() + // is called on it. Thus, the whole concatenation is also safe. + return SafeMarkup::set(implode("\n", $this->getMetaElements()) . implode("\n", $this->getLinkElements())); } /** diff --git a/core/lib/Drupal/Core/StringTranslation/TranslationManager.php b/core/lib/Drupal/Core/StringTranslation/TranslationManager.php index 23deb8c..e6d2a66 100644 --- a/core/lib/Drupal/Core/StringTranslation/TranslationManager.php +++ b/core/lib/Drupal/Core/StringTranslation/TranslationManager.php @@ -7,6 +7,7 @@ namespace Drupal\Core\StringTranslation; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\String; use Drupal\Core\Language\LanguageManagerInterface; use Drupal\Core\StringTranslation\Translator\TranslatorInterface; @@ -140,7 +141,7 @@ public function translate($string, array $args = array(), array $options = array $string = $translation === FALSE ? $string : $translation; if (empty($args)) { - return $string; + return SafeMarkup::set($string); } else { return String::format($string, $args); @@ -160,7 +161,7 @@ public function formatPlural($count, $singular, $plural, array $args = array(), $translated_array = explode(LOCALE_PLURAL_DELIMITER, $translated_strings); if ($count == 1) { - return $translated_array[0]; + return SafeMarkup::set($translated_array[0]); } // Get the plural index through the gettext formula. @@ -168,20 +169,21 @@ public function formatPlural($count, $singular, $plural, array $args = array(), $index = (function_exists('locale_get_plural')) ? locale_get_plural($count, isset($options['langcode']) ? $options['langcode'] : NULL) : -1; if ($index == 0) { // Singular form. - return $translated_array[0]; + $return = $translated_array[0]; } else { if (isset($translated_array[$index])) { // N-th plural form. - return $translated_array[$index]; + $return = $translated_array[$index]; } else { // If the index cannot be computed or there's no translation, use // the second plural form as a fallback (which allows for most flexiblity // with the replaceable @count value). - return $translated_array[1]; + $return = $translated_array[1]; } } + return SafeMarkup::set($return); } /** diff --git a/core/lib/Drupal/Core/Template/Attribute.php b/core/lib/Drupal/Core/Template/Attribute.php index ead5d05..48e2fa1 100644 --- a/core/lib/Drupal/Core/Template/Attribute.php +++ b/core/lib/Drupal/Core/Template/Attribute.php @@ -7,7 +7,7 @@ namespace Drupal\Core\Template; -use Drupal\Component\Utility\String; +use Drupal\Component\Utility\SafeMarkup; /** * A class that can be used for collecting then rendering HTML attributtes. @@ -117,7 +117,7 @@ public function __toString() { $return .= ' ' . $rendered; } } - return $return; + return SafeMarkup::set($return); } /** diff --git a/core/lib/Drupal/Core/Template/TwigExtension.php b/core/lib/Drupal/Core/Template/TwigExtension.php index 4cb31ca..69e0bb3 100644 --- a/core/lib/Drupal/Core/Template/TwigExtension.php +++ b/core/lib/Drupal/Core/Template/TwigExtension.php @@ -38,15 +38,23 @@ public function getFunctions() { public function getFilters() { return array( // Translation filters. - new \Twig_SimpleFilter('t', 't'), - new \Twig_SimpleFilter('trans', 't'), + new \Twig_SimpleFilter('t', 't', array('is_safe' => array('html'))), + new \Twig_SimpleFilter('trans', 't', array('is_safe' => array('html'))), // The "raw" filter is not detectable when parsing "trans" tags. To detect // which prefix must be used for translation (@, !, %), we must clone the // "raw" filter and give it identifiable names. These filters should only // be used in "trans" tags. // @see TwigNodeTrans::compileString() - new \Twig_SimpleFilter('passthrough', 'twig_raw_filter'), - new \Twig_SimpleFilter('placeholder', 'twig_raw_filter'), + new \Twig_SimpleFilter('passthrough', 'twig_raw_filter', array('is_safe' => array('html'))), + new \Twig_SimpleFilter('placeholder', 'twig_raw_filter', array('is_safe' => array('html'))), + + // Replace twig's escape filter with our own. + new \Twig_SimpleFilter('drupal_escape', 'twig_drupal_escape_filter', array('needs_environment' => true, 'is_safe_callback' => 'twig_escape_filter_is_safe')), + + // 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'))), // Array filters. new \Twig_SimpleFilter('without', 'twig_without'), diff --git a/core/lib/Drupal/Core/Template/TwigNodeTrans.php b/core/lib/Drupal/Core/Template/TwigNodeTrans.php index 5bfc6b3..29dea5a 100644 --- a/core/lib/Drupal/Core/Template/TwigNodeTrans.php +++ b/core/lib/Drupal/Core/Template/TwigNodeTrans.php @@ -133,7 +133,13 @@ protected function compileString(\Twig_NodeInterface $body) { while ($n instanceof \Twig_Node_Expression_Filter) { $n = $n->getNode('node'); } - $args = $n->getNode('arguments')->getNode(0); + + $args = $n; + + // Support twig_render_var function in chain. + if ($args instanceof \Twig_Node_Expression_Function) { + $args = $n->getNode('arguments')->getNode(0); + } // Detect if a token implements one of the filters reserved for // modifying the prefix of a token. The default prefix used for diff --git a/core/lib/Drupal/Core/Template/TwigNodeVisitor.php b/core/lib/Drupal/Core/Template/TwigNodeVisitor.php index a24ee50..4915c16 100644 --- a/core/lib/Drupal/Core/Template/TwigNodeVisitor.php +++ b/core/lib/Drupal/Core/Template/TwigNodeVisitor.php @@ -32,6 +32,11 @@ function leaveNode(\Twig_NodeInterface $node, \Twig_Environment $env) { // We use this to inject a call to render_var -> twig_render_var() // before anything is printed. if ($node instanceof \Twig_Node_Print) { + if (!empty($this->skipRenderVarFunction)) { + // No need to add the callback, we have escape active already. + unset($this->skipRenderVarFunction); + return $node; + } $class = get_class($node); $line = $node->getLine(); return new $class( @@ -39,6 +44,17 @@ function leaveNode(\Twig_NodeInterface $node, \Twig_Environment $env) { $line ); } + // Change the 'escape' filter to our own 'drupal_escape' filter. + else if ($node instanceof \Twig_Node_Expression_Filter) { + $name = $node->getNode('filter')->getAttribute('value'); + if ('escape' == $name || 'e' == $name) { + // Use our own escape filter that is SafeMarkup aware. + $node->getNode('filter')->setAttribute('value', 'drupal_escape'); + + // Store that we have a filter active already that knows how to deal with render arrays. + $this->skipRenderVarFunction = TRUE; + } + } return $node; } @@ -47,7 +63,8 @@ function leaveNode(\Twig_NodeInterface $node, \Twig_Environment $env) { * {@inheritdoc} */ function getPriority() { - return 1; + // Just above the Optimizer, which is the normal last one. + return 256; } } diff --git a/core/lib/Drupal/Core/Utility/Error.php b/core/lib/Drupal/Core/Utility/Error.php index e3b084f..2bfee62 100644 --- a/core/lib/Drupal/Core/Utility/Error.php +++ b/core/lib/Drupal/Core/Utility/Error.php @@ -9,6 +9,7 @@ use Drupal\Component\Utility\String; use Drupal\Component\Utility\Xss; +use Drupal\Component\Utility\SafeMarkup; /** * Drupal error utility class. @@ -101,7 +102,7 @@ public static function renderExceptionSafe(\Exception $exception) { // no longer function correctly (as opposed to a user-triggered error), so // we assume that it is safe to include a verbose backtrace. $output .= '
' . static::formatBacktrace($backtrace) . '
'; - return $output; + return SafeMarkup::set($output); } /** diff --git a/core/lib/Drupal/Core/Utility/LinkGenerator.php b/core/lib/Drupal/Core/Utility/LinkGenerator.php index b547d95..4912a05 100644 --- a/core/lib/Drupal/Core/Utility/LinkGenerator.php +++ b/core/lib/Drupal/Core/Utility/LinkGenerator.php @@ -8,11 +8,12 @@ namespace Drupal\Core\Utility; use Drupal\Component\Serialization\Json; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\String; use Drupal\Core\Extension\ModuleHandlerInterface; use Drupal\Core\Path\AliasManagerInterface; -use Drupal\Core\Template\Attribute; use Drupal\Core\Routing\UrlGeneratorInterface; +use Drupal\Core\Template\Attribute; use Drupal\Core\Url; /** @@ -122,8 +123,7 @@ public function generateFromUrl($text, Url $url) { // Sanitize the link text if necessary. $text = $variables['options']['html'] ? $variables['text'] : String::checkPlain($variables['text']); - - return '' . $text . ''; + return SafeMarkup::set('' . $text . ''); } /** diff --git a/core/modules/book/book.admin.inc b/core/modules/book/book.admin.inc index 4379582..9a55869 100644 --- a/core/modules/book/book.admin.inc +++ b/core/modules/book/book.admin.inc @@ -5,6 +5,7 @@ * Administration page callbacks for the Book module. */ +use Drupal\Component\Utility\SafeMarkup; use Drupal\Core\Render\Element; /** @@ -36,9 +37,9 @@ function theme_book_admin_table($variables) { $indentation = array('#theme' => 'indentation', '#size' => $form[$key]['depth']['#value'] - 2); $data = array( - drupal_render($indentation) . drupal_render($form[$key]['title']), + SafeMarkup::set(drupal_render($indentation) . drupal_render($form[$key]['title'])), drupal_render($form[$key]['weight']), - drupal_render($form[$key]['pid']) . drupal_render($form[$key]['nid']), + SafeMarkup::set(drupal_render($form[$key]['pid']) . drupal_render($form[$key]['nid'])), ); $links = array(); $links['view'] = array( diff --git a/core/modules/book/src/BookExport.php b/core/modules/book/src/BookExport.php index f28f855..3231802 100644 --- a/core/modules/book/src/BookExport.php +++ b/core/modules/book/src/BookExport.php @@ -7,6 +7,7 @@ namespace Drupal\book; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\node\NodeInterface; @@ -105,18 +106,16 @@ protected function exportTraverse(array $tree, $callable) { // If there is no valid callable, use the default callback. $callable = !empty($callable) ? $callable : array($this, 'bookNodeExport'); - $output = ''; + $build = array(); foreach ($tree as $data) { // Note- access checking is already performed when building the tree. if ($node = $this->nodeStorage->load($data['link']['nid'])) { $children = $data['below'] ? $this->exportTraverse($data['below'], $callable) : ''; - - $callable_output = call_user_func($callable, $node, $children); - $output .= drupal_render($callable_output); + $build[] = call_user_func($callable, $node, $children); } } - return $output; + return drupal_render($build); } /** diff --git a/core/modules/color/color.module b/core/modules/color/color.module index c82e8d1..f0137cf 100644 --- a/core/modules/color/color.module +++ b/core/modules/color/color.module @@ -7,6 +7,7 @@ use Drupal\Core\Asset\CssOptimizer; use Drupal\Component\Utility\Bytes; use Drupal\Component\Utility\Environment; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\String; use Drupal\Core\Routing\RouteMatchInterface; @@ -274,7 +275,7 @@ function template_preprocess_color_scheme_form(&$variables) { // Attempt to load preview HTML if the theme provides it. $preview_html_path = DRUPAL_ROOT . '/' . (isset($info['preview_html']) ? drupal_get_path('theme', $theme) . '/' . $info['preview_html'] : drupal_get_path('module', 'color') . '/preview.html'); - $variables['html_preview'] = file_get_contents($preview_html_path); + $variables['html_preview'] = SafeMarkup::set(file_get_contents($preview_html_path)); } /** diff --git a/core/modules/comment/comment.module b/core/modules/comment/comment.module index a98ea09..94571e9 100644 --- a/core/modules/comment/comment.module +++ b/core/modules/comment/comment.module @@ -768,7 +768,7 @@ function comment_node_update_index(EntityInterface $node, $langcode) { } } - $return = ''; + $build = array(); if ($index_comments) { foreach (\Drupal::service('comment.manager')->getFields('node') as $field_name => $info) { @@ -782,12 +782,11 @@ function comment_node_update_index(EntityInterface $node, $langcode) { if ($node->get($field_name)->status && $cids = comment_get_thread($node, $field_name, $mode, $comments_per_page)) { $comments = entity_load_multiple('comment', $cids); comment_prepare_thread($comments); - $build = comment_view_multiple($comments); - $return .= drupal_render($build); + $build[] = comment_view_multiple($comments); } } } - return $return; + return drupal_render($build); } /** diff --git a/core/modules/field/field.module b/core/modules/field/field.module index 5695f40..25e09e3 100644 --- a/core/modules/field/field.module +++ b/core/modules/field/field.module @@ -5,6 +5,7 @@ */ use Drupal\Component\Utility\Html; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\Xss; use Drupal\Core\Config\ConfigImporter; use Drupal\Core\Entity\EntityTypeInterface; @@ -259,7 +260,7 @@ function field_entity_bundle_delete($entity_type, $bundle) { * UTF-8. */ function field_filter_xss($string) { - return Html::normalize(Xss::filter($string, _field_filter_xss_allowed_tags())); + return SafeMarkup::set(Html::normalize(Xss::filter($string, _field_filter_xss_allowed_tags()))); } /** diff --git a/core/modules/field/src/Plugin/views/field/Field.php b/core/modules/field/src/Plugin/views/field/Field.php index c678554..c33a070 100644 --- a/core/modules/field/src/Plugin/views/field/Field.php +++ b/core/modules/field/src/Plugin/views/field/Field.php @@ -7,6 +7,8 @@ namespace Drupal\field\Plugin\views\field; +use Drupal\Component\Utility\SafeMarkup; +use Drupal\Component\Utility\String; use Drupal\Component\Utility\Xss; use Drupal\Core\Entity\ContentEntityDatabaseStorage; use Drupal\Core\Entity\EntityInterface; @@ -686,12 +688,22 @@ public function submitGroupByForm(&$form, &$form_state) { */ protected function renderItems($items) { if (!empty($items)) { + $output = ''; if (!$this->options['group_rows']) { - return implode('', $items); + foreach ($items as $item) { + $output .= SafeMarkup::escape($item); + } + return SafeMarkup::set($output); } - if ($this->options['multi_type'] == 'separator') { - return implode(Xss::filterAdmin($this->options['separator']), $items); + $output = ''; + $separator = ''; + $escaped_separator = Xss::filterAdmin($this->options['separator']); + foreach ($items as $item) { + $output .= $separator . SafeMarkup::escape($item); + $separator = $escaped_separator; + } + return SafeMarkup::set($output); } else { $item_list = array( diff --git a/core/modules/field_ui/src/DisplayOverviewBase.php b/core/modules/field_ui/src/DisplayOverviewBase.php index 8a98e93..f450313 100644 --- a/core/modules/field_ui/src/DisplayOverviewBase.php +++ b/core/modules/field_ui/src/DisplayOverviewBase.php @@ -8,6 +8,7 @@ namespace Drupal\field_ui; use Drupal\Component\Plugin\PluginManagerBase; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\String; use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Core\Entity\Display\EntityDisplayInterface; @@ -403,8 +404,14 @@ protected function buildFieldRow(FieldDefinitionInterface $field_definition, Ent $this->alterSettingsSummary($summary, $plugin, $field_definition); if (!empty($summary)) { + $summary_escaped = ''; + $separator = ''; + foreach ($summary as $summary_item) { + $summary_escaped .= $separator . SafeMarkup::escape($summary_item); + $separator = '
'; + } $field_row['settings_summary'] = array( - '#markup' => '
' . implode('
', $summary) . '
', + '#markup' => SafeMarkup::set('
' . $summary_escaped . '
'), '#cell_attributes' => array('class' => array('field-plugin-summary-cell')), ); } diff --git a/core/modules/field_ui/src/FieldConfigListBuilder.php b/core/modules/field_ui/src/FieldConfigListBuilder.php index 0da1bf9..4a00229 100644 --- a/core/modules/field_ui/src/FieldConfigListBuilder.php +++ b/core/modules/field_ui/src/FieldConfigListBuilder.php @@ -7,6 +7,7 @@ namespace Drupal\field_ui; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Core\Config\Entity\ConfigEntityListBuilder; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\EntityManagerInterface; @@ -117,7 +118,13 @@ public function buildRow(EntityInterface $field) { $usage[] = $this->bundles[$field->entity_type][$bundle]['label']; } } - $row['data']['usage'] = implode(', ', $usage); + $usage_escaped = ''; + $separator = ''; + foreach ($usage as $usage_item) { + $usage_escaped .= $separator . SafeMarkup::escape($usage_item); + $separator = ', '; + } + $row['data']['usage'] = SafeMarkup::set($usage_escaped); return $row; } diff --git a/core/modules/file/file.field.inc b/core/modules/file/file.field.inc index 57cd172..2eb9bf3 100644 --- a/core/modules/file/file.field.inc +++ b/core/modules/file/file.field.inc @@ -6,6 +6,7 @@ */ use Drupal\Component\Utility\Html; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\String; use Drupal\Core\Field\FieldDefinitionInterface; use Drupal\Core\Render\Element; @@ -121,7 +122,7 @@ function template_preprocess_file_widget_multiple(&$variables) { $row[] = $display; } $row[] = $weight; - $row[] = $operations; + $row[] = SafeMarkup::set($operations); $rows[] = array( 'data' => $row, 'class' => isset($widget['#attributes']['class']) ? array_merge($widget['#attributes']['class'], array('draggable')) : array('draggable'), diff --git a/core/modules/file/file.module b/core/modules/file/file.module index 97fa6ff..201f66c 100644 --- a/core/modules/file/file.module +++ b/core/modules/file/file.module @@ -5,6 +5,7 @@ * Defines a "managed_file" Form API field and a "file" field for Field module. */ +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\String; use Drupal\Core\Field\FieldDefinitionInterface; use Drupal\Core\Render\Element; @@ -931,10 +932,10 @@ function file_save_upload($form_field_name, $validators = array(), $destination '#theme' => 'item_list', '#items' => $errors, ); - $message .= drupal_render($item_list); + $message = SafeMarkup::set($message . drupal_render($item_list)); } else { - $message .= ' ' . array_pop($errors); + $message = SafeMarkup::set($message . ' ' . SafeMarkup::escape(array_pop($errors))); } drupal_set_message($message, 'error'); $files[$i] = FALSE; diff --git a/core/modules/file/templates/file-upload-help.html.twig b/core/modules/file/templates/file-upload-help.html.twig index fe4d194..8fa6b3e 100644 --- a/core/modules/file/templates/file-upload-help.html.twig +++ b/core/modules/file/templates/file-upload-help.html.twig @@ -11,4 +11,4 @@ * @ingroup themeable */ #} -{{ descriptions|join('
') }} +{{ descriptions|safe_join('
') }} diff --git a/core/modules/filter/filter.module b/core/modules/filter/filter.module index 68b445e..953924b 100644 --- a/core/modules/filter/filter.module +++ b/core/modules/filter/filter.module @@ -8,6 +8,7 @@ use Drupal\Component\Utility\Html; use Drupal\Component\Utility\Unicode; use Drupal\Component\Utility\NestedArray; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\String; use Drupal\Component\Utility\Xss; use Drupal\Core\Cache\Cache; diff --git a/core/modules/filter/src/Plugin/Filter/FilterCaption.php b/core/modules/filter/src/Plugin/Filter/FilterCaption.php index bccaf02..c283908 100644 --- a/core/modules/filter/src/Plugin/Filter/FilterCaption.php +++ b/core/modules/filter/src/Plugin/Filter/FilterCaption.php @@ -8,6 +8,7 @@ namespace Drupal\filter\Plugin\Filter; use Drupal\Component\Utility\Html; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\String; use Drupal\Component\Utility\Unicode; use Drupal\Component\Utility\Xss; @@ -82,7 +83,7 @@ public function process($text, $langcode) { // caption. $filter_caption = array( '#theme' => 'filter_caption', - '#node' => $node->C14N(), + '#node' => SafeMarkup::set($node->C14N()), '#tag' => $node->tagName, '#caption' => $caption, '#align' => $align, diff --git a/core/modules/filter/templates/filter-guidelines.html.twig b/core/modules/filter/templates/filter-guidelines.html.twig index 88a3b47..ecf9b94 100644 --- a/core/modules/filter/templates/filter-guidelines.html.twig +++ b/core/modules/filter/templates/filter-guidelines.html.twig @@ -20,6 +20,6 @@ */ #} -

{{ format.name|escape }}

+

{{ format.name }}

{{ tips }} diff --git a/core/modules/image/image.admin.inc b/core/modules/image/image.admin.inc index 84c5dc0..6e1ba87 100644 --- a/core/modules/image/image.admin.inc +++ b/core/modules/image/image.admin.inc @@ -4,8 +4,11 @@ * @file * Administration pages for image settings. */ + +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\String; use Drupal\Core\Render\Element; + /** * Returns HTML for a listing of the effects within a specific image style. * @@ -30,7 +33,8 @@ function theme_image_style_effects($variables) { } else { // Add the row for adding a new image effect. - $row[] = '
' . drupal_render($form['new']['new']) . drupal_render($form['new']['add']) . '
'; + $cell = '
' . drupal_render($form['new']['new']) . drupal_render($form['new']['add']) . '
'; + $row[] = SafeMarkup::set($cell); $row[] = drupal_render($form['new']['weight']); $row[] = ''; } diff --git a/core/modules/locale/locale.pages.inc b/core/modules/locale/locale.pages.inc index 82506d7..f894f5f 100644 --- a/core/modules/locale/locale.pages.inc +++ b/core/modules/locale/locale.pages.inc @@ -5,6 +5,7 @@ * Interface translation summary, editing and deletion user interfaces. */ +use Drupal\Component\Utility\SafeMarkup; use Drupal\Core\Render\Element; use Drupal\locale\SourceString; use Drupal\locale\TranslationString; @@ -63,7 +64,7 @@ function theme_locale_translate_edit_form_strings($variables) { } $source .= empty($string['context']) ? '' : '
' . t('In Context') . ': ' . $string['context']['#value'] . ''; $rows[] = array( - array('data' => $source), + array('data' => SafeMarkup::set($source)), array('data' => $string['translations']), ); } diff --git a/core/modules/locale/templates/locale-translation-update-info.html.twig b/core/modules/locale/templates/locale-translation-update-info.html.twig index 809028a..15cb4ce 100644 --- a/core/modules/locale/templates/locale-translation-update-info.html.twig +++ b/core/modules/locale/templates/locale-translation-update-info.html.twig @@ -21,7 +21,7 @@
Show description {% if modules %} - {% set module_list = modules|join(', ') %} + {% set module_list = modules|safe_join(', ') %} {% trans %}Updates for: {{ module_list }}{% endtrans %} {% elseif missing_updates_status %} {{ missing_updates_status }} diff --git a/core/modules/node/node.install b/core/modules/node/node.install index d0d6d1c..7df16df 100644 --- a/core/modules/node/node.install +++ b/core/modules/node/node.install @@ -5,6 +5,7 @@ * Install, update and uninstall functions for the node module. */ +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Uuid\Uuid; use Drupal\Core\Language\Language; @@ -29,7 +30,9 @@ function node_requirements($phase) { $requirements['node_access'] = array( 'title' => t('Node Access Permissions'), 'value' => $value, - 'description' => $description . ' ' . l(t('Rebuild permissions'), 'admin/reports/status/rebuild'), + // The result of t() is safe and so is the result of l(). Preserving + // safe object. + 'description' => SafeMarkup::set($description . ' ' . l(t('Rebuild permissions'), 'admin/reports/status/rebuild')), ); } return $requirements; diff --git a/core/modules/node/src/Plugin/Search/NodeSearch.php b/core/modules/node/src/Plugin/Search/NodeSearch.php index 3409383..f8e76a8 100644 --- a/core/modules/node/src/Plugin/Search/NodeSearch.php +++ b/core/modules/node/src/Plugin/Search/NodeSearch.php @@ -7,6 +7,7 @@ namespace Drupal\node\Plugin\Search; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\String; use Drupal\Core\Config\Config; use Drupal\Core\Database\Connection; @@ -266,10 +267,12 @@ public function execute() { $node = $node_storage->load($item->sid)->getTranslation($item->langcode); $build = $node_render->view($node, 'search_result', $item->langcode); unset($build['#theme']); - $node->rendered = drupal_render($build); // Fetch comment count for snippet. - $node->rendered .= ' ' . $this->moduleHandler->invoke('comment', 'node_update_index', array($node, $item->langcode)); + $node->rendered = SafeMarkup::set( + drupal_render($build) . ' ' . + SafeMarkup::escape($this->moduleHandler->invoke('comment', 'node_update_index', array($node, $item->langcode))) + ); $extra = $this->moduleHandler->invokeAll('node_search_result', array($node, $item->langcode)); diff --git a/core/modules/node/src/Plugin/views/row/Rss.php b/core/modules/node/src/Plugin/views/row/Rss.php index a8f9d50..4a7b01d 100644 --- a/core/modules/node/src/Plugin/views/row/Rss.php +++ b/core/modules/node/src/Plugin/views/row/Rss.php @@ -7,6 +7,7 @@ namespace Drupal\node\Plugin\views\row; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\String; use Drupal\views\Plugin\views\row\RowPluginBase; @@ -162,7 +163,7 @@ public function render($row) { } $item = new \stdClass(); - $item->description = $item_text; + $item->description = SafeMarkup::set($item_text); $item->title = $node->label(); $item->link = $node->link; $item->elements = $node->rss_elements; diff --git a/core/modules/rdf/rdf.module b/core/modules/rdf/rdf.module index a49b9a8..eae8606 100644 --- a/core/modules/rdf/rdf.module +++ b/core/modules/rdf/rdf.module @@ -5,6 +5,7 @@ * Enables semantically enriched output for Drupal sites in the form of RDFa. */ +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\String; use Drupal\Core\Routing\RouteMatchInterface; use Drupal\Core\Template\Attribute; @@ -444,8 +445,8 @@ function rdf_preprocess_comment(&$variables) { $author_attributes = array('rel' => $author_mapping['properties']); // Wraps the author variable and the submitted variable which are both // available in comment.html.twig. - $variables['author'] = '' . $variables['author'] . ''; - $variables['submitted'] = '' . $variables['submitted'] . ''; + $variables['author'] = SafeMarkup::set('' . $variables['author'] . ''); + $variables['submitted'] = SafeMarkup::set('' . $variables['submitted'] . ''); } // Adds RDFa markup for the date of the comment. $created_mapping = $mapping->getPreparedFieldMapping('created'); @@ -461,8 +462,8 @@ function rdf_preprocess_comment(&$variables) { $created_metadata_markup = drupal_render($rdf_metadata); // Appends the markup to the created variable and the submitted variable // which are both available in comment.html.twig. - $variables['created'] .= $created_metadata_markup; - $variables['submitted'] .= $created_metadata_markup; + $variables['created'] = SafeMarkup::set(SafeMarkup::escape($variables['created']) . $created_metadata_markup); + $variables['submitted'] = SafeMarkup::set($variables['submitted'] . $created_metadata_markup); } $title_mapping = $mapping->getPreparedFieldMapping('subject'); if (!empty($title_mapping)) { diff --git a/core/modules/responsive_image/responsive_image.module b/core/modules/responsive_image/responsive_image.module index 79e29e2..0c93cb9 100644 --- a/core/modules/responsive_image/responsive_image.module +++ b/core/modules/responsive_image/responsive_image.module @@ -6,6 +6,7 @@ */ use Drupal\breakpoint\Entity\Breakpoint; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Core\Routing\RouteMatchInterface; use \Drupal\Core\Template\Attribute; @@ -195,7 +196,6 @@ function theme_responsive_image($variables) { } $sources = array(); - $output = array(); // Fallback image, output as source with media query. $sources[] = array( @@ -239,6 +239,7 @@ function theme_responsive_image($variables) { } if (!empty($sources)) { + $output = array(); $output[] = ''; // Add source tags to the output. @@ -258,7 +259,7 @@ function theme_responsive_image($variables) { } $output[] = ''; - return implode("\n", $output); + return SafeMarkup::set(implode("\n", $output)); } } diff --git a/core/modules/search/search.module b/core/modules/search/search.module index 6c041d0..b7a071c 100644 --- a/core/modules/search/search.module +++ b/core/modules/search/search.module @@ -5,6 +5,7 @@ * Enables site-wide keyword searching. */ +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\String; use Drupal\Component\Utility\Unicode; use Drupal\Core\Routing\RouteMatchInterface; @@ -720,7 +721,7 @@ function search_excerpt($keys, $text, $langcode = NULL) { // Highlight keywords. Must be done at once to prevent conflicts ('strong' // and ''). $text = trim(preg_replace('/' . $boundary . '(?:' . implode('|', $keys) . ')' . $boundary . '/iu', '\0', ' ' . $text . ' ')); - return $text; + return SafeMarkup::set($text); } /** diff --git a/core/modules/search/tests/modules/search_embedded_form/search_embedded_form.module b/core/modules/search/tests/modules/search_embedded_form/search_embedded_form.module index 2561131..75ddd3d 100644 --- a/core/modules/search/tests/modules/search_embedded_form/search_embedded_form.module +++ b/core/modules/search/tests/modules/search_embedded_form/search_embedded_form.module @@ -9,10 +9,12 @@ * individual product (node) listed in the search results. */ +use Drupal\Component\Utility\SafeMarkup; + /** * Adds the test form to search results. */ function search_embedded_form_preprocess_search_result(&$variables) { $form = \Drupal::formBuilder()->getForm('Drupal\search_embedded_form\Form\SearchEmbeddedForm'); - $variables['snippet'] .= drupal_render($form); + $variables['snippet'] = SafeMarkup::set(SafeMarkup::escape($variables['snippet']) . drupal_render($form)); } diff --git a/core/modules/search/tests/modules/search_extra_type/src/Plugin/Search/SearchExtraTypeSearch.php b/core/modules/search/tests/modules/search_extra_type/src/Plugin/Search/SearchExtraTypeSearch.php index 5caea64..32cfb77 100644 --- a/core/modules/search/tests/modules/search_extra_type/src/Plugin/Search/SearchExtraTypeSearch.php +++ b/core/modules/search/tests/modules/search_extra_type/src/Plugin/Search/SearchExtraTypeSearch.php @@ -7,6 +7,7 @@ namespace Drupal\search_extra_type\Plugin\Search; +use Drupal\Component\Utility\SafeMarkup; use Drupal\search\Plugin\ConfigurableSearchPluginBase; /** @@ -58,7 +59,7 @@ public function execute() { 'link' => url('node'), 'type' => 'Dummy result type', 'title' => 'Dummy title', - 'snippet' => "Dummy search snippet to display. Keywords: {$this->keywords}\n\nConditions: " . print_r($this->searchParameters, TRUE), + 'snippet' => SafeMarkup::set("Dummy search snippet to display. Keywords: {$this->keywords}\n\nConditions: " . print_r($this->searchParameters, TRUE)), ), ); } diff --git a/core/modules/simpletest/src/Form/SimpletestResultsForm.php b/core/modules/simpletest/src/Form/SimpletestResultsForm.php index 3889ee1..e2b4960 100644 --- a/core/modules/simpletest/src/Form/SimpletestResultsForm.php +++ b/core/modules/simpletest/src/Form/SimpletestResultsForm.php @@ -7,6 +7,7 @@ namespace Drupal\simpletest\Form; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Core\Database\Connection; use Drupal\Core\Form\FormBase; use Drupal\simpletest\TestDiscovery; @@ -163,7 +164,8 @@ public function buildForm(array $form, array &$form_state, $test_id = NULL) { $rows = array(); foreach ($assertions as $assertion) { $row = array(); - $row[] = $assertion->message; + // Assertion messages are in code, so we assume they are safe. + $row[] = SafeMarkup::set($assertion->message); $row[] = $assertion->message_group; $row[] = drupal_basename($assertion->file); $row[] = $assertion->line; diff --git a/core/modules/system/src/Form/DateFormatFormBase.php b/core/modules/system/src/Form/DateFormatFormBase.php index 5ae6134..6a4e050 100644 --- a/core/modules/system/src/Form/DateFormatFormBase.php +++ b/core/modules/system/src/Form/DateFormatFormBase.php @@ -124,7 +124,7 @@ public function form(array $form, array &$form_state) { '#machine_name' => array( 'exists' => array($this, 'exists'), 'replace_pattern' =>'([^a-z0-9_]+)|(^custom$)', - 'error' => 'The machine-readable name must be unique, and can only contain lowercase letters, numbers, and underscores. Additionally, it can not be the reserved word "custom".', + 'error' => t('The machine-readable name must be unique, and can only contain lowercase letters, numbers, and underscores. Additionally, it can not be the reserved word "custom".'), ), ); diff --git a/core/modules/system/src/Form/ModulesListForm.php b/core/modules/system/src/Form/ModulesListForm.php index da1ecd0..2c2b6b6 100644 --- a/core/modules/system/src/Form/ModulesListForm.php +++ b/core/modules/system/src/Form/ModulesListForm.php @@ -7,6 +7,7 @@ namespace Drupal\system\Form; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\String; use Drupal\Component\Utility\Unicode; use Drupal\Core\Entity\EntityManagerInterface; diff --git a/core/modules/system/system.admin.inc b/core/modules/system/system.admin.inc index 9ba1a1c..71b0cec 100644 --- a/core/modules/system/system.admin.inc +++ b/core/modules/system/system.admin.inc @@ -5,6 +5,7 @@ * Admin page callbacks for the system module. */ +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\Xss; use Drupal\Core\Cache\Cache; use Drupal\Core\Extension\Extension; @@ -231,7 +232,7 @@ function theme_system_modules_details($variables) { // Add the module label and expand/collapse functionalty. $col2 = ''; - $row[] = array('class' => array('module'), 'data' => $col2); + $row[] = array('class' => array('module'), 'data' => SafeMarkup::set($col2)); // Add the description, along with any modules it requires. $description = ''; @@ -259,9 +260,9 @@ function theme_system_modules_details($variables) { } $details = array( '#type' => 'details', - '#title' => ' ' . drupal_render($module['description']) . '', + '#title' => SafeMarkup::set(' ' . drupal_render($module['description']) . ''), '#attributes' => array('id' => $module['enable']['#id'] . '-description'), - '#description' => $description, + '#description' => SafeMarkup::set($description), ); $col4 = drupal_render($details); $row[] = array('class' => array('description', 'expand'), 'data' => $col4); diff --git a/core/modules/system/system.install b/core/modules/system/system.install index 5edb12f..61a39d9 100644 --- a/core/modules/system/system.install +++ b/core/modules/system/system.install @@ -7,6 +7,7 @@ use Drupal\Component\Utility\Crypt; use Drupal\Component\Utility\Environment; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Core\Database\Database; use Drupal\Core\Language\Language; use Drupal\Core\Site\Settings; @@ -57,7 +58,8 @@ function system_requirements($phase) { if (function_exists('phpinfo')) { $requirements['php'] = array( 'title' => t('PHP'), - 'value' => ($phase == 'runtime') ? $phpversion .' ('. l(t('more information'), 'admin/reports/status/php') .')' : $phpversion, + // $phpversion is safe and output of l() is safe, so this value is safe. + 'value' => SafeMarkup::set(($phase == 'runtime') ? $phpversion . ' (' . l(t('more information'), 'admin/reports/status/php') . ')' : $phpversion), ); } else { @@ -320,7 +322,11 @@ function system_requirements($phase) { 'title' => t('Cron maintenance tasks'), 'severity' => $severity, 'value' => $summary, - 'description' => $description + // @todo This string is concatenated from t() calls, safe drupal_render() + // output, whitespace, and
tags, so is safe. However, as a best + // practice, we should not use SafeMarkup::set() around a variable. Fix + // in: https://www.drupal.org/node/2296929 + 'description' => SafeMarkup::set($description), ); } if ($phase != 'install') { diff --git a/core/modules/system/templates/block--system-branding-block.html.twig b/core/modules/system/templates/block--system-branding-block.html.twig index 2a12c7a..4cf0f1a 100644 --- a/core/modules/system/templates/block--system-branding-block.html.twig +++ b/core/modules/system/templates/block--system-branding-block.html.twig @@ -23,7 +23,7 @@ {% endif %} {% if site_name %} {% endif %} {% if site_slogan %} diff --git a/core/modules/system/templates/datetime.html.twig b/core/modules/system/templates/datetime.html.twig index 25ef788..183b834 100644 --- a/core/modules/system/templates/datetime.html.twig +++ b/core/modules/system/templates/datetime.html.twig @@ -25,5 +25,4 @@ * @see http://www.w3.org/TR/html5-author/the-time-element.html#attr-time-datetime */ #} -{# @todo Revisit once http://drupal.org/node/1825952 is resolved. #} -{{ html ? text|raw : text|escape }} +{{ html ? text|raw : text }} diff --git a/core/modules/system/templates/system-themes-page.html.twig b/core/modules/system/templates/system-themes-page.html.twig index fa0e748..e40545b 100644 --- a/core/modules/system/templates/system-themes-page.html.twig +++ b/core/modules/system/templates/system-themes-page.html.twig @@ -39,7 +39,7 @@

{{- theme.name }} {{ theme.version -}} {% if theme.notes %} - ({{ theme.notes|join(', ') }}) + ({{ theme.notes|safe_join(', ') }}) {%- endif -%}

{{ theme.description }}
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 ca1ea6f..6964668 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,6 +5,8 @@ * Batch callbacks for the Batch API tests. */ +use Drupal\Component\Utility\SafeMarkup; + /** * Performs a simple batch operation. */ @@ -81,7 +83,7 @@ function _batch_test_finished_helper($batch_id, $success, $results, $operations) $messages = array("results for batch $batch_id"); if ($results) { foreach ($results as $op => $op_results) { - $messages[] = 'op '. $op . ': processed ' . count($op_results) . ' elements'; + $messages[] = 'op '. SafeMarkup::escape($op) . ': processed ' . count($op_results) . ' elements'; } } else { @@ -94,7 +96,7 @@ function _batch_test_finished_helper($batch_id, $success, $results, $operations) $messages[] = t('An error occurred while processing @op with arguments:
@args', array('@op' => $error_operation[0], '@args' => print_r($error_operation[1], TRUE))); } - drupal_set_message(implode('
', $messages)); + drupal_set_message(SafeMarkup::set(implode('
', $messages))); } /** diff --git a/core/modules/system/tests/themes/test_theme/templates/theme-test-specific-suggestions--variant--foo.html.twig b/core/modules/system/tests/themes/test_theme/templates/theme-test-specific-suggestions--variant--foo.html.twig index 7e0b485..a464e47 100644 --- a/core/modules/system/tests/themes/test_theme/templates/theme-test-specific-suggestions--variant--foo.html.twig +++ b/core/modules/system/tests/themes/test_theme/templates/theme-test-specific-suggestions--variant--foo.html.twig @@ -2,4 +2,4 @@ Template overridden based on suggestion alter hook determined by the base hook.

Theme hook suggestions: -{{ theme_hook_suggestions|join("
") }}

+{{ theme_hook_suggestions|safe_join("
") }}

diff --git a/core/modules/system/tests/themes/test_theme/templates/theme-test-specific-suggestions--variant.html.twig b/core/modules/system/tests/themes/test_theme/templates/theme-test-specific-suggestions--variant.html.twig index 655db4e..8ac8cd2 100644 --- a/core/modules/system/tests/themes/test_theme/templates/theme-test-specific-suggestions--variant.html.twig +++ b/core/modules/system/tests/themes/test_theme/templates/theme-test-specific-suggestions--variant.html.twig @@ -2,4 +2,4 @@ Template matching the specific theme call.

Theme hook suggestions: -{{ theme_hook_suggestions|join("
") }}

+{{ theme_hook_suggestions|safe_join("
") }}

diff --git a/core/modules/text/src/TextProcessed.php b/core/modules/text/src/TextProcessed.php index e9f0914..321962b 100644 --- a/core/modules/text/src/TextProcessed.php +++ b/core/modules/text/src/TextProcessed.php @@ -7,6 +7,7 @@ namespace Drupal\text; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\String; use Drupal\Core\TypedData\DataDefinitionInterface; use Drupal\Core\TypedData\TypedDataInterface; @@ -60,7 +61,7 @@ public function getValue($langcode = NULL) { else { // Escape all HTML and retain newlines. // @see \Drupal\Core\Field\Plugin\Field\FieldFormatter\StringFormatter - $this->processed = nl2br(String::checkPlain($text)); + $this->processed = SafeMarkup::set(nl2br(String::checkPlain($text))); } return $this->processed; } diff --git a/core/modules/update/update.module b/core/modules/update/update.module index 06f2dd6..e34965e 100644 --- a/core/modules/update/update.module +++ b/core/modules/update/update.module @@ -11,6 +11,7 @@ * ability to install contributed modules and themes via an user interface. */ +use Drupal\Component\Utility\SafeMarkup; use Drupal\Core\Routing\RouteMatchInterface; use Drupal\Core\Site\Settings; use Symfony\Cmf\Component\Routing\RouteObjectInterface; @@ -533,7 +534,8 @@ function _update_message_text($msg_type, $msg_reason, $report_link = FALSE, $lan } } - return $text; + // All strings are t() and empty space concatenated so return SafeMarkup. + return SafeMarkup::set($text); } /** diff --git a/core/modules/update/update.report.inc b/core/modules/update/update.report.inc index b4410e0..756fbbe 100644 --- a/core/modules/update/update.report.inc +++ b/core/modules/update/update.report.inc @@ -5,6 +5,7 @@ * Code required only when rendering the available updates report. */ +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\String; /** @@ -27,8 +28,12 @@ function theme_update_report($variables) { $output = drupal_render($update_last_check); if (!is_array($data)) { + // @todo When converting this theme function to Twig, double-check with the + // caller to ensure $data is not double-escaped. At present, we cannot + // guarantee within this function that it is safe. Address in: + // https://www.drupal.org/node/1898466 $output .= '

' . $data . '

'; - return $output; + return SafeMarkup::set($output); } $header = array(); @@ -269,7 +274,7 @@ function theme_update_report($variables) { $row_key = isset($project['title']) ? drupal_strtolower($project['title']) : drupal_strtolower($project['name']); $rows[$project['project_type']][$row_key] = array( 'class' => array($class), - 'data' => array($row), + 'data' => array(SafeMarkup::set($row)), ); } @@ -305,7 +310,7 @@ function theme_update_report($variables) { ); drupal_render($assets); - return $output; + return SafeMarkup::set($output); } /** diff --git a/core/modules/views/src/Plugin/views/field/FieldPluginBase.php b/core/modules/views/src/Plugin/views/field/FieldPluginBase.php index 704a838..e3d8fac 100644 --- a/core/modules/views/src/Plugin/views/field/FieldPluginBase.php +++ b/core/modules/views/src/Plugin/views/field/FieldPluginBase.php @@ -8,6 +8,7 @@ namespace Drupal\views\Plugin\views\field; use Drupal\Component\Utility\Html; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\String; use Drupal\Component\Utility\UrlHelper; use Drupal\Component\Utility\Xss; @@ -899,7 +900,7 @@ public function buildOptionsForm(&$form, &$form_state) { $form['alter']['help'] = array( '#type' => 'details', '#title' => t('Replacement patterns'), - '#value' => $output, + '#value' => SafeMarkup::set($output), '#states' => array( 'visible' => array( array( @@ -1181,6 +1182,9 @@ public function advancedRender(ResultRow $values) { $this->last_render = $this->renderText($alter); } } + // @todo Fix this in https://www.drupal.org/node/2280961 + $this->last_render = SafeMarkup::set($this->last_render); + return $this->last_render; } diff --git a/core/modules/views/src/Plugin/views/style/Rss.php b/core/modules/views/src/Plugin/views/style/Rss.php index 0a6f8cb..362370f 100644 --- a/core/modules/views/src/Plugin/views/style/Rss.php +++ b/core/modules/views/src/Plugin/views/style/Rss.php @@ -7,6 +7,8 @@ namespace Drupal\views\Plugin\views\style; +use Drupal\Component\Utility\SafeMarkup; + /** * Default style plugin to render an RSS feed. * @@ -138,7 +140,7 @@ public function render() { '#theme' => $this->themeFunctions(), '#view' => $this->view, '#options' => $this->options, - '#rows' => $rows, + '#rows' => SafeMarkup::set($rows), ); unset($this->view->row_index); return drupal_render($build); diff --git a/core/modules/views/views.module b/core/modules/views/views.module index 745d397..6de7b71 100644 --- a/core/modules/views/views.module +++ b/core/modules/views/views.module @@ -9,6 +9,7 @@ * incoming page and block requests. */ +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\String; use Drupal\Core\Cache\Cache; use Drupal\Core\Database\Query\AlterableInterface; diff --git a/core/modules/views/views.theme.inc b/core/modules/views/views.theme.inc index 1603d81..5a06397 100644 --- a/core/modules/views/views.theme.inc +++ b/core/modules/views/views.theme.inc @@ -5,6 +5,7 @@ * Preprocessors and helper functions to make theming easier. */ +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\String; use Drupal\Component\Utility\Xss; use Drupal\Core\Template\Attribute; @@ -541,7 +542,8 @@ function template_preprocess_views_view_table(&$variables) { '#theme' => 'tablesort_indicator', '#style' => $initial, ); - $label .= drupal_render($tablesort_indicator); + $markup = drupal_render($tablesort_indicator); + $label = SafeMarkup::set($label . $markup); } $query['order'] = $field; @@ -632,7 +634,7 @@ function template_preprocess_views_view_table(&$variables) { $field_output = $handler->getField($num, $field); $element_type = $fields[$field]->elementType(TRUE, TRUE); if ($element_type) { - $field_output = '<' . $element_type . '>' . $field_output . ''; + $field_output = SafeMarkup::set('<' . $element_type . '>' . SafeMarkup::escape($field_output) . ''); } // Only bother with separators and stuff if the field shows up. @@ -640,13 +642,17 @@ function template_preprocess_views_view_table(&$variables) { // Place the field into the column, along with an optional separator. if (!empty($column_reference['content'])) { if (!empty($options['info'][$column]['separator'])) { - $column_reference['content'] .= Xss::filterAdmin($options['info'][$column]['separator']); + $safe_content = SafeMarkup::escape($column_reference['content']); + $safe_separator = Xss::filterAdmin($options['info'][$column]['separator']); + $column_reference['content'] = SafeMarkup::set($safe_content . $safe_separator); } } else { $column_reference['content'] = ''; } - $column_reference['content'] .= $field_output; + $safe_content = SafeMarkup::escape($column_reference['content']); + $safe_field_output = SafeMarkup::escape($field_output); + $column_reference['content'] = SafeMarkup::set($safe_content . $safe_field_output); } } $column_reference['attributes'] = new Attribute($column_reference['attributes']); diff --git a/core/modules/views_ui/src/Controller/ViewsUIController.php b/core/modules/views_ui/src/Controller/ViewsUIController.php index a09b063..0138266 100644 --- a/core/modules/views_ui/src/Controller/ViewsUIController.php +++ b/core/modules/views_ui/src/Controller/ViewsUIController.php @@ -7,6 +7,7 @@ namespace Drupal\views_ui\Controller; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\String; use Drupal\Core\Controller\ControllerBase; use Drupal\views\ViewExecutable; @@ -92,7 +93,7 @@ public function reportFields() { foreach ($views as $view) { $rows[$field_name]['data'][1][] = $this->l($view, 'views_ui.edit', array('view' => $view)); } - $rows[$field_name]['data'][1] = implode(', ', $rows[$field_name]['data'][1]); + $rows[$field_name]['data'][1] = SafeMarkup::set(implode(', ', $rows[$field_name]['data'][1])); } // Sort rows by field name. @@ -120,7 +121,7 @@ public function reportPlugins() { foreach ($row['views'] as $row_name => $view) { $row['views'][$row_name] = $this->l($view, 'views_ui.edit', array('view' => $view)); } - $row['views'] = implode(', ', $row['views']); + $row['views'] = SafeMarkup::set(implode(', ', $row['views'])); } // Sort rows by field name. diff --git a/core/modules/views_ui/src/ViewListBuilder.php b/core/modules/views_ui/src/ViewListBuilder.php index a80d33a..7321712 100644 --- a/core/modules/views_ui/src/ViewListBuilder.php +++ b/core/modules/views_ui/src/ViewListBuilder.php @@ -7,6 +7,7 @@ namespace Drupal\views_ui; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\String; use Drupal\Component\Plugin\PluginManagerInterface; use Drupal\Core\Entity\EntityInterface; @@ -80,6 +81,12 @@ public function load() { */ public function buildRow(EntityInterface $view) { $row = parent::buildRow($view); + $display_paths = ''; + $separator = ''; + foreach ($this->getDisplayPaths($view) as $display_path) { + $display_paths .= $separator . SafeMarkup::escape($display_path); + $separator = ', '; + } return array( 'data' => array( 'view_name' => array( @@ -96,7 +103,7 @@ public function buildRow(EntityInterface $view) { 'class' => array('views-table-filter-text-source'), ), 'tag' => $view->get('tag'), - 'path' => implode(', ', $this->getDisplayPaths($view)), + 'path' => SafeMarkup::set($display_paths), 'operations' => $row['operations'], ), 'title' => $this->t('Machine name: @name', array('@name' => $view->id())), diff --git a/core/modules/views_ui/src/ViewUI.php b/core/modules/views_ui/src/ViewUI.php index 3b9cb81..81f85fe 100644 --- a/core/modules/views_ui/src/ViewUI.php +++ b/core/modules/views_ui/src/ViewUI.php @@ -7,6 +7,7 @@ namespace Drupal\views_ui; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\String; use Drupal\Component\Utility\Timer; use Drupal\Component\Utility\Xss; @@ -202,7 +203,7 @@ public function set($property_name, $value, $notify = TRUE) { } public static function getDefaultAJAXMessage() { - return '
' . t("Click on an item to edit that item's details.") . '
'; + return SafeMarkup::set('
' . t("Click on an item to edit that item's details.") . '
'); } /** @@ -677,7 +678,10 @@ public function renderPreview($display_id, $args = array()) { } } } - $rows['query'][] = array('' . t('Query') . '', '
' . String::checkPlain(strtr($query_string, $quoted)) . '
'); + $rows['query'][] = array( + SafeMarkup::set('' . t('Query') . ''), + SafeMarkup::set('
' . String::checkPlain(strtr($query_string, $quoted)) . '
'), + ); if (!empty($this->additionalQueries)) { $queries = '' . t('These queries were run during view rendering:') . ''; foreach ($this->additionalQueries as $query) { @@ -688,18 +692,24 @@ public function renderPreview($display_id, $args = array()) { $queries .= t('[@time ms] @query', array('@time' => round($query['time'] * 100000, 1) / 100000.0, '@query' => $query_string)); } - $rows['query'][] = array('' . t('Other queries') . '', '
' . $queries . '
'); + $rows['query'][] = array( + SafeMarkup::set('' . t('Other queries') . ''), + SafeMarkup::set('
' . $queries . '
'), + ); } } if ($show_info) { - $rows['query'][] = array('' . t('Title') . '', Xss::filterAdmin($this->executable->getTitle())); + $rows['query'][] = array( + SafeMarkup::set('' . t('Title') . ''), + Xss::filterAdmin($this->executable->getTitle()), + ); if (isset($path)) { $path = l($path, $path); } else { $path = t('This display has no path.'); } - $rows['query'][] = array('' . t('Path') . '', $path); + $rows['query'][] = array(SafeMarkup::set('' . t('Path') . ''), $path); } if ($show_stats) { @@ -714,10 +724,10 @@ public function renderPreview($display_id, $args = array()) { // No query was run. Display that information in place of either the // query or the performance statistics, whichever comes first. if ($combined || ($show_location === 'above')) { - $rows['query'] = array(array('' . t('Query') . '', t('No query was run'))); + $rows['query'] = array(array(SafeMarkup::set('' . t('Query') . ''), t('No query was run'))); } else { - $rows['statistics'] = array(array('' . t('Query') . '', t('No query was run'))); + $rows['statistics'] = array(array(SafeMarkup::set('' . t('Query') . ''), t('No query was run'))); } } } diff --git a/core/modules/views_ui/templates/views-ui-display-tab-setting.html.twig b/core/modules/views_ui/templates/views-ui-display-tab-setting.html.twig index 1c67469..12cfda4 100644 --- a/core/modules/views_ui/templates/views-ui-display-tab-setting.html.twig +++ b/core/modules/views_ui/templates/views-ui-display-tab-setting.html.twig @@ -20,6 +20,6 @@ {{ description }} {%- endif %} {% if settings_links %} - {{ settings_links|join(' | ') }} + {{ settings_links|safe_join(' | ') }} {% endif %}
diff --git a/core/modules/views_ui/views_ui.theme.inc b/core/modules/views_ui/views_ui.theme.inc index 430f14a..efa54d9 100644 --- a/core/modules/views_ui/views_ui.theme.inc +++ b/core/modules/views_ui/views_ui.theme.inc @@ -5,6 +5,7 @@ * Preprocessors and theme functions for the Views UI. */ +use Drupal\Component\Utility\SafeMarkup; use Drupal\Core\Render\Element; use Drupal\Core\Template\Attribute; @@ -88,7 +89,19 @@ function template_preprocess_views_ui_display_tab_bucket(&$variables) { */ function template_preprocess_views_ui_view_info(&$variables) { $variables['title'] = $variables['view']->label(); - $variables['displays'] = empty($variables['displays']) ? t('None') : format_plural(count($variables['displays']), 'Display', 'Displays') . ': ' . '' . implode(', ', $variables['displays']) . ''; + if (empty($variables['displays'])) { + $displays = t('None'); + } + else { + $displays = format_plural(count($variables['displays']), 'Display', 'Displays') . ': '; + $separator = ''; + foreach ($variables['displays'] as $displays_item) { + $displays .= $separator . SafeMarkup::escape($displays_item); + $separator = ', '; + } + $displays = SafeMarkup::set($displays . ''); + } + $variables['displays'] = $displays; } /** diff --git a/core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php b/core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php new file mode 100644 index 0000000..cd0ee1d --- /dev/null +++ b/core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php @@ -0,0 +1,111 @@ + 'SafeMarkup tests', + 'description' => 'Confirm that SafeMarkup methods work correctly.', + 'group' => 'Common', + ); + } + + /** + * Tests SafeMarkup::set() and SafeMarkup::isSafe(). + * + * @dataProvider providerSet + * + * @param string $text + * The text or object to provide to SafeMarkup::set(). + * @param string $message + * The message to provide as output for the test. + * + * @covers ::set() + */ + public function testSet($text, $message) { + $returned = SafeMarkup::set($text); + $this->assertTrue(is_string($returned), 'The return value of SafeMarkup::set() is really a string'); + $this->assertEquals($returned, $text, 'The passed in value should be equal to the string value according to PHP'); + $this->assertTrue(SafeMarkup::isSafe($text), $message); + $this->assertTrue(SafeMarkup::isSafe($returned), 'The return value has been marked as safe'); + } + + /** + * Data provider for testSet(). + * + * @see testSet() + */ + public function providerSet() { + // Checks that invalid multi-byte sequences are rejected. + $tests[] = array("Foo\xC0barbaz", '', 'String::checkPlain() rejects invalid sequence "Foo\xC0barbaz"', TRUE); + $tests[] = array("Fooÿñ", 'SafeMarkup::set() accepts valid sequence "Fooÿñ"'); + $tests[] = array(new TextWrapper("Fooÿñ"), 'SafeMarkup::set() accepts valid sequence "Fooÿñ" in an object implementing __toString()'); + $tests[] = array("
", 'SafeMarkup::set() accepts HTML'); + + return $tests; + } + + /** + * Tests SafeMarkup::set() and SafeMarkup::isSafe() with different providers. + * + * @covers ::isSafe() + */ + public function testStrategy() { + $returned = SafeMarkup::set('string0', 'html'); + $this->assertTrue(SafeMarkup::isSafe($returned), 'String set with "html" provider is safe for default (html)'); + $returned = SafeMarkup::set('string1', 'all'); + $this->assertTrue(SafeMarkup::isSafe($returned), 'String set with "all" provider is safe for default (html)'); + $returned = SafeMarkup::set('string2', 'css'); + $this->assertFalse(SafeMarkup::isSafe($returned), 'String set with "css" provider is not safe for default (html)'); + $returned = SafeMarkup::set('string3'); + $this->assertFalse(SafeMarkup::isSafe($returned, 'all'), 'String set with "html" provider is not safe for "all"'); + } + + /** + * Tests SafeMarkup::setMultiple(). + * + * @covers ::setMultiple() + */ + public function testSetMultiple() { + $texts = array( + 'multistring0' => array('html' => TRUE), + 'multistring1' => array('all' => TRUE), + ); + SafeMarkup::setMultiple($texts); + foreach ($texts as $string => $providers) { + $this->assertTrue(SafeMarkup::isSafe($string), 'The value has been marked as safe for html'); + } + } + + /** + * Tests SafeMarkup::setMultiple(). + * + * Only TRUE may be passed in as the value. + * + * @covers ::setMultiple() + * + * @expectedException \UnexpectedValueException + */ + public function testInvalidSetMultiple() { + $texts = array( + 'invalidstring0' => array('html' => 1), + ); + SafeMarkup::setMultiple($texts); + } + +} diff --git a/core/tests/Drupal/Tests/Component/Utility/TextWrapper.php b/core/tests/Drupal/Tests/Component/Utility/TextWrapper.php new file mode 100644 index 0000000..53f25f4 --- /dev/null +++ b/core/tests/Drupal/Tests/Component/Utility/TextWrapper.php @@ -0,0 +1,39 @@ +text = $text; + } + + /** + * Magic method + * + * @return string + */ + public function __toString() { + return $this->text; + } + +} diff --git a/core/themes/bartik/templates/block--system-branding-block.html.twig b/core/themes/bartik/templates/block--system-branding-block.html.twig index 5917f58..f6147a6 100644 --- a/core/themes/bartik/templates/block--system-branding-block.html.twig +++ b/core/themes/bartik/templates/block--system-branding-block.html.twig @@ -23,7 +23,7 @@
{% if site_name %} - {{ site_name|e }} + {{ site_name }} {% endif %} {% if site_slogan %} diff --git a/core/themes/engines/twig/twig.engine b/core/themes/engines/twig/twig.engine index 1595bf8..e0cfcc2 100644 --- a/core/themes/engines/twig/twig.engine +++ b/core/themes/engines/twig/twig.engine @@ -5,6 +5,8 @@ * Handles integration of Twig templates with the Drupal theme system. */ +use Drupal\Component\Utility\SafeMarkup; +use Drupal\Component\Utility\String; use Drupal\Core\Extension\Extension; /** @@ -45,6 +47,7 @@ function twig_init(Extension $theme) { * The output generated by the template, plus any debug information. */ function twig_render_template($template_file, $variables) { + /** @var \Twig_Environment $twig_service */ $twig_service = \Drupal::service('twig'); $output = array( 'debug_prefix' => '', @@ -93,7 +96,7 @@ function twig_render_template($template_file, $variables) { $output['debug_info'] .= "\n\n"; $output['debug_suffix'] .= "\n\n\n"; } - return implode('', $output); + return SafeMarkup::set(implode('', $output)); } /** @@ -127,8 +130,8 @@ function twig_render_var($arg) { return NULL; } - // Keep Twig_Markup objects intact to prepare for later autoescaping support. - if ($arg instanceOf Twig_Markup) { + // Optimize for strings as it is likely they come from the escape filter. + if (is_string($arg)) { return $arg; } @@ -140,7 +143,9 @@ function twig_render_var($arg) { if (method_exists($arg, '__toString')) { return (string) $arg; } - throw new Exception(t('Object of type "@class" cannot be printed.', array('@class' => get_class($arg)))); + else { + throw new Exception(t('Object of type "@class" cannot be printed.', array('@class' => get_class($arg)))); + } } // This is a normal render array. @@ -179,3 +184,96 @@ function twig_without($element) { } return $filtered_element; } + +/** + * Overrides twig_escape_filter(). + * + * Replacement function for Twig's escape filter. + * + * @param Twig_Environment $env + * A Twig_Environment instance. + * @param string $string + * The value to be escaped. + * @param string $strategy + * The escaping strategy. Defaults to 'html'. + * @param string $charset + * The charset. + * @param bool $autoescape + * Whether the function is called by the auto-escaping feature (TRUE) or by + * the developer (FALSE). + * + * @return string|null + * The escaped, rendered output, or NULL if there is no valid output. + */ +function twig_drupal_escape_filter(\Twig_Environment $env, $string, $strategy = 'html', $charset = NULL, $autoescape = FALSE) { + // Check for a numeric zero. + if ($string === 0) { + return 0; + } + + // Return early for NULL or an empty array. + if ($string == NULL) { + return NULL; + } + + // Keep Twig_Markup objects intact to support autoescaping. + if ($autoescape && $string instanceOf \Twig_Markup) { + return $string; + } + + $return = NULL; + + if (is_scalar($string)) { + $return = (string) $string; + } + elseif (is_object($string)) { + if (method_exists($string, '__toString')) { + $return = (string) $string; + } + else { + throw new \Exception(t('Object of type "@class" cannot be printed.', array('@class' => get_class($string)))); + } + } + + // We have a string or an object converted to a string: Autoescape it! + if (isset($return)) { + if ($autoescape && SafeMarkup::isSafe($return, $strategy)) { + return $return; + } + // Drupal only supports the HTML escaping strategy, so provide a + // fallback for other strategies. + if ($strategy == 'html') { + return String::checkPlain($return); + } + return twig_escape_filter($env, $return, $strategy, $charset, $autoescape); + } + + // This is a normal render array, which is safe by definition. + return render($string); +} + +/** + * 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 \Drupal\Component\Utility\SafeMarkup|string + * The imploded string, which is now also marked as safe. + */ +function twig_drupal_join_filter($value, $glue = '') { + $separator = ''; + $output = ''; + foreach ($value as $item) { + $output .= $separator . SafeMarkup::escape($item); + $separator = $glue; + } + + return SafeMarkup::set($output); +}