Issue #2435493 by effulgentsia: Change String::format()'s '@' and '%'...

Issue #2435493 by effulgentsia: Change String::format()'s '@' and '%' placeholders to be auto-escaped rather than always-escaped

core/lib/Drupal/Component/Utility/String.php

@@ -74,19 +74,21 @@ public static function decodeEntities($text) {
    *   any key in $args are replaced with the corresponding value, after
    *   optional sanitization and formatting. The type of sanitization and
    *   formatting depends on the first character of the key:
-   *   - @variable: Escaped to HTML using String::checkPlain(). Use this as the
+   *   - @variable: Escaped to HTML using
+   *     \Drupal\Component\Utility\SafeMarkup::escape(). Use this as the
    *     default choice for anything displayed on a page on the site.
    *   - %variable: Escaped to HTML and formatted using String::placeholder(),
    *     which makes the following HTML code:
    *     @code
    *       <em class="placeholder">text output here.</em>
    *     @endcode
-   *   - !variable: Inserted as is, with no sanitization or formatting. Only use
-   *     this for text that has already been prepared for HTML display (for
-   *     example, user-supplied text that has already been run through
-   *     String::checkPlain() previously, or is expected to contain some limited
-   *     HTML tags and has already been run through
-   *     \Drupal\Component\Utility\Xss::filter() previously).
+   *   - !variable: Inserted as is, with no sanitization or formatting. Only
+   *     use this when the resulting string is being generated for one of:
+   *     - Non-HTML usage, such as a plain-text email.
+   *     - Non-direct HTML output, such as a plain-text variable that will be
+   *       printed as an HTML attribute value and therefore formatted with
+   *       String::checkPlain() as part of that.
+   *     - Some other special reason for suppressing sanitization.
    *
    * @return mixed
    *   The formatted string, or FALSE if no args specified.
@@ -103,7 +105,7 @@ public static function format($string, array $args = array()) {
       switch ($key[0]) {
         case '@':
           // Escaped only.
-          $args[$key] = static::checkPlain($value);
+          $args[$key] = SafeMarkup::escape($value);
           break;
 
         case '%':
@@ -140,7 +142,7 @@ public static function format($string, array $args = array()) {
    *   The formatted text (html).
    */
   public static function placeholder($text) {
-    return SafeMarkup::set('<em class="placeholder">' . static::checkPlain($text) . '</em>');
+    return SafeMarkup::set('<em class="placeholder">' . SafeMarkup::escape($text) . '</em>');
   }
 
 

core/modules/comment/comment.module

@@ -739,7 +739,7 @@ function template_preprocess_comment(&$variables) {
     $variables['permalink'] = \Drupal::l(t('Permalink'), $comment->permalink());
   }
 
-  $variables['submitted'] = t('Submitted by !username on !datetime', array('!username' => $variables['author'], '!datetime' => $variables['created']));
+  $variables['submitted'] = t('Submitted by @username on @datetime', array('@username' => $variables['author'], '@datetime' => $variables['created']));
 
   if ($comment->hasParentComment()) {
     // Fetch and store the parent comment information for use in templates.

core/modules/forum/templates/forum-submitted.html.twig

@@ -17,7 +17,7 @@
  */
 #}
 {% if time %}
-  <span>{% trans %}By {{ author|passthrough }} {{ time }} ago{% endtrans %}</span>
+  <span>{% trans %}By {{ author }} {{ time }} ago{% endtrans %}</span>
 {% else %}
   {{ 'n/a'|t }}
 {% endif %}

core/modules/node/templates/node.html.twig

@@ -82,7 +82,7 @@
     <footer>
       {{ author_picture }}
       <div{{ author_attributes }}>
-        {% trans %}Submitted by {{ author_name|passthrough }} on {{ date|passthrough }}{% endtrans %}
+        {% trans %}Submitted by {{ author_name }} on {{ date }}{% endtrans %}
         {{ metadata }}
       </div>
     </footer>

core/modules/update/templates/update-project-status.html.twig

@@ -110,10 +110,9 @@
 
   {% if base_themes %}
     {% set basethemes = base_themes|join(', ') %}
-    {# Using passthrough since placeholder is already applied in preprocess. #}
     <div class="basethemes">
       {% trans %}
-        Depends on: {{ basethemes|passthrough }}
+        Depends on: {{ basethemes }}
       {% endtrans %}
     </div>
   {% endif %}

core/tests/Drupal/Tests/Component/Utility/StringTest.php

@@ -89,7 +89,9 @@ function testFormat($string, $args, $expected, $message, $expected_is_safe) {
   function providerFormat() {
     $tests[] = array('Simple text', array(), 'Simple text', 'String::format leaves simple text alone.', TRUE);
     $tests[] = array('Escaped text: @value', array('@value' => '<script>'), 'Escaped text: &lt;script&gt;', 'String::format replaces and escapes string.', TRUE);
+    $tests[] = array('Escaped text: @value', array('@value' => SafeMarkup::set('<span>Safe HTML</span>')), 'Escaped text: <span>Safe HTML</span>', 'String::format does not escape an already safe string.', TRUE);
     $tests[] = array('Placeholder text: %value', array('%value' => '<script>'), 'Placeholder text: <em class="placeholder">&lt;script&gt;</em>', 'String::format replaces, escapes and themes string.', TRUE);
+    $tests[] = array('Placeholder text: %value', array('%value' => SafeMarkup::set('<span>Safe HTML</span>')), 'Placeholder text: <em class="placeholder"><span>Safe HTML</span></em>', 'String::format does not escape an already safe string themed as a placeholder.', TRUE);
     $tests[] = array('Verbatim text: !value', array('!value' => '<script>'), 'Verbatim text: <script>', 'String::format replaces verbatim string as-is.', FALSE);
     $tests[] = array('Verbatim text: !value', array('!value' => SafeMarkup::set('<span>Safe HTML</span>')), 'Verbatim text: <span>Safe HTML</span>', 'String::format replaces verbatim string as-is.', TRUE);
 

core/themes/bartik/templates/node.html.twig

@@ -86,7 +86,7 @@
       <div class="node__meta">
         {{ author_picture }}
         <span{{ author_attributes }}>
-          {% trans %}Submitted by {{ author_name|passthrough }} on {{ date|passthrough }}{% endtrans %}
+          {% trans %}Submitted by {{ author_name }} on {{ date }}{% endtrans %}
         </span>
         {{ metadata }}
       </div>

core/themes/classy/templates/forum/forum-submitted.html.twig

@@ -17,7 +17,7 @@
  */
 #}
 {% if time %}
-  <span class="submitted">{% trans %}By {{ author|passthrough }} {{ time }} ago{% endtrans %}</span>
+  <span class="submitted">{% trans %}By {{ author }} {{ time }} ago{% endtrans %}</span>
 {% else %}
   {{ 'n/a'|t }}
 {% endif %}

core/themes/classy/templates/node/node.html.twig

@@ -92,7 +92,7 @@
     <footer class="node__meta">
       {{ author_picture }}
       <div{{ author_attributes.addClass('node__submitted') }}>
-        {% trans %}Submitted by {{ author_name|passthrough }} on {{ date|passthrough }}{% endtrans %}
+        {% trans %}Submitted by {{ author_name }} on {{ date }}{% endtrans %}
         {{ metadata }}
       </div>
     </footer>