Merge lp:~cmiller/apport/attach_drm into lp:~apport-hackers/apport/trunk

[Chad Miller]

Open DRM files as if they're bytes, not an encoded Unicode string. Process bytes, and decode (as "latin1") as last step, to get Unicode. Avoid at least two TypeErrors in Py3.

[Martin Pitt (pitti)]

Thanks for this! But why do you decode them as latin-1? It was just encoded to base64, so ASCII should be fine. But "modes" and the others also just look like ASCII. So UTF-8 might still be a better default, or do you have a case where these attributes contain non-ASCII data which isn't UTF-8 either?

When presenting the data to the user, or downloading it from LP, apport will try and convert byte arrays to UTF-8 unicode strings; if that fails, the values are simply left as byte arrays. So either leave the entire info as a byte array, or try to convert it from ASCII or UTF-8 and if that fails with an UnicodeDecodeError, just leave it like it is?

review: Needs Information

[Chad Miller (cmiller)]

Martin, I have little confidence that the kernel will always return lower-7-bit values or valid UTF8 in those files. I *never want to guess encoding at all*, but if I have to pick, I would rather preserve bytes and get some mojibake that I can decode manually, than crash or discard.

I didn't dig into whether I could change the interface of the report hash to have byte array. If you say it's okay to make its values byte arrays, I'll happily drop the decode()ing.

[Martin Pitt (pitti)]

I modified the decoding to default to UTF-8 and be robust against wrong characters, and merged. Thanks!

review: Approve

=== modified file 'apport/hookutils.py'
--- apport/hookutils.py	2015-02-10 11:50:59 +0000
+++ apport/hookutils.py	2015-02-24 19:33:57 +0000
@@ -838,14 +838,14 @@
         path = os.path.join(con, f)
         if f == 'uevent' or not os.path.isfile(path):
             continue
-        val = open(path).read().strip()
+        val = open(path, "rb").read().strip()
         # format some well-known attributes specially
         if f == 'modes':
-            val = val.replace('\n', ' ')
+            val = val.replace(b'\n', b' ')
         if f == 'edid':
             val = base64.b64encode(val)
             f += '-base64'
-        info += '%s: %s\n' % (f, val)
+        info += '%s: %s\n' % (f, val.decode("latin1"))
     return info