Issue #2575703 by borisson_, stefan.r, Berdir, joshi.rohit100: Remove default fall-through from PlaceholderTrait::placeholderFormat()

core/lib/Drupal/Component/Render/FormattableMarkup.php

@@ -223,7 +223,6 @@ class FormattableMarkup implements MarkupInterface {
           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 @@ class FormattableMarkup implements MarkupInterface {
           // 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;
       }
     }
 

core/lib/Drupal/Core/Controller/TitleResolver.php

@@ -60,7 +60,6 @@ class TitleResolver implements TitleResolverInterface {
       if (($raw_parameters = $request->attributes->get('_raw_variables'))) {
         foreach ($raw_parameters->all() as $key => $value) {
           $args['@' . $key] = $value;
-          $args['!' . $key] = $value;
           $args['%' . $key] = $value;
         }
       }

core/modules/aggregator/src/Tests/AggregatorTestBase.php

@@ -166,7 +166,7 @@ abstract class AggregatorTestBase extends WebTestBase {
   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());

core/modules/dblog/src/Tests/Views/ViewsIntegrationTest.php

@@ -71,13 +71,13 @@ class ViewsIntegrationTest extends ViewKernelTestBase {
     );
     // 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>',
       ),
     );

core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php

@@ -22,6 +22,20 @@ 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}
    */
@@ -159,6 +173,40 @@ class SafeMarkupTest extends UnitTestCase {
 
     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);
+  }
 
 }