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
=== modified file 'lib/canonical/launchpad/doc/incomingmail.txt'
--- lib/canonical/launchpad/doc/incomingmail.txt 2009-05-12 01:39:29 +0000
+++ lib/canonical/launchpad/doc/incomingmail.txt 2010-03-03 00:35:30 +0000
@@ -82,10 +82,6 @@
82handleMail, so that every mail gets handled by the correct handler.82handleMail, so that every mail gets handled by the correct handler.
8383
84 >>> handleMail(zopeless_transaction)84 >>> handleMail(zopeless_transaction)
85 WARNING:canonical.launchpad.mail:No X-Original-To header was present ...
86 WARNING:canonical.launchpad.mail:No X-Original-To header was present ...
87 WARNING:canonical.launchpad.mail:No X-Original-To header was present ...
88 WARNING:canonical.launchpad.mail:No X-Original-To header was present ...
8985
90Now we can see that each handler handled the emails sent to its domain:86Now we can see that each handler handled the emails sent to its domain:
9187
@@ -144,7 +140,7 @@
144140
145So if we send the mail to bar.com, bar_handler will handle the mail:141So if we send the mail to bar.com, bar_handler will handle the mail:
146142
147 >>> moin_change.replace_header('X-Original-To', '123@bar.com')143 >>> moin_change.replace_header('To', '123@bar.com')
148 >>> msgid = "<%s>" % sendmail(moin_change)144 >>> msgid = "<%s>" % sendmail(moin_change)
149 >>> handleMail(zopeless_transaction)145 >>> handleMail(zopeless_transaction)
150 >>> msgid in bar_handler.handledMails146 >>> msgid in bar_handler.handledMails
@@ -179,23 +175,11 @@
179175
180== X-Original-To ==176== X-Original-To ==
181177
182If available, the X-Original-To header is used to determine to which178Ideally, we would use X-Original-To, as this reflects the actual address
183address the email was sent to:179that the mail was sent to. Unfortunately, due to mail configuration issues,
184180X-Original-To is very wrong, so we must rely on To and Cc, even though these
185 >>> msg = read_test_message('signed_detached.txt')181may be inaccurate.
186 >>> msg.replace_header('To', '123@foo.com')182
187 >>> msg['CC'] = '123@foo.com'
188 >>> msg['X-Original-To'] = '123@bar.com'
189 >>> msgid = '<%s>' % sendmail (msg, ['123@bar.com'])
190 >>> handleMail(zopeless_transaction)
191 >>> msgid in bar_handler.handledMails
192 True
193
194Only the address in X-Original-To header will be used. The addresses in
195the To and CC headers will be ignored:
196
197 >>> msgid in foo_handler.handledMails
198 False
199183
200== OOPSes processing incoming mail ==184== OOPSes processing incoming mail ==
201185
@@ -344,7 +328,6 @@
344 ...328 ...
345 DataError: division by zero329 DataError: division by zero
346 <BLANKLINE>330 <BLANKLINE>
347 WARNING...
348331
349The second mail we sent got handled despite the exception:332The second mail we sent got handled despite the exception:
350333
@@ -361,7 +344,7 @@
361== Librarian not running ==344== Librarian not running ==
362345
363If for some reason the Librarian isn't up and running, we shouldn't346If for some reason the Librarian isn't up and running, we shouldn't
364loose any emails. All that should happen is that an error should get347lose any emails. All that should happen is that an error should get
365logged.348logged.
366349
367350
368351
=== modified file 'lib/canonical/launchpad/mail/incoming.py'
--- lib/canonical/launchpad/mail/incoming.py 2009-08-04 20:43:38 +0000
+++ lib/canonical/launchpad/mail/incoming.py 2010-03-03 00:35:30 +0000
@@ -9,8 +9,8 @@
99
10from logging import getLogger10from logging import getLogger
11from cStringIO import StringIO as cStringIO11from cStringIO import StringIO as cStringIO
12from email.Utils import getaddresses, parseaddr12from email.utils import getaddresses, parseaddr
13import email.Errors13import email.errors
14import re14import re
15import sys15import sys
1616
@@ -265,19 +265,13 @@
265 continue265 continue
266266
267 # Extract the domain the mail was sent to. Mails sent to267 # Extract the domain the mail was sent to. Mails sent to
268 # Launchpad should have an X-Original-To header.268 # Launchpad should have an X-Original-To header, but
269 if mail.has_key('X-Original-To'):269 # it has an incorrect address.
270 addresses = [mail['X-Original-To']]270 # Process all addresses found as a fall back.
271 else:271 cc = mail.get_all('cc') or []
272 log = getLogger('canonical.launchpad.mail')272 to = mail.get_all('to') or []
273 log.warn(273 names_addresses = getaddresses(to + cc)
274 "No X-Original-To header was present in email: %s" %274 addresses = [addr for name, addr in names_addresses]
275 file_alias_url)
276 # Process all addresses found as a fall back.
277 cc = mail.get_all('cc') or []
278 to = mail.get_all('to') or []
279 names_addresses = getaddresses(to + cc)
280 addresses = [addr for name, addr in names_addresses]
281275
282 handler = None276 handler = None
283 for email_addr in addresses:277 for email_addr in addresses:
284278
=== modified file 'lib/lp/blueprints/doc/spec-mail-exploder.txt'
--- lib/lp/blueprints/doc/spec-mail-exploder.txt 2009-05-12 12:53:20 +0000
+++ lib/lp/blueprints/doc/spec-mail-exploder.txt 2010-03-03 00:35:30 +0000
@@ -259,7 +259,7 @@
259 True259 True
260260
261 >>> moin_change = read_test_message('moin-change.txt')261 >>> moin_change = read_test_message('moin-change.txt')
262 >>> moin_change['X-Original-To'] = "notifications@%s" % (262 >>> moin_change['To'] += ", notifications@%s" % (
263 ... config.launchpad.specs_domain)263 ... config.launchpad.specs_domain)
264 >>> moin_change['Sender'] = 'webmaster@ubuntu.com'264 >>> moin_change['Sender'] = 'webmaster@ubuntu.com'
265265