launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #26140
[Merge] ~cjwatson/launchpad:py3-MessageSet-fromEmail-bytes into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:py3-MessageSet-fromEmail-bytes into launchpad:master.
Commit message:
Make MessageSet.fromEmail require email_message as bytes
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/397159
This works better on Python 3.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:py3-MessageSet-fromEmail-bytes into launchpad:master.
diff --git a/lib/lp/bugs/mail/commands.py b/lib/lp/bugs/mail/commands.py
index b22723b..89426f0 100644
--- a/lib/lp/bugs/mail/commands.py
+++ b/lib/lp/bugs/mail/commands.py
@@ -59,6 +59,7 @@ from lp.registry.interfaces.distributionsourcepackage import (
from lp.registry.interfaces.pillar import IPillarNameSet
from lp.registry.interfaces.product import IProduct
from lp.registry.interfaces.projectgroup import IProjectGroup
+from lp.services.compat import message_as_bytes
from lp.services.mail.commands import (
EditEmailCommand,
EmailCommand,
@@ -98,7 +99,7 @@ class BugEmailCommand(EmailCommand):
if bugid == 'new':
message = getUtility(IMessageSet).fromEmail(
- parsed_msg.as_string(),
+ message_as_bytes(parsed_msg),
owner=getUtility(ILaunchBag).user,
filealias=filealias,
parsed_message=parsed_msg)
diff --git a/lib/lp/bugs/mail/handler.py b/lib/lp/bugs/mail/handler.py
index 1087fca..b64b815 100644
--- a/lib/lp/bugs/mail/handler.py
+++ b/lib/lp/bugs/mail/handler.py
@@ -29,6 +29,7 @@ from lp.bugs.interfaces.bugattachment import (
)
from lp.bugs.interfaces.bugmessage import IBugMessageSet
from lp.bugs.mail.commands import BugEmailCommands
+from lp.services.compat import message_as_bytes
from lp.services.identity.interfaces.emailaddress import IEmailAddressSet
from lp.services.mail.helpers import (
ensure_not_weakly_authenticated,
@@ -405,7 +406,7 @@ class MaloneHandler:
"""Append the message text to the bug comments."""
messageset = getUtility(IMessageSet)
message = messageset.fromEmail(
- signed_msg.as_string(), owner=getUtility(ILaunchBag).user,
+ message_as_bytes(signed_msg), owner=getUtility(ILaunchBag).user,
filealias=filealias, parsed_message=signed_msg,
fallback_parent=bug.initial_message, restricted=bug.private)
# If the new message's parent is linked to a bug watch we also link
diff --git a/lib/lp/code/browser/tests/test_branchmergeproposal.py b/lib/lp/code/browser/tests/test_branchmergeproposal.py
index a5b3520..908cfa4 100644
--- a/lib/lp/code/browser/tests/test_branchmergeproposal.py
+++ b/lib/lp/code/browser/tests/test_branchmergeproposal.py
@@ -99,6 +99,7 @@ from lp.registry.enums import (
PersonVisibility,
TeamMembershipPolicy,
)
+from lp.services.compat import message_as_bytes
from lp.services.database.constants import UTC_NOW
from lp.services.features.testing import FeatureFixture
from lp.services.job.interfaces.job import JobStatus
@@ -2189,7 +2190,7 @@ class TestCommentAttachmentRendering(TestCaseWithFactory):
msg = self.factory.makeEmailMessage(
body='testing',
attachments=[('test.diff', 'text/plain', attachment_body)])
- message = MessageSet().fromEmail(msg.as_string())
+ message = MessageSet().fromEmail(message_as_bytes(msg))
return CodeReviewDisplayComment(
bmp.createCommentFromMessage(message, None, None, msg))
diff --git a/lib/lp/code/mail/tests/test_codereviewcomment.py b/lib/lp/code/mail/tests/test_codereviewcomment.py
index 4c4a6ec..712a55f 100644
--- a/lib/lp/code/mail/tests/test_codereviewcomment.py
+++ b/lib/lp/code/mail/tests/test_codereviewcomment.py
@@ -18,6 +18,7 @@ from lp.code.mail.codereviewcomment import (
build_inline_comments_section,
CodeReviewCommentMailer,
)
+from lp.services.compat import message_as_bytes
from lp.services.config import config
from lp.services.mail.sendmail import format_address
from lp.services.messages.interfaces.message import IMessageSet
@@ -303,7 +304,8 @@ class TestCodeReviewComment(TestCaseWithFactory):
self.assertEqual(expected_lines, ctrl.body.splitlines()[1:10])
def makeComment(self, email_message):
- message = getUtility(IMessageSet).fromEmail(email_message.as_string())
+ message = getUtility(IMessageSet).fromEmail(
+ message_as_bytes(email_message))
bmp = self.factory.makeBranchMergeProposal()
comment = bmp.createCommentFromMessage(
message, None, None, email_message)
diff --git a/lib/lp/code/model/tests/test_codereviewcomment.py b/lib/lp/code/model/tests/test_codereviewcomment.py
index 6746c16..3669169 100644
--- a/lib/lp/code/model/tests/test_codereviewcomment.py
+++ b/lib/lp/code/model/tests/test_codereviewcomment.py
@@ -11,6 +11,7 @@ from lazr.lifecycle.event import ObjectCreatedEvent
from lp.code.enums import CodeReviewVote
from lp.code.model.codereviewcomment import quote_text_as_email
+from lp.services.compat import message_as_bytes
from lp.services.messages.model.message import MessageSet
from lp.testing import (
TestCase,
@@ -139,7 +140,7 @@ class TestCodeReviewCommentGetAttachments(TestCaseWithFactory):
body='This is the body of the email.',
attachments=[
(filename, content_type, 'Attachment body')])
- message = MessageSet().fromEmail(msg.as_string())
+ message = MessageSet().fromEmail(message_as_bytes(msg))
return self.bmp.createCommentFromMessage(message, None, None, msg)
def test_getAttachments_text_plain_are_displayed(self):
diff --git a/lib/lp/registry/browser/tests/test_mailinglists.py b/lib/lp/registry/browser/tests/test_mailinglists.py
index 115903c..7e13ad6 100644
--- a/lib/lp/registry/browser/tests/test_mailinglists.py
+++ b/lib/lp/registry/browser/tests/test_mailinglists.py
@@ -51,13 +51,13 @@ class MailingListTestCase(TestCaseWithFactory):
email='him@xxxxxx', name='him', displayname='Him')
raw = '\n'.join([
'From: Him <him@xxxxxx>',
- 'To: %s' % str(team.mailing_list.address),
+ 'To: %s' % team.mailing_list.address,
'Subject: monkey',
'Message-ID: <monkey>',
'Date: Fri, 01 Aug 2000 01:09:00 -0000',
'',
'First paragraph.\n\nSecond paragraph.\n\nThird paragraph.'
- ])
+ ]).encode('ASCII')
message_set = getUtility(IMessageSet)
message = message_set.fromEmail(raw)
transaction.commit()
diff --git a/lib/lp/registry/tests/test_mailinglist.py b/lib/lp/registry/tests/test_mailinglist.py
index f5416f4..a5ce917 100644
--- a/lib/lp/registry/tests/test_mailinglist.py
+++ b/lib/lp/registry/tests/test_mailinglist.py
@@ -727,15 +727,15 @@ class MailingListMessageTestCase(TestCaseWithFactory):
team, member = self.factory.makeTeamWithMailingListSubscribers(
'team', auto_subscribe=True)
sender = self.factory.makePerson()
- email = dedent(str("""\
+ email = dedent("""\
From: %s
To: %s
Subject: A question
Message-ID: <first-post>
Date: Fri, 01 Aug 2000 01:08:59 -0000\n
I have a question about this team.
- """ % (sender.preferredemail.email, team.mailing_list.address)))
- message = getUtility(IMessageSet).fromEmail(email)
+ """ % (sender.preferredemail.email, team.mailing_list.address))
+ message = getUtility(IMessageSet).fromEmail(email.encode("ASCII"))
held_message = team.mailing_list.holdMessage(message)
transaction.commit()
return team, member, sender, held_message
@@ -750,15 +750,15 @@ class MailingListHeldMessageTestCase(MailingListMessageTestCase):
team, member = self.factory.makeTeamWithMailingListSubscribers(
'team', auto_subscribe=False)
sender = self.factory.makePerson()
- email = dedent(str("""\
+ email = dedent("""\
From: %s
To: %s
Subject: =?iso-8859-1?q?Adi=C3=B3s?=
Message-ID: <first-post>
Date: Fri, 01 Aug 2000 01:08:59 -0000\n
hi.
- """ % (sender.preferredemail.email, team.mailing_list.address)))
- message = getUtility(IMessageSet).fromEmail(email)
+ """ % (sender.preferredemail.email, team.mailing_list.address))
+ message = getUtility(IMessageSet).fromEmail(email.encode("ASCII"))
pop_notifications()
held_message = team.mailing_list.holdMessage(message)
self.assertEqual(PostedMessageStatus.NEW, held_message.status)
diff --git a/lib/lp/services/messages/doc/message.txt b/lib/lp/services/messages/doc/message.txt
index 53994b5..1bd8b0d 100644
--- a/lib/lp/services/messages/doc/message.txt
+++ b/lib/lp/services/messages/doc/message.txt
@@ -145,6 +145,7 @@ add only noise to the comment. For the sake of completeness, since this
has been a problem before, let's look at text_contents of a message
containing a signature.asc attachment.
+ >>> from lp.services.compat import message_as_bytes
>>> from lp.services.mail.tests.helpers import read_test_message
>>> signed_msg = read_test_message('signed_detached.txt')
>>> signed_msg['Message-Id'] = '<signeddetached@testmsg>'
@@ -163,7 +164,7 @@ containing a signature.asc attachment.
-----END PGP SIGNATURE-----
...
- >>> signed_message = msg_set.fromEmail(signed_msg.as_string())
+ >>> signed_message = msg_set.fromEmail(message_as_bytes(signed_msg))
>>> print signed_message.text_contents
Some signed content.
@@ -196,7 +197,7 @@ doesn't specify a Content-Type, application/octet-stream will be used as
a default. If the chunk doesn't specify a charset, latin-1 will be
used as a default.
- >>> raw_msg = """\
+ >>> raw_msg = u"""\
... Content-Type: multipart/mixed; charset="latin-1";
... boundary="=====BOUNDARY====="
... MIME-Version: 1.0
@@ -217,7 +218,7 @@ used as a default.
... MIME-Version: 1.0
... Content-Transfer-Encoding: 7bit
...
- ... Plain text without a chăŕŝěţ.
+ ... Plain text without a ch\u0103\u0155\u015D\u011B\u0163.
... --=====BOUNDARY=====
... Content-Type: text/plain; charset="us-ascii"
... Content-Disposition: inline; filename="attachment.txt"
@@ -246,7 +247,7 @@ used as a default.
...
... some text in another file
... --=====BOUNDARY=====
- ... """
+ ... """.encode('UTF-8')
>>> msg = msgset.fromEmail(raw_msg)
>>> for chunk in msg.chunks:
... if chunk.content is not None:
@@ -284,7 +285,7 @@ Let's add another multipart message, this time we include a message in
the message, like it is done when forwarding an email.
>>> forwarded_msg = read_test_message('forwarded-msg.txt')
- >>> msg = msgset.fromEmail(forwarded_msg.as_string())
+ >>> msg = msgset.fromEmail(message_as_bytes(forwarded_msg))
>>> print msg.text_contents
Forwarding test message.
<BLANKLINE>
@@ -297,7 +298,7 @@ the parent if it's already in the database, though. To ensure that
threads aren't broken, if the direct parent of the message isn't in the
database, the next parent will be used.
- >>> foo_msg = msgset.fromEmail('''\
+ >>> foo_msg = msgset.fromEmail(b'''\
... From: foo.bar@xxxxxxxxxxxxx
... Subject: Test
... Date: Fri, 17 Jun 2005 10:45:13 +0100
@@ -306,7 +307,7 @@ database, the next parent will be used.
... Foo Bar
... ''')
- >>> baz_msg = msgset.fromEmail('''\
+ >>> baz_msg = msgset.fromEmail(b'''\
... From: foo.bar@xxxxxxxxxxxxx
... Subject: Test
... Date: Fri, 17 Jun 2005 10:45:13 +0100
@@ -326,7 +327,7 @@ We can specify a parent to be used, if no parent could be found for the
message. This is useful for bugs, where we want all messages except for
the first one to have a parent.
- >>> bar_msg = msgset.fromEmail('''\
+ >>> bar_msg = msgset.fromEmail(b'''\
... From: foo.bar@xxxxxxxxxxxxx
... Subject: Test
... Date: Fri, 17 Jun 2005 10:45:13 +0100
@@ -340,7 +341,7 @@ the first one to have a parent.
The fromEmail method handles non-multipart and minimalist messages
quite happily.
- >>> msg = msgset.fromEmail('''\
+ >>> msg = msgset.fromEmail(b'''\
... From: foo.bar@xxxxxxxxxxxxx
... Subject: Test
... Date: Fri, 17 Jun 2005 10:45:13 +0100
@@ -358,7 +359,7 @@ quite happily.
It also handles the case where the subject line is folded.
- >>> msg = msgset.fromEmail('''\
+ >>> msg = msgset.fromEmail(b'''\
... From: foo.bar@xxxxxxxxxxxxx
... Subject: Folded
... subject
@@ -376,7 +377,7 @@ However, there are some things it refuses to deal with. In particular, it
will not create Messages if it cannot determine the owner unless it is
explicitly told to do so:
- >>> msg = msgset.fromEmail('''\
+ >>> msg = msgset.fromEmail(b'''\
... From: invalid@xxxxxxxxxxx
... Date: Fri, 17 Jun 2005 10:45:13 +0100
... Message-Id: <fnord3>
@@ -388,7 +389,7 @@ explicitly told to do so:
[...]
UnknownSender: u'invalid@xxxxxxxxxxx'
- >>> msg = msgset.fromEmail('''\
+ >>> msg = msgset.fromEmail(b'''\
... From: invalid@xxxxxxxxxxx
... Date: Fri, 17 Jun 2005 10:45:13 +0100
... Message-Id: <fnord3>
@@ -410,7 +411,7 @@ header, or a missing Message-Id: or Date: header. These are required, and if
they are missing then the email was sent from a broken email client or
passed through a broken MTA and we have no choice but to bounce them.
- >>> msg = msgset.fromEmail('''\
+ >>> msg = msgset.fromEmail(b'''\
... From: foo.bar@xxxxxxxxxxxxx
... Date: Thu, 16 Jun 2005 14:12:54 +0100
... Subject: Dud
@@ -421,7 +422,7 @@ passed through a broken MTA and we have no choice but to bounce them.
[...]
InvalidEmailMessage: Missing Message-Id
- >>> msg = msgset.fromEmail('''\
+ >>> msg = msgset.fromEmail(b'''\
... Date: Fri, 17 Jun 2005 10:45:13 +0100
... Subject: Re: Dud
... Message-Id: <fnord6>
@@ -434,7 +435,7 @@ passed through a broken MTA and we have no choice but to bounce them.
Also, we generally insist that a message has a date associated with it.
- >>> msg = msgset.fromEmail('''\
+ >>> msg = msgset.fromEmail(b'''\
... From: foo.bar@xxxxxxxxxxxxx
... Subject: Dud without a date!
... Message-Id: <fnord6>
@@ -453,7 +454,7 @@ otherwise reject the method as invalid.
>>> from datetime import datetime
>>> import pytz
- >>> msg_text = '''\
+ >>> msg_bytes = b'''\
... From: foo.bar@xxxxxxxxxxxxx
... Subject: I have no date! Oh teh noes!
... Message-Id: <therearenofnords>
@@ -464,7 +465,7 @@ otherwise reject the method as invalid.
>>> date_created = datetime(
... 2008, 7, 9, 14, 27, 40, tzinfo=pytz.timezone('UTC'))
>>> msg = msgset.fromEmail(
- ... msg_text, date_created=date_created)
+ ... msg_bytes, date_created=date_created)
>>> msg.datecreated
datetime.datetime(2008, 7, 9, 14, 27, 40, tzinfo=<UTC>)
@@ -472,7 +473,7 @@ otherwise reject the method as invalid.
But, we make sure that we don't create a message with a date that is
futuristic:
- >>> msg = msgset.fromEmail('''\
+ >>> msg = msgset.fromEmail(b'''\
... From: foo.bar@xxxxxxxxxxxxx
... Date: Fri, 17 Jun 2016 10:45:13 +0100
... Subject: Re: Back to the future
@@ -486,7 +487,7 @@ futuristic:
And similarly, we will consider any message that claims to be older than
1990 to have been created right now:
- >>> msg = msgset.fromEmail('''\
+ >>> msg = msgset.fromEmail(b'''\
... From: foo.bar@xxxxxxxxxxxxx
... Date: Tue, 17 Jun 1986 10:45:13 +0100
... Subject: Re: Back to the future, again
@@ -500,7 +501,7 @@ And similarly, we will consider any message that claims to be older than
Finally, let's test the goldilocks message, where the date is just right:
- >>> msg = msgset.fromEmail('''\
+ >>> msg = msgset.fromEmail(b'''\
... From: foo.bar@xxxxxxxxxxxxx
... Date: Fri, 17 Jun 2005 10:45:13 +0100
... Subject: Re: Smells like the present to me
@@ -523,7 +524,8 @@ which often works.
>>> import os.path
>>> mail_path = ('../../mail/tests/emails/x-unknown-encoding.txt')
>>> msg_path = os.path.join(os.path.dirname(__file__), mail_path)
- >>> raw_msg = open(msg_path).read()
+ >>> with open(msg_path, 'rb') as f:
+ ... raw_msg = f.read()
>>> print raw_msg
MIME-Version: 1.0
...
diff --git a/lib/lp/services/messages/interfaces/message.py b/lib/lp/services/messages/interfaces/message.py
index 01ad6ec..7e18e7d 100644
--- a/lib/lp/services/messages/interfaces/message.py
+++ b/lib/lp/services/messages/interfaces/message.py
@@ -137,7 +137,7 @@ class IMessageSet(Interface):
restricted=False):
"""Construct a Message from an email message and return it.
- :param email_message: The original email as a string.
+ :param email_message: The original email as a byte string.
:param owner: Specifies the owner of the new Message. The default
is calculated using the From: or Reply-To: headers, and will raise
a UnknownSender error if they cannot be found.
diff --git a/lib/lp/services/messages/model/message.py b/lib/lp/services/messages/model/message.py
index 28d70b8..49cbef2 100644
--- a/lib/lp/services/messages/model/message.py
+++ b/lib/lp/services/messages/model/message.py
@@ -11,7 +11,6 @@ __all__ = [
]
from datetime import datetime
-import email
from email.header import (
decode_header,
make_header,
@@ -57,6 +56,7 @@ from lp.registry.interfaces.person import (
PersonCreationRationale,
validate_public_person,
)
+from lp.services.compat import message_from_bytes
from lp.services.config import config
from lp.services.database.constants import UTC_NOW
from lp.services.database.datetimecol import UtcDateTimeCol
@@ -250,7 +250,7 @@ class MessageSet:
# Unfold the header before decoding it.
header = ''.join(header.splitlines())
- bits = email.header.decode_header(header)
+ bits = decode_header(header)
# Re-encode the header parts using utf-8, replacing undecodable
# characters with question marks.
re_encoded_bits = []
@@ -268,7 +268,7 @@ class MessageSet:
decoded = word if charset is None else self.decode(word, charset)
re_encoded_bits.append((decoded.encode('utf-8'), 'utf-8'))
- return six.text_type(email.header.make_header(re_encoded_bits))
+ return six.text_type(make_header(re_encoded_bits))
def fromEmail(self, email_message, owner=None, filealias=None,
parsed_message=None, create_missing_persons=False,
@@ -277,15 +277,15 @@ class MessageSet:
# It does not make sense to handle Unicode strings, as email
# messages may contain chunks encoded in differing character sets.
# Passing Unicode in here indicates a bug.
- if not zisinstance(email_message, str):
+ if not zisinstance(email_message, bytes):
raise TypeError(
- 'email_message must be a normal string. Got: %r'
+ 'email_message must be a byte string. Got: %r'
% email_message)
# Parse the raw message into an email.message.Message instance,
# if we haven't been given one already.
if parsed_message is None:
- parsed_message = email.message_from_string(email_message)
+ parsed_message = message_from_bytes(email_message)
# We could easily generate a default, but a missing message-id
# almost certainly means a developer is using this method when
diff --git a/lib/lp/services/messages/tests/test_message.py b/lib/lp/services/messages/tests/test_message.py
index eb23342..a9e1042 100644
--- a/lib/lp/services/messages/tests/test_message.py
+++ b/lib/lp/services/messages/tests/test_message.py
@@ -15,6 +15,7 @@ from email.utils import (
import six
import transaction
+from lp.services.compat import message_as_bytes
from lp.services.messages.model.message import MessageSet
from lp.testing import (
login,
@@ -71,7 +72,7 @@ class TestMessageSet(TestCaseWithFactory):
# and an text/x-diff attachment.
msg = self._makeMessageWithAttachment()
# Now create the message from the MessageSet.
- message = MessageSet().fromEmail(msg.as_string())
+ message = MessageSet().fromEmail(message_as_bytes(msg))
text, diff = message.chunks
self.assertEqual('This is the body of the email.', text.content)
self.assertEqual('review.diff', diff.blob.filename)
@@ -85,7 +86,7 @@ class TestMessageSet(TestCaseWithFactory):
# and an text/x-diff attachment.
msg = self._makeMessageWithAttachment(filename='/tmp/foo/review.diff')
# Now create the message from the MessageSet.
- message = MessageSet().fromEmail(msg.as_string())
+ message = MessageSet().fromEmail(message_as_bytes(msg))
text, diff = message.chunks
self.assertEqual('This is the body of the email.', text.content)
self.assertEqual('review.diff', diff.blob.filename)
@@ -97,9 +98,9 @@ class TestMessageSet(TestCaseWithFactory):
def test_fromEmail_always_creates(self):
"""Even when messages are identical, fromEmail creates a new one."""
email = self.factory.makeEmailMessage()
- orig_message = MessageSet().fromEmail(email.as_string())
+ orig_message = MessageSet().fromEmail(message_as_bytes(email))
transaction.commit()
- dupe_message = MessageSet().fromEmail(email.as_string())
+ dupe_message = MessageSet().fromEmail(message_as_bytes(email))
self.assertNotEqual(orig_message.id, dupe_message.id)
def test_fromEmail_restricted_reuploads(self):
@@ -109,14 +110,15 @@ class TestMessageSet(TestCaseWithFactory):
transaction.commit()
email = self.factory.makeEmailMessage()
message = MessageSet().fromEmail(
- email.as_string(), filealias=filealias, restricted=True)
+ message_as_bytes(email), filealias=filealias, restricted=True)
self.assertTrue(message.raw.restricted)
self.assertNotEqual(message.raw.id, filealias.id)
def test_fromEmail_restricted_attachments(self):
"""fromEmail creates restricted attachments correctly."""
msg = self._makeMessageWithAttachment()
- message = MessageSet().fromEmail(msg.as_string(), restricted=True)
+ message = MessageSet().fromEmail(
+ message_as_bytes(msg), restricted=True)
text, diff = message.chunks
self.assertEqual('review.diff', diff.blob.filename)
self.assertTrue('review.diff', diff.blob.restricted)
@@ -134,7 +136,7 @@ class TestMessageSet(TestCaseWithFactory):
""""macintosh encoding is equivalent to MacRoman."""
high_decoded = self.high_characters.decode('macroman')
email = self.makeEncodedEmail('macintosh', 'macroman')
- message = MessageSet().fromEmail(email.as_string())
+ message = MessageSet().fromEmail(message_as_bytes(email))
self.assertEqual(high_decoded, message.subject)
self.assertEqual(high_decoded, message.text_contents)
@@ -142,7 +144,7 @@ class TestMessageSet(TestCaseWithFactory):
""""'booga' encoding is decoded as latin-1."""
high_decoded = self.high_characters.decode('latin-1')
email = self.makeEncodedEmail('booga', 'latin-1')
- message = MessageSet().fromEmail(email.as_string())
+ message = MessageSet().fromEmail(message_as_bytes(email))
self.assertEqual(high_decoded, message.subject)
self.assertEqual(high_decoded, message.text_contents)
