From 7ff198525701da45102c2cbe9eda036f9dc8c801 Mon Sep 17 00:00:00 2001 From: Ayush Shukla Date: Fri, 19 May 2017 15:50:21 +0530 Subject: [PATCH] Email praseaddr fix, fixes frappe/issues#3004 (#3345) * email_parse address fix * travis check for email-parseaddr * email parseaddr fix * fixes * Added test case * minor change * Added few email_ids * condition change --- .../doctype/communication/communication.py | 14 ++---- frappe/core/doctype/communication/email.py | 10 ++-- .../communication/test_communication.py | 23 ++++++++- .../email/doctype/email_group/email_group.py | 4 +- frappe/email/doctype/newsletter/newsletter.py | 7 ++- frappe/email/email_body.py | 4 +- frappe/email/receive.py | 4 +- frappe/utils/__init__.py | 48 ++++++++++++++----- 8 files changed, 77 insertions(+), 37 deletions(-) diff --git a/frappe/core/doctype/communication/communication.py b/frappe/core/doctype/communication/communication.py index 855681be5d..aa9a7c3a4e 100644 --- a/frappe/core/doctype/communication/communication.py +++ b/frappe/core/doctype/communication/communication.py @@ -11,7 +11,8 @@ from frappe.core.doctype.communication.comment import (notify_mentions, from frappe.core.doctype.communication.email import (validate_email, notify, _notify, update_parent_status) from frappe.utils.bot import BotReply -from email.utils import parseaddr +from frappe.utils import parse_addr + from collections import Counter exclude_from_linked_with = True @@ -140,14 +141,9 @@ class Communication(Document): else: if self.sent_or_received=='Sent': validate_email_add(self.sender, throw=True) - - sender_name, sender_email = parseaddr(self.sender) - - if not sender_name: - sender_name = get_fullname(sender_email) - if sender_name == sender_email: - sender_name = None - + sender_name, sender_email = parse_addr(self.sender) + if sender_name == sender_email: + sender_name = None self.sender = sender_email self.sender_full_name = sender_name or get_fullname(frappe.session.user) if frappe.session.user!='Administrator' else None diff --git a/frappe/core/doctype/communication/email.py b/frappe/core/doctype/communication/email.py index 4f1c7ccabc..3af3def992 100755 --- a/frappe/core/doctype/communication/email.py +++ b/frappe/core/doctype/communication/email.py @@ -5,9 +5,9 @@ from __future__ import unicode_literals, absolute_import from six.moves import range import frappe import json -from email.utils import formataddr, parseaddr +from email.utils import formataddr from frappe.utils import (get_url, get_formatted_email, cint, - validate_email_add, split_emails, time_diff_in_seconds) + validate_email_add, split_emails, time_diff_in_seconds, parse_addr) from frappe.utils.file_manager import get_file from frappe.email.queue import check_email_limit from frappe.utils.scheduler import log @@ -321,11 +321,11 @@ def get_cc(doc, recipients=None, fetched_from_email_account=False): # exclude unfollows, recipients and unsubscribes exclude = [] #added to remove account check exclude += [d[0] for d in frappe.db.get_all("User", ["name"], {"thread_notify": 0}, as_list=True)] - exclude += [(parseaddr(email)[1] or "").lower() for email in recipients] + exclude += [(parse_addr(email)[1] or "").lower() for email in recipients] if fetched_from_email_account: # exclude sender when pulling email - exclude += [parseaddr(doc.sender)[1]] + exclude += [parse_addr(doc.sender)[1]] if doc.reference_doctype and doc.reference_name: exclude += [d[0] for d in frappe.db.get_all("Email Unsubscribe", ["email"], @@ -356,7 +356,7 @@ def filter_email_list(doc, email_list, exclude, is_cc=False): email_address_list = [] for email in list(set(email_list)): - email_address = (parseaddr(email)[1] or "").lower() + email_address = (parse_addr(email)[1] or "").lower() if not email_address: continue diff --git a/frappe/core/doctype/communication/test_communication.py b/frappe/core/doctype/communication/test_communication.py index 8b09cf6bee..653904a07b 100644 --- a/frappe/core/doctype/communication/test_communication.py +++ b/frappe/core/doctype/communication/test_communication.py @@ -4,8 +4,27 @@ from __future__ import unicode_literals import frappe import unittest - test_records = frappe.get_test_records('Communication') + class TestCommunication(unittest.TestCase): - pass + + def test_parse_addr(self): + valid_email_list = ["me@example.com", "a.nonymous@example.com", "name@tag@example.com", + "foo@example.com", 'Full Name ', + '"Full Name with quotes and " ', + 'foo@bar@google.com', 'Surname, Name ', + 'Purchase@ABC ', 'xyz@abc2.com ', + 'Name [something else] ', + '.com@test@yahoo.com'] + + invalid_email_list = ['[invalid!email]', 'invalid-email', + 'tes2', 'e', 'rrrrrrrr', 'manas','[[[sample]]]', + '[invalid!email].com'] + + for x in valid_email_list: + self.assertTrue(frappe.utils.parse_addr(x)) + + for x in invalid_email_list: + self.assertFalse(frappe.utils.parse_addr(x)[0]) + diff --git a/frappe/email/doctype/email_group/email_group.py b/frappe/email/doctype/email_group/email_group.py index 8551c2e33d..0aad49c1e6 100755 --- a/frappe/email/doctype/email_group/email_group.py +++ b/frappe/email/doctype/email_group/email_group.py @@ -7,7 +7,7 @@ import frappe from frappe import _ from frappe.utils import validate_email_add from frappe.model.document import Document -from email.utils import parseaddr +from frappe.utils import parse_addr class EmailGroup(Document): def onload(self): @@ -26,7 +26,7 @@ class EmailGroup(Document): for user in frappe.db.get_all(doctype, [email_field, unsubscribed_field or "name"]): try: - email = parseaddr(user.get(email_field))[1] + email = parse_addr(user.get(email_field))[1] if email: frappe.get_doc({ "doctype": "Email Group Member", diff --git a/frappe/email/doctype/newsletter/newsletter.py b/frappe/email/doctype/newsletter/newsletter.py index 29ec6e376d..a54ab28c8d 100755 --- a/frappe/email/doctype/newsletter/newsletter.py +++ b/frappe/email/doctype/newsletter/newsletter.py @@ -14,6 +14,7 @@ from frappe.utils.scheduler import log from frappe.email.queue import send from frappe.email.doctype.email_group.email_group import add_subscribers from frappe.utils.file_manager import get_file +from frappe.utils import parse_addr class Newsletter(Document): @@ -135,17 +136,15 @@ def return_unsubscribed_page(email, name): def create_lead(email_id): """create a lead if it does not exist""" - from email.utils import parseaddr from frappe.model.naming import get_default_naming_series - real_name, email_id = parseaddr(email_id) - + full_name, email_id = parse_addr(email_id) if frappe.db.get_value("Lead", {"email_id": email_id}): return lead = frappe.get_doc({ "doctype": "Lead", "email_id": email_id, - "lead_name": real_name or email_id, + "lead_name": full_name or email_id, "status": "Lead", "naming_series": get_default_naming_series("Lead"), "company": frappe.db.get_default("Company"), diff --git a/frappe/email/email_body.py b/frappe/email/email_body.py index a20484b8ab..43f79a3027 100755 --- a/frappe/email/email_body.py +++ b/frappe/email/email_body.py @@ -8,6 +8,7 @@ from frappe.email.smtp import get_outgoing_email_account from frappe.utils import (get_url, scrub_urls, strip, expand_relative_urls, cint, split_emails, to_markdown, markdown, encode, random_string) import email.utils +from frappe.utils import parse_addr def get_email(recipients, sender='', msg='', subject='[No Subject]', text_content = None, footer=None, print_html=None, formatted=None, attachments=None, @@ -179,8 +180,7 @@ class EMail: def replace_sender(self): if cint(self.email_account.always_use_account_email_id_as_sender): self.set_header('X-Original-From', self.sender) - - sender_name, sender_email = email.utils.parseaddr(self.sender) + sender_name, sender_email = parse_addr(self.sender) self.sender = email.utils.formataddr((sender_name or self.email_account.name, self.email_account.email_id)) def set_message_id(self, message_id, is_notification=False): diff --git a/frappe/email/receive.py b/frappe/email/receive.py index 039dd52723..33aa9c2d0b 100644 --- a/frappe/email/receive.py +++ b/frappe/email/receive.py @@ -9,7 +9,7 @@ from email.header import decode_header import frappe from frappe import _ from frappe.utils import (extract_email_id, convert_utc_to_user_timezone, now, - cint, cstr, strip, markdown) + cint, cstr, strip, markdown, parse_addr) from frappe.utils.scheduler import log from frappe.utils.file_manager import get_random_filename, save_file, MaxFileSizeReachedError import re @@ -411,7 +411,7 @@ class Email: if self.from_email: self.from_email = self.from_email.lower() - self.from_real_name = email.utils.parseaddr(_from_email)[0] if "@" in _from_email else _from_email + self.from_real_name = parse_addr(_from_email)[0] if "@" in _from_email else _from_email def decode_email(self, email): if not email: return diff --git a/frappe/utils/__init__.py b/frappe/utils/__init__.py index 6522773093..90bdd86fba 100644 --- a/frappe/utils/__init__.py +++ b/frappe/utils/__init__.py @@ -8,10 +8,9 @@ from werkzeug.test import Client import os, re, urllib, sys, json, hashlib, requests, traceback from markdown2 import markdown as _markdown from .html_utils import sanitize_html - import frappe from frappe.utils.identicon import Identicon - +from email.utils import parseaddr, formataddr # utility functions like cint, int, flt, etc. from frappe.utils.data import * @@ -62,9 +61,8 @@ def get_formatted_email(user): def extract_email_id(email): """fetch only the email part of the Email Address""" - from email.utils import parseaddr - fullname, email_id = parseaddr(email) - if isinstance(email_id, basestring) and not isinstance(email_id, unicode): + full_name, email_id = parse_addr(email) + if email_id and isinstance(email_id, basestring) and not isinstance(email_id, unicode): email_id = email_id.decode("utf-8", "ignore") return email_id @@ -88,13 +86,12 @@ def validate_email_add(email_str, throw=False): else: e = extract_email_id(e) - match = re.match("[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*@(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?", e.lower()) + match = re.match("[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*@(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?", e.lower()) if e else None if not match: _valid = False else: matched = match.group(0) - if match: match = matched==e.lower() @@ -450,18 +447,47 @@ def markdown(text, sanitize=True, linkify=True): return html def sanitize_email(emails): - from email.utils import parseaddr, formataddr - sanitized = [] for e in split_emails(emails): if not validate_email_add(e): continue - fullname, email_id = parseaddr(e) - sanitized.append(formataddr((fullname, email_id))) + full_name, email_id = parse_addr(e) + sanitized.append(formataddr((full_name, email_id))) return ", ".join(sanitized) +def parse_addr(email_string): + """ + Return email_id and user_name based on email string + Raise error if email string is not valid + """ + name, email = parseaddr(email_string) + if check_format(email): + return (name, email) + else: + email_regex = re.compile(r"([a-zA-Z0-9_.+-]+@[a-zA-Z0-9-]+\.[a-zA-Z0-9-.]+)") + email_list = re.findall(email_regex, email_string) + if len(email_list) > 0 and check_format(email_list[0]): + #take only first email address + return (name, email_list[0]) + return (None, email) + +def check_format(email_id): + """ + Check if email_id is valid. valid email:text@example.com + String check ensures that email_id contains both '.' and + '@' and index of '@' is less than '.' + """ + is_valid = False + try: + pos = email_id.rindex("@") + is_valid = pos > 0 and (email_id.rindex(".") > pos) and (len(email_id) - pos > 4) + except Exception, e: + #print(e) + pass + return is_valid + def get_installed_apps_info(): out = [] for app in frappe.get_installed_apps():