Merge lp:~abentley/launchpad/find-merge-directive into lp:launchpad

[Aaron Bentley]

Fix content-type bugs

= Summary =
Fix bug #506835: findMergeDirectiveAndComment fails if no Content-Type
Fix bug #530455: content type fail parsing merge directive

== Proposed fix ==
Do case-insensitive comparison of content-type, and if there is no
Content-Type, pretend it was text/plain.

== Pre-implementation notes ==
None

== Implementation details ==
None

== Tests ==
bin/test -t findMergeDirective codehandler

== Demo and Q/A ==
Send a merge directive email with no content-type to <email address hidden>.
Send a merge directive email with a mixed-case content-type to
<email address hidden>.

= Launchpad lint =

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

Linting changed files:
  lib/lp/code/mail/codehandler.py
  lib/lp/code/mail/tests/test_codehandler.py

== Pylint notices ==

lib/lp/code/mail/codehandler.py
    37: [F0401] Unable to import 'lazr.uri' (No module named uri)

[Gavin Panella (allenap)]

Looks good. Two questions:

 * The test docstrings look like they've been copy-n-pasted.

 * Is it necessary to commit at the end of each test? If so, could you add a short comment explaining why?

review: Approve (code)

[Aaron Bentley (abentley)]

The commits aren't necessary, and I've fixed the docstrings.

=== modified file 'lib/lp/code/mail/codehandler.py'
--- lib/lp/code/mail/codehandler.py	2010-02-10 15:04:00 +0000
+++ lib/lp/code/mail/codehandler.py	2010-03-05 14:45:47 +0000
@@ -541,7 +541,8 @@
             if part.is_multipart():
                 continue
             payload = part.get_payload(decode=True)
-            if part['Content-type'].startswith('text/plain'):
+            content_type = part.get('Content-type', 'text/plain').lower()
+            if content_type.startswith('text/plain'):
                 body = payload
                 charset = part.get_param('charset')
                 if charset is not None:

=== modified file 'lib/lp/code/mail/tests/test_codehandler.py'
--- lib/lp/code/mail/tests/test_codehandler.py	2010-02-10 17:05:36 +0000
+++ lib/lp/code/mail/tests/test_codehandler.py	2010-03-05 14:45:47 +0000
@@ -436,6 +436,35 @@
         self.assertEqual('', comment)
         transaction.commit()
 
+    def test_findMergeDirectiveAndComment_no_content_type(self):
+        """Parts with no content-type are treated as text/plain."""
+        md = self.factory.makeMergeDirective()
+        message = self.factory.makeSignedMessage(
+            body='', attachment_contents=''.join(md.to_lines()))
+        body = message.get_payload()[0]
+        del body['Content-type']
+        body.set_payload('body')
+        self.switchDbUser(config.processmail.dbuser)
+        code_handler = CodeHandler()
+        comment, md2 = code_handler.findMergeDirectiveAndComment(message)
+        self.assertEqual('body', comment)
+
+    def test_findMergeDirectiveAndComment_case_insensitive(self):
+        """findMergeDirectiveAndComment uses case-insensitive content-type."""
+        md = self.factory.makeMergeDirective()
+        message = self.factory.makeSignedMessage(
+            body='', attachment_contents=''.join(md.to_lines()))
+        body = message.get_payload()[0]
+        # Unlike dicts, messages append when you assign to a key.  So
+        # we must delete the first Content-type before adding another.
+        del body['Content-type']
+        body['Content-type'] = 'Text/Plain'
+        body.set_payload('body')
+        self.switchDbUser(config.processmail.dbuser)
+        code_handler = CodeHandler()
+        comment, md2 = code_handler.findMergeDirectiveAndComment(message)
+        self.assertEqual('body', comment)
+
     def test_findMergeDirectiveAndCommentUnicodeBody(self):
         """findMergeDirectiveAndComment returns unicode comments."""
         md = self.factory.makeMergeDirective()