Skip to content

Missing Block: Don't use dangerouslySetInnerHTML with i18n string#10626

Merged
mcsf merged 2 commits intomasterfrom
fix/missing-block-dangerous-i18n-input
Oct 15, 2018
Merged

Missing Block: Don't use dangerouslySetInnerHTML with i18n string#10626
mcsf merged 2 commits intomasterfrom
fix/missing-block-dangerous-i18n-input

Conversation

@mcsf
Copy link
Copy Markdown
Contributor

@mcsf mcsf commented Oct 15, 2018

Description

Fixes use of dangerouslySetInnerHTML with uncontrolled external input (i18n string). Identified in this comment.

How has this been tested?

Screenshots

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.

@mcsf mcsf added the [Type] Bug An existing feature does not function as intended label Oct 15, 2018
@mcsf mcsf requested a review from aduth October 15, 2018 17:37
@mcsf mcsf added this to the 4.1 - UI freeze milestone Oct 15, 2018
<Fragment>
<Warning actions={ actions }>
<span dangerouslySetInnerHTML={ { __html: messageHTML } } />
<span>{ messageHTML }</span>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we drop span?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤦‍♂️ Yes. Pushed.

Copy link
Copy Markdown
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@mcsf mcsf merged commit dd054d2 into master Oct 15, 2018
@aduth aduth deleted the fix/missing-block-dangerous-i18n-input branch October 15, 2018 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Type] Bug An existing feature does not function as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants