From 44313c63f2e999aaf08a1b1ad7f48d391ebec054 Mon Sep 17 00:00:00 2001 From: Alex Pott Date: Fri, 16 Jan 2015 09:51:38 +0000 Subject: [PATCH] Issue #2399037 by effulgentsia: String::format() marks a resulting string as safe even when passed an unsafe passthrough argument --- core/lib/Drupal/Component/Utility/String.php | 13 ++++++++++++- core/modules/file/file.field.inc | 8 ++++---- .../views/src/Tests/Plugin/DisplayTest.php | 4 ++-- core/modules/views_ui/src/ViewEditForm.php | 4 ++-- .../Drupal/Tests/Component/Utility/StringTest.php | 15 ++++++++++----- 5 files changed, 30 insertions(+), 14 deletions(-) diff --git a/core/lib/Drupal/Component/Utility/String.php b/core/lib/Drupal/Component/Utility/String.php index 970436c747fc..3a8a2b3ed64b 100644 --- a/core/lib/Drupal/Component/Utility/String.php +++ b/core/lib/Drupal/Component/Utility/String.php @@ -96,6 +96,8 @@ class String { * @see t() */ public static function format($string, array $args = array()) { + $safe = TRUE; + // Transform arguments before inserting them. foreach ($args as $key => $value) { switch ($key[0]) { @@ -112,9 +114,18 @@ class String { case '!': // Pass-through. + if (!SafeMarkup::isSafe($value)) { + $safe = FALSE; + } } } - return SafeMarkup::set(strtr($string, $args)); + + $output = strtr($string, $args); + if ($safe) { + SafeMarkup::set($output); + } + + return $output; } /** diff --git a/core/modules/file/file.field.inc b/core/modules/file/file.field.inc index 57c402e00abb..7ec9d09ad8ec 100644 --- a/core/modules/file/file.field.inc +++ b/core/modules/file/file.field.inc @@ -189,16 +189,16 @@ function template_preprocess_file_upload_help(&$variables) { $max = $upload_validators['file_validate_image_resolution'][0]; $min = $upload_validators['file_validate_image_resolution'][1]; if ($min && $max && $min == $max) { - $descriptions[] = t('Images must be exactly !size pixels.', array('!size' => '' . $max . '')); + $descriptions[] = t('Images must be exactly @size pixels.', array('@size' => $max)); } elseif ($min && $max) { - $descriptions[] = t('Images must be larger than !min pixels. Images larger than !max pixels will be resized.', array('!min' => '' . $min . '', '!max' => '' . $max . '')); + $descriptions[] = t('Images must be larger than @min pixels. Images larger than @max pixels will be resized.', array('@min' => $min, '@max' => $max)); } elseif ($min) { - $descriptions[] = t('Images must be larger than !min pixels.', array('!min' => '' . $min . '')); + $descriptions[] = t('Images must be larger than @min pixels.', array('@min' => $min)); } elseif ($max) { - $descriptions[] = t('Images larger than !max pixels will be resized.', array('!max' => '' . $max . '')); + $descriptions[] = t('Images larger than @max pixels will be resized.', array('@max' => $max)); } } diff --git a/core/modules/views/src/Tests/Plugin/DisplayTest.php b/core/modules/views/src/Tests/Plugin/DisplayTest.php index 4b5b638cc560..ce22d2e791c1 100644 --- a/core/modules/views/src/Tests/Plugin/DisplayTest.php +++ b/core/modules/views/src/Tests/Plugin/DisplayTest.php @@ -243,7 +243,7 @@ class DisplayTest extends PluginTestBase { $this->drupalGet('test_display_invalid'); $this->assertResponse(200); - $this->assertText('The "invalid" plugin does not exist.'); + $this->assertText('The "invalid" plugin does not exist.'); // Rebuild the router, and ensure that the path is not accessible anymore. views_invalidate_cache(); @@ -273,7 +273,7 @@ class DisplayTest extends PluginTestBase { // plugin warning message. $this->drupalGet(''); $this->assertResponse(200); - $this->assertText('The "invalid" plugin does not exist.'); + $this->assertText('The "invalid" plugin does not exist.'); $this->assertNoBlockAppears($block); } diff --git a/core/modules/views_ui/src/ViewEditForm.php b/core/modules/views_ui/src/ViewEditForm.php index 055a12b17587..428775f35a38 100644 --- a/core/modules/views_ui/src/ViewEditForm.php +++ b/core/modules/views_ui/src/ViewEditForm.php @@ -137,12 +137,12 @@ class ViewEditForm extends ViewFormBase { $lock_message_substitutions = array( '!user' => drupal_render($username), '!age' => $this->dateFormatter->formatInterval(REQUEST_TIME - $view->lock->updated), - '!break' => $view->url('break-lock-form'), + '@url' => $view->url('break-lock-form'), ); $form['locked'] = array( '#type' => 'container', '#attributes' => array('class' => array('view-locked', 'messages', 'messages--warning')), - '#children' => $this->t('This view is being edited by user !user, and is therefore locked from editing by others. This lock is !age old. Click here to break this lock.', $lock_message_substitutions), + '#children' => $this->t('This view is being edited by user !user, and is therefore locked from editing by others. This lock is !age old. Click here to break this lock.', $lock_message_substitutions), '#weight' => -10, ); } diff --git a/core/tests/Drupal/Tests/Component/Utility/StringTest.php b/core/tests/Drupal/Tests/Component/Utility/StringTest.php index 358ee2df0db3..a64854f3b537 100644 --- a/core/tests/Drupal/Tests/Component/Utility/StringTest.php +++ b/core/tests/Drupal/Tests/Component/Utility/StringTest.php @@ -7,6 +7,7 @@ namespace Drupal\Tests\Component\Utility; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Tests\UnitTestCase; use Drupal\Component\Utility\String; @@ -71,10 +72,13 @@ class StringTest extends UnitTestCase { * The expected result from calling the function. * @param string $message * The message to display as output to the test. + * @param bool $expected_is_safe + * Whether the result is expected to be safe for HTML display. */ - function testFormat($string, $args, $expected, $message) { + function testFormat($string, $args, $expected, $message, $expected_is_safe) { $result = String::format($string, $args); $this->assertEquals($expected, $result, $message); + $this->assertEquals($expected_is_safe, SafeMarkup::isSafe($result), 'String::format correctly sets the result as safe or not safe.'); } /** @@ -83,10 +87,11 @@ class StringTest extends UnitTestCase { * @see testFormat() */ function providerFormat() { - $tests[] = array('Simple text', array(), 'Simple text', 'String::format leaves simple text alone.'); - $tests[] = array('Escaped text: @value', array('@value' => '