Sanitize translations instead of using _html
#3748
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
javierm
changed the title
houndci-bot
reviewed
Oct 5, 2019
javierm
changed the title
_html
javierm
force-pushed
the
locales_html
branch
2 times, most recently
from
86d44b7
to
d55f539
Compare
javierm
force-pushed
the
html_safe
branch
2 times, most recently
from
f68cde6
to
99ffafd
Compare
javierm
force-pushed
the
html_safe
branch
2 times, most recently
from
87d3bec
to
238bee5
Compare
houndci-bot
reviewed
Oct 8, 2019
javierm
force-pushed
the
html_safe
branch
2 times, most recently
from
90ff508
to
391f58e
Compare
javierm
force-pushed
the
locales_html
branch
2 times, most recently
from
1b3bfd7
to
80574b7
Compare
Although this translation has HTML, we aren't marking them as HTML safe since we're using `I18n.t` instead of Rails' helper `t` method. So using the `_html` suffix is counterintuitive in this case.
Using the `_html` suffix automatically marks texts as HTML safe, so doing so on sanitized texts is redundant. Note flash texts are not sanitized the moment they are generated, but are sanitized when displayed in the view.
Using the `_html` suffix in an i18n key is the same as using `html_safe` on it, which means that translation could potentially be used for XSS attacks.
The link tags were being stripped out by `content_tag`.
smarques
pushed a commit
to venetochevogliamo/consul
that referenced
this pull request
Apr 29, 2020
Sanitize translations instead of using `_html`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
References
Background
We were using the
_htmlsuffix in translations to indicate HTML was allowed in those cases. However, doing so is exactly the same as marking them ashtml_safe. It's safer to sanitize them instead.Objectives
Release notes
config/locales/custom/, note we've moved translations related to label forms to theactiverecord.ymlfile (pull request #3746), and we've renamed all translations whose key ended with_html, removing the_htmlsuffix (pull request #3748).