Move image size test to model to ensure all forms use it.

filer/admin/clipboardadmin.py

@@ -1,18 +1,16 @@
-from django.conf import settings
 from django.contrib import admin, messages
+from django.core.exceptions import ValidationError
 from django.forms.models import modelform_factory
 from django.http import JsonResponse
 from django.urls import path
 from django.utils.translation import gettext_lazy as _
 from django.views.decorators.csrf import csrf_exempt
 
-from PIL.Image import MAX_IMAGE_PIXELS
-
 from .. import settings as filer_settings
 from ..models import Clipboard, ClipboardItem, Folder
 from ..utils.files import handle_request_files_upload, handle_upload
 from ..utils.loader import load_model
-from ..validation import FileValidationError, validate_upload
+from ..validation import validate_upload
 from . import views
 
 
@@ -23,15 +21,6 @@
 )
 
 
-# We can never exceed the max pixel value set by Pillow's PIL Image MAX_IMAGE_PIXELS
-# as if we allow it, it will fail while thumbnailing (first in the admin thumbnails
-# and then in the page itself.
-# Refer this https://github.com/python-pillow/Pillow/blob/b723e9e62e4706a85f7e44cb42b3d838dae5e546/src/PIL/Image.py#L3148
-FILER_MAX_IMAGE_PIXELS = min(
-    getattr(settings, "FILER_MAX_IMAGE_PIXELS", MAX_IMAGE_PIXELS),
-    MAX_IMAGE_PIXELS,
-)
-
 Image = load_model(filer_settings.FILER_IMAGE_MODEL)
 
 
@@ -124,78 +113,55 @@ def ajax_upload(request, folder_id=None):
             break
     uploadform = FileForm({'original_filename': filename, 'owner': request.user.pk},
                           {'file': upload})
+    uploadform.request = request
     uploadform.instance.mime_type = mime_type
     if uploadform.is_valid():
         try:
             validate_upload(filename, upload, request.user, mime_type)
-        except FileValidationError as error:
+            file_obj = uploadform.save(commit=False)
+            # Enforce the FILER_IS_PUBLIC_DEFAULT
+            file_obj.is_public = filer_settings.FILER_IS_PUBLIC_DEFAULT
+        except ValidationError as error:
             messages.error(request, str(error))
             return JsonResponse({'error': str(error)})
-        file_obj = uploadform.save(commit=False)
-        # Enforce the FILER_IS_PUBLIC_DEFAULT
-        file_obj.is_public = filer_settings.FILER_IS_PUBLIC_DEFAULT
-        if isinstance(file_obj, Image):
-            # We check the Image size and calculate the pixel before
-            # the image gets attached to a folder and saved. We also
-            # send the error msg in the JSON and also post the message
-            # so that they know what is wrong with the image they uploaded
-            height: int = max(1, file_obj.height)
-            width: int = max(1, file_obj.width)
-            pixels: int = width * height
-            aspect: float = width / height
-            res_x: int = int((FILER_MAX_IMAGE_PIXELS * aspect) ** 0.5)
-            res_y: int = int(res_x / aspect)
-            if pixels > 2 * FILER_MAX_IMAGE_PIXELS:
-                msg = _(
-                    "Image size (%(pixels)d million pixels) exceeds limit of "
-                    "%(max_pixels)d million pixels by a factor of two or more. Resize "
-                    "image to %(width)d x %(height)d) resolution or lower."
-                ) % dict(pixels=pixels / 1000000, max_pixels=FILER_MAX_IMAGE_PIXELS / 1000000,
-                         width=res_x, height=res_y)
-                messages.error(request, msg)
-                return JsonResponse({'error': str(msg)})
-
-            if pixels > FILER_MAX_IMAGE_PIXELS:
-                msg = _(
-                    "Image size (%(pixels)d million pixels) exceeds limit of %(max_pixels)d "
-                    "million pixels. Consider resizing image to %(width)d x %(height)d resolution "
-                    "or lower."
-                ) % dict(pixels=pixels / 1000000, max_pixels=FILER_MAX_IMAGE_PIXELS / 1000000,
-                         width=res_x, height=res_y)
-                messages.warning(request, msg)
-            file_obj.folder = folder
-            file_obj.save()
-        else:
-            file_obj.folder = folder
-            file_obj.save()
+        file_obj.folder = folder
+        file_obj.save()
         # TODO: Deprecated/refactor
         # clipboard_item = ClipboardItem(
         #     clipboard=clipboard, file=file_obj)
         # clipboard_item.save()
 
-        thumbnail = None
-        data = {
-            'thumbnail': thumbnail,
-            'alt_text': '',
-            'label': str(file_obj),
-            'file_id': file_obj.pk,
-        }
-        # prepare preview thumbnail
-        if isinstance(file_obj, Image):
-            thumbnail_180_options = {
-                'size': (180, 180),
-                'crop': True,
-                'upscale': True,
+        try:
+            thumbnail = None
+            data = {
+                'thumbnail': thumbnail,
+                'alt_text': '',
+                'label': str(file_obj),
+                'file_id': file_obj.pk,
             }
-            thumbnail_180 = file_obj.file.get_thumbnail(
-                thumbnail_180_options)
-            data['thumbnail_180'] = thumbnail_180.url
-            data['original_image'] = file_obj.url
-        return JsonResponse(data)
+            # prepare preview thumbnail
+            if isinstance(file_obj, Image):
+                thumbnail_180_options = {
+                    'size': (180, 180),
+                    'crop': True,
+                    'upscale': True,
+                }
+                thumbnail_180 = file_obj.file.get_thumbnail(
+                    thumbnail_180_options)
+                data['thumbnail_180'] = thumbnail_180.url
+                data['original_image'] = file_obj.url
+            return JsonResponse(data)
+        except Exception as error:
+            messages.error(request, str(error))
+            return JsonResponse({"error": str(error)})
     else:
+        for key, error_list in uploadform.errors.items():
+            for error in error_list:
+                messages.error(request, error)
+
         form_errors = '; '.join(['{}: {}'.format(
             field,
             ', '.join(errors)) for field, errors in list(
                 uploadform.errors.items())
         ])
-        return JsonResponse({'message': str(form_errors)}, status=422)
+        return JsonResponse({'error': str(form_errors)}, status=200)

filer/fields/multistorage_file.py

@@ -43,7 +43,7 @@ class MultiStorageFileDescriptor(FileDescriptor):
     """
     This is rather similar to Django's ImageFileDescriptor.
     It calls <field name>_data_changed on model instance when new
-    value is set. The callback is suposed to update fields which
+    value is set. The callback is supposed to update fields which
     are related to file data (like size, checksum, etc.).
     When this is called from model __init__ (prev_assigned=False),
     it does nothing because related fields might not have values yet.
@@ -58,7 +58,7 @@ def __set__(self, instance, value):
         # To prevent recalculating file data related attributes when we are instantiating
         # an object from the database, update only if the field had a value before this assignment.
         # To prevent recalculating upon reassignment of the same file, update only if value is
-        # different than the previous one.
+        # different from the previous one.
         if prev_assigned and value != previous_file:
             callback_attr = f'{self.field.name}_data_changed'
             if hasattr(instance, callback_attr):

filer/models/abstract.py

@@ -1,5 +1,8 @@
 import logging
 
+from PIL.Image import MAX_IMAGE_PIXELS
+from django.conf import settings
+from django.core.exceptions import ValidationError
 from django.db import models
 from django.utils.functional import cached_property
 from django.utils.translation import gettext_lazy as _
@@ -17,6 +20,16 @@
 logger = logging.getLogger(__name__)
 
 
+# We can never exceed the max pixel value set by Pillow's PIL Image MAX_IMAGE_PIXELS
+# as if we allow it, it will fail while thumbnailing (first in the admin thumbnails
+# and then in the page itself.
+# Refer this https://github.com/python-pillow/Pillow/blob/b723e9e62e4706a85f7e44cb42b3d838dae5e546/src/PIL/Image.py#L3148
+FILER_MAX_IMAGE_PIXELS = min(
+    getattr(settings, "FILER_MAX_IMAGE_PIXELS", MAX_IMAGE_PIXELS),
+    MAX_IMAGE_PIXELS,
+)
+
+
 class BaseImage(File):
     SIDEBAR_IMAGE_WIDTH = 210
     DEFAULT_THUMBNAILS = {
@@ -112,6 +125,39 @@ def file_data_changed(self, post_init=False):
                     self._transparent = False
         return attrs_updated
 
+    def clean(self):
+        # We check the Image size and calculate the pixel before
+        # the image gets attached to a folder and saved. We also
+        # send the error msg in the JSON and also post the message
+        # so that they know what is wrong with the image they uploaded
+
+        if self._width is None or self._height is None:
+            pixels = 2 * FILER_MAX_IMAGE_PIXELS + 1
+            aspect = 16 / 9
+        else:
+            width, height = max(1, self.width), max(1, self.height)
+            pixels: int = width * height
+            aspect: float = width / height
+        res_x: int = int((FILER_MAX_IMAGE_PIXELS * aspect) ** 0.5)
+        res_y: int = int(res_x / aspect)
+        if pixels > 2 * FILER_MAX_IMAGE_PIXELS:
+            msg = _(
+                "Image format not recognized or image size exceeds limit of %(max_pixels)d million "
+                "pixels by a factor of two or more. Check file format or resize image to "
+                "%(width)d x %(height)d) resolution or lower."
+            ) % dict(max_pixels=FILER_MAX_IMAGE_PIXELS // 1000000, width=res_x, height=res_y)
+            raise ValidationError(str(msg), code="image_size")
+
+        if pixels > FILER_MAX_IMAGE_PIXELS:
+            # Can I catch warnings?
+            msg = _(
+                "Image size (%(pixels)d million pixels) exceeds limit of %(max_pixels)d "
+                "million pixels. Resize image to %(width)d x %(height)d resolution "
+                "or lower."
+            ) % dict(pixels=pixels // 1000000, max_pixels=FILER_MAX_IMAGE_PIXELS // 1000000,
+                     width=res_x, height=res_y)
+            raise ValidationError(str(msg), code="image_size")
+
     def save(self, *args, **kwargs):
         self.has_all_mandatory_data = self._check_validity()
         super().save(*args, **kwargs)

filer/validation.py

@@ -2,13 +2,14 @@
 
 from django.apps import apps
 from django.contrib.auth import get_user_model
+from django.core.exceptions import ValidationError
 from django.utils.translation import gettext as _
 
 
 User = get_user_model()  # Needed for typing
 
 
-class FileValidationError(Exception):
+class FileValidationError(ValidationError):
     pass
 
 

tests/test_admin.py

@@ -8,7 +8,7 @@
 from django.contrib.admin import helpers
 from django.contrib.auth import get_user_model
 from django.contrib.auth.models import Permission
-from django.contrib.messages import get_messages, ERROR, WARNING
+from django.contrib.messages import get_messages, ERROR
 from django.forms.models import model_to_dict as model_to_dict_django
 from django.http import HttpRequest, HttpResponseForbidden
 from django.test import RequestFactory, TestCase
@@ -20,6 +20,7 @@
 from filer import settings as filer_settings
 from filer.admin import tools
 from filer.admin.folderadmin import FolderAdmin
+from filer.models import abstract
 from filer.models.filemodels import File
 from filer.models.foldermodels import Folder, FolderPermission
 from filer.models.virtualitems import FolderRoot
@@ -478,9 +479,8 @@ def test_filer_ajax_upload_file(self):
         self.assertEqual(stored_image.mime_type, 'image/jpeg')
 
     def test_filer_ajax_decompression_bomb(self):
-        from filer.admin import clipboardadmin
-        DEFAULT_MAX_IMAGE_PIXELS = clipboardadmin.FILER_MAX_IMAGE_PIXELS
-        clipboardadmin.FILER_MAX_IMAGE_PIXELS = 800 * 200  # Triggers error
+        DEFAULT_MAX_IMAGE_PIXELS = abstract.FILER_MAX_IMAGE_PIXELS
+        abstract.FILER_MAX_IMAGE_PIXELS = 800 * 200  # Triggers error
         self.assertEqual(Image.objects.count(), 0)
         folder = Folder.objects.create(name='foo')
         with open(self.filename, 'rb') as fh:
@@ -501,7 +501,7 @@ def test_filer_ajax_decompression_bomb(self):
         self.assertEqual(len(messages), 1)
         self.assertEqual(messages[0].level, ERROR)
 
-        clipboardadmin.FILER_MAX_IMAGE_PIXELS = 800 * 300  # Triggers warning
+        abstract.FILER_MAX_IMAGE_PIXELS = 800 * 300  # Triggers warning
         folder = Folder.objects.create(name='foo')
         with open(self.filename, 'rb') as fh:
             file_obj = django.core.files.File(fh)
@@ -518,13 +518,10 @@ def test_filer_ajax_decompression_bomb(self):
         self.assertEqual(response.status_code, 200)
         messages = list(get_messages(response.wsgi_request))
         self.assertEqual(len(messages), 2)  # One more message
-        self.assertEqual(messages[1].level, WARNING)
-        self.assertEqual(Image.objects.count(), 1)
-        stored_image = Image.objects.first()
-        self.assertEqual(stored_image.original_filename, self.image_name)
-        self.assertEqual(stored_image.mime_type, 'image/jpeg')
+        self.assertEqual(messages[1].level, ERROR)
+        self.assertEqual(Image.objects.count(), 0)
 
-        clipboardadmin.FILER_MAX_IMAGE_PIXELS = DEFAULT_MAX_IMAGE_PIXELS
+        abstract.FILER_MAX_IMAGE_PIXELS = DEFAULT_MAX_IMAGE_PIXELS
 
     def test_filer_ajax_upload_file_using_content_type(self):
         self.assertEqual(Image.objects.count(), 0)

tests/test_models.py

@@ -65,6 +65,7 @@ def test_upload_image_form(self):
             'original_filename': self.image_name,
             'owner': self.superuser.pk
         }, {'file': file_obj})
+        self.assertTrue(upoad_image_form.is_valid())
         if upoad_image_form.is_valid():
             image = upoad_image_form.save()  # noqa
         self.assertEqual(Image.objects.count(), 1)