diff --git a/core/lib/Drupal/Component/Utility/SafeMarkup.php b/core/lib/Drupal/Component/Utility/SafeMarkup.php index 675a6b2..340ee9f 100644 --- a/core/lib/Drupal/Component/Utility/SafeMarkup.php +++ b/core/lib/Drupal/Component/Utility/SafeMarkup.php @@ -217,6 +217,36 @@ public static function xssFilter($string, $html_tags = NULL) { } /** + * Safely truncates an HTML string. + * + * The resulting string rendered in a browser will be free of XSS and shorter + * than the specified length. + * + * @param $string + * @param $length + * @param bool|FALSE $wordsafe + * @param bool|FALSE $add_ellipsis + * @param int $min_wordsafe_length + * @param null $html_tags + * @return string + * + * @see \Drupal\Component\Utility\Xss::truncate() + * @see \Drupal\Component\Utility\Xss::filter() + * @see \Drupal\Component\Utility\Xss::getAdminTagList() + * @see \Drupal\Component\Utility\Unicode::truncate() + * @see \Drupal\Component\Utility\SafeMarkup::isSafe() + */ + public static function xssTruncate($string, $length, $wordsafe = FALSE, $add_ellipsis = FALSE, $min_wordsafe_length = 1, $html_tags = NULL) { + if (isset($html_tags)) { + $string = Xss::truncate($string, $length, $wordsafe, $add_ellipsis, $min_wordsafe_length, $html_tags); + } + else { + $string = Xss::truncate($string, $length, $wordsafe, $add_ellipsis, $min_wordsafe_length); + } + return static::set($string); + } + + /** * Gets all strings currently marked as safe. * * This is useful for the batch and form APIs, where it is important to diff --git a/core/lib/Drupal/Component/Utility/Xss.php b/core/lib/Drupal/Component/Utility/Xss.php index a68ca2b..eace954 100644 --- a/core/lib/Drupal/Component/Utility/Xss.php +++ b/core/lib/Drupal/Component/Utility/Xss.php @@ -14,6 +14,17 @@ */ class Xss { + const HTML_TAG_PREG = '% + ( + <(?=[^a-zA-Z!/]) # a lone < + | # or + # a comment + | # or + <[^>]*(?:>|$) # a string that starts with a <, up until the > or the end of the string + | # or + > # just a > + )%x'; + /** * The list of html tags allowed by filterAdmin(). * @@ -88,16 +99,7 @@ public static function filter($string, $html_tags = array('a', 'em', 'strong', ' // for output. All other known XSS vectors have been filtered out by this // point and any HTML tags remaining will have been deliberately allowed, so // it is acceptable to call SafeMarkup::set() on the resultant string. - return preg_replace_callback('% - ( - <(?=[^a-zA-Z!/]) # a lone < - | # or - # a comment - | # or - <[^>]*(>|$) # a string that starts with a <, up until the > or the end of the string - | # or - > # just a > - )%x', $splitter, $string); + return preg_replace_callback(static::HTML_TAG_PREG, $splitter, $string); } /** @@ -135,6 +137,55 @@ public static function filterAdmin($string) { } /** + * Safely truncates an HTML string. + * + * The resulting string rendered in a browser will be free of XSS and shorter + * than the specified length. + * + * @param $string + * The string with raw HTML in it. It will be stripped of everything that + * can cause an XSS attack and truncated to $length. + * @param $length + * The maximum length of the rendered string. + * @param bool $wordsafe + * See \Drupal\Component\Unicode::truncate() for this parameter. + * @param bool $add_ellipsis + * See \Drupal\Component\Unicode::truncate() for this parameter. + * @param int $min_wordsafe_length + * See \Drupal\Component\Unicode::truncate() for this parameter. + * @param array $html_tags + * See \Drupal\Component\Xss::filter() for this parameter. + * + * @return string + * The filtered and truncated string. + */ + public static function truncate($string, $length, $wordsafe = FALSE, $add_ellipsis = FALSE, $min_wordsafe_length = 1, $html_tags = array('a', 'em', 'strong', 'cite', 'blockquote', 'code', 'ul', 'ol', 'li', 'dl', 'dt', 'dd')) { + $html_tags = array_flip($html_tags); + $class = get_called_class(); + $pieces = preg_split(static::HTML_TAG_PREG, $string, -1 , PREG_SPLIT_DELIM_CAPTURE | PREG_SPLIT_NO_EMPTY); + $final = ''; + $remaining_length = $length; + foreach ($pieces as $piece) { + // If it is a HTML tag, pass it to the normal XSS filter pipeline. + if ($piece[0] == '<' || $piece[0] == '>') { + $piece = static::split($piece, $html_tags, $class); + } + else { + // Would adding this piece of text make the string too long? + $current_length = Unicode::strlen($piece); + if ($current_length > $remaining_length) { + $final .= Unicode::truncate($piece, $remaining_length, $wordsafe, $add_ellipsis, $min_wordsafe_length); + break; + } + $remaining_length -= $current_length; + } + $final .= $piece; + } + // Close the tags. + return Html::normalize($final); + } + + /** * Processes an HTML tag. * * @param string $string diff --git a/core/modules/dblog/src/Controller/DbLogController.php b/core/modules/dblog/src/Controller/DbLogController.php index 3982da6..ab354fd 100644 --- a/core/modules/dblog/src/Controller/DbLogController.php +++ b/core/modules/dblog/src/Controller/DbLogController.php @@ -185,9 +185,7 @@ public function overview() { $message = $this->formatMessage($dblog); if ($message && isset($dblog->wid)) { // Truncate link_text to 56 chars of message. - // @todo Reevaluate the SafeMarkup::set() in - // https://www.drupal.org/node/2399261. - $log_text = SafeMarkup::set(Unicode::truncate(Xss::filter($message, array()), 56, TRUE, TRUE)); + $log_text = SafeMarkup::xssTruncate($message, 56, TRUE, TRUE); $message = $this->l($log_text, new Url('dblog.event', array('event_id' => $dblog->wid), array( 'attributes' => array( // Provide a title for the link for useful hover hints. diff --git a/core/modules/dblog/src/Tests/DbLogTest.php b/core/modules/dblog/src/Tests/DbLogTest.php index 820b8e0..8700fca 100644 --- a/core/modules/dblog/src/Tests/DbLogTest.php +++ b/core/modules/dblog/src/Tests/DbLogTest.php @@ -7,8 +7,8 @@ namespace Drupal\dblog\Tests; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\Unicode; -use Drupal\Component\Utility\Xss; use Drupal\Core\Logger\RfcLogLevel; use Drupal\Core\Url; use Drupal\dblog\Controller\DbLogController; @@ -332,17 +332,17 @@ private function doUser() { // Add user. // Default display includes name and email address; if too long, the email // address is replaced by three periods. - $this->assertLogMessage(t('New user: %name %email.', array('%name' => $name, '%email' => '<' . $user->getEmail() . '>')), 'DBLog event was recorded: [add user]'); + $this->assertLogMessage('New user: %name %email.', array('%name' => $name, '%email' => '<' . $user->getEmail() . '>'), 'DBLog event was recorded: [add user]'); // Login user. - $this->assertLogMessage(t('Session opened for %name.', array('%name' => $name)), 'DBLog event was recorded: [login user]'); + $this->assertLogMessage('Session opened for %name.', array('%name' => $name), 'DBLog event was recorded: [login user]'); // Logout user. - $this->assertLogMessage(t('Session closed for %name.', array('%name' => $name)), 'DBLog event was recorded: [logout user]'); + $this->assertLogMessage('Session closed for %name.', array('%name' => $name), 'DBLog event was recorded: [logout user]'); // Delete user. $message = t('Deleted user: %name %email.', array('%name' => $name, '%email' => '<' . $user->getEmail() . '>')); - $message_text = Unicode::truncate(Xss::filter($message, array()), 56, TRUE, TRUE); + $message_text = SafeMarkup::xssTruncate($message, 56, TRUE, TRUE); // Verify that the full message displays on the details page. $link = FALSE; - if ($links = $this->xpath('//a[text()="' . html_entity_decode($message_text) . '"]')) { + if ($links = $this->xpath('//a[string()="' . html_entity_decode(strip_tags($message_text)) . '"]')) { // Found link with the message text. $links = array_shift($links); foreach ($links->attributes() as $attr => $value) { @@ -412,11 +412,11 @@ private function doNode($type) { // Verify that node events were recorded. // Was node content added? - $this->assertLogMessage(t('@type: added %title.', array('@type' => $type, '%title' => $title)), 'DBLog event was recorded: [content added]'); + $this->assertLogMessage('@type: added %title.', array('@type' => $type, '%title' => $title), 'DBLog event was recorded: [content added]'); // Was node content updated? - $this->assertLogMessage(t('@type: updated %title.', array('@type' => $type, '%title' => $title)), 'DBLog event was recorded: [content updated]'); + $this->assertLogMessage('@type: updated %title.', array('@type' => $type, '%title' => $title), 'DBLog event was recorded: [content updated]'); // Was node content deleted? - $this->assertLogMessage(t('@type: deleted %title.', array('@type' => $type, '%title' => $title)), 'DBLog event was recorded: [content deleted]'); + $this->assertLogMessage('@type: deleted %title.', array('@type' => $type, '%title' => $title), 'DBLog event was recorded: [content deleted]'); // View the database log access-denied report page. $this->drupalGet('admin/reports/access-denied'); @@ -694,12 +694,19 @@ protected function asText(\SimpleXMLElement $element) { * @param string $message * The message to pass to simpletest. */ - protected function assertLogMessage($log_message, $message) { - $message_text = Unicode::truncate(Xss::filter($log_message, array()), 56, TRUE, TRUE); + protected function assertLogMessage($log_message, $log_variables, $message) { + $stripped_text = SafeMarkup::xssTruncate(SafeMarkup::format($log_message, $log_variables), 56, TRUE, TRUE, 1, []); // After \Drupal\Component\Utility\Xss::filter(), HTML entities should be // converted to their character equivalents because assertLink() uses this // string in xpath() to query the Document Object Model (DOM). - $this->assertLink(html_entity_decode($message_text), 0, $message); + $xpath = "//a[string() = '" . html_entity_decode($stripped_text) . "'"; + foreach ($log_variables as $key => $variable) { + if ($key[0] == '%') { + $xpath .= ' and em[@class="placeholder" and .="' . html_entity_decode($variable) .'"]'; + } + } + $xpath .= ']'; + $this->assertTrue($this->xpath($xpath), $message); } /** diff --git a/core/modules/search/src/Tests/SearchConfigSettingsFormTest.php b/core/modules/search/src/Tests/SearchConfigSettingsFormTest.php index a99ea45..5f6b1e4 100644 --- a/core/modules/search/src/Tests/SearchConfigSettingsFormTest.php +++ b/core/modules/search/src/Tests/SearchConfigSettingsFormTest.php @@ -7,6 +7,7 @@ namespace Drupal\search\Tests; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Core\Url; /** @@ -100,7 +101,7 @@ function testSearchSettingsPage() { $text = $this->randomMachineName(5); $this->drupalPostForm('search/node', array('keys' => $text), t('Search')); $this->drupalGet('admin/reports/dblog'); - $this->assertLink('Searched Content for ' . $text . '.', 0, 'Search was logged'); + $this->assertRaw('>' . SafeMarkup::format('Searched %type for %keys.', ['%type' => 'Content', '%keys' => $text]) . '', 0, 'Search was logged'); }