diff --git a/core/lib/Drupal/Component/Utility/SafeMarkup.php b/core/lib/Drupal/Component/Utility/SafeMarkup.php index bfa6eed..e1c7524 100644 --- a/core/lib/Drupal/Component/Utility/SafeMarkup.php +++ b/core/lib/Drupal/Component/Utility/SafeMarkup.php @@ -60,8 +60,7 @@ public static function isSafe($string, $strategy = 'html') { * Safely implodes strings by escaping any that are not known to be safe. * * @param string $delimiter - * The delimiter to use when imploding. Note that the delimiter is assumed - * to be safe, so do not pass unsanitized input! + * The delimiter to use when imploding. * @param array $array * An array of the strings to implode. * @param string $strategy @@ -69,18 +68,19 @@ public static function isSafe($string, $strategy = 'html') { * * @return \Drupal\Component\Utility\SafeMarkup|string * The imploded string, which is now also marked as safe. - * - * @todo Should $delimeter also be processed with checkPlain() if it's not a - * known-safe string? */ public static function implode($delimiter, array $array, $strategy = 'html') { + $array[] = $delimiter; foreach ($array as $key => $string) { if (!(isset(static::$safeStrings[(string) $string][$strategy]) || isset(static::$safeStrings[(string) $string]['all']))) { $array[$key] = String::checkPlain($string); } } - return SafeMarkup::set(implode($delimiter, $array), $strategy); + // Recover the delimiter. + $delimiter = array_pop($array); + static::set($delimiter, $strategy); + return static::set(implode($delimiter, $array), $strategy); } /** diff --git a/core/modules/simpletest/src/Form/SimpletestResultsForm.php b/core/modules/simpletest/src/Form/SimpletestResultsForm.php index e7106ff..2008206 100644 --- a/core/modules/simpletest/src/Form/SimpletestResultsForm.php +++ b/core/modules/simpletest/src/Form/SimpletestResultsForm.php @@ -156,7 +156,7 @@ public function buildForm(array $form, array &$form_state, $test_id = NULL) { $rows = array(); foreach ($assertions as $assertion) { $row = array(); - // @todo Need to preserve safe markup, not create it. + // 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); 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..64b96f9 --- /dev/null +++ b/core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php @@ -0,0 +1,142 @@ + '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. + */ + 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. + */ + 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 string formatting with SafeMarkup::implode(). + * + * @dataProvider providerImplode + * + * @param string $delimiter + * The delimiter to run through SafeMarkup::implode(). + * @param string $args + * The arguments to pass into SafeMarkup::implode(). + * @param string $expected + * The expected result from calling the function. + * @param string $message + * The message to display as output to the test. + * + * @see SafeMarkup::implode() + */ + public function testImplode($delimiter, $args, $expected, $message) { + $result = SafeMarkup::implode($delimiter, $args); + $this->assertEquals($expected, $result, $message); + $this->assertTrue(SafeMarkup::isSafe($result), 'The return value has been marked as safe'); + } + + /** + * Data provider for testImplode(). + * + * @see testImplode() + */ + public function providerImplode() { + $tests[] = array('|', array('Simple text'), 'Simple text', 'SafeMarkup::implode() leaves simple text alone.'); + $tests[] = array(' | ', array('Escaped text: