add in arbitrary size limit and put shorter download links for all files (bug #651794)

apps/files/helpers.py

@@ -7,6 +7,7 @@ import stat
 from django.conf import settings
 from django.utils.datastructures import SortedDict
 from django.utils.encoding import smart_unicode
+from django.template.defaultfilters import filesizeformat
 
 import jinja2
 import commonware.log
@@ -96,11 +97,16 @@ class FileViewer:
                            'xml-dtd', 'vnd.mozilla.xul+xml']:
                 return False
         elif os.path.splitext(filename)[1] in ['.dtd', '.xul', '.properties',
-                                               '.src', '.mf', '.sf', '.json']:
+                                               '.src', '.mf', '.sf', '.json',
+                                               '.manifest']:
             return False
         return True
 
     def read_file(self, selected):
+        if selected['size'] > settings.FILE_VIEWER_SIZE_LIMIT:
+            return '', _('File size is over the limit of %s.'
+                         % (filesizeformat(settings.FILE_VIEWER_SIZE_LIMIT)))
+
         with open(selected['full'], 'r') as opened:
             cont = opened.read()
             codec = 'utf-16' if cont.startswith(codecs.BOM_UTF16) else 'utf-8'
@@ -165,6 +171,7 @@ class FileViewer:
                           'mimetype': mime or 'application/octet-stream',
                           'modified': os.stat(path)[stat.ST_MTIME],
                           'short': short,
+                          'size': os.stat(path)[stat.ST_SIZE],
                           'truncated': self.truncate(filename),
                           'url': reverse('files.list', args=args),
                           'url_serve': reverse('files.redirect', args=args)}

apps/files/templates/files/file.html

@@ -1,7 +1,10 @@
-<dl>
-    <dt>{{ _('Version:') }}</dt><dd>{{ file.version }}</dd>
-    {# L10n: {0} is the filename #}
-    <dt>{{ _('File:') }}</dt><dd><a href="{{ selected['url_serve'] }}">{{ _('Download {0}').format(selected['filename']) }}</a></dd>
-    <dt>{{ _('MD5 hash:') }}</dt><dd>{{ selected['md5'] }}</dd>
-    <dt>{{ _('Mimetype:') }}</dt><dd>{{ selected['mimetype'] }}</dd>
-</dl>
+<p>
+    <a href="{{ selected['url_serve'] }}">{{ _('Download {0}').format(selected['filename']) }}</a><br/>
+    {% trans version=file.version, size=selected['size']|filesizeformat,
+             md5=selected['md5'], mimetype=selected['mimetype'] %}
+        Version: {{ version }} &bull;
+        Size: {{ size }} &bull;
+        MD5 hash: {{ md5 }} &bull;
+        Mimetype: {{ mimetype }}
+    {% endtrans %}
+</p>

apps/files/templates/files/viewer.html

@@ -43,23 +43,18 @@
         </div>
         <div id="content-wrapper">
             {% if msg %}
-                <p>{{ msg }}</p>
+                <p class="notification-box warning">{{ msg }}</p>
             {% endif %}
             <div>
             {% if viewer %}
-                {% if selected['binary'] and not selected['directory'] %}
-                    {% include "files/file.html" %}
-                {% else %}
-                    {% if selected and content %}
-                        <pre id="content" class="wrapped">{{ content }}</pre>
-                    {% elif content == '' %}
-                        <p>{{ _('No content.') }}</p>
-                    {% endif %}
+                {% if selected and not selected['binary'] and not selected['directory'] and content %}
+                    <pre id="content" class="wrapped">{{ content }}</pre>
                 {% endif %}
+                {% include "files/file.html" %}
             {% endif %}
             {% if diff %}
                 {% if diff.is_diffable() %}
-                    <p>
+                    <p class="notification-box info">
                     {% if diff.is_different() %}
                         {{ _('Files are different.') }}<br/>
                       {% else %}
@@ -67,20 +62,17 @@
                     {% endif %}
                     </p>
                 {% endif %}
-                {% if diff.is_binary() %}
-                    {% with selected = diff.one, file = diff.file_one.file %}
-                        {% include "files/file.html" %}
-                    {% endwith %}
-                    {% with selected = diff.two, file = diff.file_two.file %}
-                        {% include "files/file.html" %}
-                    {% endwith %}
-                {% else %}
-                    {% if text_one and text_two %}
+                {% if text_one and text_two %}
                         <pre id="diff" class="wrapped hidden"></pre>
                         <pre class="left hidden">{{ text_one }}</pre>
                         <pre class="right hidden">{{ text_two }}</pre>
-                    {% endif %}
                 {% endif %}
+                {% with selected = diff.one, file = diff.file_one.file %}
+                    {% include "files/file.html" %}
+                {% endwith %}
+                {% with selected = diff.two, file = diff.file_two.file %}
+                    {% include "files/file.html" %}
+                {% endwith %}
             {% endif %}
             </div>
         </div>

apps/files/tests/test_helpers.py

@@ -6,7 +6,7 @@ import tempfile
 from django.conf import settings
 from django.core.cache import cache
 
-from mock import Mock
+from mock import Mock, patch_object
 from nose.tools import eq_
 import test_utils
 
@@ -118,7 +118,7 @@ class TestFileHelper(test_utils.TestCase):
     def test_bom(self):
         dest = tempfile.mkstemp()[1]
         open(dest, 'w').write('foo'.encode('utf-16'))
-        eq_(self.viewer.read_file({'full': dest}), (u'foo', ''))
+        eq_(self.viewer.read_file({'full': dest, 'size': 1}), (u'foo', ''))
 
     def test_file_order(self):
         self.viewer.extract()
@@ -129,8 +129,16 @@ class TestFileHelper(test_utils.TestCase):
         open(os.path.join(subdir, 'foo'), 'w')
         cache.clear()
         files = self.viewer.get_files().keys()
-        root = files.index(u'chrome')
-        eq_(files[root:root+3], [u'chrome', u'chrome/foo', u'chrome.manifest'])
+        rt = files.index(u'chrome')
+        eq_(files[rt:rt + 3], [u'chrome', u'chrome/foo', u'chrome.manifest'])
+
+    @patch_object(settings, 'FILE_VIEWER_SIZE_LIMIT', 5)
+    def test_file_size(self):
+        self.viewer.extract()
+        files = self.viewer.get_files()
+        res = self.viewer.read_file(files.get('install.js'))
+        eq_(res[0], '')
+        assert res[1].startswith('File size is')
 
 
 class TestDiffHelper(test_utils.TestCase):

apps/files/tests/test_views.py

@@ -8,6 +8,7 @@ from django.conf import settings
 from django.core.cache import cache
 from django.utils.encoding import iri_to_uri
 
+from mock import patch_object
 from nose.tools import eq_
 from pyquery import PyQuery as pq
 import test_utils
@@ -311,6 +312,13 @@ class TestFileViewer(FilesBase, test_utils.TestCase):
         eq_(res['X-SENDFILE'],
             self.file_viewer.get_files().get(binary)['full'])
 
+    @patch_object(settings._wrapped, 'FILE_VIEWER_SIZE_LIMIT', 5)
+    def test_file_size(self):
+        self.file_viewer.extract()
+        res = self.client.get(self.file_url(not_binary))
+        doc = pq(res.content)
+        assert doc('p.notification-box').text().startswith('File size is')
+
 
 class TestDiffViewer(FilesBase, test_utils.TestCase):
     fixtures = ['base/addon_3615', 'base/users']

media/css/zamboni/files.css

@@ -22,7 +22,7 @@
     padding-left: 1em;
 }
 
-#thinking {
+#content-wrapper, #thinking {
     padding-left: 25%;
 }
 

settings.py

@@ -882,3 +882,5 @@ BLOCKLIST_COOKIE = 'BLOCKLIST_v1'
 
 # Responsys id used for newsletter subscribing
 RESPONSYS_ID = ''
+
+FILE_VIEWER_SIZE_LIMIT = 1048576