Merge lp:~dholbach/click-reviewers-tools/1530894 into lp:click-reviewers-tools

[Daniel Holbach]

[lp:~dholbach/click-reviewers-tools/1530894]

565. By Daniel Holbach on 2016-01-06

return False if message catalog can't be opened

[Daniel Holbach (dholbach)]

http://paste.ubuntu.com/14418943/ is a sketch of a test, but with _list_all_compiled_binaries not being run during tests currently not useful.

[Daniel Holbach (dholbach)]

<beuno> and I'll comment on the MP that I wouldn't even probe for .mo's to be mo's
<dholbach> so fn.endswith('.mo') would be good enough for you?
<beuno> dholbach, it would, yes

[Jamie Strandboge (jdstrand)]

I agree that only checking for .mo is enough (it is cheap and sufficient). Marking as 'Needs fixing' for this comment, buy I will be merging an updated commit with a test case momentarily.

review: Needs Fixing

[Jamie Strandboge (jdstrand)]

12:11 < jdstrand> beuno: I committed dholbach's change with some changes in r564. It
                  sounds like it would be a good time for a sync of the review tools
12:11 < jdstrand> beuno: and hi!
12:11 < beuno> jdstrand, o/
12:11 < beuno> ack

=== modified file 'clickreviews/cr_common.py'
--- clickreviews/cr_common.py	2015-12-02 19:27:17 +0000
+++ clickreviews/cr_common.py	2016-01-06 09:32:03 +0000
@@ -18,6 +18,7 @@
 import atexit
 import codecs
 from debian.deb822 import Deb822
+import gettext
 import glob
 import inspect
 import json
@@ -114,6 +115,12 @@
                            "security-template",
                            "security-override",
                            "security-policy"]
+    magic_binary_file_descriptions = [
+        'application/x-executable; charset=binary',
+        'application/x-sharedlib; charset=binary',
+        'application/x-object; charset=binary',
+        'application/octet-stream; charset=binary'
+    ]
 
     def __init__(self, fn, review_type, peer_hooks=None, overrides=None,
                  peer_hooks_link=None):
@@ -317,14 +324,23 @@
             for f in filenames:
                 self.pkg_files.append(os.path.join(root, f))
 
+    def _check_if_message_catalog(self, fn):
+        '''Check if file is a message catalog (.mo file).'''
+        if fn.endswith('.mo'):
+            try:
+                gettext.GNUTranslations(open(fn, 'rb'))
+                return True
+            except:
+                return False
+        return False
+
+
     def _list_all_compiled_binaries(self):
         '''List all compiled binaries in this click package.'''
         for i in self.pkg_files:
             res = self.mime.file(i)
-            if res in ['application/x-executable; charset=binary',
-                       'application/x-sharedlib; charset=binary',
-                       'application/x-object; charset=binary',
-                       'application/octet-stream; charset=binary']:
+            if res in self.magic_binary_file_descriptions and \
+               not self._check_if_message_catalog(i):
                 self.pkg_bin_files.append(i)
 
     def _verify_manifest_structure(self):

=== modified file 'debian/changelog'
--- debian/changelog	2015-12-14 22:35:22 +0000
+++ debian/changelog	2016-01-06 09:32:03 +0000
@@ -1,6 +1,8 @@
 click-reviewers-tools (0.36) UNRELEASED; urgency=medium
 
-  * 
+  [ Daniel Holbach ]
+  * Add check if suspected (using python-magic) compiled binaries
+    aren't actually just message catalogs (.mo files) (LP: #1530894).
 
  -- Jamie Strandboge <[e-mail]>  Mon, 14 Dec 2015 16:34:46 -0600