This is an automated email from the ASF dual-hosted git repository. brondsem pushed a commit to branch db/8533 in repository https://gitbox.apache.org/repos/asf/allura.git
commit bf020d137405070eb9bd0d2413d7f0cda05b8a98 Author: Dave Brondsema <dbronds...@slashdotmedia.com> AuthorDate: Thu Jan 4 14:37:16 2024 -0500 [#8533] switch to RFC-compliant email formatting which will wrap long unbreaking subjects correctly --- Allura/allura/lib/mail_util.py | 33 ++++++++++++---------- Allura/allura/tests/test_mail_util.py | 21 ++++++++------ Allura/allura/tests/test_tasks.py | 27 +++++++++--------- .../forgediscussion/tests/functional/test_forum.py | 22 +++++++++------ .../forgetracker/tests/functional/test_root.py | 3 +- 5 files changed, 59 insertions(+), 47 deletions(-) diff --git a/Allura/allura/lib/mail_util.py b/Allura/allura/lib/mail_util.py index fb8459ec9..5e7907d65 100644 --- a/Allura/allura/lib/mail_util.py +++ b/Allura/allura/lib/mail_util.py @@ -48,25 +48,28 @@ EMAIL_VALIDATOR = fev.Email(not_empty=True) # http://www.jebriggs.com/blog/2010/07/smtp-maximum-line-lengths/ MAX_MAIL_LINE_OCTETS = 990 +email_policy = email.policy.SMTP + email.policy.strict -def Header(text, *more_text): - '''Helper to make sure we encode headers properly''' +def Header(text, *more_text) -> str: + ''' + Helper to make sure we encode headers properly + This used to return an email.header.Header instance + But needs to be a plain str now that we're using email.policy.SMTP + ''' if isinstance(text, header.Header): - return text - # email.header.Header handles str vs unicode differently - # see - # http://docs.python.org/library/email.header.html#email.header.Header.append + return str(text) + if not isinstance(text, str): raise TypeError('This must be unicode: %r' % text) - head = header.Header(text) + + hdr_text = text for m in more_text: if not isinstance(m, str): raise TypeError('This must be unicode: %r' % text) - head.append(m) - return head - + hdr_text += ' ' + m + return hdr_text -def AddrHeader(fromaddr): +def AddrHeader(fromaddr) -> str: '''Accepts any of: Header() instance f...@bar.com @@ -201,7 +204,7 @@ def encode_email_part(content, content_type): # switch to Quoted-Printable encoding to avoid too-long lines # we could always Quoted-Printabl, but it makes the output a little messier and less human-readable # the particular order of all these operations seems to be very important for everything to end up right - msg = MIMEText('', content_type) + msg = MIMEText('', content_type, policy=email_policy) msg.replace_header('content-transfer-encoding', 'quoted-printable') cs = email.charset.Charset('utf-8') cs.header_encoding = email.charset.QP @@ -210,13 +213,13 @@ def encode_email_part(content, content_type): msg.set_payload(payload, 'utf-8') return msg else: - return MIMEText(encoded_content, content_type, encoding) + return MIMEText(encoded_content, content_type, encoding, policy=email_policy) def make_multipart_message(*parts): - msg = MIMEMultipart('related') + msg = MIMEMultipart('related', policy=email_policy) msg.preamble = 'This is a multi-part message in MIME format.' - alt = MIMEMultipart('alternative') + alt = MIMEMultipart('alternative', policy=email_policy) msg.attach(alt) for part in parts: alt.attach(part) diff --git a/Allura/allura/tests/test_mail_util.py b/Allura/allura/tests/test_mail_util.py index 76fd42965..50b5b208b 100644 --- a/Allura/allura/tests/test_mail_util.py +++ b/Allura/allura/tests/test_mail_util.py @@ -35,6 +35,7 @@ from allura.lib.mail_util import ( is_autoreply, identify_sender, _parse_message_id, + email_policy, ) from allura.lib.exceptions import AddressException from allura.tests import decorators as td @@ -91,7 +92,8 @@ class TestReactor(unittest.TestCase): Толпой со всех концов земли К богатым пристаням стремятся;'''.encode(charset), 'plain', - charset) + charset, + policy=email_policy) msg1['Message-ID'] = '<f...@bar.com>' s_msg = msg1.as_string() msg2 = parse_message(s_msg) @@ -189,15 +191,17 @@ Content-Type: text/html; charset="utf-8" Толпой со всех концов земли К богатым пристаням стремятся;'''.encode(charset), 'plain', - charset) + charset, + policy=email_policy) p2 = MIMEText('''<p>По оживлённым берегам Громады стройные теснятся Дворцов и башен; корабли Толпой со всех концов земли К богатым пристаням стремятся;</p>'''.encode(charset), 'plain', - charset) - msg1 = MIMEMultipart() + charset, + policy=email_policy) + msg1 = MIMEMultipart(policy=email_policy) msg1['Message-ID'] = '<f...@bar.com>' msg1.attach(p1) msg1.attach(p2) @@ -214,20 +218,19 @@ class TestHeader: def test_bytestring(self): with pytest.raises(TypeError): our_header = Header(b'[asdf2:wiki] Discussion for Home page') - assert our_header.encode() == '[asdf2:wiki] Discussion for Home page' + assert our_header == '[asdf2:wiki] Discussion for Home page' def test_ascii(self): our_header = Header('[asdf2:wiki] Discussion for Home page') - assert our_header.encode() == '[asdf2:wiki] Discussion for Home page' + assert our_header == '[asdf2:wiki] Discussion for Home page' def test_utf8(self): our_header = Header('теснятся') - assert our_header.encode() == '=?utf-8?b?0YLQtdGB0L3Rj9GC0YHRjw==?=' + assert our_header == 'теснятся' def test_name_addr(self): our_header = Header('"теснятся"', '<d...@b.com>') - assert (our_header.encode() == - '=?utf-8?b?ItGC0LXRgdC90Y/RgtGB0Y8i?= <d...@b.com>') + assert our_header == '"теснятся" <d...@b.com>' class TestIsAutoreply: diff --git a/Allura/allura/tests/test_tasks.py b/Allura/allura/tests/test_tasks.py index 10baaceca..9e00970b6 100644 --- a/Allura/allura/tests/test_tasks.py +++ b/Allura/allura/tests/test_tasks.py @@ -38,7 +38,7 @@ from alluratest.controller import setup_basic_test, setup_global_objects, TestCo from allura import model as M from allura.command.taskd import TaskdCommand from allura.lib import helpers as h -from allura.lib.mail_util import MAX_MAIL_LINE_OCTETS +from allura.lib.mail_util import MAX_MAIL_LINE_OCTETS, email_policy from allura.tasks import event_tasks from allura.tasks import index_tasks from allura.tasks import mail_tasks @@ -265,7 +265,7 @@ class TestMailTasks(unittest.TestCase): message_id=h.gen_message_id()) assert _client.sendmail.call_count == 1 return_path, rcpts, body = _client.sendmail.call_args[0] - body = body.split('\n') + body = body.split(email_policy.linesep) assert rcpts == [c.user.get_pref('email_address')] assert 'Reply-To: %s' % g.noreply in body @@ -288,14 +288,13 @@ class TestMailTasks(unittest.TestCase): message_id=h.gen_message_id()) assert _client.sendmail.call_count == 1 return_path, rcpts, body = _client.sendmail.call_args[0] - body = body.split('\n') + body = body.split(email_policy.linesep) assert rcpts == ['b...@blah.com'] assert 'Reply-To: %s' % g.noreply in body # The address portion must not be encoded, only the name portion can be. - # Also py2 and py3 vary in handling of double-quote separators when the name portion is encoded - unquoted_cyrillic_No = '=?utf-8?b?0J/Qvg==?=' # По + unquoted_cyrillic_No = '=?utf-8?q?=D0=9F=D0=BE?=' # По quoted_cyrillic_No = '=?utf-8?b?ItCf0L4i?=' # "По" assert (f'From: {quoted_cyrillic_No} <f...@bar.com>' in body or f'From: {unquoted_cyrillic_No} <f...@bar.com>' in body), body @@ -321,7 +320,7 @@ class TestMailTasks(unittest.TestCase): message_id=h.gen_message_id()) assert _client.sendmail.call_count == 1 return_path, rcpts, body = _client.sendmail.call_args[0] - body = body.split('\n') + body = body.split(email_policy.linesep) assert 'From: %s' % g.noreply in body def test_send_email_with_disabled_destination_user(self): @@ -352,7 +351,7 @@ class TestMailTasks(unittest.TestCase): message_id=h.gen_message_id()) assert _client.sendmail.call_count == 1 return_path, rcpts, body = _client.sendmail.call_args[0] - body = body.split('\n') + body = body.split(email_policy.linesep) assert 'From: "Test Admin" <test-admin@users.localhost>' in body c.user.disabled = True @@ -366,7 +365,7 @@ class TestMailTasks(unittest.TestCase): message_id=h.gen_message_id()) assert _client.sendmail.call_count == 2 return_path, rcpts, body = _client.sendmail.call_args[0] - body = body.split('\n') + body = body.split(email_policy.linesep) assert 'From: %s' % g.noreply in body def test_email_sender_to_headers(self): @@ -382,7 +381,7 @@ class TestMailTasks(unittest.TestCase): message_id=h.gen_message_id()) assert _client.sendmail.call_count == 1 return_path, rcpts, body = _client.sendmail.call_args[0] - body = body.split('\n') + body = body.split(email_policy.linesep) assert 'From: "Test Admin" <test-admin@users.localhost>' in body assert 'Sender: tick...@test.p.domain.net' in body assert 'To: t...@mail.com' in body @@ -398,7 +397,7 @@ class TestMailTasks(unittest.TestCase): message_id=h.gen_message_id()) assert _client.sendmail.call_count == 1 return_path, rcpts, body = _client.sendmail.call_args[0] - body = body.split('\n') + body = body.split(email_policy.linesep) assert 'From: "Test Admin" <test-admin@users.localhost>' in body assert 'Sender: tick...@test.p.domain.net' in body assert 'To: 1...@tickets.test.p.domain.net' in body @@ -416,7 +415,7 @@ class TestMailTasks(unittest.TestCase): message_id=h.gen_message_id()) assert _client.sendmail.call_count == 1 return_path, rcpts, body = _client.sendmail.call_args[0] - body = body.split('\n') + body = body.split(email_policy.linesep) assert 'From: "Test Admin" <test-admin@users.localhost>' in body assert 'References: <a> <b> <c>' in body @@ -431,7 +430,7 @@ class TestMailTasks(unittest.TestCase): message_id=h.gen_message_id()) assert _client.sendmail.call_count == 1 return_path, rcpts, body = _client.sendmail.call_args[0] - body = body.split('\n') + body = body.split(email_policy.linesep) assert 'From: "Test Admin" <test-admin@users.localhost>' in body assert 'References: <ref>' in body @@ -472,11 +471,11 @@ class TestMailTasks(unittest.TestCase): toaddr='b...@blah.com', text=('0123456789' * 100) + '\n\n' + ('Громады стро ' * 100), reply_to=g.noreply, - subject='По оживлённым берегам', + subject='123451234512345' * 100, references=['f...@example.com'] * 100, # needs to handle really long headers as well message_id=h.gen_message_id()) return_path, rcpts, body = _client.sendmail.call_args[0] - body = body.split('\n') + body = body.split(email_policy.linesep) for line in body: assert len(line) <= MAX_MAIL_LINE_OCTETS diff --git a/ForgeDiscussion/forgediscussion/tests/functional/test_forum.py b/ForgeDiscussion/forgediscussion/tests/functional/test_forum.py index 18e2a3f93..c0a9512db 100644 --- a/ForgeDiscussion/forgediscussion/tests/functional/test_forum.py +++ b/ForgeDiscussion/forgediscussion/tests/functional/test_forum.py @@ -34,6 +34,7 @@ from tg import config import feedparser from allura import model as M +from allura.lib.mail_util import email_policy from allura.tasks import mail_tasks from alluratest.controller import TestController from allura.lib import helpers as h @@ -61,7 +62,7 @@ class TestForumEmail(TestController): self.forum = FM.Forum.query.get(shortname='testforum') def test_simple_email(self): - msg = MIMEText('This is a test message') + msg = MIMEText('This is a test message', policy=email_policy) self._post_email( self.email_address, [self.forum.email_address], @@ -74,8 +75,9 @@ class TestForumEmail(TestController): msg = MIMEMultipart( 'alternative', _subparts=[ - MIMEText('This is a test message'), - MIMEText('This is a <em>test</em> message', 'html')]) + MIMEText('This is a test message', policy=email_policy), + MIMEText('This is a <em>test</em> message', 'html', policy=email_policy)], + policy=email_policy) self._post_email( self.email_address, [self.forum.email_address], @@ -94,13 +96,17 @@ class TestForumEmail(TestController): MIMEMultipart( 'alternative', _subparts=[ - MIMEText('This is a test message'), - MIMEText('This is a <em>test</em> message', 'html') - ]) - ]) + MIMEText('This is a test message', policy=email_policy), + MIMEText('This is a <em>test</em> message', 'html', policy=email_policy) + ], + policy=email_policy + ), + ], + policy=email_policy + ) with open(pkg_resources.resource_filename( 'forgediscussion', 'tests/data/python-logo.png'), 'rb') as fp: - img = MIMEImage(fp.read()) + img = MIMEImage(fp.read(), policy=email_policy) img.add_header('Content-Disposition', 'attachment', filename='python-logo.png') msg.attach(img) diff --git a/ForgeTracker/forgetracker/tests/functional/test_root.py b/ForgeTracker/forgetracker/tests/functional/test_root.py index 4ed46201b..7926b5d15 100644 --- a/ForgeTracker/forgetracker/tests/functional/test_root.py +++ b/ForgeTracker/forgetracker/tests/functional/test_root.py @@ -33,6 +33,7 @@ from tg import tmpl_context as c from tg import app_globals as g from tg import config +from allura.lib.mail_util import email_policy from allura.tests.decorators import assert_equivalent_urls from allura.tests.test_globals import squish_spaces from alluratest.controller import TestController, setup_basic_test @@ -2526,7 +2527,7 @@ class TestFunctionalController(TrackerTestController): message_id=h.gen_message_id()) assert _client.sendmail.call_count == 1 return_path, rcpts, body = _client.sendmail.call_args[0] - body = body.split('\n') + body = body.split(email_policy.linesep) # check subject assert 'Subject: [test:bugs] #1 test <h2> ticket' in body # check html, need tags escaped