#description is only XSS admin filtered by default. This makes it akin to #markup, #prefix and #suffix - but it is not the same at all. Let's make it auto-escape like most other things.

CommentFileSizeAuthor
#18 interdiff.txt3.65 KBlauriii
#18 description_should-2573743-18.patch15.24 KBlauriii
#18 description_should-2573743-18-reroll.patch15.16 KBlauriii
#11 2573743-11.patch15.16 KBalexpott
#11 10-11-interdiff.txt2.85 KBalexpott
#10 2573743-10.patch13.99 KBalexpott
#10 9-10-interdiff.txt10.06 KBalexpott
#9 2573743-9.patch6.69 KBalexpott
#9 6-9-interdiff.txt4.48 KBalexpott
#6 2573743-6.patch2.21 KBalexpott
#6 2-6-interdiff.txt1.7 KBalexpott
#2 2573743-2.patch527 bytesalexpott

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Active » Needs review
StatusFileSize
new 2573743-2.patch527 bytes

Let's see what breaks.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Related issues: +#2324371: Fix common HTML escaped render #key values due to Twig autoescape

The admin filtering of #description was added in #2324371: Fix common HTML escaped render #key values due to Twig autoescape

Status: Needs review » Needs work

The last submitted patch, 2: 2573743-2.patch, failed testing.

The last submitted patch, 2: 2573743-2.patch, failed testing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs work » Needs review
StatusFileSize
new 2-6-interdiff.txt1.7 KB
new 2573743-6.patch2.21 KB

Fixing the test fails. I think the fixes show that this is good idea.

Status: Needs review » Needs work

The last submitted patch, 6: 2573743-6.patch, failed testing.

The last submitted patch, 6: 2573743-6.patch, failed testing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs work » Needs review
StatusFileSize
new 6-9-interdiff.txt4.48 KB
new 2573743-9.patch6.69 KB

Fixing more fails and some @todos.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
StatusFileSize
new 9-10-interdiff.txt10.06 KB
new 2573743-10.patch13.99 KB

Introducing RenderVariable to produce not very nice looking render arrays.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
StatusFileSize
new 10-11-interdiff.txt2.85 KB
new 2573743-11.patch15.16 KB

More tests

Status: Needs review » Needs work

The last submitted patch, 11: 2573743-11.patch, failed testing.

stefan.r’s picture

stefan.r commented

Discussed this with @dawehner - it may be fine, considering we autoescape elsewhere as well. Not autoescaping used to be inconvenient a year ago but we have better tools now. Also #description contains a t() string in most cases.

The problem is when we concatenate. This shows we need a helper function that helps concatenate and mark the resulting string as safe or not, santizing where needed.

One worry with this change is... this used work, and now we break it. Do we truly need to?

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Yes this is an api change - generally the worst that can happen is some escaped HTML where it's not supposed to be. But this does mean that the only special cased render variables are #markup, #prefix, #suffix , #field_prefix and #field_suffix. Which are all similar - the outlier is #description. Even the fact that it is touched at all in Renderer should ring alarm bells.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented

In order to make the API more user friendly, we have to remove as many special cases as possible. We want to teach people to use #markup, #prefix and #suffix if they need to print some static HTML from PHP. What does #description do with that? For me it seems like #description doesn't belong to that list because its trying to solve special use case, and the other render variables are general solutions for printing markup.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

@lauriii nice comment that sums it up well.

However doing this patch will make #2571935: Fix use of !placeholder for imploding in views.views.inc harder because then we'll have to mark the result of concatenating everything together as safe somehow.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented

I know this is a weird suggestion but I'd suggest to postpone this till #2571935: Fix use of !placeholder for imploding in views.views.inc is in and then we could discover how to fix the concatenating in this issue. That way we wouldn't make solving criticals any harder :)

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Needs work » Needs review
StatusFileSize
new description_should-2573743-18-reroll.patch15.16 KB
new description_should-2573743-18.patch15.24 KB
new interdiff.txt3.65 KB

The last submitted patch, 18: description_should-2573743-18-reroll.patch, failed testing.

The last submitted patch, 18: description_should-2573743-18-reroll.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 18: description_should-2573743-18.patch, failed testing.

The last submitted patch, 18: description_should-2573743-18.patch, failed testing.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Issue tags: +D8 major triage deferred

Discussed with @xjm, @Cottser, @joelpittet and @laurii. All things should autoescape, support translatable markup, support render arrays. We need to add documentation of the sanitisation behaviour of ALL render elements (and workarounds to change them) to the scope of the docs meta. I proposed resolution to add version key to render arrays as a separate 8.x issue.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Created #2722747: Discuss being able to version render API to discuss a possible way to achieve this in D8

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

xjm’s picture

xjm
she/her
Primary language English
commented
Issue tags: -D8 major triage deferred

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

smustgrave’s picture

smustgrave commented
Issue tags: +Bug Smash Initiative, +Needs issue summary update

This came up as a daily BSI target

This definitely appears to be relevant, from checking a few of the ['#description'] instances in core.

This will need an issue summary update but @alexpott would you say this also just needs a reroll?

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.