Merge lp:~mbp/launchpad/893612-mail-too-big into lp:launchpad

Proposed by Martin Pool on 2012-04-13
Status: Merged
Approved by: Ian Booth on 2012-04-13
Approved revision: no longer in the source branch.
Merged at revision: 15100
Proposed branch: lp:~mbp/launchpad/893612-mail-too-big
Merge into: lp:launchpad
Diff against target: 141 lines (+56/-12)
4 files modified
lib/lp/services/mail/incoming.py (+21/-1)
lib/lp/services/mail/signedmessage.py (+6/-1)
lib/lp/services/mail/tests/test_incoming.py (+27/-1)
lib/lp/services/messages/model/message.py (+2/-9)
To merge this branch: bzr merge lp:~mbp/launchpad/893612-mail-too-big
Reviewer Review Type Date Requested Status
Ian Booth (community) 2012-04-13 Approve on 2012-04-13
Review via email: mp+101865@code.launchpad.net

Commit message

Detect incoming too-big email messages early, and give a notication not an oops

Description of the change

At the moment, if you send an >10MB message to Launchpad, it records an oops, and sends a message back to the user saying there was an oops.

This happens fairly late in processing, while running the handler.

This patch changes it so we do an up-front check as early as is easy, and give the user a regular error notification. This ought to eliminate one stream of oopses (I don't know if it's very common.)

There's a unit test and I also tried running a message through process-one-mail. I also confirmed the bug's still live on lp.

To post a comment you must log in.
Ian Booth (wallyworld) wrote :

Excellent fix. Thank you.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/services/mail/incoming.py'
2--- lib/lp/services/mail/incoming.py 2012-01-04 03:23:19 +0000
3+++ lib/lp/services/mail/incoming.py 2012-04-13 07:22:18 +0000
4@@ -1,4 +1,4 @@
5-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
6+# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
7 # GNU Affero General Public License version 3 (see the file LICENSE).
8
9 """Functions dealing with mails coming into Launchpad."""
10@@ -62,6 +62,10 @@
11 # Match trailing whitespace.
12 trailing_whitespace = re.compile(r'[ \t]*((?=\r\n)|$)')
13
14+# this is a hard limit on the size of email we will be willing to store in
15+# the database.
16+MAX_EMAIL_SIZE = 10 * 1024 * 1024
17+
18
19 def canonicalise_line_endings(text):
20 r"""Canonicalise the line endings to '\r\n'.
21@@ -461,6 +465,22 @@
22 log.info("Got a message with a precedence header.")
23 return
24
25+ if mail.raw_length > MAX_EMAIL_SIZE:
26+ complaint = (
27+ "The mail you sent to Launchpad is too long.\n\n"
28+ "Your message <%s>\nwas %d MB and the limit is %d MB." %
29+ (mail['message-id'], mail.raw_length / 1e6, MAX_EMAIL_SIZE / 1e6))
30+ log.info(complaint)
31+ # It's probably big and it's probably mostly binary, so trim it pretty
32+ # aggressively.
33+ send_process_error_notification(
34+ mail['From'],
35+ 'Mail to Launchpad was too large',
36+ complaint,
37+ mail,
38+ max_return_size=8192)
39+ return
40+
41 try:
42 principal = authenticateEmail(
43 mail, signature_timestamp_checker)
44
45=== modified file 'lib/lp/services/mail/signedmessage.py'
46--- lib/lp/services/mail/signedmessage.py 2011-08-12 14:36:25 +0000
47+++ lib/lp/services/mail/signedmessage.py 2012-04-13 07:22:18 +0000
48@@ -1,4 +1,4 @@
49-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
50+# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
51 # GNU Affero General Public License version 3 (see the file LICENSE).
52
53 """Classes for simpler handling of PGP signed email messages."""
54@@ -153,6 +153,11 @@
55 signature, signed_content = self._getSignatureAndSignedContent()
56 return signature
57
58+ @property
59+ def raw_length(self):
60+ """Return the length in bytes of the underlying raw form."""
61+ return len(self.parsed_string)
62+
63
64 def strip_pgp_signature(text):
65 """Strip any PGP signature from the supplied text."""
66
67=== modified file 'lib/lp/services/mail/tests/test_incoming.py'
68--- lib/lp/services/mail/tests/test_incoming.py 2012-01-20 15:42:44 +0000
69+++ lib/lp/services/mail/tests/test_incoming.py 2012-04-13 07:22:18 +0000
70@@ -1,4 +1,4 @@
71-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
72+# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
73 # GNU Affero General Public License version 3 (see the file LICENSE).
74
75 from doctest import DocTestSuite
76@@ -69,6 +69,32 @@
77 "(7, 58, u'No data')",
78 body)
79
80+ def test_mail_too_big(self):
81+ """Much-too-big mail should generate a bounce, not an OOPS.
82+
83+ See <https://bugs.launchpad.net/launchpad/+bug/893612>.
84+ """
85+ person = self.factory.makePerson()
86+ transaction.commit()
87+ email_address = person.preferredemail.email
88+ fat_body = '\n'.join(
89+ ['some big mail with this line repeated many many times\n']
90+ * 1000000)
91+ ctrl = MailController(
92+ email_address, 'to@example.com', 'subject', fat_body,
93+ bulk=False)
94+ ctrl.send()
95+ handleMail()
96+ self.assertEqual([], self.oopses)
97+ [notification] = pop_notifications()
98+ body = notification.get_payload()[0].get_payload(decode=True)
99+ self.assertIn(
100+ "The mail you sent to Launchpad is too long.",
101+ body)
102+ self.assertIn(
103+ "was 55 MB\nand the limit is 10 MB.",
104+ body)
105+
106 def test_invalid_to_addresses(self):
107 """Invalid To: header should not be handled as an OOPS."""
108 raw_mail = open(os.path.join(
109
110=== modified file 'lib/lp/services/messages/model/message.py'
111--- lib/lp/services/messages/model/message.py 2012-01-06 15:07:17 +0000
112+++ lib/lp/services/messages/model/message.py 2012-04-13 07:22:18 +0000
113@@ -1,4 +1,4 @@
114-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
115+# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
116 # GNU Affero General Public License version 3 (see the file LICENSE).
117
118 # pylint: disable-msg=E0611,W0212
119@@ -85,10 +85,6 @@
120 )
121 from lp.services.propertycache import cachedproperty
122
123-# this is a hard limit on the size of email we will be willing to store in
124-# the database.
125-MAX_EMAIL_SIZE = 10 * 1024 * 1024
126-
127
128 def utcdatetime_from_field(field_value):
129 """Turn an RFC 2822 Date: header value into a Python datetime (UTC).
130@@ -310,10 +306,7 @@
131 if not rfc822msgid:
132 raise InvalidEmailMessage('Missing Message-Id')
133
134- # make sure we don't process anything too long
135- if len(email_message) > MAX_EMAIL_SIZE:
136- raise InvalidEmailMessage('Msg %s size %d exceeds limit %d' % (
137- rfc822msgid, len(email_message), MAX_EMAIL_SIZE))
138+ # Over-long messages are checked for at the handle_on_message level.
139
140 # Stuff a copy of the raw email into the Librarian, if it isn't
141 # already in there.