diff --git a/core/lib/Drupal/Component/Render/FormattableMarkup.php b/core/lib/Drupal/Component/Render/FormattableMarkup.php index d9fbf2f4393..6797b970fd4 100644 --- a/core/lib/Drupal/Component/Render/FormattableMarkup.php +++ b/core/lib/Drupal/Component/Render/FormattableMarkup.php @@ -227,11 +227,18 @@ class FormattableMarkup implements MarkupInterface, \Countable { default: // We do not trigger an error for placeholder that start with an // alphabetic character. + // @todo https://www.drupal.org/node/2807743 Change to an exception + // and always throw regardless of the first 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 . ') in string: ' . $string, E_USER_ERROR); } + elseif (strpos($string, $key) !== FALSE) { + trigger_error('Invalid placeholder (' . $key . ') in string: ' . $string, E_USER_DEPRECATED); + } + // No replacement possible therefore we can discard the argument. + unset($args[$key]); break; } } diff --git a/core/tests/Drupal/Tests/Component/Render/FormattableMarkupTest.php b/core/tests/Drupal/Tests/Component/Render/FormattableMarkupTest.php index e39617dd1d7..5d4cad944c1 100644 --- a/core/tests/Drupal/Tests/Component/Render/FormattableMarkupTest.php +++ b/core/tests/Drupal/Tests/Component/Render/FormattableMarkupTest.php @@ -13,6 +13,20 @@ use Drupal\Tests\UnitTestCase; */ class FormattableMarkupTest 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; + /** * @covers ::__toString * @covers ::jsonSerialize @@ -35,4 +49,54 @@ class FormattableMarkupTest extends UnitTestCase { $this->assertEquals(strlen($string), $formattable_string->count()); } + /** + * 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; + } + + /** + * @covers ::__toString + * @dataProvider providerTestUnexpectedPlaceholder + */ + public function testUnexpectedPlaceholder($string, $arguments, $error_number, $error_message) { + // 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. + $markup = new FormattableMarkup($string, $arguments); + // Cast it to a string which will generate the errors. + $output = (string) $markup; + restore_error_handler(); + // The string should not change. + $this->assertEquals($string, $output); + $this->assertEquals($error_number, $this->lastErrorNumber); + $this->assertEquals($error_message, $this->lastErrorMessage); + } + + /** + * Data provider for FormattableMarkupTest::testUnexpectedPlaceholder(). + * + * @return array + */ + public function providerTestUnexpectedPlaceholder() { + return [ + ['Non alpha starting character: ~placeholder', ['~placeholder' => 'replaced'], E_USER_ERROR, 'Invalid placeholder (~placeholder) in string: Non alpha starting character: ~placeholder'], + ['Alpha starting character: placeholder', ['placeholder' => 'replaced'], E_USER_DEPRECATED, 'Invalid placeholder (placeholder) in string: Alpha starting character: placeholder'], + // Ensure that where the placeholder is located in the the string is + // irrelevant. + ['placeholder', ['placeholder' => 'replaced'], E_USER_DEPRECATED, 'Invalid placeholder (placeholder) in string: placeholder'], + ]; + } + } diff --git a/core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php b/core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php index cbf86d209d4..8c1fdcec13a 100644 --- a/core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php +++ b/core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php @@ -22,20 +22,6 @@ use Drupal\Tests\UnitTestCase; */ 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} */ @@ -137,7 +123,7 @@ class SafeMarkupTest extends UnitTestCase { UrlHelper::setAllowedProtocols(['http', 'https', 'mailto']); $result = SafeMarkup::format($string, $args); - $this->assertEquals($expected, $result, $message); + $this->assertEquals($expected, (string) $result, $message); $this->assertEquals($expected_is_safe, $result instanceof MarkupInterface, 'SafeMarkup::format correctly sets the result as safe or not safe.'); foreach ($args as $arg) { @@ -171,42 +157,10 @@ class SafeMarkupTest extends UnitTestCase { $tests['non-url-with-colon'] = ['Hey giraffe MUUUH', [':url' => "llamas: they are not URLs"], 'Hey giraffe MUUUH', '', TRUE]; $tests['non-url-with-html'] = ['Hey giraffe MUUUH', [':url' => "not a url"], 'Hey giraffe MUUUH', '', TRUE]; + // Tests non-standard placeholders that will not replace. + $tests['non-standard-placeholder'] = ['Hey hey', ['risky' => ""], 'Hey hey', '', TRUE]; 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) in string: Broken placeholder: ~placeholder', $this->lastErrorMessage); - } }