diff --git a/addons/link_tracker/models/mail_render_mixin.py b/addons/link_tracker/models/mail_render_mixin.py index f1c4491465b..2c7fa9c5e09 100644 --- a/addons/link_tracker/models/mail_render_mixin.py +++ b/addons/link_tracker/models/mail_render_mixin.py @@ -35,24 +35,29 @@ class MailRenderMixin(models.AbstractModel): """ base_url = base_url or self.env['ir.config_parameter'].sudo().get_param('web.base.url') short_schema = base_url + '/r/' - for match in re.findall(tools.HTML_TAG_URL_REGEX, html): - href = markupsafe.Markup(match[0]) + for match in set(re.findall(tools.HTML_TAG_URL_REGEX, html)): long_url = match[1] + # Don't shorten already-shortened links + if long_url.startswith(short_schema): + continue + # Don't shorten urls present in blacklist (aka to skip list) + if blacklist and any(s in long_url for s in blacklist): + continue label = (match[3] or '').strip() - if not blacklist or not [s for s in blacklist if s in long_url] and not long_url.startswith(short_schema): - create_vals = dict(link_tracker_vals, url=utils.unescape(long_url), label=utils.unescape(label)) - link = self.env['link.tracker'].search_or_create(create_vals) - if link.short_url: - new_href = href.replace(long_url, link.short_url) - html = html.replace(href, new_href) + create_vals = dict(link_tracker_vals, url=utils.unescape(long_url), label=utils.unescape(label)) + link = self.env['link.tracker'].search_or_create(create_vals) + if link.short_url: + # `str` manipulation required to support replacing "&" characters, common in urls + new_href = match[0].replace(long_url, link.short_url) + html = html.replace(markupsafe.Markup(match[0]), markupsafe.Markup(new_href)) return html @api.model def _shorten_links_text(self, content, link_tracker_vals, blacklist=None, base_url=None): """ Shorten links in a string content. Works like ``_shorten_links`` but - targetting string content, not html. + targeting string content, not html. :return: updated content """ @@ -61,7 +66,7 @@ class MailRenderMixin(models.AbstractModel): base_url = base_url or self.env['ir.config_parameter'].sudo().get_param('web.base.url') shortened_schema = base_url + '/r/' unsubscribe_schema = base_url + '/sms/' - for original_url in re.findall(tools.TEXT_URL_REGEX, content): + for original_url in set(re.findall(tools.TEXT_URL_REGEX, content)): # don't shorten already-shortened links or links towards unsubscribe page if original_url.startswith(shortened_schema) or original_url.startswith(unsubscribe_schema): continue @@ -70,9 +75,10 @@ class MailRenderMixin(models.AbstractModel): if blacklist and any(item in parsed.path for item in blacklist): continue - create_vals = dict(link_tracker_vals, url= utils.unescape(original_url)) + create_vals = dict(link_tracker_vals, url=utils.unescape(original_url)) link = self.env['link.tracker'].search_or_create(create_vals) if link.short_url: - content = content.replace(original_url, link.short_url, 1) + # Ensures we only replace the same link and not a subpart of a longer one, multiple times if applicable + content = re.sub(re.escape(original_url) + r'(?![\w@:%.+&~#=/-])', link.short_url, content) return content diff --git a/addons/link_tracker/tests/test_mail_render_mixin.py b/addons/link_tracker/tests/test_mail_render_mixin.py index 23a74b5aae0..d1295fca2f8 100644 --- a/addons/link_tracker/tests/test_mail_render_mixin.py +++ b/addons/link_tracker/tests/test_mail_render_mixin.py @@ -1,10 +1,19 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. +import re -from odoo.tests import common +from odoo.tests import common, tagged +from odoo.tools import TEXT_URL_REGEX +@tagged('-at_install', 'post_install') class TestMailRenderMixin(common.TransactionCase): + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.base_url = cls.env["mail.render.mixin"].get_base_url() + def test_shorten_links(self): test_links = [ 'test_label', @@ -50,3 +59,93 @@ class TestMailRenderMixin(common.TransactionCase): for tracker_to_fail in trackers_to_fail: self.assertFalse(self.env["link.tracker"].search(tracker_to_fail)) + + def test_shorten_links_html_skip_shorts(self): + old_content = self.env["mail.render.mixin"]._shorten_links( + 'This is a link: old', {}) + created_short_url_match = re.search(TEXT_URL_REGEX, old_content) + self.assertIsNotNone(created_short_url_match) + created_short_url = created_short_url_match[0] + self.assertRegex(created_short_url, "{base_url}/r/[\\w]+".format(base_url=self.base_url)) + + new_content = self.env["mail.render.mixin"]._shorten_links( + 'Reusing this old link with a new one' + .format(old_short_url=created_short_url), + {} + ) + expected = re.compile( + 'Reusing this old link with a new one' + .format(old_short_url=created_short_url, base_url=self.base_url) + ) + self.assertRegex(new_content, expected) + + def test_shorten_links_html_including_base_url(self): + content = ( + 'This is a link: https://www.worldcommunitygrid.org
\n' + 'This is another: {base_url}
\n' + 'And a third: Here\n' + 'And a forth: Here\n' + 'And a fifth: Here too\n' + 'And a last, more complex: There!' + .format(base_url=self.base_url) + ) + expected_pattern = re.compile( + 'This is a link: https://www.worldcommunitygrid.org
\n' + 'This is another: {base_url}
\n' + 'And a third: Here\n' + 'And a forth: Here\n' + 'And a fifth: Here too\n' + 'And a last, more complex: There!' + .format(base_url=self.base_url) + ) + new_content = self.env["mail.render.mixin"]._shorten_links(content, {}) + + self.assertRegex(new_content, expected_pattern) + matches = expected_pattern.search(new_content).groups() + # 3rd and 4th lead to the same short_url + self.assertEqual(matches[0], matches[1]) + # 5th has different label but should currently lead to the same link + self.assertEqual(matches[1], matches[2]) + + def test_shorten_links_text_including_base_url(self): + content = ( + 'This is a link: https://www.worldcommunitygrid.org\n' + 'This is another: {base_url}/web#debug=1&more=2\n' + 'A third: {base_url}\n' + 'A forth: {base_url}\n' + 'And a last, with question mark: https://boinc.berkeley.edu/forum_thread.php?id=14544&postid=106833' + .format(base_url=self.base_url) + ) + expected_pattern = re.compile( + 'This is a link: {base_url}/r/[\\w]+\n' + 'This is another: {base_url}/r/[\\w]+\n' + 'A third: {base_url}/r/([\\w]+)\n' + 'A forth: {base_url}/r/([\\w]+)\n' + 'And a last, with question mark: {base_url}/r/([\\w]+)' + .format(base_url=self.base_url) + ) + new_content = self.env["mail.render.mixin"]._shorten_links_text(content, {}) + + self.assertRegex(new_content, expected_pattern) + matches = expected_pattern.search(new_content).groups() + # 3rd and 4th lead to the same short_url + self.assertEqual(matches[0], matches[1]) + + def test_shorten_links_text_skip_shorts(self): + old_content = self.env["mail.render.mixin"]._shorten_links_text( + 'This is a link: https://test_542152qsdqsd.com', {}) + created_short_url_match = re.search(TEXT_URL_REGEX, old_content) + self.assertIsNotNone(created_short_url_match) + created_short_url = created_short_url_match[0] + self.assertRegex(created_short_url, "{base_url}/r/[\\w]+".format(base_url=self.base_url)) + + new_content = self.env["mail.render.mixin"]._shorten_links_text( + 'Reusing this old link {old_short_url} with a new one, https://odoo.com' + .format(old_short_url=created_short_url), + {} + ) + expected = re.compile( + 'Reusing this old link {old_short_url} with a new one, {base_url}/r/[\\w]+' + .format(old_short_url=created_short_url, base_url=self.base_url) + ) + self.assertRegex(new_content, expected) diff --git a/addons/mass_mailing/models/mail_mail.py b/addons/mass_mailing/models/mail_mail.py index f9446867c70..71b4c2b6a8d 100644 --- a/addons/mass_mailing/models/mail_mail.py +++ b/addons/mass_mailing/models/mail_mail.py @@ -36,7 +36,7 @@ class MailMail(models.Model): body = super(MailMail, self)._send_prepare_body() if self.mailing_id and body and self.mailing_trace_ids: - for match in re.findall(tools.URL_REGEX, self.body_html): + for match in set(re.findall(tools.URL_REGEX, self.body_html)): href = match[0] url = match[1] diff --git a/addons/mass_mailing_sms/models/sms_sms.py b/addons/mass_mailing_sms/models/sms_sms.py index 298801f3c0e..31802af24ba 100644 --- a/addons/mass_mailing_sms/models/sms_sms.py +++ b/addons/mass_mailing_sms/models/sms_sms.py @@ -22,9 +22,9 @@ class SmsSms(models.Model): continue body = sms.body - for url in re.findall(tools.TEXT_URL_REGEX, body): + for url in set(re.findall(tools.TEXT_URL_REGEX, body)): if url.startswith(sms.get_base_url() + '/r/'): - body = body.replace(url, url + '/s/%s' % sms.id) + body = re.sub(re.escape(url) + r'(?![\w@:%.+&~#=/-])', url + f'/s/{sms.id}', body) res[sms.id] = body return res diff --git a/addons/test_mail_full/tests/test_sms_sms.py b/addons/test_mail_full/tests/test_sms_sms.py index a467c75678f..8ad82017b13 100644 --- a/addons/test_mail_full/tests/test_sms_sms.py +++ b/addons/test_mail_full/tests/test_sms_sms.py @@ -99,12 +99,21 @@ class TestSMSPost(TestMailFullCommon, MockLinkTracker): 'number': '13', 'mailing_id': mailing.id, }) + sms_4 = self.env['sms.sms'].create({ + 'body': 'Welcome to https://test.odoo.com/r/RAOUL\nAnd again,\n' + 'https://test.odoo.com/r/RAOUL', + 'number': '14', + 'mailing_id': mailing.id, + }) - res = (sms_0 | sms_1 | sms_2 | sms_3)._update_body_short_links() + res = (sms_0 | sms_1 | sms_2 | sms_3 | sms_4)._update_body_short_links() self.assertEqual(res[sms_0.id], 'Welcome to https://test.odoo.com') self.assertEqual(res[sms_1.id], 'Welcome to https://test.odoo.com/r/RAOUL') self.assertEqual(res[sms_2.id], 'Welcome to https://test.odoo.com/r/RAOUL/s/%s' % sms_2.id) self.assertEqual(res[sms_3.id], 'Welcome to https://test.odoo.com/leodagan/r/RAOUL') + self.assertEqual( + res[sms_4.id], + f'Welcome to https://test.odoo.com/r/RAOUL/s/{sms_4.id}\nAnd again,\nhttps://test.odoo.com/r/RAOUL/s/{sms_4.id}') def test_sms_send_batch_size(self): self.count = 0 diff --git a/odoo/tools/mail.py b/odoo/tools/mail.py index 2d320d4125f..15b94d199c7 100644 --- a/odoo/tools/mail.py +++ b/odoo/tools/mail.py @@ -257,7 +257,7 @@ def html_sanitize(src, silent=True, sanitize_tags=True, sanitize_attributes=Fals # ---------------------------------------------------------- URL_REGEX = r'(\bhref=[\'"](?!mailto:|tel:|sms:)([^\'"]+)[\'"])' -TEXT_URL_REGEX = r'https?://[a-zA-Z0-9@:%._\+~#=/-]+(?:\?\S+)?' +TEXT_URL_REGEX = r'https?://[\w@:%.+&~#=/-]+(?:\?\S+)?' # retrieve inner content of the link HTML_TAG_URL_REGEX = URL_REGEX + r'([^<>]*>([^<>]+)<\/)?'