Merge lp:~benji/launchpad/bug-612754-2 into lp:launchpad
Status: | Merged |
---|---|
Approved by: | Benji York |
Approved revision: | no longer in the source branch. |
Merged at revision: | 11744 |
Proposed branch: | lp:~benji/launchpad/bug-612754-2 |
Merge into: | lp:launchpad |
Diff against target: |
76 lines (+27/-7) 2 files modified
lib/lp/services/mail/incoming.py (+5/-1) lib/lp/services/mail/tests/test_signedmessage.py (+22/-6) |
To merge this branch: | bzr merge lp:~benji/launchpad/bug-612754-2 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Edwin Grubbs (community) | code | Approve | |
Review via email: mp+37797@code.launchpad.net |
Description of the change
Bug 612754 is about people having perfectly valid GPG signed messages
rejected by LP.
It turns out that they were rejected because LP doesn't remove trailing
whitespace before checking the signature (as required by RFC 4880 and
implemented in GPG). The fix was to remove the trailing whitespace as
part of the newline canonicalization.
The test added by this branch can be run by
bin/test -c test_signedmessage
while more email tests can be run by
bin/test -c mail
Lint fixed:
./lib/canonical
64: E302 expected 2 blank lines, found 1
156: W291 trailing whitespace
345: E202 whitespace before ')'
358: E202 whitespace before ')'
407: E202 whitespace before ')'
156: Line has trailing whitespace.
./lib/lp/
39: E501 line too long (86 characters)
169: W391 blank line at end of file
39: Line exceeds 78 characters.
Recommended listening for this MP: http://
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' launchpad/ mail/incoming. py 2010-10-03 15:30:06 +0000 launchpad/ mail/incoming. py 2010-10-06 22:26:05 +0000 mail.signedmess age import signed_ message_ from_string ed_line_ endings = re.compile( '((?<!\ r)\n)|( \r(?!\n) )') whitespace = re.compile(' *$')
>--- lib/canonical/
>+++ lib/canonical/
>@@ -55,9 +55,11 @@
> from lp.services.
>
> # 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_canonicalis
>
>+# Match trailing whitespace.
>+trailing_
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): ed_line_ endings. search( text): ed_line_ endings. sub('\r\ n', text) whitespace. search( text): whitespace. sub('', text) services/ mail/tests/ test_signedmess age.py' services/ mail/tests/ test_signedmess age.py 2010-08-20 20:31:18 +0000 services/ mail/tests/ test_signedmess age.py 2010-10-06 22:26:05 +0000 catedPrincipal. providedBy( principle) ) d_for_person( self, sender): d_for_person( self, sender, body=None): secret_ test_key( ) xt(key. fingerprint, password='test') makeSignedMessa ge( sender. preferredemail. email, context= signing_ context) catedPrincipal. providedBy( principle) ) whitespace( self): IPersonSet) .getByEmail( '<email address hidden>')
> r"""Canonicalise the line endings to '\r\n'.
>@@ -73,6 +75,8 @@
> """
> if non_canonicalis
> text = non_canonicalis
>+ if trailing_
>+ text = trailing_
> return text
>
>
>
>=== modified file 'lib/lp/
>--- lib/lp/
>+++ lib/lp/
>@@ -59,17 +59,18 @@
> IWeaklyAuthenti
> self.assertIs(None, msg.signature)
>
>- def _get_clearsigne
>+ def _get_clearsigne
> # Create a signed message for the sender specified with the test
> # secret key.
> key = import_
> signing_context = GPGSigningConte
>- 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.
> email_address=
> body=body, signing_
>@@ -97,6 +98,18 @@
> self.assertFalse(
> IWeaklyAuthenti
>
>+ def test_trailing_
>+ # Trailing whitespace should be ignored when verifying a message's
>+ # signature.
>+ sender = getUtility(
>+ 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) il(msg) t(None, msg.sig...
>+ principle = authenticateEma
>+ self.assertIsNo