From 35ed46b43ee936a32f1bc4adfa5a14e16129d69d Mon Sep 17 00:00:00 2001
From: Alex Pott <alex.a.pott@googlemail.com>
Date: Sun, 1 Nov 2015 23:00:34 +0000
Subject: [PATCH] Issue #2575703 by borisson_, stefan.r, Berdir,
joshi.rohit100: Remove default fall-through from
PlaceholderTrait::placeholderFormat()
---
.../Component/Render/FormattableMarkup.php | 11 ++++-
.../Drupal/Core/Controller/TitleResolver.php | 1 -
.../src/Tests/AggregatorTestBase.php | 2 +-
.../src/Tests/Views/ViewsIntegrationTest.php | 4 +-
.../Component/Utility/SafeMarkupTest.php | 48 +++++++++++++++++++
5 files changed, 61 insertions(+), 5 deletions(-)
diff --git a/core/lib/Drupal/Component/Render/FormattableMarkup.php b/core/lib/Drupal/Component/Render/FormattableMarkup.php
index cde8e86bdda3..ec42e068a793 100644
--- a/core/lib/Drupal/Component/Render/FormattableMarkup.php
+++ b/core/lib/Drupal/Component/Render/FormattableMarkup.php
@@ -223,7 +223,6 @@ protected static function placeholderFormat($string, array $args) {
break;
case '%':
- default:
// Similarly to @, escape non-safe values. Also, add wrapping markup
// in order to render as a placeholder. Not for use within attributes,
// per the warning above about
@@ -231,6 +230,16 @@ protected static function placeholderFormat($string, array $args) {
// due to the wrapping markup.
$args[$key] = '<em class="placeholder">' . static::placeholderEscape($value) . '</em>';
break;
+
+ default:
+ // We do not trigger an error for placeholder that start with an
+ // alphabetic character.
+ if (!ctype_alpha($key[0])) {
+ // We trigger an error as we may want to introduce new placeholders
+ // in the future without breaking backward compatibility.
+ trigger_error('Invalid placeholder: ' . $key, E_USER_ERROR);
+ }
+ break;
}
}
diff --git a/core/lib/Drupal/Core/Controller/TitleResolver.php b/core/lib/Drupal/Core/Controller/TitleResolver.php
index 4fe90a6a7efc..9adff8c2f470 100644
--- a/core/lib/Drupal/Core/Controller/TitleResolver.php
+++ b/core/lib/Drupal/Core/Controller/TitleResolver.php
@@ -60,7 +60,6 @@ public function getTitle(Request $request, Route $route) {
if (($raw_parameters = $request->attributes->get('_raw_variables'))) {
foreach ($raw_parameters->all() as $key => $value) {
$args['@' . $key] = $value;
- $args['!' . $key] = $value;
$args['%' . $key] = $value;
}
}
diff --git a/core/modules/aggregator/src/Tests/AggregatorTestBase.php b/core/modules/aggregator/src/Tests/AggregatorTestBase.php
index e6d5124df0da..d5b09e5256a5 100644
--- a/core/modules/aggregator/src/Tests/AggregatorTestBase.php
+++ b/core/modules/aggregator/src/Tests/AggregatorTestBase.php
@@ -166,7 +166,7 @@ public function getDefaultFeedItemCount() {
public function updateFeedItems(FeedInterface $feed, $expected_count = NULL) {
// First, let's ensure we can get to the rss xml.
$this->drupalGet($feed->getUrl());
- $this->assertResponse(200, format_string('!url is reachable.', array('!url' => $feed->getUrl())));
+ $this->assertResponse(200, format_string(':url is reachable.', array(':url' => $feed->getUrl())));
// Attempt to access the update link directly without an access token.
$this->drupalGet('admin/config/services/aggregator/update/' . $feed->id());
diff --git a/core/modules/dblog/src/Tests/Views/ViewsIntegrationTest.php b/core/modules/dblog/src/Tests/Views/ViewsIntegrationTest.php
index ae088e004088..50659ae283ae 100644
--- a/core/modules/dblog/src/Tests/Views/ViewsIntegrationTest.php
+++ b/core/modules/dblog/src/Tests/Views/ViewsIntegrationTest.php
@@ -71,13 +71,13 @@ public function testIntegration() {
);
// Setup a watchdog entry with two tokens.
$entries[] = array(
- 'message' => '@token1 !token2',
+ 'message' => '@token1 @token2',
// Setup a link with a tag which is filtered by
// \Drupal\Component\Utility\Xss::filterAdmin() in order to make sure
// that strings which are not marked as safe get filtered.
'variables' => array(
'@token1' => $this->randomMachineName(),
- '!token2' => $this->randomMachineName(),
+ '@token2' => $this->randomMachineName(),
'link' => '<a href="' . \Drupal::url('<front>') . '"><object>Link</object></a>',
),
);
diff --git a/core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php b/core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php
index 0a2e0ea180c8..7b9cfaaba491 100644
--- a/core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php
+++ b/core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php
@@ -22,6 +22,20 @@
*/
class SafeMarkupTest extends UnitTestCase {
+ /**
+ * The error message of the last error in the error handler.
+ *
+ * @var string
+ */
+ protected $lastErrorMessage;
+
+ /**
+ * The error number of the last error in the error handler.
+ *
+ * @var int
+ */
+ protected $lastErrorNumber;
+
/**
* {@inheritdoc}
*/
@@ -159,6 +173,40 @@ function providerFormat() {
return $tests;
}
+ /**
+ * Custom error handler that saves the last error.
+ *
+ * We need this custom error handler because we cannot rely on the error to
+ * exception conversion as __toString is never allowed to leak any kind of
+ * exception.
+ *
+ * @param int $error_number
+ * The error number.
+ * @param string $error_message
+ * The error message.
+ */
+ public function errorHandler($error_number, $error_message) {
+ $this->lastErrorNumber = $error_number;
+ $this->lastErrorMessage = $error_message;
+ }
+
+ /**
+ * String formatting with SafeMarkup::format() and an unsupported placeholder.
+ *
+ * When you call SafeMarkup::format() with an unsupported placeholder, an
+ * InvalidArgumentException should be thrown.
+ */
+ public function testUnexpectedFormat() {
+
+ // We set a custom error handler because of https://github.com/sebastianbergmann/phpunit/issues/487
+ set_error_handler([$this, 'errorHandler']);
+ // We want this to trigger an error.
+ $error = SafeMarkup::format('Broken placeholder: ~placeholder', ['~placeholder' => 'broken'])->__toString();
+ restore_error_handler();
+
+ $this->assertEquals(E_USER_ERROR, $this->lastErrorNumber);
+ $this->assertEquals('Invalid placeholder: ~placeholder', $this->lastErrorMessage);
+ }
}
--
GitLab