Merge lp:~bac/launchpadlib/bug-745801 into lp:launchpadlib

[Brad Crittenden]

Encode the serialized credentials before saving.

Since some users experienced problems saving the configuration containing lplib keys into their keyring, this branch base64 encodes the values before putting into the keyring in an attempt to avoid freaking out the Gnome keyring or KDE wallet.

When decoding credentials a test is made to see if we have an unencoded string to provide backwards compatibility.

[j.c.sackett (jcsackett)]

I think this looks fine.

I'm a little confused about the addition of krdebug; it seems a little irregular to have a file like this living in the root directory, esp as it seems to be meant as a debug tool while working on this branch.

Was this definitely intended to be added?

review: Needs Information

[Brad Crittenden (bac)]

Jonathan, you're correct about the debug file. I had it there for a user affected by this bug to use to gather some data. It will not be submitted.

Thanks for the review.

[lp:~bac/launchpadlib/bug-745801]

128. By Brad Crittenden on 2011-11-17

Removed debugging script

=== modified file 'src/launchpadlib/credentials.py'
--- src/launchpadlib/credentials.py	2011-11-10 15:29:38 +0000
+++ src/launchpadlib/credentials.py	2011-11-17 22:23:23 +0000
@@ -36,6 +36,11 @@
 from urllib import urlencode
 from urlparse import urljoin
 import webbrowser
+import ConfigParser
+from base64 import (
+    b64decode,
+    b64encode,
+    )
 
 import simplejson
 
@@ -69,6 +74,8 @@
 
     URI_TOKEN_FORMAT = "uri"
     DICT_TOKEN_FORMAT = "dict"
+    ITEM_SEPARATOR = '<BR>'
+    NEWLINE = '\n'
 
     def serialize(self):
         """Turn this object into a string.
@@ -77,7 +84,12 @@
         """
         sio = StringIO()
         self.save(sio)
-        return sio.getvalue()
+        serialized = sio.getvalue()
+        # Some users have reported problems with corrupted keyrings, both in
+        # Gnome and KDE, when newlines are included in the password.  Avoid
+        # this problem by base 64 encoding the serialized value.
+        serialized = b64encode(serialized)
+        return serialized
 
     @classmethod
     def from_string(cls, value):
@@ -86,6 +98,8 @@
         This should probably be moved into OAuthAuthorizer.
         """
         credentials = cls()
+        if 'consumer_key' not in value:
+            value = b64decode(value)
         credentials.load(StringIO(value))
         return credentials
 
@@ -121,7 +135,7 @@
             oauth_signature_method='PLAINTEXT',
             oauth_signature='&')
         url = web_root + request_token_page
-        headers = {'Referer' : web_root}
+        headers = {'Referer': web_root}
         if token_format == self.DICT_TOKEN_FORMAT:
             headers['Accept'] = 'application/json'
         response, content = httplib2.Http().request(
@@ -310,8 +324,9 @@
     def do_save(self, credentials, unique_key):
         """Store newly-authorized credentials in the keyring."""
         self._ensure_keyring_imported()
+        serialized = credentials.serialize()
         keyring.set_password(
-            'launchpadlib', unique_key, credentials.serialize())
+            'launchpadlib', unique_key, serialized)
 
     def do_load(self, unique_key):
         """Retrieve credentials from the keyring."""
@@ -320,7 +335,14 @@
             'launchpadlib', unique_key)
         if credential_string is not None:
             credential_string = credential_string.encode('utf8')
-            return Credentials.from_string(credential_string)
+            try:
+                credentials = Credentials.from_string(credential_string)
+                return credentials
+            except ConfigParser.NoOptionError as noe:
+                print "OOPS"
+                print "%r" % credential_string
+                print noe
+                return None
         return None