-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Issue #87 Make sanitization happen in Post.htmlize() #88
Merged
janetriley
merged 5 commits into
DistrictDataLabs:develop
from
janetriley:feature-87_extract_html_sanitizer
Jun 12, 2017
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
1507ffc
Issue #87 Make sanitization happen in Post.htmlize()
janetriley 8b4ef79
Incorporate PR feedback - default sanitize to none
janetriley bfafd43
Issue #87 Make sanitization happen in Post.htmlize()
janetriley 00415a3
Issue #87 Make sanitization happen in Post.htmlize()
janetriley 6eb8e58
Fix test; tweak docs
janetriley File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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 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 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
# baleen.utils.text | ||
# Utility functions for Baleen | ||
# | ||
# Author: Benjamin Bengfort <[email protected]> | ||
# Created: Sat Jun 03 18:48:00 2017 -0400 | ||
# | ||
# Copyright (C) 2017 Bengfort.com | ||
# For license information, see LICENSE.txt | ||
# | ||
# ID: text.py [caaaaca] [email protected] $ | ||
|
||
""" | ||
Text-related Utility functions for Baleenc | ||
""" | ||
|
||
########################################################################## | ||
## Imports | ||
########################################################################## | ||
|
||
import bleach | ||
from readability.readability import Document | ||
|
||
########################################################################## | ||
## Constants | ||
########################################################################## | ||
|
||
RAW = 'raw' | ||
SAFE = 'safe' | ||
TEXT = 'text' | ||
SANITIZE_LEVELS = (RAW, SAFE, TEXT) | ||
|
||
|
||
def get_raw_html(html): | ||
""" | ||
:param html: html content | ||
:return: the unmodified html | ||
""" | ||
return html | ||
|
||
|
||
def get_safe_html(html): | ||
""" | ||
Applies Readability's sanitize() method to content. | ||
:param html: the content to sanitize | ||
:return: the body of the html content minus html tags | ||
""" | ||
if html is None: | ||
return None | ||
return Document(html).summary() | ||
|
||
|
||
def get_text_from_html(html): | ||
""" | ||
Applies the 'safe' level of sanitization, removes newlines, | ||
and converts the html entity for ampersand into the ampersand character. | ||
:param html: the content to sanitize | ||
:return: sanitized content | ||
""" | ||
if html is None: | ||
return html | ||
|
||
text = get_safe_html(html) | ||
text = bleach.clean(text, tags=[], strip=True) | ||
text = text.strip() | ||
text = text.replace("\n", "") | ||
text = text.replace("&", "&") | ||
return text | ||
|
||
|
||
def sanitize_html(html, level): | ||
""" | ||
Return a sanitized version of html content | ||
:param html: the content to sanitized | ||
:param level: the type of sanitization - one of ['raw', 'safe', 'text', None] | ||
:return: sanitized content | ||
""" | ||
if level == SAFE: | ||
return get_safe_html(html) | ||
elif level == RAW: | ||
return get_raw_html(html) | ||
elif level == TEXT: | ||
return get_text_from_html(html) | ||
elif level is None: | ||
return html | ||
|
||
raise ValueError( | ||
"{level} is not a supported sanitize_html level.".format( | ||
level=level | ||
) | ||
) |
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 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
# test.utils_tests.test_text | ||
# Testing for the text helpers library. | ||
# | ||
# Author: Benjamin Bengfort <[email protected]> | ||
# Created: Sat Jun 03 18:48:00 2017 -0400 | ||
# | ||
# Copyright (C) 2017 Bengfort.com | ||
# For license information, see LICENSE.txt | ||
# | ||
# ID: test_text.py [df0c71b] [email protected] $ | ||
|
||
""" | ||
Testing for the text helpers library. | ||
""" | ||
|
||
########################################################################## | ||
## Imports | ||
########################################################################## | ||
|
||
import unittest | ||
|
||
from baleen.exceptions import ExportError | ||
from baleen.utils.text import sanitize_html, RAW, SAFE, TEXT | ||
|
||
|
||
class SanitizeHtmlTests(unittest.TestCase): | ||
""" Tests the exporter's HTML sanitize methods """ | ||
|
||
@classmethod | ||
def setUpClass(cls): | ||
cls.sample_html = ('<html>' | ||
'<head><script>javascript here</script></head>' | ||
'<body><b>body &\n mind</b></body>' | ||
'</html>') | ||
|
||
@classmethod | ||
def tearDownClass(self): | ||
""" | ||
Drop the mongomock connection | ||
""" | ||
pass | ||
|
||
def test_sanitize_requires_a_valid_level(self): | ||
""" Sanitize_html requires a supported level """ | ||
with self.assertRaises(ValueError): | ||
sanitize_html(self.sample_html, "bogus") | ||
|
||
def test_sanitize_returns_input_for_level_none(self): | ||
""" sanitize_html returns unmodified input for level None """ | ||
self.assertEqual(sanitize_html(self.sample_html, None), self.sample_html) | ||
|
||
def test_sanitize_raw(self): | ||
""" Sanitize level raw returns the content as submitted """ | ||
self.assertEqual(sanitize_html(self.sample_html, RAW), self.sample_html) | ||
|
||
def test_sanitize_raw_handles_none(self): | ||
""" | ||
Sanitize level raw accepts None gracefully | ||
""" | ||
self.assertEqual(sanitize_html(None, RAW), None) | ||
|
||
def test_sanitize_safe(self): | ||
""" Sanitize level safe applies Readability and returns the body """ | ||
|
||
# Give Readability a simpler HTML sample to keep its parse strategy simple | ||
sample_html = ('<html>' | ||
'<head><script>javascript here</script></head>' | ||
'<body>body</body>' | ||
'</html>') | ||
expected = '<body id="readabilityBody">body</body>' | ||
self.assertEqual(sanitize_html(sample_html, SAFE), expected) | ||
|
||
def test_sanitize_safe_handles_none(self): | ||
""" | ||
Sanitize level safe accepts None gracefully | ||
""" | ||
self.assertEqual(sanitize_html(None, SAFE), None) | ||
|
||
def test_sanitize_text(self): | ||
""" | ||
Sanitize level text strips HTML tags, removes newlines, | ||
and converts the html entity ampersand into an ampersand character | ||
""" | ||
expected = 'body & mind' | ||
self.assertEqual(sanitize_html(self.sample_html, TEXT), expected) | ||
|
||
def test_sanitize_text_handles_none(self): | ||
""" | ||
Sanitize level text accepts None gracefully | ||
""" | ||
self.assertEqual(sanitize_html(None, TEXT), None) |
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It absolutely makes sense to remove these tests, should we have a test for
Post.htmlize()
? It will just be mockingsanitize_html
and making sure it is called with the right arguments. If you think that's overkill though I understand.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's verging on overkill but I like making percentages go up. Let me look at it this weekend.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hehe, ok -- well do you want me to merge this PR and make a new one, or would you rather keep this PR open until you write the tests?
BTW - do you see a button at the bottom entitled "squash and merge", once everything is reviewed and CI approved, please feel free to merge yourself if you do see that button.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll merge this and take up the test in another feature, possibly for #89 .