Something' + normalized_val = '
Something' + write_vals = {'comment5': val} + + record.with_user(internal_user).write(write_vals) + self.assertEqual(record.comment5, normalized_val, + "should be normalized (not in groups)") + record.with_user(bypass_user).write(write_vals) + self.assertEqual(record.comment5, val, + "should not be normalized (has group)") + record.with_user(internal_user).write(write_vals) + self.assertEqual(record.comment5, normalized_val, + "should be normalized (not in groups) despite admin previous diff") + + # 2. Test main use case: prevent restricted user to wipe non restricted # user previous change val = '' write_vals = {'comment5': val} @@ -3061,7 +3077,7 @@ class TestHtmlField(common.TransactionCase): # other user that bypassed the sanitize) record.with_user(internal_user).write(write_vals) - # 2. Make sure field compare in `_convert` is working as expected with + # 3. Make sure field compare in `_convert` is working as expected with # special content / format val = 'é@
' write_vals = {'comment5': val} @@ -3078,6 +3094,13 @@ class TestHtmlField(common.TransactionCase): # Next write shouldn't raise a sanitize right error record.with_user(internal_user).write(write_vals) + # 4. Ensure our exception handling is fine + val = '' + write_vals = {'comment5': val} + record.with_user(internal_user).write(write_vals) + self.assertEqual(record.comment5, '', + "should be sanitized (not in groups)") + class TestMagicFields(common.TransactionCase): diff --git a/odoo/fields.py b/odoo/fields.py index bc3e235a431..17bb3750cc0 100644 --- a/odoo/fields.py +++ b/odoo/fields.py @@ -27,9 +27,10 @@ from difflib import get_close_matches from hashlib import sha256 from .tools import ( - float_repr, float_round, float_compare, float_is_zero, html_sanitize, human_size, + float_repr, float_round, float_compare, float_is_zero, human_size, pg_varchar, ustr, OrderedSet, pycompat, sql, date_utils, unique, image_process, merge_sequences, SQL_ORDER_BY_TYPE, is_list_of, has_list_types, + html_normalize, html_sanitize, ) from .tools import DEFAULT_SERVER_DATE_FORMAT as DATE_FORMAT from .tools import DEFAULT_SERVER_DATETIME_FORMAT as DATETIME_FORMAT @@ -1988,14 +1989,13 @@ class Html(_String): original_value = record[self.name] if original_value: + # Note that sanitize also normalize original_value_sanitized = html_sanitize(original_value, **sanitize_vals) - - def get_parsed(val): - return etree.tostring(html.fromstring(val)) + original_value_normalized = html_normalize(original_value) if ( not original_value_sanitized # sanitizer could empty it - or get_parsed(original_value) != get_parsed(original_value_sanitized) + or original_value_normalized != original_value_sanitized ): # The field contains element(s) that would be removed if # sanitized. It means that someone who was part of a group diff --git a/odoo/tools/mail.py b/odoo/tools/mail.py index 26b43998e01..b2f80cd7e60 100644 --- a/odoo/tools/mail.py +++ b/odoo/tools/mail.py @@ -14,7 +14,7 @@ from urllib.parse import urlparse import idna import markupsafe -from lxml import etree +from lxml import etree, html from lxml.html import clean from werkzeug import urls @@ -74,10 +74,6 @@ class _Cleaner(clean.Cleaner): sanitize_style = False def __call__(self, doc): - # perform quote detection before cleaning and class removal - for el in doc.iter(tag=etree.Element): - tag_quote(el) - super(_Cleaner, self).__call__(doc) # if we keep attributes but still remove classes @@ -178,68 +174,110 @@ def tag_quote(el): el.set('data-o-mail-quote', '1') -def html_sanitize(src, silent=True, sanitize_tags=True, sanitize_attributes=False, sanitize_style=False, sanitize_form=True, strip_style=False, strip_classes=False): +def html_normalize(src, filter_callback=None): + """ Normalize `src` for storage as an html field value. + + The string is parsed as an html tag soup, made valid, then decorated for + "email quote" detection, and prepared for an optional filtering. + The filtering step (e.g. sanitization) should be performed by the + `filter_callback` function (to avoid multiple parsing operations, and + normalize the result). + + :param src: the html string to normalize + :param filter_callback: optional callable taking a single `etree._Element` + document parameter, to be called during normalization in order to + filter the output document + """ + if not src: return src + src = ustr(src, errors='replace') # html: remove encoding attribute inside tags doctype = re.compile(r'(<[^>]*\s)(encoding=(["\'][^"\']*?["\']|[^\s\n\r>]+)(\s[^>]*|/)?>)', re.IGNORECASE | re.DOTALL) src = doctype.sub(u"", src) - logger = logging.getLogger(__name__ + '.html_sanitize') - - kwargs = { - 'page_structure': True, - 'style': strip_style, # True = remove style tags/attrs - 'sanitize_style': sanitize_style, # True = sanitize styling - 'forms': sanitize_form, # True = remove form tags - 'remove_unknown_tags': False, - 'comments': False, - 'processing_instructions': False - } - if sanitize_tags: - kwargs.update(SANITIZE_TAGS) - - if sanitize_attributes: # We keep all attributes in order to keep "style" - if strip_classes: - current_safe_attrs = safe_attrs - frozenset(['class']) - else: - current_safe_attrs = safe_attrs - kwargs.update({ - 'safe_attrs_only': True, - 'safe_attrs': current_safe_attrs, - }) - else: - kwargs.update({ - 'safe_attrs_only': False, # keep oe-data attributes + style - 'strip_classes': strip_classes, # remove classes, even when keeping other attributes - }) - try: - # some corner cases make the parser crash (such as in test_mail) - cleaner = _Cleaner(**kwargs) - cleaned = cleaner.clean_html(src) - assert isinstance(cleaned, str) - # html considerations so real html content match database value - cleaned = cleaned.replace(u'\xa0', u' ') + doc = html.fromstring(src) except etree.ParserError as e: + # HTML comment only string, whitespace only.. if 'empty' in str(e): return u"" + raise + + # perform quote detection before cleaning and class removal + if doc is not None: + for el in doc.iter(tag=etree.Element): + tag_quote(el) + + if filter_callback: + doc = filter_callback(doc) + + src = html.tostring(doc, encoding='unicode') + + # this is ugly, but lxml/etree tostring want to put everything in a + # 'div' that breaks the editor -> remove that + if src.startswith('
ParserError when sanitizing
' + sanitized = 'ParserError when sanitizing
' except Exception: if not silent: raise logger.warning(u'unknown error obtained when sanitizing %r', src, exc_info=True) - cleaned = u'Unknown error when sanitizing
' + sanitized = 'Unknown error when sanitizing
' - # this is ugly, but lxml/etree tostring want to put everything in a 'div' that breaks the editor -> remove that - if cleaned.startswith(u'