!message in t() can lead to double-escaping when used in drupal_set_message()

Here is a patch to fix the two places where this occurs in the User module.

Thanks for doing further research into this issue, @edutrul.

Instances where "!" are used when there is no html in the message are okay, because it is innocuous to check_plain() text that is already plain. The first two you quote might sometimes be an issue, though, because %label will be escaped, which will cause double-escaping due to "!message" if the contents of $values['label'] contains any content that is altered when escaped. So, these two could probably become @message.

So, here is a screenshot after adding Joe's "special" field. I wasn't sure how to force the exception to be thrown, so I temporarily added a drupal_set_message() to the non-failure case just to take the screenshot.

Temporary code (wrong, %label is double-escapped):

        drupal_set_message($this->t('There was no problem creating field %label: !message', array('%label' => $values['label'], '!message' => "This is just a test.")));

Screenshot:

I've added an updated patch to also correct this issue.

I was thinking about this overnight, and realized that using '!message' with drupal_set_message is always a problem in Drupal 8. To demonstrate this, I ran the following test:

drupal_set_message($this->t('Test of bang substitutions: !message', array('!message' => 'This is a test')));

That produced the following output:

So, !message works the same as @message, but !message has the additional undesirable side-effect of escaping other parts of the message unrelated to the substitution that used "!". !message is therefore only usable in cases where there is no content that would be altered by output escaping. In that instance, though, @message works just as well. Therefore, the other places in Drupal 8 where this pattern occurs should be changed to use @message instead of !message, to avoid the side-effect, and reduce the chance that this pattern might spread through emulation. For example, if someone copies a druap_set_message that uses !message, and then adds an @other variable, then a double-escaping bug will be introduced. This can linger if the message comes up only rarely, or if the substituted values usually do not contain any characters that need escaping.

Okay, I made an important realization: !message is useful and works as intended, but only if the replacement value is already marked as a safe string.

Here is a working example from Drupal 8 core, in core/modules/config/src/Form/ConfigSync.php buildForm():

      $change_list_render = array(
        '#theme' => 'item_list',
        '#items' => $change_list,
      );
      $change_list_html = drupal_render($change_list_render);
      drupal_set_message($this->t('Your current configuration has changed. Changes to these configuration items will be lost on the next synchronization: !changes', array('!changes' => $change_list_html)), 'warning');

Here is a screenshot of a copy of this code, executed with a hardcoded list of two items, "item 1" and "item 2":

So, the output of drupal_render() is marked as "safe", so when this safe value is used as the replacement value for !changes, above, the result of t() is also marked as safe, and no double-escaping occurs. This is different than the erroneous values above, because in the failure cases, the replacement values were simple strings that had never passed through any function that marked them safe with SafeMarkup::set(). Calling SafeMarkup::set() directly is deprecated, so the usage model for using !replacement with drupal_set_message() is to always insure that the replacement value came from drupal_render(), or some other function that will mark the result safe.

There was only one other place in Drupal core where a !replacement needed to be changed to @replacement. I have attached an updated patch that fixes it, plus #6 and #1.