Issue #2580723 by AdamPS, Berdir, andypost, darvanen, larowlan, alexpott, effulgentsia, catch, dawehner: Fix token system confusion, with new function Token::replacePlain()

(cherry picked from commit 74b25722ac)

core/lib/Drupal/Core/Utility/Token.php

@@ -4,6 +4,7 @@ namespace Drupal\Core\Utility;
 
 use Drupal\Component\Render\HtmlEscapedText;
 use Drupal\Component\Render\MarkupInterface;
+use Drupal\Component\Render\PlainTextOutput;
 use Drupal\Core\Cache\CacheableDependencyInterface;
 use Drupal\Core\Cache\CacheBackendInterface;
 use Drupal\Core\Cache\CacheTagsInvalidatorInterface;
@@ -134,12 +135,10 @@ class Token {
   }
 
   /**
-   * Replaces all tokens in a given string with appropriate values.
+   * Replaces all tokens in given markup with appropriate values.
    *
-   * @param string $text
-   *   An HTML string containing replaceable tokens. The caller is responsible
-   *   for calling \Drupal\Component\Utility\Html::escape() in case the $text
-   *   was plain text.
+   * @param string $markup
+   *   An HTML string containing replaceable tokens.
    * @param array $data
    *   (optional) An array of keyed objects. For simple replacement scenarios
    *   'node', 'user', and others are common keys, with an accompanying node or
@@ -175,14 +174,58 @@ class Token {
    *
    * @return string
    *   The token result is the entered HTML text with tokens replaced. The
-   *   caller is responsible for choosing the right escaping / sanitization. If
-   *   the result is intended to be used as plain text, using
-   *   PlainTextOutput::renderFromHtml() is recommended. If the result is just
-   *   printed as part of a template relying on Twig autoescaping is possible,
-   *   otherwise for example the result can be put into #markup, in which case
-   *   it would be sanitized by Xss::filterAdmin().
+   *   caller is responsible for choosing the right sanitization, for example
+   *   the result can be put into #markup, in which case it would be sanitized
+   *   by Xss::filterAdmin().
+   *
+   *   The return value must be treated as unsafe even if the input was safe
+   *   markup. This is necessary because an attacker could craft an input
+   *   string and token value that, although each safe individually, would be
+   *   unsafe when combined by token replacement.
+   *
+   * @see static::replacePlain()
    */
-  public function replace($text, array $data = [], array $options = [], BubbleableMetadata $bubbleable_metadata = NULL) {
+  public function replace($markup, array $data = [], array $options = [], BubbleableMetadata $bubbleable_metadata = NULL) {
+    return $this->doReplace(TRUE, (string) $markup, $data, $options, $bubbleable_metadata);
+  }
+
+  /**
+   * Replaces all tokens in a given plain text string with appropriate values.
+   *
+   * @param string $plain
+   *   Plain text string.
+   * @param array $data
+   *   (optional) An array of keyed objects. See replace().
+   * @param array $options
+   *   (optional) A keyed array of options. See replace().
+   * @param \Drupal\Core\Render\BubbleableMetadata|null $bubbleable_metadata
+   *   (optional) Target for adding metadata. See replace().
+   *
+   * @return string
+   *   The entered plain text with tokens replaced.
+   */
+  public function replacePlain(string $plain, array $data = [], array $options = [], BubbleableMetadata $bubbleable_metadata = NULL): string {
+    return $this->doReplace(FALSE, $plain, $data, $options, $bubbleable_metadata);
+  }
+
+  /**
+   * Replaces all tokens in a given string with appropriate values.
+   *
+   * @param bool $markup
+   *   TRUE to convert token values to markup, FALSE to convert to plain text.
+   * @param string $text
+   *   A string containing replaceable tokens.
+   * @param array $data
+   *   An array of keyed objects. See replace().
+   * @param array $options
+   *   A keyed array of options. See replace().
+   * @param \Drupal\Core\Render\BubbleableMetadata|null $bubbleable_metadata
+   *   (optional) Target for adding metadata. See replace().
+   *
+   * @return string
+   *   The token result is the entered string with tokens replaced.
+   */
+  protected function doReplace(bool $markup, string $text, array $data, array $options, BubbleableMetadata $bubbleable_metadata = NULL): string {
     $text_tokens = $this->scan($text);
     if (empty($text_tokens)) {
       return $text;
@@ -199,9 +242,19 @@ class Token {
       }
     }
 
-    // Escape the tokens, unless they are explicitly markup.
+    // Each token value is markup if it implements MarkupInterface otherwise it
+    // is plain text. Convert them, but only if needed. It can cause corruption
+    // to render a string that's already plain text or to escape a string
+    // that's already markup.
     foreach ($replacements as $token => $value) {
-      $replacements[$token] = $value instanceof MarkupInterface ? $value : new HtmlEscapedText($value);
+      if ($markup) {
+        // Escape plain text tokens.
+        $replacements[$token] = $value instanceof MarkupInterface ? $value : new HtmlEscapedText($value);
+      }
+      else {
+        // Render markup tokens to plain text.
+        $replacements[$token] = $value instanceof MarkupInterface ? PlainTextOutput::renderFromHtml($value) : $value;
+      }
     }
 
     // Optionally alter the list of replacement values.

core/tests/Drupal/Tests/Core/Utility/TokenTest.php

@@ -294,4 +294,29 @@ class TokenTest extends UnitTestCase {
     return $data;
   }
 
+  /**
+   * @covers ::replacePlain
+   */
+  public function testReplacePlain() {
+    $this->setupSiteTokens();
+    $base = 'Wow, great "[site:name]" has a slogan "[site:slogan]"';
+    $plain = $this->token->replacePlain($base);
+    $this->assertEquals($plain, 'Wow, great "Your <best> buys" has a slogan "We are best"');
+  }
+
+  /**
+   * Sets up the token library to return site tokens.
+   */
+  protected function setupSiteTokens() {
+    // The site name is plain text, but the slogan is markup.
+    $tokens = [
+      '[site:name]' => 'Your <best> buys',
+      '[site:slogan]' => Markup::Create('We are <b>best</b>'),
+    ];
+
+    $this->moduleHandler->expects($this->any())
+      ->method('invokeAll')
+      ->willReturn($tokens);
+  }
+
 }