launchpad-reviewers team mailing list archive

[Tim Penhey <tim.penhey@xxxxxxxxxxxxx>]

Tim Penhey has proposed merging lp:~thumper/launchpad/strip-email-attachment-path into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #221938 Email interface crashes when an attachment file name contains a slash
  https://bugs.launchpad.net/bugs/221938

For more details, see:
https://code.launchpad.net/~thumper/launchpad/strip-email-attachment-path/+merge/52159

Another relatively trivial email OOPS.

The attachment disposition may specify a path in the filename, but the librarian database table really doesn't like that, so we strip it off.
-- 
https://code.launchpad.net/~thumper/launchpad/strip-email-attachment-path/+merge/52159
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~thumper/launchpad/strip-email-attachment-path into lp:launchpad.

=== modified file 'lib/canonical/launchpad/database/message.py'
--- lib/canonical/launchpad/database/message.py	2011-02-17 17:02:54 +0000
+++ lib/canonical/launchpad/database/message.py	2011-03-04 03:57:02 +0000
@@ -14,7 +14,6 @@
     'UserToUserEmail',
     ]
 
-
 from cStringIO import StringIO as cStringIO
 from datetime import datetime
 import email
@@ -29,6 +28,7 @@
     parsedate_tz,
     )
 from operator import attrgetter
+import os.path
 
 from lazr.config import as_timedelta
 from lazr.enum import (
@@ -476,6 +476,8 @@
                     sequence += 1
             else:
                 filename = part.get_filename() or 'unnamed'
+                # Strip off any path information.
+                filename = os.path.basename(filename)
                 # Note we use the Content-Type header instead of
                 # part.get_content_type() here to ensure we keep
                 # parameters as sent. If Content-Type is None we default

=== modified file 'lib/canonical/launchpad/database/tests/test_message.py'
--- lib/canonical/launchpad/database/tests/test_message.py	2010-11-06 12:50:22 +0000
+++ lib/canonical/launchpad/database/tests/test_message.py	2011-03-04 03:57:02 +0000
@@ -114,6 +114,33 @@
         transaction.commit()
         self.assertEqual('This is the diff, honest.', diff.blob.read())
 
+    def test_fromEmail_strips_attachment_paths(self):
+        # Build a simple multipart message with a plain text first part
+        # and an text/x-diff attachment.
+        sender = self.factory.makePerson()
+        msg = MIMEMultipart()
+        msg['Message-Id'] = make_msgid('launchpad')
+        msg['Date'] = formatdate()
+        msg['To'] = 'to@xxxxxxxxxxx'
+        msg['From'] = sender.preferredemail.email
+        msg['Subject'] = 'Sample'
+        msg.attach(MIMEText('This is the body of the email.'))
+        attachment = Message()
+        attachment.set_payload('This is the diff, honest.')
+        attachment['Content-Type'] = 'text/x-diff'
+        attachment['Content-Disposition'] = (
+            'attachment; filename="/tmp/foo/review.diff"')
+        msg.attach(attachment)
+        # Now create the message from the MessageSet.
+        message = MessageSet().fromEmail(msg.as_string())
+        text, diff = message.chunks
+        self.assertEqual('This is the body of the email.', text.content)
+        self.assertEqual('review.diff', diff.blob.filename)
+        self.assertEqual('text/x-diff', diff.blob.mimetype)
+        # Need to commit in order to read back out of the librarian.
+        transaction.commit()
+        self.assertEqual('This is the diff, honest.', diff.blob.read())
+
 
 class TestMessageJob(TestCaseWithFactory):
     """Tests for MessageJob."""