From f6d785e56c4312e8ee40c7232ce9fad79a0133bd Mon Sep 17 00:00:00 2001
From: xjm <xjm@65776.no-reply.drupal.org>
Date: Fri, 19 Jun 2015 12:16:55 -0500
Subject: [PATCH] Issue #2506133 by alexpott, joelpittet, dawehner, pwolanin:
 Replace SafeMarkup::set() in \Drupal\Core\Template\Attribute

---
 .../Drupal/Component/Utility/SafeMarkup.php   |  6 ++-
 .../Component/Utility/SafeStringInterface.php | 36 +++++++++++++++++
 core/lib/Drupal/Core/Template/Attribute.php   | 10 ++---
 .../Drupal/Core/Template/TwigExtension.php    |  3 +-
 .../Component/Utility/SafeMarkupTest.php      | 26 +++++++++++++
 .../Tests/Core/Template/TwigExtensionTest.php | 39 +++++++++++++++++++
 6 files changed, 110 insertions(+), 10 deletions(-)
 create mode 100644 core/lib/Drupal/Component/Utility/SafeStringInterface.php

diff --git a/core/lib/Drupal/Component/Utility/SafeMarkup.php b/core/lib/Drupal/Component/Utility/SafeMarkup.php
index 69d12fedc20a..d97dc2f24d37 100644
--- a/core/lib/Drupal/Component/Utility/SafeMarkup.php
+++ b/core/lib/Drupal/Component/Utility/SafeMarkup.php
@@ -82,7 +82,7 @@ class SafeMarkup {
   /**
    * Checks if a string is safe to output.
    *
-   * @param string $string
+   * @param string|\Drupal\Component\Utility\SafeStringInterface $string
    *   The content to be checked.
    * @param string $strategy
    *   The escaping strategy. See self::set(). Defaults to 'html'.
@@ -91,7 +91,9 @@ class SafeMarkup {
    *   TRUE if the string has been marked secure, FALSE otherwise.
    */
   public static function isSafe($string, $strategy = 'html') {
-    return isset(static::$safeStrings[(string) $string][$strategy]) ||
+    // Do the instanceof checks first to save unnecessarily casting the object
+    // to a string.
+    return $string instanceOf SafeStringInterface || isset(static::$safeStrings[(string) $string][$strategy]) ||
       isset(static::$safeStrings[(string) $string]['all']);
   }
 
diff --git a/core/lib/Drupal/Component/Utility/SafeStringInterface.php b/core/lib/Drupal/Component/Utility/SafeStringInterface.php
new file mode 100644
index 000000000000..372902711127
--- /dev/null
+++ b/core/lib/Drupal/Component/Utility/SafeStringInterface.php
@@ -0,0 +1,36 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Component\Utility\SafeStringInterface.
+ */
+
+namespace Drupal\Component\Utility;
+
+/**
+ * Marks an object's __toString() method as returning safe markup.
+ *
+ * This interface should only be used on objects that emit known safe strings
+ * from their __toString() method. If there is any risk of the method returning
+ * user-entered data that has not been filtered first, it must not be used.
+ *
+ * @internal
+ *   This interface is marked as internal because it should only be used by
+ *   objects used during rendering. Currently, there is no use case for this
+ *   interface in contrib or custom code.
+ *
+ * @see \Drupal\Component\Utility\SafeMarkup::set()
+ * @see \Drupal\Component\Utility\SafeMarkup::isSafe()
+ * @see \Drupal\Core\Template\TwigExtension::escapeFilter()
+ */
+interface SafeStringInterface {
+
+  /**
+   * Returns a safe string.
+   *
+   * @return string
+   *   The safe string.
+   */
+  public function __toString();
+
+}
diff --git a/core/lib/Drupal/Core/Template/Attribute.php b/core/lib/Drupal/Core/Template/Attribute.php
index ad4e1b1ecea8..b8c05909bd15 100644
--- a/core/lib/Drupal/Core/Template/Attribute.php
+++ b/core/lib/Drupal/Core/Template/Attribute.php
@@ -7,7 +7,7 @@
 
 namespace Drupal\Core\Template;
 
-use Drupal\Component\Utility\SafeMarkup;
+use Drupal\Component\Utility\SafeStringInterface;
 
 /**
  * Collects, sanitizes, and renders HTML attributes.
@@ -42,8 +42,7 @@ use Drupal\Component\Utility\SafeMarkup;
  * The attribute keys and values are automatically sanitized for output with
  * htmlspecialchars() and the entire attribute string is marked safe for output.
  */
-class Attribute implements \ArrayAccess, \IteratorAggregate {
-
+class Attribute implements \ArrayAccess, \IteratorAggregate, SafeStringInterface {
   /**
    * Stores the attribute data.
    *
@@ -259,10 +258,7 @@ class Attribute implements \ArrayAccess, \IteratorAggregate {
         $return .= ' ' . $rendered;
       }
     }
-    // The implementations of AttributeValueBase::render() call
-    // htmlspecialchars() on the attribute name and value so we are confident
-    // that the return value can be set as safe.
-    return SafeMarkup::set($return);
+    return $return;
   }
 
   /**
diff --git a/core/lib/Drupal/Core/Template/TwigExtension.php b/core/lib/Drupal/Core/Template/TwigExtension.php
index 5a736400fb2d..b3bc4253269e 100644
--- a/core/lib/Drupal/Core/Template/TwigExtension.php
+++ b/core/lib/Drupal/Core/Template/TwigExtension.php
@@ -13,6 +13,7 @@
 namespace Drupal\Core\Template;
 
 use Drupal\Component\Utility\SafeMarkup;
+use Drupal\Component\Utility\SafeStringInterface;
 use Drupal\Core\Render\RendererInterface;
 use Drupal\Core\Routing\UrlGeneratorInterface;
 use Drupal\Core\Theme\ThemeManagerInterface;
@@ -397,7 +398,7 @@ class TwigExtension extends \Twig_Extension {
     }
 
     // Keep Twig_Markup objects intact to support autoescaping.
-    if ($autoescape && $arg instanceOf \Twig_Markup) {
+    if ($autoescape && ($arg instanceOf \Twig_Markup || $arg instanceOf SafeStringInterface)) {
       return $arg;
     }
 
diff --git a/core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php b/core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php
index b7c253846621..1776e752ffff 100644
--- a/core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php
+++ b/core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php
@@ -69,6 +69,18 @@ class SafeMarkupTest extends UnitTestCase {
     $this->assertFalse(SafeMarkup::isSafe($returned, 'all'), 'String set with "html" provider is not safe for "all"');
   }
 
+  /**
+   * Tests SafeMarkup::isSafe() with different objects.
+   *
+   * @covers ::isSafe
+   */
+  public function testIsSafe() {
+    $safe_string = $this->getMock('\Drupal\Component\Utility\SafeStringInterface');
+    $this->assertTrue(SafeMarkup::isSafe($safe_string));
+    $string_object = new SafeMarkupTestString('test');
+    $this->assertFalse(SafeMarkup::isSafe($string_object));
+  }
+
   /**
    * Tests SafeMarkup::setMultiple().
    *
@@ -258,3 +270,17 @@ class SafeMarkupTest extends UnitTestCase {
   }
 
 }
+
+class SafeMarkupTestString {
+
+  protected $string;
+
+  public function __construct($string) {
+    $this->string = $string;
+  }
+
+  public function __toString() {
+    return $this->string;
+  }
+
+}
diff --git a/core/tests/Drupal/Tests/Core/Template/TwigExtensionTest.php b/core/tests/Drupal/Tests/Core/Template/TwigExtensionTest.php
index 75d0ef543945..e8718d5fd739 100644
--- a/core/tests/Drupal/Tests/Core/Template/TwigExtensionTest.php
+++ b/core/tests/Drupal/Tests/Core/Template/TwigExtensionTest.php
@@ -99,4 +99,43 @@ class TwigExtensionTest extends UnitTestCase {
     $this->assertEquals('test_theme', $result);
   }
 
+  /**
+   * Tests the escaping of objects implementing SafeStringInterface.
+   *
+   * @covers ::escapeFilter
+   */
+  public function testSafeStringEscaping() {
+    $renderer = $this->getMock('\Drupal\Core\Render\RendererInterface');
+    $twig = new \Twig_Environment(NULL, array(
+      'debug' => TRUE,
+      'cache' => FALSE,
+      'autoescape' => TRUE,
+      'optimizations' => 0
+    ));
+    $twig_extension = new TwigExtension($renderer);
+
+    // By default, TwigExtension will attempt to cast objects to strings.
+    // Ensure objects that implement SafeStringInterface are unchanged.
+    $safe_string = $this->getMock('\Drupal\Component\Utility\SafeStringInterface');
+    $this->assertSame($safe_string, $twig_extension->escapeFilter($twig, $safe_string, 'html', 'UTF-8', TRUE));
+
+    // Ensure objects that do not implement SafeStringInterface are escaped.
+    $string_object = new TwigExtensionTestString("<script>alert('here');</script>");
+    $this->assertSame('&lt;script&gt;alert(&#039;here&#039;);&lt;/script&gt;', $twig_extension->escapeFilter($twig, $string_object, 'html', 'UTF-8', TRUE));
+  }
+
+}
+
+class TwigExtensionTestString {
+
+  protected $string;
+
+  public function __construct($string) {
+    $this->string = $string;
+  }
+
+  public function __toString() {
+    return $this->string;
+  }
+
 }