Hi Benji,

I think the regex and the test need some work to handle trailing whitespace on any line.

-Edwin


>=== modified file 'lib/canonical/launchpad/mail/incoming.py'
>--- lib/canonical/launchpad/mail/incoming.py	2010-10-03 15:30:06 +0000
>+++ lib/canonical/launchpad/mail/incoming.py	2010-10-06 22:26:05 +0000
>@@ -55,9 +55,11 @@
> from lp.services.mail.signedmessage import signed_message_from_string
> 
> # Match '\n' and '\r' line endings. That is, all '\r' that are not
>-# followed by a # '\n', and all '\n' that are not preceded by a '\r'.
>+# followed by a '\n', and all '\n' that are not preceded by a '\r'.
> non_canonicalised_line_endings = re.compile('((?<!\r)\n)|(\r(?!\n))')
> 
>+# Match trailing whitespace.
>+trailing_whitespace = re.compile(' *$')



Since you are not using rstrip(), I assume you want the regex to
strip the whitespace at the end of each line. In that case, you need
to tell re.compile() that you want to use a multiline expression.
Also, " *$" will not match " \r\n".



> def canonicalise_line_endings(text):
>     r"""Canonicalise the line endings to '\r\n'.
>@@ -73,6 +75,8 @@
>     """
>     if non_canonicalised_line_endings.search(text):
>         text = non_canonicalised_line_endings.sub('\r\n', text)
>+    if trailing_whitespace.search(text):
>+        text = trailing_whitespace.sub('', text)
>     return text
> 
> 
>
>=== modified file 'lib/lp/services/mail/tests/test_signedmessage.py'
>--- lib/lp/services/mail/tests/test_signedmessage.py	2010-08-20 20:31:18 +0000
>+++ lib/lp/services/mail/tests/test_signedmessage.py	2010-10-06 22:26:05 +0000
>@@ -59,17 +59,18 @@
>             IWeaklyAuthenticatedPrincipal.providedBy(principle))
>         self.assertIs(None, msg.signature)
> 
>-    def _get_clearsigned_for_person(self, sender):
>+    def _get_clearsigned_for_person(self, sender, body=None):
>         # Create a signed message for the sender specified with the test
>         # secret key.
>         key = import_secret_test_key()
>         signing_context = GPGSigningContext(key.fingerprint, password='test')
>-        body = dedent("""\
>-            This is a multi-line body.
>+        if body is None:
>+            body = dedent("""\
>+                This is a multi-line body.
> 
>-            Sincerely,
>-            Your friendly tester.
>-            """)
>+                Sincerely,
>+                Your friendly tester.
>+                """)
>         msg = self.factory.makeSignedMessage(
>             email_address=sender.preferredemail.email,
>             body=body, signing_context=signing_context)
>@@ -97,6 +98,18 @@
>         self.assertFalse(
>             IWeaklyAuthenticatedPrincipal.providedBy(principle))
> 
>+    def test_trailing_whitespace(self):
>+        # Trailing whitespace should be ignored when verifying a message's
>+        # signature.
>+        sender = getUtility(IPersonSet).getByEmail('<email address hidden>')
>+        body = 'A message with trailing whitespace.   '



A multiline body with trailing whitespace on each line will fail.



>+        msg = self._get_clearsigned_for_person(sender, body)
>+        principle = authenticateEmail(msg)
>+        self.assertIsNot(None, msg.signature)
>+        self.assertEqual(sender, principle.person)
>+        self.assertFalse(
>+            IWeaklyAuthenticatedPrincipal.providedBy(principle))
>+
>     def _get_detached_message_for_person(self, sender):
>         # Return a signed message that contains a detached signature.
>         body = dedent("""\
>