From 3786e1dcb0ce4b20c5072ec897d118a3bfeda608 Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Thu, 4 Mar 2021 13:57:41 +0000 Subject: [PATCH] [FIX] core, mail: mark HTML manipulation results as markup-safe. * The result of `plaintext2html` is fully controlled and markup-safe (the first thing we do is escape the input). * For `append_content_to_html`, we assume the inputs are HTML and the output is thus always properly HTML. Alternatively, we may want to `Markup("%s%s") % ...` and require the inputs to be properly marked? That seems like a good idea. * In `_replace_local_links`, applying re.sub will strip out the markup mark, so store it and reapply it on output if necessary. That one is a big gnarly, because if the input to ustr is markup-safe bytes (e.g. qweb rendering output) then the output is a Markup object, but if the input is str then the output is str, so we need to check before and after unless... we update ustr to check for subclasses instead of exact type? * In `_prepend_preview` the issue is similar to that of `append_content_to_html`, though in this case we should *not* trust the input, so we can flag the "parent document" as Markup and format the preview bit in. * And since we're marking mail's jinja output as safe, do the same for web and iot. Sadly there doesn't seem to be any hook for doing that at the environment level of jinja, so every `Template.render` site has to be marked. --- .../iot_handlers/drivers/DisplayDriver.py | 1 + addons/mail/models/mail_render_mixin.py | 14 +++++++++----- addons/web/controllers/main.py | 15 +++++++-------- odoo/tools/mail.py | 13 ++++++------- 4 files changed, 23 insertions(+), 20 deletions(-) diff --git a/addons/hw_drivers/iot_handlers/drivers/DisplayDriver.py b/addons/hw_drivers/iot_handlers/drivers/DisplayDriver.py index 4f110946234..4a3d917cfab 100644 --- a/addons/hw_drivers/iot_handlers/drivers/DisplayDriver.py +++ b/addons/hw_drivers/iot_handlers/drivers/DisplayDriver.py @@ -9,6 +9,7 @@ import os import subprocess import threading import time + import urllib3 from odoo import http diff --git a/addons/mail/models/mail_render_mixin.py b/addons/mail/models/mail_render_mixin.py index 68728726b11..1f4b97c5ab4 100644 --- a/addons/mail/models/mail_render_mixin.py +++ b/addons/mail/models/mail_render_mixin.py @@ -8,6 +8,7 @@ import logging import re import dateutil.relativedelta as relativedelta +from markupsafe import Markup from werkzeug import urls from odoo import _, api, fields, models, tools @@ -173,7 +174,10 @@ class MailRenderMixin(models.AbstractModel): if not html: return html + wrapper = Markup if isinstance(html, Markup) else str html = tools.ustr(html) + if isinstance(html, Markup): + wrapper = Markup def _sub_relative2absolute(match): # compute here to do it only if really necessary + cache will ensure it is done only once @@ -187,7 +191,7 @@ class MailRenderMixin(models.AbstractModel): html = re.sub(r"""(]*\shref=")(/[^/][^"]+)""", _sub_relative2absolute, html) html = re.sub(r"""(<[^>]+\bstyle="[^"]+\burl\('?)(/[^/'][^'")]+)""", _sub_relative2absolute, html) - return html + return wrapper(html) @api.model def _render_encapsulate(self, layout_xmlid, html, add_context=None, context_record=None): @@ -226,11 +230,11 @@ class MailRenderMixin(models.AbstractModel): preview = preview.strip() if preview: - html_preview = f""" + html_preview = Markup("""
- {tools.html_escape(preview)} + {}
- """ + """).format(preview) return tools.prepend_html_content(html, html_preview) return html @@ -362,7 +366,7 @@ class MailRenderMixin(models.AbstractModel): raise UserError(_("Failed to render template : %s", e)) if render_result == u"False": render_result = u"" - results[record.id] = render_result + results[record.id] = Markup(render_result) return results diff --git a/addons/web/controllers/main.py b/addons/web/controllers/main.py index b5d3f684646..e119a309b19 100644 --- a/addons/web/controllers/main.py +++ b/addons/web/controllers/main.py @@ -1,16 +1,13 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -import babel.messages.pofile import base64 import copy import datetime import functools -import glob import hashlib import io import itertools -import jinja2 import json import logging import operator @@ -18,17 +15,19 @@ import os import re import sys import tempfile +import unicodedata +from collections import OrderedDict, defaultdict +import babel.messages.pofile +import jinja2 import werkzeug import werkzeug.exceptions import werkzeug.utils import werkzeug.wrappers import werkzeug.wsgi -from collections import OrderedDict, defaultdict, Counter -from werkzeug.urls import url_encode, url_decode, iri_to_uri from lxml import etree -import unicodedata - +from markupsafe import Markup +from werkzeug.urls import url_encode, url_decode, iri_to_uri import odoo import odoo.modules.registry @@ -1063,7 +1062,7 @@ class Database(http.Controller): monodb = db_monodb() if monodb: d['databases'] = [monodb] - return env.get_template("database_manager.html").render(d) + return Markup(env.get_template("database_manager.html").render(d)) @http.route('/web/database/selector', type='http', auth="none") def selector(self, **kw): diff --git a/odoo/tools/mail.py b/odoo/tools/mail.py index 0a5e33cde99..85c18d890ec 100644 --- a/odoo/tools/mail.py +++ b/odoo/tools/mail.py @@ -373,8 +373,7 @@ def plaintext2html(text, container_tag=False): text = misc.html_escape(ustr(text)) # 1. replace \n and \r - text = text.replace('\n', '
') - text = text.replace('\r', '
') + text = re.sub(r'(\r\n|\r|\n)', '
', text) # 2. clickable links text = html_keep_url(text) @@ -382,16 +381,16 @@ def plaintext2html(text, container_tag=False): # 3-4: form paragraphs idx = 0 final = '

' - br_tags = re.compile(r'(([<]\s*[bB][rR]\s*\/?[>]\s*){2,})') + br_tags = re.compile(r'(([<]\s*[bB][rR]\s*/?[>]\s*){2,})') for item in re.finditer(br_tags, text): final += text[idx:item.start()] + '

' idx = item.end() final += text[idx:] + '

' # 5. container - if container_tag: + if container_tag: # FIXME: validate that container_tag is just a simple tag? final = '<%s>%s' % (container_tag, final, container_tag) - return ustr(final) + return markupsafe.Markup(final) def append_content_to_html(html, content, plaintext=True, preserve=False, container_tag=False): """ Append extra content at the end of an HTML snippet, trying @@ -428,8 +427,8 @@ def append_content_to_html(html, content, plaintext=True, preserve=False, contai if insert_location == -1: insert_location = html.find('') if insert_location == -1: - return '%s%s' % (html, content) - return '%s%s%s' % (html[:insert_location], content, html[insert_location:]) + return markupsafe.Markup('%s%s' % (html, content)) + return markupsafe.Markup('%s%s%s' % (html[:insert_location], content, html[insert_location:])) def prepend_html_content(html_body, html_content):