Wordpress-seo: Switch toggles: consider to use proper escaping

Created on 16 Feb 2018  路  2Comments  路  Source: Yoast/wordpress-seo

See Yoast_Form::light_switch() and Yoast_Form::toggle_switch() that generate the "switch toggle" controls.

Most of the output is escaped (HTML attributes, screen reader text) but the visible "labels" are not escaped.

Proper escaping is made more difficult now that the new design uses HTML (for bold text) in the visible "labels":

screen shot 2018-02-16 at 16 02 28

  • ideally, escaping should happen immediately before the output but then it wouldn't be possible to use bold or any other HTML
  • or, less ideal, the passed strings should be escaped on a case by case basis, which is made difficult by the fact these toggle functions are now often used within loops that pass dynamically generated strings containing variables
UX translations development

Most helpful comment

One of the problems with these is that they are used in our other plugins as well. So to properly apply escaping when generating the HTML, we need to make sure all plugins are only passing non-escaped strings.

As this is not an easy task and requires backwards compatible solutions, we need to make sure all input to these methods is escaped when calling them.

When we decide to properly fix this problem, we will need to introduce new methods that assume the entered strings are not escaped yet. These could even be presenters to allow links and styling to be applied within the string.

Thus we need to verify on a case-by-case basis that they are passed with the proper escaping.
@jrfnl has been doing a lot in this regard, pinging to get her context on how this is addressed in the current Coding Standards upgrades and if we need to have this issue to make sure we verify and check everything again.

All 2 comments

@afercia This is one of the things I'm looking into during the CS round. No guarantee that I'll fix it as all relevant strings would need to be examined (if not, a technical-debt issue will be opened for it), but I'll probably make an attempt with having a wp_kses() in combination with a allowed HTML list within some of the Yoast form methods.

One of the problems with these is that they are used in our other plugins as well. So to properly apply escaping when generating the HTML, we need to make sure all plugins are only passing non-escaped strings.

As this is not an easy task and requires backwards compatible solutions, we need to make sure all input to these methods is escaped when calling them.

When we decide to properly fix this problem, we will need to introduce new methods that assume the entered strings are not escaped yet. These could even be presenters to allow links and styling to be applied within the string.

Thus we need to verify on a case-by-case basis that they are passed with the proper escaping.
@jrfnl has been doing a lot in this regard, pinging to get her context on how this is addressed in the current Coding Standards upgrades and if we need to have this issue to make sure we verify and check everything again.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

PENTAGON4 picture PENTAGON4  路  6Comments

tholu picture tholu  路  3Comments

isaumya picture isaumya  路  4Comments

cristopherrosenberg picture cristopherrosenberg  路  3Comments

danieltj27 picture danieltj27  路  6Comments