Merge lp:~roadmr/checkbox/fix-gconf-resource into lp:checkbox

[Daniel Manrique]

Fixes the attached bug by changing the decoding charset.

I moved a bit of code around to factor command execution out of the get_gconf function, mainly to ease testing. I didn't find a way or place to put a proper test of the function :( Suggestions welcome, but the code seems to work now. To test it, download the gconf.txt file from the bug (comment #3), and replace COMMAND with something like "cat gconf.txt". Change the encoding from ascii to utf-8 and viceversa to see things pass/fail.

[Marc Tardif (cr3)]

Hm, I remember specifically setting the encoding to ascii but I was probably only thinking about the keys. I now see that the values can conceivably be encoded in utf-8!

The changes look good but I made a couple modifications while merging:

1. The parentheses around the check_output were not necessary for the arguments to be split on multiple lines.
2. The length of the command and its arguments was short, so they didn't really need to be split in the first place :)

review: Approve

=== modified file 'debian/changelog'
--- debian/changelog	2012-07-24 18:45:55 +0000
+++ debian/changelog	2012-07-25 14:36:24 +0000
@@ -11,6 +11,8 @@
     (LP: #1028065)
   * Instruct Chromium browser to accept file:// URLs so it can correctly
     open the checkbox submission.xml report (LP: #1026614)
+  * scripts/gconf_resource: decode gconf output as utf-8 rather than ascii
+    (LP: #1022593)
 
   [Jeff Marcom]
   * Added timeout to job call for disk smart test.
@@ -18,7 +20,7 @@
   [Marc Tardif]
   * Escaping encoded strings in udevadm output (LP: #1025381)
 
- -- Daniel Manrique <[e-mail]>  Tue, 24 Jul 2012 13:32:37 -0400
+ -- Daniel Manrique <[e-mail]>  Wed, 25 Jul 2012 10:22:40 -0400
 
 checkbox (0.14.2) quantal; urgency=low
 
 
=== modified file 'scripts/gconf_resource'
--- scripts/gconf_resource	2012-05-31 20:43:58 +0000
+++ scripts/gconf_resource	2012-07-25 14:36:24 +0000
@@ -21,7 +21,7 @@
 import sys
 import posixpath
 
-from subprocess import Popen, PIPE
+from subprocess import check_output, PIPE
 
 from checkbox.lib.conversion import string_to_type
 
@@ -33,7 +33,7 @@
 SOURCE = "~/.gconf"
 
 
-def get_gconf(command):
+def get_gconf(output):
     id = None
     id_pattern = re.compile(r"\s+(?P<id>\/.+):")
     key_value_pattern = re.compile(r"\s+(?P<key>[\w\-]+) = (?P<value>.*)")
@@ -42,9 +42,7 @@
     # TODO: add support for multi-line values
 
     gconf = {}
-    output = (Popen(command, stdout=PIPE, stderr=PIPE, shell=True)
-              .communicate()[0])
-    for line in output.decode("ascii").split("\n"):
+    for line in output.decode("utf-8").split("\n"):
         if not line:
             continue
 
@@ -80,7 +78,10 @@
 def main():
     source = posixpath.expanduser(SOURCE)
     command = COMMAND.replace("$source", source)
-    gconf = get_gconf(command)
+    command_output = (check_output(command, 
+                                   stderr=PIPE, 
+                                   shell=True))
+    gconf = get_gconf(command_output)
 
     for name, value in gconf.items():
         print("name: %s" % name)