Bump sanitize gem #127
Merged
Bump sanitize gem #127
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
This fixes CVE-2018-2740 (See: rgrove/sanitize#176) We also have to fix some tests around table tags, because as of sanitize 3.x it uses a parser more like a browser which means it will strip invalid HTML and correct it when it's less-broken. Tables are one of the things it does this for.
Sanitize::Config objects are now frozen by default so we can't manipulate them in place. It does now come with a `merge` method that lets you combine them though, and this means we no longer need our `deep_merge` method. While we're manipulating the config we can also remove some of the settings that we've added because they have been rolled into the default RELAXED settings. Specifically: `div`, `aside`, `span` are now allowed elements, `id`, `class` are allowed attributes, and `rel` is allowed on an `a` tag.
h-lame
force-pushed
the
bump-sanitize-gem
branch
from
8e3a400 to
95a9a02
Compare
thomasleese
approved these changes
Mar 21, 2018
suzannehamilton
approved these changes
Mar 22, 2018
kevindew
added a commit
that referenced
this pull request
Jul 17, 2019
This is to resolve a blocking issue that prevents Whitehall from upgrading to Govspeak > 5.5. A problem we have is that a number of pieces of Whitehall content have unusual unicode characters that the Santize gem strips out, this then makes the content fail HTML Validation and was introduced as part of a Santize update in #127. The approach I've taken to resolve this is to remove forbidden unicode characters from the source markdown that is converted to HTML. Since these characters are invalid in a HTML output it seems reasonable to consider them as invalid as part of govspeak input. Looking through Whitehalls database I found there to be 765 Edition::Translation models and 248 GovspeakContent models that have at last one of these characters in their body fields. Initially I intended to resolve this with a data migration on Whitehall, however that would have caused a problem if anyone used these characters again as it would be very difficult to determine why content was failing govspeak validation. I figure it's actually more user friendly to strip the character out and then if they notice a space or similar is missing they can correct that with their keyboard. I noticed that in 2019 there had been 74 of these items added so this doesn't appear to be a legacy issue. My current theory is that perhaps these are characters that are present in content that is being pasted from such as a PDF document. Anyway, with this merged in Whitehall can proceed to be upgraded to Govspeak 6 as with this and #159 all GovspeakContent models are valid govspeak and only 14 Edition::Translation bodies are invalid, all of which are also invalid under Govspeak 5.5.
kevindew
added a commit
that referenced
this pull request
Jul 17, 2019
This is to resolve a blocking issue that prevents Whitehall from upgrading to Govspeak > 5.5. A problem we have is that a number of pieces of Whitehall content have unusual unicode characters that the Santize gem strips out, this then makes the content fail HTML Validation and was introduced as part of a Santize update in #127. The approach I've taken to resolve this is to remove forbidden unicode characters from the source markdown that is converted to HTML. Since these characters are invalid in a HTML output it seems reasonable to consider them as invalid as part of govspeak input. Looking through Whitehalls database I found there to be 765 Edition::Translation models and 248 GovspeakContent models that have at last one of these characters in their body fields. Initially I intended to resolve this with a data migration on Whitehall, however that would have caused a problem if anyone used these characters again as it would be very difficult to determine why content was failing govspeak validation. I figure it's actually more user friendly to strip the character out and then if they notice a space or similar is missing they can correct that with their keyboard. I noticed that in 2019 there had been 74 of these items added so this doesn't appear to be a legacy issue. My current theory is that perhaps these are characters that are present in content that is being pasted from such as a PDF document. Anyway, with this merged in Whitehall can proceed to be upgraded to Govspeak 6 as with this and #159 all GovspeakContent models are valid govspeak and only 14 Edition::Translation bodies are invalid, all of which are also invalid under Govspeak 5.5.
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.
For: https://trello.com/c/X6CWuGOX/134-rails-security-vulnerability-sanitize-463-libxml-292
This mitigates against rgrove/sanitize#176. While we're at it we tidy up some config that we no longer need to set because it's handled by default by the gem now. Check individual commits for details.