Launchpad itself

Merge lp:~abentley/launchpad/no-original-to into lp:launchpad

  1. no-original-to
  2. Merge into devel
Proposed by Aaron Bentley
Status: Merged
Merged at revision: not available
Proposed branch: lp:~abentley/launchpad/no-original-to
Merge into: lp:launchpad
Diff against target: 123 lines (+17/-40)
3 files modified
lib/canonical/launchpad/doc/incomingmail.txt (+7/-24)
lib/canonical/launchpad/mail/incoming.py (+9/-15)
lib/lp/blueprints/doc/spec-mail-exploder.txt (+1/-1)
To merge this branch: bzr merge lp:~abentley/launchpad/no-original-to
Reviewer Review Type Date Requested Status
Francis J. Lacoste (community) release-critical Approve
Brad Crittenden (community) code Approve
Tim Penhey release-critical Pending
Review via email: mp+20451@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Aaron Bentley (abentley) wrote :

= Summary =
Stop using X-Original-To header for dispatching incoming mail, because it's
wrong.

== Proposed fix ==
Remove the code that prefers X-Original-To as the header, falling back on the
code that uses To and Cc, which was already present. These code changes have
been manually applied to production by the Losas for the past two months.

== Pre-implementation notes ==
Preimplementation was with Thumper.

== Implementation details ==
Some driveby fixes-- email.Utils -> email.utils to fix lint complaints, and a
spelling correction in a doctest.

== Tests ==
bin/test -t incoming.txt

== Demo and Q/A ==
Hard to assess.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/canonical/launchpad/mail/incoming.py
  lib/canonical/launchpad/doc/incomingmail.txt

Revision history for this message
Brad Crittenden (bac) :
review: Approve (code)
Revision history for this message
Francis J. Lacoste (flacoste) :
review: Approve (release-critical)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/doc/incomingmail.txt'
2--- lib/canonical/launchpad/doc/incomingmail.txt 2009-05-12 01:39:29 +0000
3+++ lib/canonical/launchpad/doc/incomingmail.txt 2010-03-03 00:35:30 +0000
4@@ -82,10 +82,6 @@
5 handleMail, so that every mail gets handled by the correct handler.
6
7 >>> handleMail(zopeless_transaction)
8- WARNING:canonical.launchpad.mail:No X-Original-To header was present ...
9- WARNING:canonical.launchpad.mail:No X-Original-To header was present ...
10- WARNING:canonical.launchpad.mail:No X-Original-To header was present ...
11- WARNING:canonical.launchpad.mail:No X-Original-To header was present ...
12
13 Now we can see that each handler handled the emails sent to its domain:
14
15@@ -144,7 +140,7 @@
16
17 So if we send the mail to bar.com, bar_handler will handle the mail:
18
19- >>> moin_change.replace_header('X-Original-To', '123@bar.com')
20+ >>> moin_change.replace_header('To', '123@bar.com')
21 >>> msgid = "<%s>" % sendmail(moin_change)
22 >>> handleMail(zopeless_transaction)
23 >>> msgid in bar_handler.handledMails
24@@ -179,23 +175,11 @@
25
26 == X-Original-To ==
27
28-If available, the X-Original-To header is used to determine to which
29-address the email was sent to:
30-
31- >>> msg = read_test_message('signed_detached.txt')
32- >>> msg.replace_header('To', '123@foo.com')
33- >>> msg['CC'] = '123@foo.com'
34- >>> msg['X-Original-To'] = '123@bar.com'
35- >>> msgid = '<%s>' % sendmail (msg, ['123@bar.com'])
36- >>> handleMail(zopeless_transaction)
37- >>> msgid in bar_handler.handledMails
38- True
39-
40-Only the address in X-Original-To header will be used. The addresses in
41-the To and CC headers will be ignored:
42-
43- >>> msgid in foo_handler.handledMails
44- False
45+Ideally, we would use X-Original-To, as this reflects the actual address
46+that the mail was sent to. Unfortunately, due to mail configuration issues,
47+X-Original-To is very wrong, so we must rely on To and Cc, even though these
48+may be inaccurate.
49+
50
51 == OOPSes processing incoming mail ==
52
53@@ -344,7 +328,6 @@
54 ...
55 DataError: division by zero
56 <BLANKLINE>
57- WARNING...
58
59 The second mail we sent got handled despite the exception:
60
61@@ -361,7 +344,7 @@
62 == Librarian not running ==
63
64 If for some reason the Librarian isn't up and running, we shouldn't
65-loose any emails. All that should happen is that an error should get
66+lose any emails. All that should happen is that an error should get
67 logged.
68
69
70
71=== modified file 'lib/canonical/launchpad/mail/incoming.py'
72--- lib/canonical/launchpad/mail/incoming.py 2009-08-04 20:43:38 +0000
73+++ lib/canonical/launchpad/mail/incoming.py 2010-03-03 00:35:30 +0000
74@@ -9,8 +9,8 @@
75
76 from logging import getLogger
77 from cStringIO import StringIO as cStringIO
78-from email.Utils import getaddresses, parseaddr
79-import email.Errors
80+from email.utils import getaddresses, parseaddr
81+import email.errors
82 import re
83 import sys
84
85@@ -265,19 +265,13 @@
86 continue
87
88 # Extract the domain the mail was sent to. Mails sent to
89- # Launchpad should have an X-Original-To header.
90- if mail.has_key('X-Original-To'):
91- addresses = [mail['X-Original-To']]
92- else:
93- log = getLogger('canonical.launchpad.mail')
94- log.warn(
95- "No X-Original-To header was present in email: %s" %
96- file_alias_url)
97- # Process all addresses found as a fall back.
98- cc = mail.get_all('cc') or []
99- to = mail.get_all('to') or []
100- names_addresses = getaddresses(to + cc)
101- addresses = [addr for name, addr in names_addresses]
102+ # Launchpad should have an X-Original-To header, but
103+ # it has an incorrect address.
104+ # Process all addresses found as a fall back.
105+ cc = mail.get_all('cc') or []
106+ to = mail.get_all('to') or []
107+ names_addresses = getaddresses(to + cc)
108+ addresses = [addr for name, addr in names_addresses]
109
110 handler = None
111 for email_addr in addresses:
112
113=== modified file 'lib/lp/blueprints/doc/spec-mail-exploder.txt'
114--- lib/lp/blueprints/doc/spec-mail-exploder.txt 2009-05-12 12:53:20 +0000
115+++ lib/lp/blueprints/doc/spec-mail-exploder.txt 2010-03-03 00:35:30 +0000
116@@ -259,7 +259,7 @@
117 True
118
119 >>> moin_change = read_test_message('moin-change.txt')
120- >>> moin_change['X-Original-To'] = "notifications@%s" % (
121+ >>> moin_change['To'] += ", notifications@%s" % (
122 ... config.launchpad.specs_domain)
123 >>> moin_change['Sender'] = 'webmaster@ubuntu.com'
124