Summaries should not be decoded with Html::decodeEntities

Created on 16 March 2023, over 1 year ago
Updated 16 July 2024, about 2 months ago

Problem/Motivation

Currently, asset injector summaries are decoded, however this results in unnecessarily escaped HTML.

e.g. if the summary returns:

$this->t('Condition filter values: %theme.', [
  '%theme' => $this->configFactory->get('system.theme')->get('default'),
]);

Steps to reproduce

Use a condition which returns a summary containing Drupal placeholders.

Proposed resolution

Don't escape as the underlying module providing the summary should've already handled all the necessary sanitization.

We could also potentially wrap it in something along the lines of this (but shouldn't be necessary since it's not user generated content):

Markup::create(Xss::filterAdmin($summary->render()));

Remaining tasks

User interface changes

N/A

🐛 Bug report
Status

Needs review

Version

2.0

Component

Code

Created by

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @codebymikey
  • Status changed to Needs review over 1 year ago
  • Added issue fork, the current approach also allows it to work if the third party module happens to return a simple string rather than markup.

  • First commit to issue fork.
  • 🇩🇪Germany Anybody Porta Westfalica

    The module's implementation changed in the meantime:
    $data['conditions'][$condition_id] = is_string($summary) ? Html::decodeEntities($summary) : Html::decodeEntities($summary->render());

    Could the maintainer eventually explain why Html::decodeEntities() is used here at all?

    I also can't really see a benefit, theoretically it's even more a risk:

    Be careful when using this function, as it will revert previous sanitization efforts (<script> will become

    ).
    https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Component%21Utility%21Html.php/function/Html%3A%3AdecodeEntities/8.9.x So I think it should be removed for good reasons, if it's not needed.
  • Pipeline finished with Success
    about 2 months ago
    Total: 177s
    #225335
Production build 0.71.5 2024