allow changelog through to make clouserw happy (bug #656093)

apps/files/helpers.py

@@ -18,6 +18,8 @@ import amo
 from amo.utils import memoize
 from amo.urlresolvers import reverse
 from files.utils import extract_xpi, get_md5
+from validator.testcases.packagelayout import (blacklisted_extensions,
+                                               blacklisted_magic_numbers)
 
 
 task_log = commonware.log.getLogger('z.task')
@@ -98,22 +100,24 @@ class FileViewer:
         """If the file has been extracted or not."""
         return os.path.exists(self.dest)
 
-    def _is_binary(self, mimetype, filename):
+    def _is_binary(self, mimetype, path):
         """Uses the filename to see if the file can be shown in HTML or not."""
+        # Re-use the blacklisted data from amo-validator to spot binaries.
+        ext = os.path.splitext(path)[1][1:]
+        if ext in blacklisted_extensions:
+            return True
+
+        if os.path.exists(path) and not os.path.isdir(path):
+            bytes = tuple([ord(x) for x in open(path, 'r').read(4)])
+            if [x for x in blacklisted_magic_numbers if bytes[0:len(x)] == x]:
+                return True
+
         if mimetype:
             major, minor = mimetype.split('/')
-            if major == 'text' and minor in ['plain', 'html', 'css']:
-                return False
-            elif major == 'application' and minor in ['json']:
-                return False
-            elif minor in ['xml', 'rdf+xml', 'javascript', 'x-javascript',
-                           'xml-dtd', 'vnd.mozilla.xul+xml']:
-                return False
-        if os.path.splitext(filename)[1] in ['.dtd', '.xul', '.properties',
-                                             '.src', '.mf', '.sf', '.json',
-                                             '.manifest']:
-            return False
-        return True
+            if major == 'image':
+                return True
+
+        return False
 
     def read_file(self, allow_empty=False):
         """
@@ -146,10 +150,16 @@ class FileViewer:
 
     def is_binary(self):
         if self.selected:
+            if self.selected['binary']:
+                self.selected['msg'] = _('This file is not viewable online. '
+                                         'Please download the file to view '
+                                         'the contents.')
             return self.selected['binary']
 
     def is_directory(self):
         if self.selected:
+            if self.selected['directory']:
+                self.selected['msg'] = _('This file is a directory.')
             return self.selected['directory']
 
     def get_default(self, key=None):
@@ -223,7 +233,7 @@ class FileViewer:
             mime, encoding = mimetypes.guess_type(filename)
             directory = os.path.isdir(path)
             args = [self.file.id, short]
-            res[short] = {'binary': self._is_binary(mime, filename),
+            res[short] = {'binary': self._is_binary(mime, path),
                           'depth': short.count(os.sep),
                           'directory': directory,
                           'filename': filename,
@@ -247,7 +257,7 @@ class DiffHelper:
     def __init__(self, left, right):
         self.left = FileViewer(left)
         self.right = FileViewer(right)
-        self.status, self.key = None, None
+        self.key = None
 
     def __str__(self):
         return '%s:%s' % (self.left, self.right)
@@ -343,8 +353,8 @@ class DiffHelper:
             return False
 
         for obj in [self.left, self.right]:
+            if obj.is_binary():
+                return False
             if obj.is_directory():
-                self.status = _('%s is a directory in file %s.' %
-                                (self.key, obj.file.id))
                 return False
         return True

apps/files/templates/files/content.html

@@ -11,7 +11,7 @@
         {% endif %}
     {% endif %}
     {% if diff %}
-        {% if not diff.left.selected['binary'] %}
+        {% if left or right %}
             <div class="diff-bar js-hidden"></div>
             <pre id="diff" class="diff-bar-height js-hidden brush: diff; toolbar: false"></pre>
             <pre class="left js-hidden">{{ left }}</pre>

apps/files/tests/test_helpers.py

@@ -76,7 +76,7 @@ class TestFileHelper(test_utils.TestCase):
         binary = self.viewer._is_binary
         for f in ['foo.rdf', 'foo.xml', 'foo.js', 'foo.py'
                   'foo.html', 'foo.txt', 'foo.dtd', 'foo.xul',
-                  'foo.properties', 'foo.json', 'foo.src']:
+                  'foo.properties', 'foo.json', 'foo.src', 'CHANGELOG']:
             m, encoding = mimetypes.guess_type(f)
             assert not binary(m, f), '%s should not be binary' % f
 
@@ -84,10 +84,21 @@ class TestFileHelper(test_utils.TestCase):
             m, encoding = mimetypes.guess_type(f)
             assert not binary(None, f), '%s should not be binary' % f
 
-        for f in ['foo.png', 'foo.gif', 'foo.xls', 'foo.dic']:
+        for f in ['foo.png', 'foo.gif', 'foo.exe', 'foo.swf']:
             m, encoding = mimetypes.guess_type(f)
             assert binary(m, f), '%s should be binary' % f
 
+        filename = tempfile.mktemp()
+        for txt in ['#python', u'\0x2']:
+            open(filename, 'w').write(txt)
+            m, encoding = mimetypes.guess_type(filename)
+            assert not binary(m, filename), '%s should not be binary' % txt
+
+        for txt in ['#!/usr/bin/python', 'MZ']:
+            open(filename, 'w').write(txt)
+            m, encoding = mimetypes.guess_type(filename)
+            assert binary(m, filename), '%s should be binary' % txt
+
     def test_truncate(self):
         truncate = self.viewer.truncate
         for x, y in (['foo.rdf', 'foo.rdf'],
@@ -112,7 +123,7 @@ class TestFileHelper(test_utils.TestCase):
         eq_(files['install.js']['directory'], False)
         eq_(files['install.js']['binary'], False)
         eq_(files['__MACOSX']['directory'], True)
-        eq_(files['__MACOSX']['binary'], True)
+        eq_(files['__MACOSX']['binary'], False)
 
     def test_url_file(self):
         self.viewer.extract()
@@ -285,8 +296,7 @@ class TestDiffHelper(test_utils.TestCase):
         self.helper.select('install.js')
         self.helper.left.selected['directory'] = True
         assert not self.helper.is_diffable()
-        eq_(unicode(self.helper.status),
-            'install.js is a directory in file 1.')
+        assert self.helper.left.selected['msg'].startswith('This file')
 
     def test_diffable_parent(self):
         self.helper.extract()

apps/files/tests/test_views.py

@@ -398,6 +398,21 @@ class TestDiffViewer(FilesBase, test_utils.TestCase):
         eq_(len(doc('pre')), 3)
         eq_(len(doc('#content-wrapper p')), 1)
 
+    def test_view_left_binary(self):
+        self.file_viewer.extract()
+        filename = os.path.join(self.file_viewer.left.dest, 'install.js')
+        open(filename, 'w').write('#!')
+        res = self.client.get(self.file_url(not_binary))
+        assert 'This file is not viewable online' in res.content
+
+    def test_view_right_binary(self):
+        self.file_viewer.extract()
+        filename = os.path.join(self.file_viewer.right.dest, 'install.js')
+        open(filename, 'w').write('#!')
+        assert not self.file_viewer.is_diffable()
+        res = self.client.get(self.file_url(not_binary))
+        assert 'This file is not viewable online' in res.content
+
     def test_different_tree(self):
         self.file_viewer.extract()
         os.remove(os.path.join(self.file_viewer.left.dest, not_binary))

apps/files/views.py

@@ -118,10 +118,7 @@ def compare(request, diff, key=None):
 
         diff.select(key)
         data['key'] = key
-        if not diff.is_diffable():
-            data['msgs'] = [diff.status]
-
-        elif not diff.is_binary():
+        if diff.is_diffable():
             data['left'], data['right'] = diff.read_file()
 
     else: