Code review comment for lp:~wgrant/launchpad/emailauthentication.txt-2.6-fix

Revision history for this message
Michael Nelson (michael.nelson) wrote :

Hi William,

Thanks for sorting this out. Just a suggestion below, see what you think.

> === modified file 'lib/canonical/launchpad/doc/emailauthentication.txt'
> --- lib/canonical/launchpad/doc/emailauthentication.txt 2009-02-19 23:55:18 +0000
> +++ lib/canonical/launchpad/doc/emailauthentication.txt 2010-04-15 01:53:20 +0000
> @@ -137,7 +137,10 @@
>
> Python's email library unfolds the headers, which means that we have to
> be careful when extracting the signed content when folded headers are
> -signed.
> +signed. This is done by manually parsing boundaries in
> +SignedMessage._getSignatureAndSignedContent. If the second test here
> +starts failing, Python is probably fixed, so the manual boundary parsing
> +hack can be removed.

Although that makes sense to me at the moment, if I look at that in 6months time when the second test fails, I'll struggle. I was wondering if instead we could say: If the second test here starts failing, [link to remaining python issue if it exists] has probably been fixed and it can be switched back to simply use as_string().

I'll wait to hear from you before sending this off to land.

>
> >>> msg = read_test_message('signed_folded_header.txt')
> >>> print msg.parsed_string #doctest: -NORMALIZE_WHITESPACE
> @@ -147,9 +150,7 @@
> boundary="--------------------EuxKj2iCbKjpUGkD"
> ...
>
> - >>> print msg.as_string() #doctest: -NORMALIZE_WHITESPACE
> - Date:...
> - ...
> + >>> print msg.get_payload(i=0).as_string() #doctest: -NORMALIZE_WHITESPACE
> Content-Type: multipart/mixed; boundary="--------------------EuxKj2iCbKjpUGkD"
> ...

review: Approve

« Back to merge proposal