Discovered in #2560863: #options for radios and checkboxes uses SafeMarkup::checkPlain() to escape - use Html::escape() instead

Problem/Motivation

Flipping between auto-escaping and auto XSS admin filtering is really confusing for developers and that is exactly what happens with #options for select (escape), and radios and checkboxes (filter). The admin filtering is applied by template_preprocess_fieldset() and template_preprocess_form_element_label().

Proposed resolution

Remove the XSS admin filtering and allow twig to auto escape. If calling code wants to support HTML here it should use a render array or mark it safe. The configurable options field is not affected because it use FieldFilteredString for this.

Remaining tasks

Do it
Review
Commit

User interface changes

None

API changes

#options is autoescaped and more tba

Data model changes

None

CommentFileSizeAuthor
#14 interdiff.txt785 bytesmr.baileys
#14 autoescape_title_element_in_form-2568647-14.patch11.89 KBmr.baileys
#9 autoescape_title_element_in_form-2568647-9.patch11.17 KBmr.baileys
#9 interdiff.txt948 bytesmr.baileys
#6 autoescape_title_element_in_form-2568647-6.patch10.25 KBmr.baileys
#6 interdiff.txt8.52 KBmr.baileys
#3 autoescape_title_element_in_form-2568647-3.patch1.73 KBmr.baileys

Comments

alexpott created an issue. See original summary.

mr.baileys’s picture

mr.baileys
Primary language Dutch
Location 🇧🇪 (Ghent)
commented
Assigned: Unassigned » mr.baileys

Will be working on this during the Barcelona sprints

mr.baileys’s picture

mr.baileys
Primary language Dutch
Location 🇧🇪 (Ghent)
commented
Status: Active » Needs review
StatusFileSize
new autoescape_title_element_in_form-2568647-3.patch1.73 KB

This removes the '#markup' from the form #title element in template_preprocess_fieldset() and template_preprocess_form_element_label() so it's no longer admin filtered, but rather falls back on twig auto-escaping.

I'm not sure if we need to add explicit test coverage for this?

Status: Needs review » Needs work

The last submitted patch, 3: autoescape_title_element_in_form-2568647-3.patch, failed testing.

The last submitted patch, 3: autoescape_title_element_in_form-2568647-3.patch, failed testing.

mr.baileys’s picture

mr.baileys
Primary language Dutch
Location 🇧🇪 (Ghent)
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt8.52 KB
new autoescape_title_element_in_form-2568647-6.patch10.25 KB

This should take care of most of the failures.

Status: Needs review » Needs work

The last submitted patch, 6: autoescape_title_element_in_form-2568647-6.patch, failed testing.

The last submitted patch, 6: autoescape_title_element_in_form-2568647-6.patch, failed testing.

mr.baileys’s picture

mr.baileys
Primary language Dutch
Location 🇧🇪 (Ghent)
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt948 bytes
new autoescape_title_element_in_form-2568647-9.patch11.17 KB

Thanks to @lauriii I managed to fix the remaining failure.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Needs review » Reviewed & tested by the community

Looks good for me :)

catch’s picture

catch
he/him
Primary language English
commented

Looks great to me but I either need a second opinion or to take another good look at it before I feel 100% happy committing,

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs change record

I really like this change as it brings consistency but it needs a CR.

alexpott’s picture

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

Also need to convert $role_options = array_map('\Drupal\Component\Utility\Html::escape', user_role_names()); in core/modules/views/src/Plugin/views/filter/FilterPluginBase.php - Also with this patch that would by double escaped so we're missing test coverage.

mr.baileys’s picture

mr.baileys
Primary language Dutch
Location 🇧🇪 (Ghent)
commented
StatusFileSize
new autoescape_title_element_in_form-2568647-14.patch11.89 KB
new interdiff.txt785 bytes

I have openend #2579829: Missing test coverage for views exposed filter "remember last selection" to add the missing test coverage for core/modules/views/src/Plugin/views/filter/FilterPluginBase.php

Patch attached fixes #13, I'm working on the CR.

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
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Issue tags: +Bug Smash Initiative

What's the next steps here?

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.

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.