From baafa119404299ebcc97aef8e97d5510dad8ae73 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 15 2016 15:07:58 +0000 Subject: [PATCH 1/10] Encode the from email to UTF-8 This ensure email having non-ascii characters are properly encoded and displayed in the email clients. Fixes https://pagure.io/pagure/issue/1496 --- diff --git a/pagure/lib/notify.py b/pagure/lib/notify.py index 49d1290..45bb3d2 100644 --- a/pagure/lib/notify.py +++ b/pagure/lib/notify.py @@ -197,6 +197,7 @@ def send_email(text, subject, to_mail, 'FROM_EMAIL', 'pagure@fedoraproject.org') if user_from: from_email = '%s <%s>' % (user_from, from_email) + from_email = from_email.encode('utf-8') if not pagure.APP.config.get('EMAIL_SEND', True): print '******EMAIL******' From 65d42e3577e0e8a0483e645ecd4eaa5d8873e0fc Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 15 2016 15:07:58 +0000 Subject: [PATCH 2/10] Rely on email.header to do the encoding of the email for us This avoids letting us to the encoding and takes care of it properly --- diff --git a/pagure/lib/notify.py b/pagure/lib/notify.py index 45bb3d2..2166b99 100644 --- a/pagure/lib/notify.py +++ b/pagure/lib/notify.py @@ -25,6 +25,7 @@ import warnings import flask import pagure +from email.header import Header from email.mime.text import MIMEText @@ -196,8 +197,8 @@ def send_email(text, subject, to_mail, from_email = pagure.APP.config.get( 'FROM_EMAIL', 'pagure@fedoraproject.org') if user_from: - from_email = '%s <%s>' % (user_from, from_email) - from_email = from_email.encode('utf-8') + header = Header(user_from, 'utf-8') + from_email = '%s <%s>' % (header.encode(), from_email) if not pagure.APP.config.get('EMAIL_SEND', True): print '******EMAIL******' From 9add6e06cd7843776cb9461fc746381ffb3b1d5b Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 15 2016 15:07:58 +0000 Subject: [PATCH 3/10] Convert to bytes if mailto or mail_id are unicode This is also in order to support user's email address having non-ascii characters --- diff --git a/pagure/lib/notify.py b/pagure/lib/notify.py index 2166b99..f23402f 100644 --- a/pagure/lib/notify.py +++ b/pagure/lib/notify.py @@ -244,6 +244,10 @@ def send_email(text, subject, to_mail, # envelope header. msg['To'] = mailto salt = pagure.APP.config.get('SALT_EMAIL') + if isinstance(mailto, unicode): + mailto = mailto.encode('utf-8') + if isinstance(mail_id, unicode): + mail_id = mail_id.encode('utf-8') mhash = hashlib.sha512('<%s>%s%s' % (mail_id, salt, mailto)) msg['Reply-To'] = 'reply+%s@%s' % ( mhash.hexdigest(), From d07721de12ca7ced94f246c1fc3a1657c034b670 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 15 2016 15:07:58 +0000 Subject: [PATCH 4/10] Improve printing the email sent to the logs for debugging purposes --- diff --git a/pagure/lib/notify.py b/pagure/lib/notify.py index f23402f..b15bed9 100644 --- a/pagure/lib/notify.py +++ b/pagure/lib/notify.py @@ -8,6 +8,7 @@ pagure notifications. """ +from __future__ import print_function # pylint: disable=too-many-branches # pylint: disable=too-many-arguments @@ -200,18 +201,6 @@ def send_email(text, subject, to_mail, header = Header(user_from, 'utf-8') from_email = '%s <%s>' % (header.encode(), from_email) - if not pagure.APP.config.get('EMAIL_SEND', True): - print '******EMAIL******' - print 'From: %s' % from_email - print 'To: %s' % to_mail - print 'Subject: %s' % subject - print 'in_reply_to: %s' % in_reply_to - print 'mail_id: %s' % mail_id - print 'Contents:' - print text.encode('utf-8') - print '*****/EMAIL******' - return - if project_name is not None: subject_tag = project_name else: @@ -253,6 +242,19 @@ def send_email(text, subject, to_mail, mhash.hexdigest(), pagure.APP.config['DOMAIN_EMAIL_NOTIFICATIONS']) msg['Mail-Followup-To'] = msg['Reply-To'] + if not pagure.APP.config.get('EMAIL_SEND', True): + print('******EMAIL******') + print('From: %s' % from_email) + print('To: %s' % to_mail) + print('Subject: %s' % subject) + print('in_reply_to: %s' % in_reply_to) + print('mail_id: %s' % mail_id) + print('Contents:') + print(text.encode('utf-8')) + print('*****************') + print(msg.as_string()) + print('*****/EMAIL******') + continue try: if pagure.APP.config['SMTP_USERNAME'] \ and pagure.APP.config['SMTP_PASSWORD']: From 4c524454b1781285e230afc52354fba683a821ba Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 15 2016 15:07:58 +0000 Subject: [PATCH 5/10] Add unit-tests for pagure.lib.notify.send_email --- diff --git a/tests/test_pagure_lib_notify.py b/tests/test_pagure_lib_notify.py index 16316c9..5a692b6 100644 --- a/tests/test_pagure_lib_notify.py +++ b/tests/test_pagure_lib_notify.py @@ -176,6 +176,35 @@ class PagureLibNotifytests(tests.Modeltests): out = pagure.lib.notify._get_emails_for_obj(req) self.assertEqual(out, exp) + def test_send_email(self): + """ Test the notify_new_comment method from pagure.lib.notify. """ + email = pagure.lib.notify.send_email( + 'Email content', + 'Email Subject', + 'foo@bar.com,zöé@foo.net', + mail_id='test-pull-request-2edbf96ebe644f4bb31b94605e-1@pagure', + in_reply_to='test-pull-request-2edbf96ebe644f4bb31b94605e@pagure', + project_name='namespace/project', + user_from='Zöé', + ) + exp = '''Content-Type: text/plain; charset="utf-8" +MIME-Version: 1.0 +Content-Transfer-Encoding: base64 +Subject: [namespace/project] Email Subject +From: =?utf-8?b?WsO2w6k=?= +mail-id: test-pull-request-2edbf96ebe644f4bb31b94605e-1@pagure +Message-Id: +In-Reply-To: +X-pagure: https://pagure.org/ +X-pagure-project: namespace/project +To: zöé@foo.net +Reply-To: reply+42f5809bca16d73f59180bdcc76c981e939b5eab5c02930d7d7dd38f45118b89e9ceb877e94e7f22376fbf35aab1d0e8e83dfb074ee82640cc82da12ea8019ca@pagure.org +Mail-Followup-To: reply+42f5809bca16d73f59180bdcc76c981e939b5eab5c02930d7d7dd38f45118b89e9ceb877e94e7f22376fbf35aab1d0e8e83dfb074ee82640cc82da12ea8019ca@pagure.org + +RW1haWwgY29udGVudA== +''' + self.assertEqual(email.as_string(), exp) + if __name__ == '__main__': SUITE = unittest.TestLoader().loadTestsFromTestCase(PagureLibNotifytests) From 4fe3bd1acb6f7f881dc89fa7bbf8d37f5092de6f Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 15 2016 15:07:58 +0000 Subject: [PATCH 6/10] Drop the call to .encode() that is indeed not needed Thanks @jcline for pointing it out --- diff --git a/pagure/lib/notify.py b/pagure/lib/notify.py index b15bed9..b29871a 100644 --- a/pagure/lib/notify.py +++ b/pagure/lib/notify.py @@ -199,7 +199,7 @@ def send_email(text, subject, to_mail, 'FROM_EMAIL', 'pagure@fedoraproject.org') if user_from: header = Header(user_from, 'utf-8') - from_email = '%s <%s>' % (header.encode(), from_email) + from_email = '%s <%s>' % (header, from_email) if project_name is not None: subject_tag = project_name From d940a8a2e01401ebf6a41e8719fcc15f2a3391a2 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 15 2016 15:07:58 +0000 Subject: [PATCH 7/10] Also encode the subject lines to display properly non-ascii ticket/PR titles --- diff --git a/pagure/lib/notify.py b/pagure/lib/notify.py index b29871a..93b8328 100644 --- a/pagure/lib/notify.py +++ b/pagure/lib/notify.py @@ -215,7 +215,8 @@ def send_email(text, subject, to_mail, for mailto in to_mail.split(','): msg = MIMEText(text.encode('utf-8'), 'plain', 'utf-8') - msg['Subject'] = '[%s] %s' % (subject_tag, subject) + msg['Subject'] = header = Header( + '[%s] %s' % (subject_tag, subject), 'utf-8') msg['From'] = from_email if mail_id: From 773eb0ccbbbb1ec19cb93793ae11befeb3c06c7e Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 15 2016 15:07:58 +0000 Subject: [PATCH 8/10] Expand the test for send_email to include a non-ascii subject --- diff --git a/tests/test_pagure_lib_notify.py b/tests/test_pagure_lib_notify.py index 5a692b6..124a311 100644 --- a/tests/test_pagure_lib_notify.py +++ b/tests/test_pagure_lib_notify.py @@ -180,7 +180,7 @@ class PagureLibNotifytests(tests.Modeltests): """ Test the notify_new_comment method from pagure.lib.notify. """ email = pagure.lib.notify.send_email( 'Email content', - 'Email Subject', + 'Email “Subject“', 'foo@bar.com,zöé@foo.net', mail_id='test-pull-request-2edbf96ebe644f4bb31b94605e-1@pagure', in_reply_to='test-pull-request-2edbf96ebe644f4bb31b94605e@pagure', @@ -190,7 +190,7 @@ class PagureLibNotifytests(tests.Modeltests): exp = '''Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 -Subject: [namespace/project] Email Subject +Subject: =?utf-8?b?W25hbWVzcGFjZS9wcm9qZWN0XSBFbWFpbCDigJxTdWJqZWN04oCc?= From: =?utf-8?b?WsO2w6k=?= mail-id: test-pull-request-2edbf96ebe644f4bb31b94605e-1@pagure Message-Id: From 186cd421eec8d6a0fb11e5741a436eebcfb650ed Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 15 2016 15:07:58 +0000 Subject: [PATCH 9/10] Ensure we work if the mailto given is unicode, just encode it to utf-8 is so --- diff --git a/pagure/lib/notify.py b/pagure/lib/notify.py index 93b8328..c7a8a92 100644 --- a/pagure/lib/notify.py +++ b/pagure/lib/notify.py @@ -232,6 +232,8 @@ def send_email(text, subject, to_mail, # Send the message via our own SMTP server, but don't include the # envelope header. + if isinstance(mailto, unicode): + mailto = mailto.encode('utf-8') msg['To'] = mailto salt = pagure.APP.config.get('SALT_EMAIL') if isinstance(mailto, unicode): diff --git a/tests/test_pagure_lib_notify.py b/tests/test_pagure_lib_notify.py index 124a311..855ac01 100644 --- a/tests/test_pagure_lib_notify.py +++ b/tests/test_pagure_lib_notify.py @@ -205,6 +205,17 @@ RW1haWwgY29udGVudA== ''' self.assertEqual(email.as_string(), exp) + email = pagure.lib.notify.send_email( + 'Email content', + 'Email “Subject“', + u'foo@bar.com,zöé@foo.net', + mail_id='test-pull-request-2edbf96ebe644f4bb31b94605e-1@pagure', + in_reply_to='test-pull-request-2edbf96ebe644f4bb31b94605e@pagure', + project_name='namespace/project', + user_from='Zöé', + ) + self.assertEqual(email.as_string(), exp) + if __name__ == '__main__': SUITE = unittest.TestLoader().loadTestsFromTestCase(PagureLibNotifytests) From 87aa1fd31fe816cdc64a524ccde496356d623ed7 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 15 2016 15:07:58 +0000 Subject: [PATCH 10/10] Drop duplicated line --- diff --git a/pagure/lib/notify.py b/pagure/lib/notify.py index c7a8a92..2683f58 100644 --- a/pagure/lib/notify.py +++ b/pagure/lib/notify.py @@ -236,8 +236,6 @@ def send_email(text, subject, to_mail, mailto = mailto.encode('utf-8') msg['To'] = mailto salt = pagure.APP.config.get('SALT_EMAIL') - if isinstance(mailto, unicode): - mailto = mailto.encode('utf-8') if isinstance(mail_id, unicode): mail_id = mail_id.encode('utf-8') mhash = hashlib.sha512('<%s>%s%s' % (mail_id, salt, mailto))