move the size check to a form and don't save the file contents if big (bug 777171)

mkt/developers/forms.py

@@ -1,11 +1,13 @@
 # -*- coding: utf-8 -*-
 from datetime import datetime
+import json
 import os
 import socket
 
 from django import forms
 from django.conf import settings
 from django.forms.models import formset_factory, modelformset_factory
+from django.template.defaultfilters import filesizeformat
 
 import commonware
 import happyforms
@@ -21,8 +23,10 @@ from addons.forms import clean_name, icons, IconWidgetRenderer, slug_validator
 from addons.models import (Addon, AddonCategory, AddonUpsell, AddonUser,
                            BlacklistedSlug, Category, Preview)
 from addons.widgets import CategoriesSelectMultiple
+from amo import get_user
 from amo.utils import raise_required, remove_icons
 from editors.models import RereviewQueue
+from files.models import FileUpload
 from lib.video import tasks as vtasks
 from market.models import AddonPremium, Price, PriceCurrency
 from translations.fields import TransField
@@ -31,7 +35,7 @@ from translations.models import Translation
 from translations.widgets import TransInput, TransTextarea
 
 import mkt
-from mkt.constants import APP_IMAGE_SIZES
+from mkt.constants import APP_IMAGE_SIZES, MAX_PACKAGED_APP_SIZE
 from mkt.constants.ratingsbodies import (RATINGS_BY_NAME, ALL_RATINGS,
                                          RATINGS_BODIES)
 from mkt.inapp_pay.models import InappConfig
@@ -468,6 +472,42 @@ class NewManifestForm(happyforms.Form):
         return manifest
 
 
+class NewPackagedAppForm(happyforms.Form):
+    upload = forms.FileField()
+
+    def __init__(self, *args, **kwargs):
+        self.max_size = kwargs.pop('max_size', MAX_PACKAGED_APP_SIZE)
+        self.user = kwargs.pop('user', get_user())
+        self.file_upload = None
+        super(NewPackagedAppForm, self).__init__(*args, **kwargs)
+
+    def clean_upload(self):
+        upload = self.cleaned_data['upload']
+        if upload.size > self.max_size:
+            msg = 'Packaged app too large for submission.'
+            big = json.dumps({
+                'errors': 1,
+                'success': False,
+                'messages': [{
+                    'type': 'error',
+                    'message': [
+                        msg,
+                        'Packages must be less than %s.' %
+                        filesizeformat(self.max_size)],
+                    'tier': 1}]})
+            # Persist the error with this into FileUpload, but do not persist
+            # the file contents, which are too large.
+            self.file_upload = FileUpload.objects.create(is_webapp=True,
+                                                         user=self.user,
+                                                         validation=big)
+            # Raise an error so the form is invalid.
+            raise forms.ValidationError(msg)
+        else:
+            self.file_upload = FileUpload.from_post(upload, upload.name,
+                                                    upload.size,
+                                                    is_webapp=True)
+
+
 class PremiumForm(happyforms.Form):
     """
     The premium details for an addon, which is unfortunately

mkt/developers/tests/test_forms.py

@@ -1,8 +1,10 @@
+import json
 import os
 import shutil
 
 from django.conf import settings
 from django.core.files.storage import default_storage as storage
+from django.core.files.uploadedfile import SimpleUploadedFile
 
 import mock
 from nose.tools import eq_
@@ -237,3 +239,31 @@ class TestRegionForm(amo.tests.WebappTestCase):
         eq_(form.errors,
             {'__all__': ['You must select at least one region or '
                          '"Other and new regions."']})
+
+
+class TestPackagedAppForm(amo.tests.AMOPaths, amo.tests.WebappTestCase):
+
+    def setUp(self):
+        path = self.packaged_app_path('mozball.zip')
+        self.files = {'upload': SimpleUploadedFile('mozball.zip',
+                                                   open(path).read())}
+
+    def test_not_there(self):
+        form = forms.NewPackagedAppForm({}, {})
+        eq_(form.is_valid(), False)
+        eq_(form.errors['upload'], [u'This field is required.'])
+        eq_(form.file_upload, None)
+
+    def test_right_size(self):
+        form = forms.NewPackagedAppForm({}, self.files)
+        eq_(form.is_valid(), True)
+        assert form.file_upload
+
+    def test_too_big(self):
+        form = forms.NewPackagedAppForm({}, self.files, max_size=5)
+        eq_(form.is_valid(), False)
+        validation = json.loads(form.file_upload.validation)
+        assert 'messages' in validation, 'No messages in validation.'
+        eq_(validation['messages'][0]['message'],
+            [u'Packaged app too large for submission.',
+             u'Packages must be less than 5 bytes.'])

mkt/developers/views.py

@@ -12,7 +12,6 @@ from django.core.exceptions import PermissionDenied
 from django.db import models, transaction
 from django.forms.models import model_to_dict
 from django.shortcuts import get_object_or_404, redirect
-from django.template.defaultfilters import filesizeformat
 from django.views.decorators.cache import never_cache
 from django.views.decorators.csrf import csrf_view_exempt
 
@@ -55,13 +54,14 @@ from users.views import _login
 from versions.models import Version
 
 from mkt.api.models import Access, generate
-from mkt.constants import APP_IMAGE_SIZES, MAX_PACKAGED_APP_SIZE, regions
+from mkt.constants import APP_IMAGE_SIZES, regions
 from mkt.developers.decorators import dev_required
 from mkt.developers.forms import (AppFormBasic, AppFormDetails, AppFormMedia,
                                   AppFormSupport, AppFormTechnical,
                                   CategoryForm, ImageAssetFormSet,
-                                  InappConfigForm,PaypalSetupForm,
+                                  InappConfigForm, NewPackagedAppForm,
-                                  PreviewFormSet, RegionForm, trap_duplicate)
+                                  PaypalSetupForm, PreviewFormSet, RegionForm,
+                                  trap_duplicate)
 from mkt.developers.models import AddonBlueViaConfig, BlueViaConfig
 from mkt.developers.utils import check_upload
 from mkt.inapp_pay.models import InappConfig
@@ -849,36 +849,20 @@ def validate_addon(request):
 @login_required
 @post_required
 def upload(request, addon_slug=None, is_standalone=False):
-    filedata = request.FILES['upload']
+    form = NewPackagedAppForm(request.POST, request.FILES,
-
+                              user=request.amo_user)
-    fu = FileUpload.from_post(filedata, filedata.name, filedata.size,
+    if form.is_valid():
-                              is_webapp=True)
+        tasks.validator.delay(form.file_upload.pk)
-    log.info('Packaged App FileUpload created: %s' % fu.pk)
-    if request.user.is_authenticated():
-        fu.user = request.amo_user
-        fu.save()
-
-    if filedata.size > MAX_PACKAGED_APP_SIZE:
-        fu.validation = json.dumps(
-            {'errors': 1,
-             'success': False,
-             'messages': [{'type': 'error',
-                           'message': [
-                               'Packaged app too large for submission.',
-                               'Packages must be less than %s.' %
-                                   filesizeformat(MAX_PACKAGED_APP_SIZE)],
-                           'tier': 1}]})
-        fu.save()
-    else:
-        tasks.validator.delay(fu.pk)
 
     if addon_slug:
         return redirect('mkt.developers.upload_detail_for_addon',
-                        addon_slug, fu.pk)
+                        addon_slug, form.file_upload.pk)
     elif is_standalone:
-        return redirect('mkt.developers.standalone_upload_detail', fu.pk)
+        return redirect('mkt.developers.standalone_upload_detail',
+                        form.file_upload.pk)
     else:
-        return redirect('mkt.developers.upload_detail', fu.pk, 'json')
+        return redirect('mkt.developers.upload_detail',
+                        form.file_upload.pk, 'json')
 
 
 @dev_required