[IMP] tools: mail: parse style in sanitizer
Style attribute is now parsed and sanitized. Only a while list of accepted properties is kept. The behavior is implemented directly in the cleaner itself. As there is no easy possible inheritance, the styling cleaning is appended directly after the legacy processing in __call__. The styling sanitizer is called only if the style attribute is kept. If the sanitizer is called with strip_style=True, the styling is removed and therefore no sanitizing is performed. Tests about html fields have been updated. Indeed the styling is now sanitized and the test was not correct anymore, as the test html was stripped. It now contains styling that is kept. The strip_classes is also tested.
This commit is contained in:
@@ -116,13 +116,45 @@ class TestSanitizer(unittest.TestCase):
|
||||
sanitized,
|
||||
'html_sanitize escaped valid address-like')
|
||||
|
||||
def test_style_parsing(self):
|
||||
test_data = [
|
||||
(
|
||||
'<span style="position: fixed; top: 0px; left: 50px; width: 40%; height: 50%; background-color: red;">Coin coin </span>',
|
||||
['background-color: red', 'Coin coin'],
|
||||
['position', 'top', 'left']
|
||||
), (
|
||||
"""<div style='before: "Email Address; coincoin cheval: lapin";
|
||||
font-size: 30px; max-width: 100%; after: "Not sure
|
||||
|
||||
this; means: anything ?#ùµ"
|
||||
; some-property: 2px; top: 3'>youplaboum</div>""",
|
||||
['font-size: 30px', 'youplaboum'],
|
||||
['some-property', 'top', 'cheval']
|
||||
), (
|
||||
'<span style="width">Coincoin</span>',
|
||||
[],
|
||||
['width']
|
||||
)
|
||||
]
|
||||
|
||||
for test, in_lst, out_lst in test_data:
|
||||
new_html = html_sanitize(test, strict=False, strip_style=False, strip_classes=False)
|
||||
for text in in_lst:
|
||||
self.assertIn(text, new_html)
|
||||
for text in out_lst:
|
||||
self.assertNotIn(text, new_html)
|
||||
|
||||
# style should not be sanitized if removed
|
||||
new_html = html_sanitize(test_data[0][0], strict=False, strip_style=True, strip_classes=False)
|
||||
self.assertEqual(new_html, u'<span>Coin coin </span>')
|
||||
|
||||
def test_edi_source(self):
|
||||
html = html_sanitize(test_mail_examples.EDI_LIKE_HTML_SOURCE)
|
||||
self.assertIn('div style="font-family: \'Lucida Grande\', Ubuntu, Arial, Verdana, sans-serif; font-size: 12px; color: rgb(34, 34, 34); background-color: #FFF;', html,
|
||||
'html_sanitize removed valid style attribute')
|
||||
self.assertIn('<span style="color: #222; margin-bottom: 5px; display: block; ">', html,
|
||||
'html_sanitize removed valid style attribute')
|
||||
self.assertIn('img class="oe_edi_paypal_button" src="https://www.paypal.com/en_US/i/btn/btn_paynowCC_LG.gif"', html,
|
||||
self.assertIn(
|
||||
'font-family: \'Lucida Grande\', Ubuntu, Arial, Verdana, sans-serif;', html,
|
||||
'html_sanitize removed valid styling')
|
||||
self.assertIn(
|
||||
'src="https://www.paypal.com/en_US/i/btn/btn_paynowCC_LG.gif"', html,
|
||||
'html_sanitize removed valid img')
|
||||
self.assertNotIn('</body></html>', html, 'html_sanitize did not remove extra closing tags')
|
||||
|
||||
@@ -404,5 +436,6 @@ class TestEmailTools(unittest.TestCase):
|
||||
for text, expected in cases:
|
||||
self.assertEqual(email_split(text), expected, 'email_split is broken')
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
unittest.main()
|
||||
|
||||
@@ -138,10 +138,13 @@ class TestHtmlField(common.TransactionCase):
|
||||
% if object.some_field and not object.oriented:
|
||||
<table>
|
||||
% if object.other_field:
|
||||
<tr style="border: 10px solid black;">
|
||||
<tr style="margin: 0px; border: 10px solid black;">
|
||||
${object.mako_thing}
|
||||
<td>
|
||||
</tr>
|
||||
<tr class="custom_class">
|
||||
This is some html.
|
||||
</tr>
|
||||
% endif
|
||||
<tr>
|
||||
%if object.dummy_field:
|
||||
@@ -156,7 +159,17 @@ class TestHtmlField(common.TransactionCase):
|
||||
self.assertEqual(partner.comment, some_ugly_html, 'Error in HTML field: content was sanitized but field has sanitize=False')
|
||||
|
||||
self.partner._columns.update({
|
||||
'comment': fields.html('Unsecure Html', sanitize=True),
|
||||
'comment': fields.html('Unsecure Html', sanitize=True, strip_classes=False),
|
||||
})
|
||||
self.partner.write(cr, uid, [pid], {
|
||||
'comment': some_ugly_html,
|
||||
}, context=context)
|
||||
partner = self.partner.browse(cr, uid, pid, context=context)
|
||||
# classes are kept
|
||||
self.assertIn('<tr class="', partner.comment)
|
||||
|
||||
self.partner._columns.update({
|
||||
'comment': fields.html('Unsecure Html', sanitize=True, strip_classes=True),
|
||||
})
|
||||
self.partner.write(cr, uid, [pid], {
|
||||
'comment': some_ugly_html,
|
||||
@@ -166,6 +179,8 @@ class TestHtmlField(common.TransactionCase):
|
||||
self.assertIn('</table>', partner.comment, 'Error in HTML field: content does not seem to have been sanitized despise sanitize=True')
|
||||
self.assertIn('</td>', partner.comment, 'Error in HTML field: content does not seem to have been sanitized despise sanitize=True')
|
||||
self.assertIn('<tr style="', partner.comment, 'Style attr should not have been stripped')
|
||||
# sanitize does not keep classes if asked to
|
||||
self.assertNotIn('<tr class="', partner.comment)
|
||||
|
||||
self.partner._columns['comment'] = fields.html('Stripped Html', sanitize=True, strip_style=True)
|
||||
self.partner.write(cr, uid, [pid], {'comment': some_ugly_html}, context=context)
|
||||
|
||||
@@ -38,11 +38,46 @@ safe_attrs = clean.defs.safe_attrs | frozenset(
|
||||
|
||||
|
||||
class _Cleaner(clean.Cleaner):
|
||||
|
||||
_style_re = re.compile('''([\w-]+)\s*:\s*((?:[^;"']|"[^"]*"|'[^']*')+)''')
|
||||
|
||||
_style_whitelist = [
|
||||
'font-size', 'font-family', 'background-color', 'color', 'text-align',
|
||||
'padding', 'padding-top', 'padding-left', 'padding-bottom', 'padding-right',
|
||||
'margin', 'margin-top', 'margin-left', 'margin-bottom', 'margin-right'
|
||||
# box model
|
||||
'border', 'border-color', 'border-radius', 'height', 'margin', 'padding', 'width', 'max-width', 'min-width',
|
||||
# tables
|
||||
'border-collapse', 'border-spacing', 'caption-side', 'empty-cells', 'table-layout']
|
||||
|
||||
def __call__(self, doc):
|
||||
super(_Cleaner, self).__call__(doc)
|
||||
|
||||
# if we keep style attribute, sanitize them
|
||||
if not self.style:
|
||||
for el in doc.iter():
|
||||
self.parse_style(el)
|
||||
|
||||
def parse_style(self, el):
|
||||
attributes = el.attrib
|
||||
styling = attributes.get('style')
|
||||
if styling:
|
||||
valid_styles = {}
|
||||
styles = self._style_re.findall(styling)
|
||||
for style in styles:
|
||||
if style[0].lower() in self._style_whitelist:
|
||||
valid_styles[style[0].lower()] = style[1]
|
||||
if valid_styles:
|
||||
el.attrib['style'] = '; '.join('%s: %s' % (key, val) for (key, val) in valid_styles.iteritems())
|
||||
else:
|
||||
del el.attrib['style']
|
||||
|
||||
def allow_element(self, el):
|
||||
if el.tag == 'object' and el.get('type') == "image/svg+xml":
|
||||
return True
|
||||
return super(_Cleaner, self).allow_element(el)
|
||||
|
||||
|
||||
def html_sanitize(src, silent=True, strict=False, strip_style=False, strip_classes=False):
|
||||
if not src:
|
||||
return src
|
||||
|
||||
Reference in New Issue
Block a user