Commit 03ac04f3 authored by alexpott's avatar alexpott

Issue #2572929 by dawehner, pwolanin, mikeker, xjm, stefan.r, David_Rothstein,...

Issue #2572929 by dawehner, pwolanin, mikeker, xjm, stefan.r, David_Rothstein, hussainweb, lauriii, Wim Leers, catch, chx, alexpott: Document lack of auto-escape in theme functions and add a theme autoescape helper function
parent 3d0a938e
......@@ -12,10 +12,12 @@
use Drupal\Component\Utility\Crypt;
use Drupal\Component\Utility\Html;
use Drupal\Component\Utility\SafeMarkup;
use Drupal\Component\Utility\SafeStringInterface;
use Drupal\Component\Utility\Unicode;
use Drupal\Component\Utility\Xss;
use Drupal\Core\Config\Config;
use Drupal\Core\Config\StorageException;
use Drupal\Core\Render\RenderableInterface;
use Drupal\Core\Template\Attribute;
use Drupal\Core\Theme\ThemeSettings;
use Drupal\Component\Utility\NestedArray;
......@@ -357,6 +359,75 @@ function theme_get_setting($setting_name, $theme = NULL) {
return $cache[$theme]->get($setting_name);
}
/**
* Escapes and renders variables for theme functions.
*
* This method is used in theme functions to ensure that the result is safe for
* output inside HTML fragments. This mimics the behavior of the auto-escape
* functionality in Twig.
*
* Note: This function should be kept in sync with
* \Drupal\Core\Template\TwigExtension::escapeFilter().
*
* @param mixed $arg
* The string, object, or render array to escape if needed.
*
* @return string
* The rendered string, safe for use in HTML. The string is not safe when used
* as any part of an HTML attribute name or value.
*
* @throws \Exception
* Thrown when an object is passed in which cannot be printed.
*
* @see \Drupal\Core\Template\TwigExtension::escapeFilter()
*
* @todo Discuss deprecating this in https://www.drupal.org/node/2575081.
* @todo Refactor this to keep it in sync with Twig filtering in
* https://www.drupal.org/node/2575065
*/
function theme_render_and_autoescape($arg) {
if ($arg instanceOf SafeStringInterface) {
return (string) $arg;
}
$return = NULL;
if (is_scalar($arg)) {
$return = (string) $arg;
}
elseif (is_object($arg)) {
if ($arg instanceof RenderableInterface) {
$arg = $arg->toRenderable();
}
elseif (method_exists($arg, '__toString')) {
$return = (string) $arg;
}
// You can't throw exceptions in the magic PHP __toString methods, see
// http://php.net/manual/en/language.oop5.magic.php#object.tostring so
// we also support a toString method.
elseif (method_exists($arg, 'toString')) {
$return = $arg->toString();
}
else {
throw new \Exception(t('Object of type "@class" cannot be printed.', array('@class' => get_class($arg))));
}
}
// We have a string or an object converted to a string: Escape it!
if (isset($return)) {
return SafeMarkup::isSafe($return, 'html') ? $return : Html::escape($return);
}
// This is a normal render array, which is safe by definition, with special
// simple cases already handled.
// Early return if this element was pre-rendered (no need to re-render).
if (isset($arg['#printed']) && $arg['#printed'] == TRUE && isset($arg['#markup']) && strlen($arg['#markup']) > 0) {
return (string) $arg['#markup'];
}
$arg['#printed'] = FALSE;
return (string) \Drupal::service('renderer')->render($arg);
}
/**
* Converts theme settings to configuration.
*
......
......@@ -70,7 +70,10 @@
* hook_theme() implementations can also specify that a theme hook
* implementation is a theme function, but that is uncommon. It is only used for
* special cases, for performance reasons, because rendering using theme
* functions is somewhat faster than theme templates.
* functions is somewhat faster than theme templates. Note that while Twig
* templates will auto-escape variables, theme functions must explicitly escape
* any variables by using theme_render_and_autoescape(). Failure to do so is
* likely to result in security vulnerabilities.
*
* @section sec_overriding_theme_hooks Overriding Theme Hooks
* Themes may register new theme hooks within a hook_theme() implementation, but
......@@ -93,6 +96,9 @@
* bartik_search_result() in the bartik.theme file, if the search_result hook
* implementation was a function instead of a template). Normally, copying the
* default function is again a good starting point for overriding its behavior.
* Again, note that theme functions (unlike templates) must explicitly escape
* variables using theme_render_and_autoescape() or risk security
* vulnerabilities.
*
* @section sec_preprocess_templates Preprocessing for Template Files
* If the theme implementation is a template file, several functions are called
......
......@@ -350,6 +350,9 @@ public function escapePlaceholder($env, $string) {
*
* Replacement function for Twig's escape filter.
*
* Note: This function should be kept in sync with
* theme_render_and_autoescape().
*
* @param \Twig_Environment $env
* A Twig_Environment instance.
* @param mixed $arg
......@@ -364,6 +367,9 @@ public function escapePlaceholder($env, $string) {
*
* @return string|null
* The escaped, rendered output, or NULL if there is no valid output.
*
* @todo Refactor this to keep it in sync with theme_render_and_autoescape()
* in https://www.drupal.org/node/2575065
*/
public function escapeFilter(\Twig_Environment $env, $arg, $strategy = 'html', $charset = NULL, $autoescape = FALSE) {
// Check for a numeric zero int or float.
......
......@@ -201,5 +201,5 @@ function template_preprocess_language_content_settings_table(&$variables) {
* @ingroup themeable
*/
function theme_language_content_settings_table($variables) {
return '<h4>' . $variables['build']['#title'] . '</h4>' . drupal_render($variables['build']);
return '<h4>' . theme_render_and_autoescape($variables['build']['#title']) . '</h4>' . theme_render_and_autoescape($variables['build']);
}
<?php
/**
* @file
* Contains \Drupal\KernelTests\Core\Theme\ThemeRenderAndAutoescapeTest.
*/
namespace Drupal\KernelTests\Core\Theme;
use Drupal\Component\Utility\Html;
use Drupal\Core\Link;
use Drupal\Core\Render\RenderContext;
use Drupal\Core\Render\SafeString;
use Drupal\KernelTests\KernelTestBase;
/**
* Tests the theme_render_and_autoescape() function.
*
* @group Theme
*/
class ThemeRenderAndAutoescapeTest extends KernelTestBase {
/**
* {@inheritdoc}
*/
public static $modules = ['system'];
/**
* {@inheritdoc}
*/
protected function setUp() {
parent::setUp();
$this->installSchema('system', 'router');
\Drupal::service('router.builder')->rebuild();
}
/**
* @dataProvider providerTestThemeRenderAndAutoescape
*/
public function testThemeRenderAndAutoescape($arg, $expected) {
if (is_array($arg) && isset($arg['#type']) && $arg['#type'] === 'link') {
$arg = Link::createFromRoute($arg['#title'], $arg['#url']);
}
$context = new RenderContext();
// Use a closure here since we need to render with a render context.
$theme_render_and_autoescape = function () use ($arg) {
return theme_render_and_autoescape($arg);
};
/** @var \Drupal\Core\Render\RendererInterface $renderer */
$renderer = \Drupal::service('renderer');
$output = $renderer->executeInRenderContext($context, $theme_render_and_autoescape);
$this->assertEquals($expected, $output);
$this-> assertInternalType('string', $output);
}
/**
* Provide test examples.
*/
public function providerTestThemeRenderAndAutoescape() {
return [
'empty string unchanged' => ['', ''],
'simple string unchanged' => ['ab', 'ab'],
'int (scalar) cast to string' => [111, '111'],
'float (scalar) cast to string' => [2.10, '2.10'],
'> is escaped' => ['>', '&gt;'],
'SafeString EM tag is unchanged' => [SafeString::create('<em>hi</em>'), '<em>hi</em>'],
'SafeString SCRIPT tag is unchanged' => [SafeString::create('<script>alert("hi");</script>'), '<script>alert("hi");</script>'],
'EM tag in string is escaped' => ['<em>hi</em>', Html::escape('<em>hi</em>')],
'type link render array is rendered' => [['#type' => 'link', '#title' => 'Text', '#url' => '<none>'], '<a href="">Text</a>'],
'type markup with EM tags is rendered' => [['#markup' => '<em>hi</em>'], '<em>hi</em>'],
'SCRIPT tag in string is escaped' => [
'<script>alert(123)</script>',
Html::escape('<script>alert(123)</script>')
],
'type plain_text render array EM tag is escaped' => [['#plain_text' => '<em>hi</em>'], Html::escape('<em>hi</em>')],
'type hidden render array is rendered' => [['#type' => 'hidden', '#name' => 'foo', '#value' => 'bar'], "<input type=\"hidden\" name=\"foo\" value=\"bar\" />\n"],
];
}
/**
* Ensures invalid content is handled correctly.
*
* @expectedException \Exception
*/
public function testThemeEscapeAndRenderNotPrintable() {
theme_render_and_autoescape(new NonPrintable());
}
}
class NonPrintable { }
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment