Contrib.social

Make it impossible to double escape with #plain_text

Open on Drupal.org →
Created on 14 September 2015, over 9 years ago
Updated 7 February 2023, about 2 years ago

Problem/Motivation

There is no render #thingie the behaves like twig.

  • #markup admin filters if not safe
  • #plain_text always escapes regardless of safety
  • Twig will escape if not safe

This means that issues like #2567159: Fix improper usage of t() in ViewsBlock → are resorting to inline templates to get this behaviour. Funnily enough we use to be able to flip #markup between filtering and escaping but that got changed in #2555931: Add #plain_text to escape text in render arrays → . I think the rationale behind that change was good since

$render_array = ['#plain_text' => SafeString::create('<em>I win</em>')];

looks awful.

But

$render_array = ['#plain_text' => t('@thing: @subthing', $placeholders)];

the fact that this will double escape is also awful.

Proposed resolution

Not sure. We could make #plain_text use the inbuilt double escaping prevention in htmlspecialchars() or maybe once t() and SafeMarkup::format return objects we could glean information as to how they are made safe.

Remaining tasks

User interface changes

API changes

Data model changes

🐛 Bug report
Status

Needs review

Version

10.1 ✨

Component
Render  →

Last updated about 3 hours ago

Created by

🇬🇧United Kingdom alexpott 🇪🇺🌍

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

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment about 2 years ago →
    🇺🇸United States smustgrave

    @alexpott should a change be made for #31?

  • Comment about 2 years ago →
    🇬🇧United Kingdom alexpott 🇪🇺🌍

    We could try #31 but I worry about things like the filter admin page which need things double escaped. I think that #plain_text indicates an intention beyond other places where Html::escape() is called so the patch in #28 is correct but moving this to Html::escape() could have unintended impacts.

  • Status changed to Needs work about 2 years ago
  • Comment about 2 years ago →
    🇺🇸United States smustgrave

    Wonder if the issue summary could be updated though.

    The proposed solution, correct me if I'm wrong, doesn't seem to match up with the solution in the patch.

    Solution wise though this looks good.

  • Comment 8 months ago →
    🇦🇺Australia acbramley

    This was triaged as part of BSI. At the very least needs a reroll onto an MR.

  • Comment 2 months ago →
    🇮🇳India prem suthar Ahemdabad- Gujrat , Jodhpur - Rajsthan

    prem suthar → made their first commit to this issue’s fork.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024