Merge lp:~brunonova/software-properties/lp1381050_2 into lp:software-properties

[Bruno Nova]

This should fix LP: #1381050.

This change was already proposed and accepted (https://code.launchpad.net/~brunonova/software-properties/lp1381050/+merge/238873), but the reviewer simplified it by removing this and two other lines. I tested it and I thought it worked, but I didn't test it correctly. A line like this is needed.

As mentioned in my last comment in the bug report, I think that the software-properties-dbus service is started with a clean environment, and thus in the C/POSIX locale. This makes special characters trigger an encoding error in this method.
This patch forces the path to be handled in UTF-8, fixing the bug (tested).

Michael Vogt reviewed the other merge requests, so he's free to review this one as well. :)

[Brian Murray (brian-murray)]

I've tested the change provided in Wily and it doesn't seem to fix the Traceback for me. Here is a result of trying import the key from the related bug report:

ipdb> n
dbus.exceptions.DBusException: org.freedesktop.DBus.Python.UnicodeEncodeError: Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/dbus/service.py", line 707, in _message_cb
    retval = candidate_method(self, *args, **keywords)
  File "/usr/lib/python3/dist-packages/softwareproperties/dbus/SoftwarePropertiesDBus.py", line 288, in AddKey
    return self.add_key(path)
  File "/usr/lib/python3/dist-packages/softwareproperties/SoftwareProperties.py", line 804, in add_key
    path = path.encode("utf-8")
  File "/usr/lib/python3.4/genericpath.py", line 19, in exists
    os.stat(path)
UnicodeEncodeError: 'ascii' codec can't encode character '\xed' in position 58: ordinal not in range(128)

review: Needs Fixing

[Bruno Nova (brunonova)]

I just booted the Wily Daily Live ISO, patched the system SoftwareProperties.py file directly, then started software-properties-gtk and added the VLC key that I downloaded to "~/Transferências" (and I also renamed the key file so it contained accented characters).
It worked! The key was added successfully.

Why didn't it work for you? 'path.encode("utf-8")' failed on your system.
Did you test this on a system without UTF-8 support?

Any ideas?

I think the code of the method should be all inside of the try...except method, so that the method always returns and the GUI can show an error message, instead of failing silently. The 'except' block should also print the stack trace.
This won't fix the bug, but is an improvement I think.

[Brian Murray (brian-murray)]

Sorry, for the delay in response. I tried as you've suggested booting a Wily Daily Live ISO and and patched /usr/lib/python3/dist-packages/softwareproperties/SoftwareProperties.py and the video lan asc file from the bug's description which I've renamed to contain an accent still fails to import. To be extra sure I renamed the file to a filename w/o an accent and it imported fine.

I did boot with English (US). Are you booting with a different language?

[Bruno Nova (brunonova)]

Yes, I'm booting in Portuguese.

I'm running out of ideas.
Is any UTF-8 locale available in English by default? It probably is.

We'll probably need someone who understands more about this Python filename encoding/decoding stuff.

[Bruno Nova (brunonova)]

@Brian, I now booted the daily live ISO in English, applied the patch, and tried adding a key with an accented character, and it worked. No error occurred.
What's going on here? What am I doing different from you? :S

[lp:~brunonova/software-properties/lp1381050_2]

939. By Bruno Nova on 2015-09-29

Ensure that True/False is returned in add_key method in SoftwareProperties.py

[Bruno Nova (brunonova)]

I've added a commit.

The "try:" line was moved to the top, putting the entire body of the method inside of a try...except block.
That way, when this encoding exception is raised, the GUI will report an error (it wasn't doing this).
Though it will also probably suppress exception tracebacks in the console/log.

I did nothing to try to fix the error that Brian reported, and I don't think I can fix it.

But it works for me at least! So, if someone else other than me confirms that this fix works for them, maybe the fix should be accepted (and a new bug opened for that error)?
A partial fix is better than no fix (usually), and I don't think there is regression potential. Except maybe if the system has no UTF-8 support and locales (can that happen?).

Unmerged revisions

939. By Bruno Nova on 2015-09-29

Ensure that True/False is returned in add_key method in SoftwareProperties.py

938. By Bruno Nova on 2015-07-13

Actually fix LP: #1381050

=== modified file 'softwareproperties/SoftwareProperties.py'
--- softwareproperties/SoftwareProperties.py	2015-06-05 13:05:08 +0000
+++ softwareproperties/SoftwareProperties.py	2015-09-29 21:31:27 +0000
@@ -801,9 +801,10 @@
 
   def add_key(self, path):
     """Add a gnupg key to the list of trusted software vendors"""
-    if not os.path.exists(path):
-        return False
     try:
+        path = path.encode('utf-8') # ensure path is encoded in UTF-8
+        if not os.path.exists(path):
+            return False
         res = self.apt_key.add(path)
         self.KeysModified()
         return res