From 2f82b95dcaf8e9773d47c369e9218b9f778642e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Mon, 1 Oct 2018 14:39:35 +0200 Subject: [PATCH] [FIX] mail: insert only sanitized and lower-case emails in blacklist Purpose of this commit is to standardize content of blacklist records and ensure only real email addresses are stored. We introduce a sanitize method to extract email address from string (like Raoul Grosbedon -> raoul@example.com) and return a lower case version of it. Purpose is to gradually get rid of lower() that appear a lot in the code and speedup computation by being sure everything is always lower case. Moreover indexing on blacklist model will be more performant is using directly email field content instead of having to lowerize it. This commit is related to task ID 33224 (original blacklist implementation done for v12) and its PR #25966 as well as task ID 1889703 (tests and fixes) and its PR #27330. Done with collaboration of @dbeguin. --- addons/mail/models/mail_blacklist.py | 39 ++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/addons/mail/models/mail_blacklist.py b/addons/mail/models/mail_blacklist.py index 1b3ccb69775..b329d0bbb11 100644 --- a/addons/mail/models/mail_blacklist.py +++ b/addons/mail/models/mail_blacklist.py @@ -1,9 +1,13 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -from odoo import api, fields, models, _ +import logging + +from odoo import api, fields, models, tools, _ from odoo.exceptions import UserError +_logger = logging.getLogger(__name__) + class MailBlackList(models.Model): """ Model of blacklisted email addresses to stop sending emails.""" @@ -22,18 +26,34 @@ class MailBlackList(models.Model): @api.model_create_multi def create(self, values): + # First of all, extract values to ensure emails are really unique (and don't modify values in place) + new_values = [] + for value in values: + email = self._sanitize_email(value.get('email')) + if not email: + _logger.warning('Blacklist: invalid email value %s' % value['email']) + continue + new_value = dict(value, email=email) + new_values.append(new_value) + """ To avoid crash during import due to unique email, return the existing records if any """ - sql = '''SELECT LOWER(email), id FROM mail_blacklist WHERE LOWER(email) = ANY(%s)''' - emails = [(v['email'] or '').lower() for v in values] + sql = '''SELECT email, id FROM mail_blacklist WHERE email = ANY(%s)''' + emails = [v['email'] for v in new_values] self._cr.execute(sql, (emails,)) bl_entries = dict(self._cr.fetchall()) - to_create = [v for v in values - if (v['email'] or '').lower() not in bl_entries] + to_create = [v for v in new_values + if v['email'] not in bl_entries] # TODO DBE Fixme : reorder ids according to incoming ids. results = super(MailBlackList, self).create(to_create) return self.env['mail.blacklist'].browse(bl_entries.values()) | results + @api.multi + def write(self, values): + if 'email' in values: + values['email'] = self._sanitize_email(values['email']) + return super(MailBlackList, self).write(values) + def _add(self, email): record = self.env["mail.blacklist"].with_context(active_test=False).search([('email', '=', email)]) if len(record) > 0: @@ -50,6 +70,15 @@ class MailBlackList(models.Model): record = record.create({'email': email, 'active': False}) return record + def _sanitize_email(self, email): + """ Sanitize and standardize blacklist entries: all emails should be + only real email extracted from strings (A -> a@a) and should be + lower case. """ + emails = tools.email_split(email) + if not emails or len(emails) != 1: + return False + return emails[0].lower() + class MailBlackListMixin(models.AbstractModel): """ Mixin that is inherited by all model with opt out.